From b8f56faa82d6be55da35c122f278c797e6b35475 Mon Sep 17 00:00:00 2001 From: Tyler Leonhardt Date: Thu, 21 Mar 2019 16:31:35 -0700 Subject: [PATCH 1/5] Add new PSSA 1.18 ParseError Severity --- .../Workspace/ScriptFileMarker.cs | 53 +++++++++---------- 1 file changed, 25 insertions(+), 28 deletions(-) diff --git a/src/PowerShellEditorServices/Workspace/ScriptFileMarker.cs b/src/PowerShellEditorServices/Workspace/ScriptFileMarker.cs index 3737b7169..0a36b313a 100644 --- a/src/PowerShellEditorServices/Workspace/ScriptFileMarker.cs +++ b/src/PowerShellEditorServices/Workspace/ScriptFileMarker.cs @@ -33,20 +33,22 @@ public class MarkerCorrection /// public enum ScriptFileMarkerLevel { - /// - /// The marker represents an informational message. - /// - Information = 0, - - /// - /// The marker represents a warning message. - /// - Warning, - - /// - /// The marker represents an error message. - /// - Error + ///  +        /// Information: This warning is trivial, but may be useful. They are recommended by PowerShell best practice. +        ///  +        Information = 0, +        ///  +        /// WARNING: This warning may cause a problem or does not follow PowerShell's recommended guidelines. +        ///  +        Warning = 1, +        ///  +        /// ERROR: This warning is likely to cause a problem or does not follow PowerShell's required guidelines. +        ///  +        Error = 2, +        ///  +        /// ERROR: This diagnostic is caused by an actual parsing error, and is generated only by the engine. +        ///  +        ParseError = 3 }; /// @@ -62,7 +64,7 @@ public class ScriptFileMarker /// Gets or sets the marker's message string. /// public string Message { get; set; } - + /// /// Gets or sets the ruleName associated with this marker. /// @@ -176,21 +178,16 @@ internal static ScriptFileMarker FromDiagnosticRecord(PSObject psObject) private static ScriptFileMarkerLevel GetMarkerLevelFromDiagnosticSeverity( string diagnosticSeverity) { - switch (diagnosticSeverity) + if(Enum.TryParse(diagnosticSeverity, out ScriptFileMarkerLevel level)) { - case "Information": - return ScriptFileMarkerLevel.Information; - case "Warning": - return ScriptFileMarkerLevel.Warning; - case "Error": - return ScriptFileMarkerLevel.Error; - default: - throw new ArgumentException( - string.Format( - "The provided DiagnosticSeverity value '{0}' is unknown.", - diagnosticSeverity), - "diagnosticSeverity"); + return level; } + + throw new ArgumentException( + string.Format( + "The provided DiagnosticSeverity value '{0}' is unknown.", + diagnosticSeverity), + "diagnosticSeverity"); } #endregion } From 4937f56ea465265f54246e589766be70498adeaf Mon Sep 17 00:00:00 2001 From: Tyler Leonhardt Date: Fri, 22 Mar 2019 11:41:24 -0700 Subject: [PATCH 2/5] ignore parseerrors from PSSA --- .../Server/LanguageServer.cs | 16 +++++++----- .../Analysis/AnalysisService.cs | 18 +++++++------ .../Workspace/ScriptFile.cs | 4 +-- .../Workspace/ScriptFileMarker.cs | 26 ++++++++----------- .../Session/ScriptFileTests.cs | 4 +-- 5 files changed, 34 insertions(+), 34 deletions(-) diff --git a/src/PowerShellEditorServices.Protocol/Server/LanguageServer.cs b/src/PowerShellEditorServices.Protocol/Server/LanguageServer.cs index 0e97b8edd..89119a846 100644 --- a/src/PowerShellEditorServices.Protocol/Server/LanguageServer.cs +++ b/src/PowerShellEditorServices.Protocol/Server/LanguageServer.cs @@ -1192,7 +1192,7 @@ protected async Task HandleCommentHelpRequestAsync( funcText = string.Join("\n", lines); } - ScriptFileMarker[] analysisResults = await this.editorSession.AnalysisService.GetSemanticMarkersAsync( + List analysisResults = await this.editorSession.AnalysisService.GetSemanticMarkersAsync( funcText, AnalysisService.GetCommentHelpRuleSettings( enable: true, @@ -1743,7 +1743,7 @@ await PublishScriptDiagnosticsAsync( // Get the requested files foreach (ScriptFile scriptFile in filesToAnalyze) { - ScriptFileMarker[] semanticMarkers = null; + List semanticMarkers = null; if (isScriptAnalysisEnabled && editorSession.AnalysisService != null) { using (Logger.LogExecutionTime($"Script analysis of {scriptFile.FilePath} completed.")) @@ -1755,13 +1755,15 @@ await PublishScriptDiagnosticsAsync( { // Semantic markers aren't available if the AnalysisService // isn't available - semanticMarkers = new ScriptFileMarker[0]; + semanticMarkers = new List(); } + scriptFile.SyntaxMarkers.AddRange(semanticMarkers); + await PublishScriptDiagnosticsAsync( scriptFile, // Concat script analysis errors to any existing parse errors - scriptFile.SyntaxMarkers.Concat(semanticMarkers).ToArray(), + scriptFile.SyntaxMarkers, correctionIndex, eventSender); } @@ -1772,14 +1774,14 @@ private async Task ClearMarkersAsync(ScriptFile scriptFile, EventContext eventCo // send empty diagnostic markers to clear any markers associated with the given file await PublishScriptDiagnosticsAsync( scriptFile, - new ScriptFileMarker[0], + new List(), this.codeActionsPerFile, eventContext); } private static async Task PublishScriptDiagnosticsAsync( ScriptFile scriptFile, - ScriptFileMarker[] markers, + List markers, Dictionary> correctionIndex, EventContext eventContext) { @@ -1792,7 +1794,7 @@ await PublishScriptDiagnosticsAsync( private static async Task PublishScriptDiagnosticsAsync( ScriptFile scriptFile, - ScriptFileMarker[] markers, + List markers, Dictionary> correctionIndex, Func, PublishDiagnosticsNotification, Task> eventSender) { diff --git a/src/PowerShellEditorServices/Analysis/AnalysisService.cs b/src/PowerShellEditorServices/Analysis/AnalysisService.cs index d87226377..406c4193b 100644 --- a/src/PowerShellEditorServices/Analysis/AnalysisService.cs +++ b/src/PowerShellEditorServices/Analysis/AnalysisService.cs @@ -55,7 +55,7 @@ public class AnalysisService : IDisposable /// /// An empty script marker result to return when no script markers can be returned. /// - private static readonly ScriptFileMarker[] s_emptyScriptMarkerResult = new ScriptFileMarker[0]; + private static readonly List s_emptyScriptMarkerResult = new List(); private static readonly string[] s_emptyGetRuleResult = new string[0]; @@ -266,7 +266,7 @@ public static Hashtable GetPSSASettingsHashtable(IDictionary /// /// The ScriptFile which will be analyzed for semantic markers. /// An array of ScriptFileMarkers containing semantic analysis results. - public async Task GetSemanticMarkersAsync(ScriptFile file) + public async Task> GetSemanticMarkersAsync(ScriptFile file) { return await GetSemanticMarkersAsync(file, ActiveRules, SettingsPath); } @@ -277,7 +277,7 @@ public async Task GetSemanticMarkersAsync(ScriptFile file) /// The ScriptFile to be analyzed. /// ScriptAnalyzer settings /// - public async Task GetSemanticMarkersAsync(ScriptFile file, Hashtable settings) + public async Task> GetSemanticMarkersAsync(ScriptFile file, Hashtable settings) { return await GetSemanticMarkersAsync(file, null, settings); } @@ -288,7 +288,7 @@ public async Task GetSemanticMarkersAsync(ScriptFile file, H /// The script content to be analyzed. /// ScriptAnalyzer settings /// - public async Task GetSemanticMarkersAsync( + public async Task> GetSemanticMarkersAsync( string scriptContent, Hashtable settings) { @@ -379,7 +379,7 @@ public async Task FormatAsync( #region Private Methods - private async Task GetSemanticMarkersAsync( + private async Task> GetSemanticMarkersAsync( ScriptFile file, string[] rules, TSettings settings) where TSettings : class @@ -398,7 +398,7 @@ private async Task GetSemanticMarkersAsync( } } - private async Task GetSemanticMarkersAsync( + private async Task> GetSemanticMarkersAsync( string scriptContent, string[] rules, TSettings settings) where TSettings : class @@ -407,7 +407,7 @@ private async Task GetSemanticMarkersAsync( && (rules != null || settings != null)) { var scriptFileMarkers = await GetDiagnosticRecordsAsync(scriptContent, rules, settings); - return scriptFileMarkers.Select(ScriptFileMarker.FromDiagnosticRecord).ToArray(); + return scriptFileMarkers.Select(ScriptFileMarker.FromDiagnosticRecord).ToList(); } else { @@ -514,7 +514,9 @@ private async Task GetDiagnosticRecordsAsync( new Dictionary { { "ScriptDefinition", scriptContent }, - { settingParameter, settingArgument } + { settingParameter, settingArgument }, + // We ignore ParseErrors from PSSA because we already send them when we parse the file. + { "Severity", new [] { ScriptFileMarkerLevel.Error, ScriptFileMarkerLevel.Information, ScriptFileMarkerLevel.Warning }} }); diagnosticRecords = result?.Output; diff --git a/src/PowerShellEditorServices/Workspace/ScriptFile.cs b/src/PowerShellEditorServices/Workspace/ScriptFile.cs index c2806a7fb..cafed0cf8 100644 --- a/src/PowerShellEditorServices/Workspace/ScriptFile.cs +++ b/src/PowerShellEditorServices/Workspace/ScriptFile.cs @@ -100,7 +100,7 @@ public string Contents /// Gets the list of syntax markers found by parsing this /// file's contents. /// - public ScriptFileMarker[] SyntaxMarkers + public List SyntaxMarkers { get; private set; @@ -657,7 +657,7 @@ private void ParseFileContents() this.SyntaxMarkers = parseErrors .Select(ScriptFileMarker.FromParseError) - .ToArray(); + .ToList(); // Untitled files have no directory // Discussed in https://github.com/PowerShell/PowerShellEditorServices/pull/815. diff --git a/src/PowerShellEditorServices/Workspace/ScriptFileMarker.cs b/src/PowerShellEditorServices/Workspace/ScriptFileMarker.cs index 0a36b313a..b48107bc3 100644 --- a/src/PowerShellEditorServices/Workspace/ScriptFileMarker.cs +++ b/src/PowerShellEditorServices/Workspace/ScriptFileMarker.cs @@ -164,31 +164,27 @@ internal static ScriptFileMarker FromDiagnosticRecord(PSObject psObject) }; } + string severity = diagnosticRecord.Severity.ToString(); + if(!Enum.TryParse(severity, out ScriptFileMarkerLevel level)) + { + throw new ArgumentException( + string.Format( + "The provided DiagnosticSeverity value '{0}' is unknown.", + severity), + "diagnosticSeverity"); + } + return new ScriptFileMarker { Message = $"{diagnosticRecord.Message as string}", RuleName = $"{diagnosticRecord.RuleName as string}", - Level = GetMarkerLevelFromDiagnosticSeverity((diagnosticRecord.Severity as Enum).ToString()), + Level = level, ScriptRegion = ScriptRegion.Create(diagnosticRecord.Extent as IScriptExtent), Correction = correction, Source = "PSScriptAnalyzer" }; } - private static ScriptFileMarkerLevel GetMarkerLevelFromDiagnosticSeverity( - string diagnosticSeverity) - { - if(Enum.TryParse(diagnosticSeverity, out ScriptFileMarkerLevel level)) - { - return level; - } - - throw new ArgumentException( - string.Format( - "The provided DiagnosticSeverity value '{0}' is unknown.", - diagnosticSeverity), - "diagnosticSeverity"); - } #endregion } } diff --git a/test/PowerShellEditorServices.Test/Session/ScriptFileTests.cs b/test/PowerShellEditorServices.Test/Session/ScriptFileTests.cs index 24ca67104..75fc76d8a 100644 --- a/test/PowerShellEditorServices.Test/Session/ScriptFileTests.cs +++ b/test/PowerShellEditorServices.Test/Session/ScriptFileTests.cs @@ -550,7 +550,7 @@ public void PropertiesInitializedCorrectlyForFile() Assert.True(scriptFile.IsAnalysisEnabled); Assert.False(scriptFile.IsInMemory); Assert.Empty(scriptFile.ReferencedFiles); - Assert.Empty(scriptFile.SyntaxMarkers); + Assert.Null(scriptFile.SyntaxMarkers); Assert.Single(scriptFile.ScriptTokens); Assert.Single(scriptFile.FileLines); } @@ -576,7 +576,7 @@ public void PropertiesInitializedCorrectlyForUntitled() Assert.True(scriptFile.IsAnalysisEnabled); Assert.True(scriptFile.IsInMemory); Assert.Empty(scriptFile.ReferencedFiles); - Assert.Empty(scriptFile.SyntaxMarkers); + Assert.Null(scriptFile.SyntaxMarkers); Assert.Equal(10, scriptFile.ScriptTokens.Length); Assert.Equal(3, scriptFile.FileLines.Count); } From a81d4ae24cc67dfadd966bd73cc0de28e9a6239d Mon Sep 17 00:00:00 2001 From: Tyler Leonhardt Date: Fri, 22 Mar 2019 13:22:02 -0700 Subject: [PATCH 3/5] address feedback --- .../Server/LanguageServer.cs | 6 +++--- src/PowerShellEditorServices/Workspace/ScriptFile.cs | 4 ++-- src/PowerShellEditorServices/Workspace/ScriptFileMarker.cs | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/PowerShellEditorServices.Protocol/Server/LanguageServer.cs b/src/PowerShellEditorServices.Protocol/Server/LanguageServer.cs index 89119a846..a5d3d3c86 100644 --- a/src/PowerShellEditorServices.Protocol/Server/LanguageServer.cs +++ b/src/PowerShellEditorServices.Protocol/Server/LanguageServer.cs @@ -1725,7 +1725,7 @@ private static async Task DelayThenInvokeDiagnosticsAsync( { await PublishScriptDiagnosticsAsync( script, - script.SyntaxMarkers, + script.DiagnosticMarkers, correctionIndex, eventSender); } @@ -1758,12 +1758,12 @@ await PublishScriptDiagnosticsAsync( semanticMarkers = new List(); } - scriptFile.SyntaxMarkers.AddRange(semanticMarkers); + scriptFile.DiagnosticMarkers.AddRange(semanticMarkers); await PublishScriptDiagnosticsAsync( scriptFile, // Concat script analysis errors to any existing parse errors - scriptFile.SyntaxMarkers, + scriptFile.DiagnosticMarkers, correctionIndex, eventSender); } diff --git a/src/PowerShellEditorServices/Workspace/ScriptFile.cs b/src/PowerShellEditorServices/Workspace/ScriptFile.cs index cafed0cf8..958778240 100644 --- a/src/PowerShellEditorServices/Workspace/ScriptFile.cs +++ b/src/PowerShellEditorServices/Workspace/ScriptFile.cs @@ -100,7 +100,7 @@ public string Contents /// Gets the list of syntax markers found by parsing this /// file's contents. /// - public List SyntaxMarkers + public List DiagnosticMarkers { get; private set; @@ -654,7 +654,7 @@ private void ParseFileContents() } // Translate parse errors into syntax markers - this.SyntaxMarkers = + this.DiagnosticMarkers = parseErrors .Select(ScriptFileMarker.FromParseError) .ToList(); diff --git a/src/PowerShellEditorServices/Workspace/ScriptFileMarker.cs b/src/PowerShellEditorServices/Workspace/ScriptFileMarker.cs index b48107bc3..e50bfb236 100644 --- a/src/PowerShellEditorServices/Workspace/ScriptFileMarker.cs +++ b/src/PowerShellEditorServices/Workspace/ScriptFileMarker.cs @@ -165,7 +165,7 @@ internal static ScriptFileMarker FromDiagnosticRecord(PSObject psObject) } string severity = diagnosticRecord.Severity.ToString(); - if(!Enum.TryParse(severity, out ScriptFileMarkerLevel level)) + if (!Enum.TryParse(severity, out ScriptFileMarkerLevel level)) { throw new ArgumentException( string.Format( From 0e05c920076bad0174ed125af5172c8a817c60f1 Mon Sep 17 00:00:00 2001 From: Tyler Leonhardt Date: Fri, 22 Mar 2019 13:29:55 -0700 Subject: [PATCH 4/5] back to empty list since we supply parse errors in PSES again --- test/PowerShellEditorServices.Test/Session/ScriptFileTests.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/PowerShellEditorServices.Test/Session/ScriptFileTests.cs b/test/PowerShellEditorServices.Test/Session/ScriptFileTests.cs index 75fc76d8a..b41376dc5 100644 --- a/test/PowerShellEditorServices.Test/Session/ScriptFileTests.cs +++ b/test/PowerShellEditorServices.Test/Session/ScriptFileTests.cs @@ -550,7 +550,7 @@ public void PropertiesInitializedCorrectlyForFile() Assert.True(scriptFile.IsAnalysisEnabled); Assert.False(scriptFile.IsInMemory); Assert.Empty(scriptFile.ReferencedFiles); - Assert.Null(scriptFile.SyntaxMarkers); + Assert.Empty(scriptFile.DiagnosticMarkers); Assert.Single(scriptFile.ScriptTokens); Assert.Single(scriptFile.FileLines); } @@ -576,7 +576,7 @@ public void PropertiesInitializedCorrectlyForUntitled() Assert.True(scriptFile.IsAnalysisEnabled); Assert.True(scriptFile.IsInMemory); Assert.Empty(scriptFile.ReferencedFiles); - Assert.Null(scriptFile.SyntaxMarkers); + Assert.Empty(scriptFile.DiagnosticMarkers); Assert.Equal(10, scriptFile.ScriptTokens.Length); Assert.Equal(3, scriptFile.FileLines.Count); } From a57fbd0230e526fb200fbd54188b7c5429cbfa85 Mon Sep 17 00:00:00 2001 From: Tyler Leonhardt Date: Fri, 22 Mar 2019 16:11:17 -0700 Subject: [PATCH 5/5] address feedback --- src/PowerShellEditorServices/Analysis/AnalysisService.cs | 9 ++------- .../Workspace/ScriptFileMarker.cs | 4 +--- 2 files changed, 3 insertions(+), 10 deletions(-) diff --git a/src/PowerShellEditorServices/Analysis/AnalysisService.cs b/src/PowerShellEditorServices/Analysis/AnalysisService.cs index 406c4193b..045a50d31 100644 --- a/src/PowerShellEditorServices/Analysis/AnalysisService.cs +++ b/src/PowerShellEditorServices/Analysis/AnalysisService.cs @@ -52,11 +52,6 @@ public class AnalysisService : IDisposable /// private static readonly PSObject[] s_emptyDiagnosticResult = new PSObject[0]; - /// - /// An empty script marker result to return when no script markers can be returned. - /// - private static readonly List s_emptyScriptMarkerResult = new List(); - private static readonly string[] s_emptyGetRuleResult = new string[0]; /// @@ -394,7 +389,7 @@ private async Task> GetSemanticMarkersAsync( else { // Return an empty marker list - return s_emptyScriptMarkerResult; + return new List(); } } @@ -412,7 +407,7 @@ private async Task> GetSemanticMarkersAsync( else { // Return an empty marker list - return s_emptyScriptMarkerResult; + return new List(); } } diff --git a/src/PowerShellEditorServices/Workspace/ScriptFileMarker.cs b/src/PowerShellEditorServices/Workspace/ScriptFileMarker.cs index e50bfb236..9eb73cb6c 100644 --- a/src/PowerShellEditorServices/Workspace/ScriptFileMarker.cs +++ b/src/PowerShellEditorServices/Workspace/ScriptFileMarker.cs @@ -168,9 +168,7 @@ internal static ScriptFileMarker FromDiagnosticRecord(PSObject psObject) if (!Enum.TryParse(severity, out ScriptFileMarkerLevel level)) { throw new ArgumentException( - string.Format( - "The provided DiagnosticSeverity value '{0}' is unknown.", - severity), + $"The provided DiagnosticSeverity value '{severity}' is unknown.", "diagnosticSeverity"); }