Skip to content
This repository was archived by the owner on Apr 14, 2022. It is now read-only.

Improve diagnostic generation/handling, enable diagnostics with setting #289

Closed
Jake Bailey (jakebailey) wants to merge 4 commits into from
Closed

Improve diagnostic generation/handling, enable diagnostics with setting #289

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
72c5f2a
diagnostic improvments
jakebailey Oct 19, 2018
6e5adfb
add setting for enabling/disabling diagnostics as a whole
jakebailey Oct 19, 2018
6c9ed87
formatting
jakebailey Oct 19, 2018
9cf31f1
change default diagnostics setting to true
jakebailey Oct 22, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 0 additions & 8 deletions src/Analysis/Engine/Impl/PythonAnalyzer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -618,10 +618,6 @@ public AnalysisLimits Limits {
public bool EnableDiagnostics { get; set; }

public void AddDiagnostic(Node node, AnalysisUnit unit, string message, DiagnosticSeverity severity, string code = null) {
if (!EnableDiagnostics) {
return;
}

lock (_diagnostics) {
if (!_diagnostics.TryGetValue(unit.ProjectEntry, out var diags)) {
_diagnostics[unit.ProjectEntry] = diags = new Dictionary<Node, Diagnostic>();
Expand Down Expand Up @@ -656,10 +652,6 @@ public IReadOnlyDictionary<IProjectEntry, IReadOnlyList<Diagnostic>> GetAllDiagn
}

public void ClearDiagnostic(Node node, AnalysisUnit unit, string code = null) {
if (!EnableDiagnostics) {
return;
}

lock (_diagnostics) {
if (_diagnostics.TryGetValue(unit.ProjectEntry, out var diags) && diags.TryGetValue(node, out var d)) {
if (code == null || d.code == code) {
Expand Down
15 changes: 11 additions & 4 deletions src/LanguageServer/Impl/Definitions/ServerSettings.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,12 @@ public class PythonAnalysisOptions {
public int symbolsHierarchyDepthLimit = 10;
public int symbolsHierarchyMaxSymbols = 1000;

public string[] errors { get; } = Array.Empty<string>();
public string[] warnings { get; } = Array.Empty<string>();
public string[] information { get; } = Array.Empty<string>();
public string[] disabled { get; } = Array.Empty<string>();
public bool diagnostics;

public string[] errors { get; private set; } = Array.Empty<string>();
public string[] warnings { get; private set; } = Array.Empty<string>();
public string[] information { get; private set; } = Array.Empty<string>();
public string[] disabled { get; private set; } = Array.Empty<string>();

public DiagnosticSeverity GetEffectiveSeverity(string code, DiagnosticSeverity defaultSeverity)
=> _map.TryGetValue(code, out var severity) ? severity : defaultSeverity;
Expand All @@ -50,6 +52,11 @@ public void SetErrorSeverityOptions(string[] errors, string[] warnings, string[]
foreach (var x in disabled) {
_map[x] = DiagnosticSeverity.Unspecified;
}

this.errors = errors;
this.warnings = warnings;
this.information = information;
this.disabled = disabled;
}
}
public readonly PythonAnalysisOptions analysis = new PythonAnalysisOptions();
Expand Down
20 changes: 7 additions & 13 deletions src/LanguageServer/Impl/Implementation/EditorFiles.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,36 +176,30 @@ public void UpdateParseDiagnostics(VersionCookie vc, Uri documentUri) {

public void UpdateAnalysisDiagnostics(IProjectEntry projectEntry, int version) {
lock (_lock) {
var diags = _server.Analyzer.GetDiagnostics(projectEntry);
var settings = _server.Settings;
IEnumerable<Diagnostic> diags = _server.Analyzer.GetDiagnostics(projectEntry);

for (var i = 0; i < diags.Count; i++) {
diags[i].severity = settings.analysis.GetEffectiveSeverity(diags[i].code, diags[i].severity);
}

var severity = settings.analysis.GetEffectiveSeverity(ErrorMessages.UnresolvedImportCode, DiagnosticSeverity.Warning);
var pythonProjectEntry = projectEntry as IPythonProjectEntry;
var parse = pythonProjectEntry?.GetCurrentParse();

// TODO: move this to the normal analysis process
if (parse != null && severity != DiagnosticSeverity.Unspecified) {
var walker = new ImportStatementWalker(parse.Tree, pythonProjectEntry, _server.Analyzer, severity);
if (parse != null) {
var walker = new ImportStatementWalker(parse.Tree, pythonProjectEntry, _server.Analyzer, DiagnosticSeverity.Warning);
parse.Tree.Walk(walker);
diags = diags.Concat(walker.Diagnostics).ToArray();
diags = diags.Concat(walker.Diagnostics);
}

if (pythonProjectEntry is IDocument doc) {
if (_lastReportedParseDiagnostics != null) {
diags = diags.Concat(_lastReportedParseDiagnostics.SelectMany(d => d.diagnostics)).ToArray();
diags = diags.Concat(_lastReportedParseDiagnostics.SelectMany(d => d.diagnostics));
}
}

var lastPublishedVersion = _lastReportedAnalysisDiagnostics?.FirstOrDefault()?._version;
version = version >= 0 ? version : (lastPublishedVersion.HasValue ? lastPublishedVersion.Value : 0);
version = version >= 0 ? version : (lastPublishedVersion ?? 0);

_lastReportedAnalysisDiagnostics = new[] { new PublishDiagnosticsEventArgs {
uri = pythonProjectEntry.DocumentUri,
diagnostics = diags,
diagnostics = diags.ToArray(),
_version = version
}};

Expand Down
12 changes: 5 additions & 7 deletions src/LanguageServer/Impl/Implementation/Server.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,7 @@ public override async Task DidChangeConfiguration(DidChangeConfigurationParams @
var reanalyze = true;
if (@params.settings != null) {
if (@params.settings is ServerSettings settings) {
Analyzer.EnableDiagnostics = settings.analysis.diagnostics;

@MikhailArkhipov Mikhail Arkhipov (MikhailArkhipov) Oct 21, 2018

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The setting needs to be added to the package.json in the extension defaulting to true

@jakebailey Jake Bailey (jakebailey) Oct 21, 2018

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, I've done that locally for now (until I PR it), since I wasn't 100% sure on the naming.

@MikhailArkhipov Mikhail Arkhipov (MikhailArkhipov) Oct 22, 2018

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don Jayamanne (@DonJayamanne) can suggest name of the setting

reanalyze = HandleConfigurationChanges(settings);
} else {
LogMessage(MessageType.Error, "change configuration notification sent unsupported settings");
Expand Down Expand Up @@ -696,15 +697,12 @@ private bool HandleConfigurationChanges(ServerSettings newSettings) {
return true;
}

if (newSettings.analysis.openFilesOnly != oldSettings.analysis.openFilesOnly) {
_editorFiles.UpdateDiagnostics();
return false;
}

if (!newSettings.analysis.errors.SetEquals(oldSettings.analysis.errors) ||
if (newSettings.analysis.openFilesOnly != oldSettings.analysis.openFilesOnly ||
!newSettings.analysis.errors.SetEquals(oldSettings.analysis.errors) ||
!newSettings.analysis.warnings.SetEquals(oldSettings.analysis.warnings) ||
!newSettings.analysis.information.SetEquals(oldSettings.analysis.information) ||
!newSettings.analysis.disabled.SetEquals(oldSettings.analysis.disabled)) {
!newSettings.analysis.disabled.SetEquals(oldSettings.analysis.disabled) ||
newSettings.analysis.diagnostics != oldSettings.analysis.diagnostics) {
_editorFiles.UpdateDiagnostics();
}

Expand Down
34 changes: 28 additions & 6 deletions src/LanguageServer/Impl/LanguageServer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,17 +14,17 @@
// See the Apache Version 2.0 License for specific language governing
// permissions and limitations under the License.

using System;
using System.Collections.Generic;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.Python.LanguageServer.Services;
using Microsoft.PythonTools.Analysis;
using Microsoft.PythonTools.Analysis.Infrastructure;
using Newtonsoft.Json;
using Newtonsoft.Json.Linq;
using StreamJsonRpc;
using System;
using System.Collections.Generic;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;

namespace Microsoft.Python.LanguageServer.Implementation {
/// <summary>
Expand Down Expand Up @@ -145,6 +145,7 @@ public async Task DidChangeConfiguration(JToken token, CancellationToken cancell
settings.diagnosticPublishDelay = GetSetting(analysis, "diagnosticPublishDelay", 1000);
settings.symbolsHierarchyDepthLimit = GetSetting(analysis, "symbolsHierarchyDepthLimit", 10);
settings.symbolsHierarchyMaxSymbols = GetSetting(analysis, "symbolsHierarchyMaxSymbols", 1000);
settings.analysis.diagnostics = GetSetting(analysis, "diagnostics", true);

_ui.SetLogLevel(GetLogLevel(analysis));

Expand Down Expand Up @@ -419,9 +420,30 @@ private void PublishPendingDiagnostics() {
}

foreach (var kvp in list) {
var diagnostics = kvp.Value;

if (_server.Settings.analysis.diagnostics) {
diagnostics = kvp.Value
.Select(d => new Diagnostic { // Copy the diagnostic so we aren't just modifying the severity for everyone who has access.
range = d.range,
severity = _server.Settings.analysis.GetEffectiveSeverity(d.code, d.severity),
code = d.code,
source = d.source,
message = d.message,
})
.Where(d => d.severity != DiagnosticSeverity.Unspecified) // Don't send unspecified/disabled diagnostics.
.ToArray();
} else {
// If diagnostics are disabled globally, don't send any diagnostics,
// except for the explicitly empty ones which are used to clear.
if (diagnostics.Length != 0) {
continue;
}
}

var parameters = new PublishDiagnosticsParams {
uri = kvp.Key,
diagnostics = kvp.Value
diagnostics = diagnostics
};
_rpc.NotifyWithParameterObjectAsync("textDocument/publishDiagnostics", parameters).DoNotWait();
}
Expand Down