From 72c5f2a99efc349d019ccc9317d45822ac3d6e17 Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Fri, 19 Oct 2018 13:33:35 -0700 Subject: [PATCH 1/4] diagnostic improvments --- .../Impl/Definitions/ServerSettings.cs | 13 ++++++++---- .../Impl/Implementation/EditorFiles.cs | 20 +++++++------------ .../Impl/Implementation/Server.cs | 8 ++------ src/LanguageServer/Impl/LanguageServer.cs | 12 ++++++++++- 4 files changed, 29 insertions(+), 24 deletions(-) diff --git a/src/LanguageServer/Impl/Definitions/ServerSettings.cs b/src/LanguageServer/Impl/Definitions/ServerSettings.cs index 463331da5..982446351 100644 --- a/src/LanguageServer/Impl/Definitions/ServerSettings.cs +++ b/src/LanguageServer/Impl/Definitions/ServerSettings.cs @@ -27,10 +27,10 @@ public class PythonAnalysisOptions { public int symbolsHierarchyDepthLimit = 10; public int symbolsHierarchyMaxSymbols = 1000; - public string[] errors { get; } = Array.Empty(); - public string[] warnings { get; } = Array.Empty(); - public string[] information { get; } = Array.Empty(); - public string[] disabled { get; } = Array.Empty(); + public string[] errors { get; private set; } = Array.Empty(); + public string[] warnings { get; private set; } = Array.Empty(); + public string[] information { get; private set; } = Array.Empty(); + public string[] disabled { get; private set; } = Array.Empty(); public DiagnosticSeverity GetEffectiveSeverity(string code, DiagnosticSeverity defaultSeverity) => _map.TryGetValue(code, out var severity) ? severity : defaultSeverity; @@ -50,6 +50,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(); diff --git a/src/LanguageServer/Impl/Implementation/EditorFiles.cs b/src/LanguageServer/Impl/Implementation/EditorFiles.cs index 487952676..d42450bb4 100644 --- a/src/LanguageServer/Impl/Implementation/EditorFiles.cs +++ b/src/LanguageServer/Impl/Implementation/EditorFiles.cs @@ -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 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 }}; diff --git a/src/LanguageServer/Impl/Implementation/Server.cs b/src/LanguageServer/Impl/Implementation/Server.cs index 9f398d473..27b976b21 100644 --- a/src/LanguageServer/Impl/Implementation/Server.cs +++ b/src/LanguageServer/Impl/Implementation/Server.cs @@ -696,12 +696,8 @@ 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)) { diff --git a/src/LanguageServer/Impl/LanguageServer.cs b/src/LanguageServer/Impl/LanguageServer.cs index ff0a46f80..8628ab384 100644 --- a/src/LanguageServer/Impl/LanguageServer.cs +++ b/src/LanguageServer/Impl/LanguageServer.cs @@ -419,9 +419,19 @@ private void PublishPendingDiagnostics() { } foreach (var kvp in list) { + var 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); + var parameters = new PublishDiagnosticsParams { uri = kvp.Key, - diagnostics = kvp.Value + diagnostics = diagnostics.ToArray() }; _rpc.NotifyWithParameterObjectAsync("textDocument/publishDiagnostics", parameters).DoNotWait(); } From 6e5adfb53345a80449d4093e82194fff629092e4 Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Fri, 19 Oct 2018 14:13:36 -0700 Subject: [PATCH 2/4] add setting for enabling/disabling diagnostics as a whole --- src/Analysis/Engine/Impl/PythonAnalyzer.cs | 8 -------- .../Impl/Definitions/ServerSettings.cs | 2 ++ .../Impl/Implementation/Server.cs | 4 +++- src/LanguageServer/Impl/LanguageServer.cs | 18 +++++++++++++++--- 4 files changed, 20 insertions(+), 12 deletions(-) diff --git a/src/Analysis/Engine/Impl/PythonAnalyzer.cs b/src/Analysis/Engine/Impl/PythonAnalyzer.cs index d03fc9ab8..ecb7bb41f 100644 --- a/src/Analysis/Engine/Impl/PythonAnalyzer.cs +++ b/src/Analysis/Engine/Impl/PythonAnalyzer.cs @@ -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(); @@ -656,10 +652,6 @@ public IReadOnlyDictionary> 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) { diff --git a/src/LanguageServer/Impl/Definitions/ServerSettings.cs b/src/LanguageServer/Impl/Definitions/ServerSettings.cs index 982446351..98c540029 100644 --- a/src/LanguageServer/Impl/Definitions/ServerSettings.cs +++ b/src/LanguageServer/Impl/Definitions/ServerSettings.cs @@ -27,6 +27,8 @@ public class PythonAnalysisOptions { public int symbolsHierarchyDepthLimit = 10; public int symbolsHierarchyMaxSymbols = 1000; + public bool diagnostics; + public string[] errors { get; private set; } = Array.Empty(); public string[] warnings { get; private set; } = Array.Empty(); public string[] information { get; private set; } = Array.Empty(); diff --git a/src/LanguageServer/Impl/Implementation/Server.cs b/src/LanguageServer/Impl/Implementation/Server.cs index 27b976b21..2f359c10c 100644 --- a/src/LanguageServer/Impl/Implementation/Server.cs +++ b/src/LanguageServer/Impl/Implementation/Server.cs @@ -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; reanalyze = HandleConfigurationChanges(settings); } else { LogMessage(MessageType.Error, "change configuration notification sent unsupported settings"); @@ -700,7 +701,8 @@ private bool HandleConfigurationChanges(ServerSettings newSettings) { !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(); } diff --git a/src/LanguageServer/Impl/LanguageServer.cs b/src/LanguageServer/Impl/LanguageServer.cs index 8628ab384..207009462 100644 --- a/src/LanguageServer/Impl/LanguageServer.cs +++ b/src/LanguageServer/Impl/LanguageServer.cs @@ -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", false); _ui.SetLogLevel(GetLogLevel(analysis)); @@ -419,7 +420,10 @@ private void PublishPendingDiagnostics() { } foreach (var kvp in list) { - var diagnostics = kvp.Value + 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), @@ -427,11 +431,19 @@ private void PublishPendingDiagnostics() { source = d.source, message = d.message, }) - .Where(d => d.severity != DiagnosticSeverity.Unspecified); + .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 = diagnostics.ToArray() + diagnostics = diagnostics }; _rpc.NotifyWithParameterObjectAsync("textDocument/publishDiagnostics", parameters).DoNotWait(); } From 6c9ed8786b9ee8bd219941808443624b211766c1 Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Fri, 19 Oct 2018 14:21:11 -0700 Subject: [PATCH 3/4] formatting --- src/LanguageServer/Impl/LanguageServer.cs | 28 +++++++++++------------ 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/src/LanguageServer/Impl/LanguageServer.cs b/src/LanguageServer/Impl/LanguageServer.cs index 207009462..aefe2fe36 100644 --- a/src/LanguageServer/Impl/LanguageServer.cs +++ b/src/LanguageServer/Impl/LanguageServer.cs @@ -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 { /// @@ -424,15 +424,15 @@ private void PublishPendingDiagnostics() { 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(); + .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. From 9cf31f15e7944a94c7751fb5f5f0d71859fba34d Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Mon, 22 Oct 2018 09:08:07 -0700 Subject: [PATCH 4/4] change default diagnostics setting to true --- src/LanguageServer/Impl/LanguageServer.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/LanguageServer/Impl/LanguageServer.cs b/src/LanguageServer/Impl/LanguageServer.cs index aefe2fe36..8eb5fe631 100644 --- a/src/LanguageServer/Impl/LanguageServer.cs +++ b/src/LanguageServer/Impl/LanguageServer.cs @@ -145,7 +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", false); + settings.analysis.diagnostics = GetSetting(analysis, "diagnostics", true); _ui.SetLogLevel(GetLogLevel(analysis));