Invoke-ScriptAnalyzer: Print summary only once per invocation, include parse errors

Also enumerate diagnostics one by one instead of collecting
and fix incorrect Pester test grouping.

Author: MatejKafka

Engine/Commands/InvokeScriptAnalyzerCommand.cs

@@ -34,7 +34,9 @@ public class InvokeScriptAnalyzerCommand : PSCmdlet, IOutputWriter
 
         #region Private variables
         List<string> processedPaths;
-        private int totalDiagnosticCount = 0;
+        // initialize to zero for all severity enum values
+        private Dictionary<DiagnosticSeverity, int> diagnosticCounts =
+                Enum.GetValues<DiagnosticSeverity>().ToDictionary(s => s, _ => 0);
         #endregion // Private variables
 
         #region Parameters
@@ -414,8 +416,35 @@ protected override void EndProcessing()
             ScriptAnalyzer.Instance.CleanUp();
             base.EndProcessing();
 
-            if (EnableExit) {
-                this.Host.SetShouldExit(totalDiagnosticCount);
+            var diagnosticCount = diagnosticCounts.Values.Sum();
+
+            if (ReportSummary.IsPresent)
+            {
+                if (diagnosticCount == 0)
+                {
+                    Host.UI.WriteLine("0 rule violations found.");
+                }
+                else
+                {
+                    var infoCount = diagnosticCounts[DiagnosticSeverity.Information];
+                    var warningCount = diagnosticCounts[DiagnosticSeverity.Warning];
+                    var errorCount = diagnosticCounts[DiagnosticSeverity.Error] + diagnosticCounts[DiagnosticSeverity.ParseError];
+                    var severeDiagnosticCount = diagnosticCount - infoCount;
+
+                    var colorPropertyPrefix = severeDiagnosticCount == 0 ? "Warning" : "Error";
+                    var pluralS = diagnosticCount > 1 ? "s" : string.Empty;
+                    ConsoleHostHelper.DisplayMessageUsingSystemProperties(
+                            Host, colorPropertyPrefix + "ForegroundColor", colorPropertyPrefix + "BackgroundColor",
+                            $"{diagnosticCount} rule violation{pluralS} found. Severity distribution: " +
+                            $"{DiagnosticSeverity.Error} = {errorCount}, " +
+                            $"{DiagnosticSeverity.Warning} = {warningCount}, " +
+                            $"{DiagnosticSeverity.Information} = {infoCount}");
+                }
+            }
+
+            if (EnableExit)
+            {
+                this.Host.SetShouldExit(diagnosticCount);
             }
         }
 
@@ -431,86 +460,47 @@ protected override void StopProcessing()
 
         private void ProcessInput()
         {
-            var diagnosticRecords = RunAnalysis();
-            WriteToOutput(diagnosticRecords);
-            totalDiagnosticCount += diagnosticRecords.Count;
-        }
-
-        private List<DiagnosticRecord> RunAnalysis()
-        {
-            if (!IsFileParameterSet())
+            foreach (var diagnostic in RunAnalysis())
             {
-                return ScriptAnalyzer.Instance.AnalyzeScriptDefinition(scriptDefinition, out _, out _);
-            }
+                diagnosticCounts[diagnostic.Severity]++;
 
-            var diagnostics = new List<DiagnosticRecord>();
-            foreach (string path in this.processedPaths)
-            {
-                if (fix)
+                foreach (var logger in ScriptAnalyzer.Instance.Loggers)
                 {
-                    ShouldProcess(path, $"Analyzing and fixing path with Recurse={this.recurse}");
-                    diagnostics.AddRange(ScriptAnalyzer.Instance.AnalyzeAndFixPath(path, this.ShouldProcess, this.recurse));
-                }
-                else
-                {
-                    ShouldProcess(path, $"Analyzing path with Recurse={this.recurse}");
-                    diagnostics.AddRange(ScriptAnalyzer.Instance.AnalyzePath(path, this.ShouldProcess, this.recurse));
+                    logger.LogObject(diagnostic, this);
                 }
             }
-
-            return diagnostics;
         }
 
-        private void WriteToOutput(List<DiagnosticRecord> diagnosticRecords)
+        private IEnumerable<DiagnosticRecord> RunAnalysis()
         {
-            foreach (ILogger logger in ScriptAnalyzer.Instance.Loggers)
+            if (!IsFileParameterSet())
             {
-                var errorCount = 0;
-                var warningCount = 0;
-                var infoCount = 0;
-                var parseErrorCount = 0;
+                foreach (var record in ScriptAnalyzer.Instance.AnalyzeScriptDefinition(scriptDefinition, out _, out _))
+                {
+                    yield return record;
+                }
+                yield break;
+            }
 
-                foreach (DiagnosticRecord diagnostic in diagnosticRecords)
+            foreach (var path in this.processedPaths)
+            {
+                if (!ShouldProcess(path, $"Analyzing path with Fix={this.fix} and Recurse={this.recurse}"))
                 {
-                    logger.LogObject(diagnostic, this);
-                    switch (diagnostic.Severity)
-                    {
-                        case DiagnosticSeverity.Information:
-                            infoCount++;
-                            break;
-                        case DiagnosticSeverity.Warning:
-                            warningCount++;
-                            break;
-                        case DiagnosticSeverity.Error:
-                            errorCount++;
-                            break;
-                        case DiagnosticSeverity.ParseError:
-                            parseErrorCount++;
-                            break;
-                        default:
-                            throw new ArgumentOutOfRangeException(nameof(diagnostic.Severity), $"Severity '{diagnostic.Severity}' is unknown");
-                    }
+                    continue;
                 }
 
-                if (ReportSummary.IsPresent)
+                if (fix)
                 {
-                    var numberOfRuleViolations = infoCount + warningCount + errorCount;
-                    if (numberOfRuleViolations == 0)
+                    foreach (var record in ScriptAnalyzer.Instance.AnalyzeAndFixPath(path, this.ShouldProcess, this.recurse))
                     {
-                        Host.UI.WriteLine("0 rule violations found.");
+                        yield return record;
                     }
-                    else
+                }
+                else
+                {
+                    foreach (var record in ScriptAnalyzer.Instance.AnalyzePath(path, this.ShouldProcess, this.recurse))
                     {
-                        var pluralS = numberOfRuleViolations > 1 ? "s" : string.Empty;
-                        var message = $"{numberOfRuleViolations} rule violation{pluralS} found.    Severity distribution:  {DiagnosticSeverity.Error} = {errorCount}, {DiagnosticSeverity.Warning} = {warningCount}, {DiagnosticSeverity.Information} = {infoCount}";
-                        if (warningCount + errorCount == 0)
-                        {
-                            ConsoleHostHelper.DisplayMessageUsingSystemProperties(Host, "WarningForegroundColor", "WarningBackgroundColor", message);
-                        }
-                        else
-                        {
-                            ConsoleHostHelper.DisplayMessageUsingSystemProperties(Host, "ErrorForegroundColor", "ErrorBackgroundColor", message);
-                        }
+                        yield return record;
                     }
                 }
             }

Tests/Engine/InvokeScriptAnalyzer.tests.ps1

@@ -585,65 +585,65 @@ Describe "Test -EnableExit Switch" {
 
         $LASTEXITCODE | Should -Be 2
     }
+}
 
-    Describe "-ReportSummary switch" {
-        BeforeAll {
-            $pssaPath = (Get-Module PSScriptAnalyzer).Path
-
-            if ($IsCoreCLR)
-            {
-                $pwshExe = (Get-Process -Id $PID).Path
-            }
-            else
-            {
-                $pwshExe = 'powershell'
-            }
+Describe "-ReportSummary switch" {
+    BeforeAll {
+        $pssaPath = (Get-Module PSScriptAnalyzer).Path
 
-            $reportSummaryFor1Warning = '*1 rule violation found.    Severity distribution:  Error = 0, Warning = 1, Information = 0*'
+        if ($IsCoreCLR)
+        {
+            $pwshExe = (Get-Process -Id $PID).Path
+        }
+        else
+        {
+            $pwshExe = 'powershell'
         }
 
-        It "prints the correct report summary using the -NoReportSummary switch" {
-            $result = & $pwshExe -Command "Import-Module '$pssaPath'; Invoke-ScriptAnalyzer -ScriptDefinition gci -ReportSummary"
+        $reportSummaryFor1Warning = '*1 rule violation found. Severity distribution: Error = 0, Warning = 1, Information = 0*'
+    }
 
-            "$result" | Should -BeLike $reportSummaryFor1Warning
-        }
-        It "does not print the report summary when not using -NoReportSummary switch" {
-            $result = & $pwshExe -Command "Import-Module '$pssaPath'; Invoke-ScriptAnalyzer -ScriptDefinition gci"
+    It "prints the correct report summary using the -NoReportSummary switch" {
+        $result = & $pwshExe -Command "Import-Module '$pssaPath'; Invoke-ScriptAnalyzer -ScriptDefinition gci -ReportSummary"
 
-            "$result" | Should -Not -BeLike $reportSummaryFor1Warning
-        }
+        "$result" | Should -BeLike $reportSummaryFor1Warning
     }
+    It "does not print the report summary when not using -NoReportSummary switch" {
+        $result = & $pwshExe -Command "Import-Module '$pssaPath'; Invoke-ScriptAnalyzer -ScriptDefinition gci"
 
-    # using statements are only supported in v5+
-    Describe "Handles parse errors due to unknown types" -Skip:($testingLibraryUsage -or ($PSVersionTable.PSVersion -lt '5.0')) {
-        BeforeAll {
-            $script = @'
-                using namespace Microsoft.Azure.Commands.ResourceManager.Cmdlets.SdkModels
-                using namespace Microsoft.Azure.Commands.Common.Authentication.Abstractions
-                Import-Module "AzureRm"
-                class MyClass { [IStorageContext]$StorageContext } # This will result in a parser error due to [IStorageContext] type that comes from the using statement but is not known at parse time
+        "$result" | Should -Not -BeLike $reportSummaryFor1Warning
+    }
+}
+
+# using statements are only supported in v5+
+Describe "Handles parse errors due to unknown types" -Skip:($testingLibraryUsage -or ($PSVersionTable.PSVersion -lt '5.0')) {
+    BeforeAll {
+        $script = @'
+            using namespace Microsoft.Azure.Commands.ResourceManager.Cmdlets.SdkModels
+            using namespace Microsoft.Azure.Commands.Common.Authentication.Abstractions
+            Import-Module "AzureRm"
+            class MyClass { [IStorageContext]$StorageContext } # This will result in a parser error due to [IStorageContext] type that comes from the using statement but is not known at parse time
 '@
-        }
-        It "does not throw and detect one expected warning after the parse error has occured when using -ScriptDefintion parameter set" {
-            $warnings = Invoke-ScriptAnalyzer -ScriptDefinition $script
-            $warnings.Count | Should -Be 1
-            $warnings.RuleName | Should -Be 'TypeNotFound'
-        }
+    }
+    It "does not throw and detect one expected warning after the parse error has occured when using -ScriptDefintion parameter set" {
+        $warnings = Invoke-ScriptAnalyzer -ScriptDefinition $script
+        $warnings.Count | Should -Be 1
+        $warnings.RuleName | Should -Be 'TypeNotFound'
+    }
 
-        It "does not throw and detect one expected warning after the parse error has occured when using -Path parameter set" {
-            $testFilePath = "TestDrive:\testfile.ps1"
-            Set-Content $testFilePath -Value $script
-            $warnings = Invoke-ScriptAnalyzer -Path $testFilePath
-            $warnings.Count | Should -Be 1
-            $warnings.RuleName | Should -Be 'TypeNotFound'
-        }
+    It "does not throw and detect one expected warning after the parse error has occured when using -Path parameter set" {
+        $testFilePath = "TestDrive:\testfile.ps1"
+        Set-Content $testFilePath -Value $script
+        $warnings = Invoke-ScriptAnalyzer -Path $testFilePath
+        $warnings.Count | Should -Be 1
+        $warnings.RuleName | Should -Be 'TypeNotFound'
     }
+}
 
-    Describe 'Handles static Singleton (issue 1182)' -Skip:($testingLibraryUsage -or ($PSVersionTable.PSVersion -lt '5.0')) {
-        It 'Does not throw or return diagnostic record' {
-            $scriptDefinition = 'class T { static [T]$i }; function foo { [CmdletBinding()] param () $script:T.WriteLog() }'
-            Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -ErrorAction Stop | Should -BeNullOrEmpty
-        }
+Describe 'Handles static Singleton (issue 1182)' -Skip:($testingLibraryUsage -or ($PSVersionTable.PSVersion -lt '5.0')) {
+    It 'Does not throw or return diagnostic record' {
+        $scriptDefinition = 'class T { static [T]$i }; function foo { [CmdletBinding()] param () $script:T.WriteLog() }'
+        Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -ErrorAction Stop | Should -BeNullOrEmpty
     }
 }