Add new ParseError level to ScriptFileMarkerLevel and only have it send parse errors (#888)

* Add new PSSA 1.18 ParseError Severity

* ignore parseerrors from PSSA

* address feedback

* back to empty list since we supply parse errors in PSES again

* address feedback

Author: TylerLeonhardt

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<ScriptFileMarker> analysisResults = await this.editorSession.AnalysisService.GetSemanticMarkersAsync(
                 funcText,
                 AnalysisService.GetCommentHelpRuleSettings(
                     enable: true,
@@ -1725,7 +1725,7 @@ private static async Task DelayThenInvokeDiagnosticsAsync(
                 {
                     await PublishScriptDiagnosticsAsync(
                         script,
-                        script.SyntaxMarkers,
+                        script.DiagnosticMarkers,
                         correctionIndex,
                         eventSender);
                 }
@@ -1743,7 +1743,7 @@ await PublishScriptDiagnosticsAsync(
             // Get the requested files
             foreach (ScriptFile scriptFile in filesToAnalyze)
             {
-                ScriptFileMarker[] semanticMarkers = null;
+                List<ScriptFileMarker> 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<ScriptFileMarker>();
                 }
 
+                scriptFile.DiagnosticMarkers.AddRange(semanticMarkers);
+
                 await PublishScriptDiagnosticsAsync(
                     scriptFile,
                     // Concat script analysis errors to any existing parse errors
-                    scriptFile.SyntaxMarkers.Concat(semanticMarkers).ToArray(),
+                    scriptFile.DiagnosticMarkers,
                     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<ScriptFileMarker>(),
                     this.codeActionsPerFile,
                     eventContext);
         }
 
         private static async Task PublishScriptDiagnosticsAsync(
             ScriptFile scriptFile,
-            ScriptFileMarker[] markers,
+            List<ScriptFileMarker> markers,
             Dictionary<string, Dictionary<string, MarkerCorrection>> correctionIndex,
             EventContext eventContext)
         {
@@ -1792,7 +1794,7 @@ await PublishScriptDiagnosticsAsync(
 
         private static async Task PublishScriptDiagnosticsAsync(
             ScriptFile scriptFile,
-            ScriptFileMarker[] markers,
+            List<ScriptFileMarker> markers,
             Dictionary<string, Dictionary<string, MarkerCorrection>> correctionIndex,
             Func<NotificationType<PublishDiagnosticsNotification, object>, PublishDiagnosticsNotification, Task> eventSender)
         {

src/PowerShellEditorServices/Analysis/AnalysisService.cs

@@ -52,11 +52,6 @@ public class AnalysisService : IDisposable
         /// </summary>
         private static readonly PSObject[] s_emptyDiagnosticResult = new PSObject[0];
 
-        /// <summary>
-        /// An empty script marker result to return when no script markers can be returned.
-        /// </summary>
-        private static readonly ScriptFileMarker[] s_emptyScriptMarkerResult = new ScriptFileMarker[0];
-
         private static readonly string[] s_emptyGetRuleResult = new string[0];
 
         /// <summary>
@@ -266,7 +261,7 @@ public static Hashtable GetPSSASettingsHashtable(IDictionary<string, Hashtable>
         /// </summary>
         /// <param name="file">The ScriptFile which will be analyzed for semantic markers.</param>
         /// <returns>An array of ScriptFileMarkers containing semantic analysis results.</returns>
-        public async Task<ScriptFileMarker[]> GetSemanticMarkersAsync(ScriptFile file)
+        public async Task<List<ScriptFileMarker>> GetSemanticMarkersAsync(ScriptFile file)
         {
             return await GetSemanticMarkersAsync<string>(file, ActiveRules, SettingsPath);
         }
@@ -277,7 +272,7 @@ public async Task<ScriptFileMarker[]> GetSemanticMarkersAsync(ScriptFile file)
         /// <param name="file">The ScriptFile to be analyzed.</param>
         /// <param name="settings">ScriptAnalyzer settings</param>
         /// <returns></returns>
-        public async Task<ScriptFileMarker[]> GetSemanticMarkersAsync(ScriptFile file, Hashtable settings)
+        public async Task<List<ScriptFileMarker>> GetSemanticMarkersAsync(ScriptFile file, Hashtable settings)
         {
             return await GetSemanticMarkersAsync<Hashtable>(file, null, settings);
         }
@@ -288,7 +283,7 @@ public async Task<ScriptFileMarker[]> GetSemanticMarkersAsync(ScriptFile file, H
         /// <param name="scriptContent">The script content to be analyzed.</param>
         /// <param name="settings">ScriptAnalyzer settings</param>
         /// <returns></returns>
-        public async Task<ScriptFileMarker[]> GetSemanticMarkersAsync(
+        public async Task<List<ScriptFileMarker>> GetSemanticMarkersAsync(
            string scriptContent,
            Hashtable settings)
         {
@@ -379,7 +374,7 @@ public async Task<string> FormatAsync(
 
         #region Private Methods
 
-        private async Task<ScriptFileMarker[]> GetSemanticMarkersAsync<TSettings>(
+        private async Task<List<ScriptFileMarker>> GetSemanticMarkersAsync<TSettings>(
             ScriptFile file,
             string[] rules,
             TSettings settings) where TSettings : class
@@ -394,11 +389,11 @@ private async Task<ScriptFileMarker[]> GetSemanticMarkersAsync<TSettings>(
             else
             {
                 // Return an empty marker list
-                return s_emptyScriptMarkerResult;
+                return new List<ScriptFileMarker>();
             }
         }
 
-        private async Task<ScriptFileMarker[]> GetSemanticMarkersAsync<TSettings>(
+        private async Task<List<ScriptFileMarker>> GetSemanticMarkersAsync<TSettings>(
             string scriptContent,
             string[] rules,
             TSettings settings) where TSettings : class
@@ -407,12 +402,12 @@ private async Task<ScriptFileMarker[]> GetSemanticMarkersAsync<TSettings>(
                 && (rules != null || settings != null))
             {
                 var scriptFileMarkers = await GetDiagnosticRecordsAsync(scriptContent, rules, settings);
-                return scriptFileMarkers.Select(ScriptFileMarker.FromDiagnosticRecord).ToArray();
+                return scriptFileMarkers.Select(ScriptFileMarker.FromDiagnosticRecord).ToList();
             }
             else
             {
                 // Return an empty marker list
-                return s_emptyScriptMarkerResult;
+                return new List<ScriptFileMarker>();
             }
         }
 
@@ -514,7 +509,9 @@ private async Task<PSObject[]> GetDiagnosticRecordsAsync<TSettings>(
                     new Dictionary<string, object>
                     {
                         { "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;

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.
         /// </summary>
-        public ScriptFileMarker[] SyntaxMarkers
+        public List<ScriptFileMarker> DiagnosticMarkers
         {
             get;
             private set;
@@ -654,10 +654,10 @@ private void ParseFileContents()
             }
 
             // Translate parse errors into syntax markers
-            this.SyntaxMarkers =
+            this.DiagnosticMarkers =
                 parseErrors
                     .Select(ScriptFileMarker.FromParseError)
-                    .ToArray();
+                    .ToList();
 
             // Untitled files have no directory
             // Discussed in https://github.com/PowerShell/PowerShellEditorServices/pull/815.

src/PowerShellEditorServices/Workspace/ScriptFileMarker.cs

@@ -33,20 +33,22 @@ public class MarkerCorrection
     /// </summary>
     public enum ScriptFileMarkerLevel
     {
-        /// <summary>
-        /// The marker represents an informational message.
-        /// </summary>
-        Information = 0,
-
-        /// <summary>
-        /// The marker represents a warning message.
-        /// </summary>
-        Warning,
-
-        /// <summary>
-        /// The marker represents an error message.
-        /// </summary>
-        Error
+        /// <summary>
+        /// Information: This warning is trivial, but may be useful. They are recommended by PowerShell best practice.
+        /// </summary>
+        Information = 0,
+        /// <summary>
+        /// WARNING: This warning may cause a problem or does not follow PowerShell's recommended guidelines.
+        /// </summary>
+        Warning = 1,
+        /// <summary>
+        /// ERROR: This warning is likely to cause a problem or does not follow PowerShell's required guidelines.
+        /// </summary>
+        Error = 2,
+        /// <summary>
+        /// ERROR: This diagnostic is caused by an actual parsing error, and is generated only by the engine.
+        /// </summary>
+        ParseError = 3
     };
 
     /// <summary>
@@ -62,7 +64,7 @@ public class ScriptFileMarker
         /// Gets or sets the marker's message string.
         /// </summary>
         public string Message { get; set; }
-        
+
         /// <summary>
         /// Gets or sets the ruleName associated with this marker.
         /// </summary>
@@ -162,36 +164,25 @@ internal static ScriptFileMarker FromDiagnosticRecord(PSObject psObject)
                 };
             }
 
+            string severity = diagnosticRecord.Severity.ToString();
+            if (!Enum.TryParse(severity, out ScriptFileMarkerLevel level))
+            {
+                throw new ArgumentException(
+                    $"The provided DiagnosticSeverity value '{severity}' is unknown.",
+                    "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)
-        {
-            switch (diagnosticSeverity)
-            {
-                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");
-            }
-        }
         #endregion
     }
 }

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.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.Empty(scriptFile.SyntaxMarkers);
+                Assert.Empty(scriptFile.DiagnosticMarkers);
                 Assert.Equal(10, scriptFile.ScriptTokens.Length);
                 Assert.Equal(3, scriptFile.FileLines.Count);
             }