From 18c43e6b955a55ba208ceaca4448895eda922e70 Mon Sep 17 00:00:00 2001 From: James Truher Date: Thu, 17 Jan 2019 15:50:34 -0800 Subject: [PATCH 01/10] update analyzer to create diagnostic records for parser errors, rather than emitting error records Embedded tools such as VSCode have real difficulty when there are parser errors. We need to be sure to just return what we find and let the down stream consumers filter. --- .../Commands/InvokeScriptAnalyzerCommand.cs | 6 +- Engine/Generic/DiagnosticRecord.cs | 5 + Engine/ScriptAnalyzer.cs | 114 +++++++++--------- Tests/Engine/InvokeScriptAnalyzer.tests.ps1 | 16 ++- .../UseUTF8EncodingForHelpFile.tests.ps1 | 16 +-- 5 files changed, 89 insertions(+), 68 deletions(-) diff --git a/Engine/Commands/InvokeScriptAnalyzerCommand.cs b/Engine/Commands/InvokeScriptAnalyzerCommand.cs index e38baac92..c51fc9f38 100644 --- a/Engine/Commands/InvokeScriptAnalyzerCommand.cs +++ b/Engine/Commands/InvokeScriptAnalyzerCommand.cs @@ -133,7 +133,7 @@ public string[] IncludeRule /// /// IncludeRule: Array of the severity types to be enabled. /// - [ValidateSet("Warning", "Error", "Information", IgnoreCase = true)] + [ValidateSet("Warning", "Error", "Information", "ParseError", IgnoreCase = true)] [Parameter(Mandatory = false)] [SuppressMessage("Microsoft.Performance", "CA1819:PropertiesShouldNotReturnArrays")] public string[] Severity @@ -432,6 +432,7 @@ private void WriteToOutput(IEnumerable diagnosticRecords) var errorCount = 0; var warningCount = 0; var infoCount = 0; + var parseErrorCount = 0; foreach (DiagnosticRecord diagnostic in diagnosticRecords) { @@ -447,6 +448,9 @@ private void WriteToOutput(IEnumerable diagnosticRecords) case DiagnosticSeverity.Error: errorCount++; break; + case DiagnosticSeverity.ParseError: + parseErrorCount++; + break; default: throw new ArgumentOutOfRangeException(nameof(diagnostic.Severity), $"Severity '{diagnostic.Severity}' is unknown"); } diff --git a/Engine/Generic/DiagnosticRecord.cs b/Engine/Generic/DiagnosticRecord.cs index 28d0e87bd..a02f8a273 100644 --- a/Engine/Generic/DiagnosticRecord.cs +++ b/Engine/Generic/DiagnosticRecord.cs @@ -142,5 +142,10 @@ public enum DiagnosticSeverity : uint /// ERROR: This diagnostic 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, }; } diff --git a/Engine/ScriptAnalyzer.cs b/Engine/ScriptAnalyzer.cs index 01b4ef3cd..e5adbf1c3 100644 --- a/Engine/ScriptAnalyzer.cs +++ b/Engine/ScriptAnalyzer.cs @@ -1526,23 +1526,26 @@ public IEnumerable AnalyzeScriptDefinition(string scriptDefini var relevantParseErrors = RemoveTypeNotFoundParseErrors(errors, out List diagnosticRecords); - if (relevantParseErrors != null && relevantParseErrors.Count > 0) + // Add parse errors first! + if ( relevantParseErrors != null ) { - foreach (var parseError in relevantParseErrors) + List results = new List(); + foreach ( var parseError in relevantParseErrors ) { string parseErrorMessage = String.Format(CultureInfo.CurrentCulture, Strings.ParseErrorFormatForScriptDefinition, parseError.Message.TrimEnd('.'), parseError.Extent.StartLineNumber, parseError.Extent.StartColumnNumber); - this.outputWriter.WriteError(new ErrorRecord(new ParseException(parseErrorMessage), parseErrorMessage, ErrorCategory.ParserError, parseError.ErrorId)); + results.Add(new DiagnosticRecord( + parseError.Message, + parseError.Extent, + parseError.ErrorId.ToString(), + DiagnosticSeverity.ParseError, + "" // no script file + ) + ); } + diagnosticRecords.AddRange(results); } - if (relevantParseErrors != null && relevantParseErrors.Count > 10) - { - string manyParseErrorMessage = String.Format(CultureInfo.CurrentCulture, Strings.ParserErrorMessageForScriptDefinition); - this.outputWriter.WriteError(new ErrorRecord(new ParseException(manyParseErrorMessage), manyParseErrorMessage, ErrorCategory.ParserError, scriptDefinition)); - - return new List(); - } - + // now, analyze the script definition return diagnosticRecords.Concat(this.AnalyzeSyntaxTree(scriptAst, scriptTokens, String.Empty)); } @@ -1839,49 +1842,8 @@ private IEnumerable AnalyzeFile(string filePath) this.outputWriter.WriteVerbose(string.Format(CultureInfo.CurrentCulture, Strings.VerboseFileMessage, filePath)); var diagnosticRecords = new List(); - //Parse the file - if (File.Exists(filePath)) - { - // processing for non help script - if (!(Path.GetFileName(filePath).ToLower().StartsWith("about_") && Path.GetFileName(filePath).ToLower().EndsWith(".help.txt"))) - { - try - { - scriptAst = Parser.ParseFile(filePath, out scriptTokens, out errors); - } - catch (Exception e) - { - this.outputWriter.WriteWarning(e.ToString()); - return null; - } -#if !PSV3 - //try parsing again - if (TrySaveModules(errors, scriptAst)) - { - scriptAst = Parser.ParseFile(filePath, out scriptTokens, out errors); - } -#endif //!PSV3 - var relevantParseErrors = RemoveTypeNotFoundParseErrors(errors, out diagnosticRecords); - - //Runspace.DefaultRunspace = oldDefault; - if (relevantParseErrors != null && relevantParseErrors.Count > 0) - { - foreach (var parseError in relevantParseErrors) - { - string parseErrorMessage = String.Format(CultureInfo.CurrentCulture, Strings.ParserErrorFormat, parseError.Extent.File, parseError.Message.TrimEnd('.'), parseError.Extent.StartLineNumber, parseError.Extent.StartColumnNumber); - this.outputWriter.WriteError(new ErrorRecord(new ParseException(parseErrorMessage), parseErrorMessage, ErrorCategory.ParserError, parseError.ErrorId)); - } - } - - if (relevantParseErrors != null && relevantParseErrors.Count > 10) - { - string manyParseErrorMessage = String.Format(CultureInfo.CurrentCulture, Strings.ParserErrorMessage, System.IO.Path.GetFileName(filePath)); - this.outputWriter.WriteError(new ErrorRecord(new ParseException(manyParseErrorMessage), manyParseErrorMessage, ErrorCategory.ParserError, filePath)); - return new List(); - } - } - } - else + // If the file doesn't exist, return + if (! File.Exists(filePath)) { this.outputWriter.ThrowTerminatingError(new ErrorRecord(new FileNotFoundException(), string.Format(CultureInfo.CurrentCulture, Strings.InvalidPath, filePath), @@ -1890,6 +1852,50 @@ private IEnumerable AnalyzeFile(string filePath) return null; } + // short-circuited processing for a help file + // no parsing can really be done, but there are other rules to run (specifically encoding). + if ( Regex.Matches(Path.GetFileName(filePath), @"^about_.*help.txt$", RegexOptions.IgnoreCase).Count != 0) + { + return diagnosticRecords.Concat(this.AnalyzeSyntaxTree(scriptAst, scriptTokens, filePath)); + } + + // Process script + try + { + scriptAst = Parser.ParseFile(filePath, out scriptTokens, out errors); + } + catch (Exception e) + { + this.outputWriter.WriteWarning(e.ToString()); + return null; + } +#if !PSV3 + //try parsing again + if (TrySaveModules(errors, scriptAst)) + { + scriptAst = Parser.ParseFile(filePath, out scriptTokens, out errors); + } +#endif //!PSV3 + var relevantParseErrors = RemoveTypeNotFoundParseErrors(errors, out diagnosticRecords); + + // First, add all parse errors + if ( relevantParseErrors != null ) + { + List results = new List(); + foreach ( var parseError in relevantParseErrors ) + { + string parseErrorMessage = String.Format(CultureInfo.CurrentCulture, Strings.ParseErrorFormatForScriptDefinition, parseError.Message.TrimEnd('.'), parseError.Extent.StartLineNumber, parseError.Extent.StartColumnNumber); + results.Add(new DiagnosticRecord( + parseError.Message, + parseError.Extent, + parseError.ErrorId.ToString(), + DiagnosticSeverity.ParseError, + filePath) + ); + } + diagnosticRecords.AddRange(results); + } + return diagnosticRecords.Concat(this.AnalyzeSyntaxTree(scriptAst, scriptTokens, filePath)); } diff --git a/Tests/Engine/InvokeScriptAnalyzer.tests.ps1 b/Tests/Engine/InvokeScriptAnalyzer.tests.ps1 index c43cac665..e44669430 100644 --- a/Tests/Engine/InvokeScriptAnalyzer.tests.ps1 +++ b/Tests/Engine/InvokeScriptAnalyzer.tests.ps1 @@ -109,9 +109,11 @@ Describe "Test available parameters" { Describe "Test ScriptDefinition" { Context "When given a script definition" { - It "Does not run rules on script with more than 10 parser errors" { - $moreThanTenErrors = Invoke-ScriptAnalyzer -ErrorAction SilentlyContinue -ScriptDefinition (Get-Content -Raw "$directory\CSharp.ps1") - $moreThanTenErrors.Count | Should -Be 0 + It "Runs rules on script with more than 10 parser errors" { + # this is a script with 12 parse errors + $script = ');' * 12 + $moreThanTenErrors = Invoke-ScriptAnalyzer -ScriptDefinition $script + $moreThanTenErrors.Count | Should -Be 12 } } } @@ -124,9 +126,11 @@ Describe "Test Path" { $withPath.Count -eq $withoutPath.Count | Should -BeTrue } - It "Does not run rules on script with more than 10 parser errors" { - $moreThanTenErrors = Invoke-ScriptAnalyzer -ErrorAction SilentlyContinue $directory\CSharp.ps1 - $moreThanTenErrors.Count | Should -Be 0 + It "Runs rules on script with more than 10 parser errors" { + # this is a script with 12 parse errors + 1..12 | Foreach-Object { ')' } | Out-File "TestDrive:\badfile.ps1" + $moreThanTenErrors = Invoke-ScriptAnalyzer -ErrorAction SilentlyContinue "TestDrive:\badfile.ps1" + $moreThanTenErrors.Count | Should -Be 12 } } diff --git a/Tests/Rules/UseUTF8EncodingForHelpFile.tests.ps1 b/Tests/Rules/UseUTF8EncodingForHelpFile.tests.ps1 index 352702415..1f99549d5 100644 --- a/Tests/Rules/UseUTF8EncodingForHelpFile.tests.ps1 +++ b/Tests/Rules/UseUTF8EncodingForHelpFile.tests.ps1 @@ -1,11 +1,13 @@ -$violationMessage = "File about_utf16.help.txt has to use UTF8 instead of System.Text.UTF32Encoding encoding because it is a powershell help file." -$violationName = "PSUseUTF8EncodingForHelpFile" -$directory = Split-Path -Parent $MyInvocation.MyCommand.Path -$violations = Invoke-ScriptAnalyzer $directory\about_utf16.help.txt | Where-Object {$_.RuleName -eq $violationName} -$noViolations = Invoke-ScriptAnalyzer $directory\about_utf8.help.txt | Where-Object {$_.RuleName -eq $violationName} -$notHelpFileViolations = Invoke-ScriptAnalyzer $directory\utf16.txt | Where-Object {$_.RuleName -eq $violationName} - +$directory = Split-Path -Parent $MyInvocation.MyCommand.Path Describe "UseUTF8EncodingForHelpFile" { + BeforeAll { + $violationMessage = "File about_utf16.help.txt has to use UTF8 instead of System.Text.UTF32Encoding encoding because it is a powershell help file." + $violationName = "PSUseUTF8EncodingForHelpFile" + $violations = Invoke-ScriptAnalyzer $directory\about_utf16.help.txt | Where-Object {$_.RuleName -eq $violationName} + $noViolations = Invoke-ScriptAnalyzer $directory\about_utf8.help.txt | Where-Object {$_.RuleName -eq $violationName} + $notHelpFileViolations = Invoke-ScriptAnalyzer $directory\utf16.txt | Where-Object {$_.RuleName -eq $violationName} + } + Context "When there are violations" { It "has 1 avoid use utf8 encoding violation" { $violations.Count | Should -Be 1 From 36061f5bea7d9da9c0644ce6ebb11a981afa489a Mon Sep 17 00:00:00 2001 From: James Truher Date: Thu, 17 Jan 2019 15:51:16 -0800 Subject: [PATCH 02/10] update the output in the case that we don't get a build. I've noticed it's sometimes hard to determine went wrong in the build, this may help a bit --- build.psm1 | 1 + 1 file changed, 1 insertion(+) diff --git a/build.psm1 b/build.psm1 index f9f216554..d283eef00 100644 --- a/build.psm1 +++ b/build.psm1 @@ -197,6 +197,7 @@ function Start-ScriptAnalyzerBuild if ( $LASTEXITCODE -ne 0 ) { throw "$buildOutput" } } catch { + Write-Warning $_ Write-Error "Failure to build for PSVersion '$PSVersion' using framework '$framework' and configuration '$config'" return } From edaca8e80d1e1f5cf7dd05575f3eabd6f15650bb Mon Sep 17 00:00:00 2001 From: James Truher Date: Thu, 31 Jan 2019 13:13:53 -0800 Subject: [PATCH 03/10] Fix test failures Some tests are checking ErrorVariable for parse errors which are now part of the output stream --- Tests/Engine/InvokeScriptAnalyzer.tests.ps1 | 7 ++++--- Tests/Engine/ModuleDependencyHandler.tests.ps1 | 17 ++++++++++++----- Tests/Rules/AlignAssignmentStatement.tests.ps1 | 1 + 3 files changed, 17 insertions(+), 8 deletions(-) diff --git a/Tests/Engine/InvokeScriptAnalyzer.tests.ps1 b/Tests/Engine/InvokeScriptAnalyzer.tests.ps1 index e44669430..0f52ab941 100644 --- a/Tests/Engine/InvokeScriptAnalyzer.tests.ps1 +++ b/Tests/Engine/InvokeScriptAnalyzer.tests.ps1 @@ -121,9 +121,10 @@ Describe "Test ScriptDefinition" { Describe "Test Path" { Context "When given a single file" { It "Has the same effect as without Path parameter" { - $withPath = Invoke-ScriptAnalyzer $directory\TestScript.ps1 - $withoutPath = Invoke-ScriptAnalyzer -Path $directory\TestScript.ps1 - $withPath.Count -eq $withoutPath.Count | Should -BeTrue + $scriptPath = Join-Path $directory "TestScript.ps1" + $withPath = Invoke-ScriptAnalyzer $scriptPath + $withoutPath = Invoke-ScriptAnalyzer -Path $scriptPath + $withPath.Count | Should -Be $withoutPath.Count } It "Runs rules on script with more than 10 parser errors" { diff --git a/Tests/Engine/ModuleDependencyHandler.tests.ps1 b/Tests/Engine/ModuleDependencyHandler.tests.ps1 index 0b00c9147..c3393ced9 100644 --- a/Tests/Engine/ModuleDependencyHandler.tests.ps1 +++ b/Tests/Engine/ModuleDependencyHandler.tests.ps1 @@ -139,8 +139,12 @@ Describe "Resolve DSC Resource Dependency" { Context "Invoke-ScriptAnalyzer without switch" { It "Has parse errors" -skip:$skipTest { - $dr = Invoke-ScriptAnalyzer -Path $violationFilePath -ErrorVariable parseErrors -ErrorAction SilentlyContinue - $parseErrors.Count | Should -Be 1 + $dr = Invoke-ScriptAnalyzer -Path $violationFilePath -ErrorVariable analyzerErrors -ErrorAction SilentlyContinue + $analyzerErrors.Count | Should -Be 0 + + $dr | + Where-Object { $_.Severity -eq "ParseError" } | + Get-Count | Should -Be 1 } } @@ -182,10 +186,13 @@ Describe "Resolve DSC Resource Dependency" { Copy-Item -Recurse -Path $modulePath -Destination $tempModulePath } - It "Doesn't have parse errors" -skip:$skipTest { + It "has a single parse error" -skip:$skipTest { # invoke script analyzer - $dr = Invoke-ScriptAnalyzer -Path $violationFilePath -ErrorVariable parseErrors -ErrorAction SilentlyContinue - $dr.Count | Should -Be 0 + $dr = Invoke-ScriptAnalyzer -Path $violationFilePath -ErrorVariable analyzerErrors -ErrorAction SilentlyContinue + $analyzerErrors.Count | Should -Be 0 + $dr | + Where-Object { $_.Severity -eq "ParseError" } | + Get-Count | Should -Be 1 } It "Keeps PSModulePath unchanged before and after invocation" -skip:$skipTest { diff --git a/Tests/Rules/AlignAssignmentStatement.tests.ps1 b/Tests/Rules/AlignAssignmentStatement.tests.ps1 index 29a39eeee..c8e7081e8 100644 --- a/Tests/Rules/AlignAssignmentStatement.tests.ps1 +++ b/Tests/Rules/AlignAssignmentStatement.tests.ps1 @@ -127,6 +127,7 @@ Configuration Sample_ChangeDescriptionAndPermissions # SilentlyContinue Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings -ErrorAction SilentlyContinue | Get-Count | + Where-Object { $_.Severity -ne "ParseError" } | Should -Be 4 } } From 37c095caee18a8c300f503c043d48bbee370f3db Mon Sep 17 00:00:00 2001 From: James Truher Date: Thu, 31 Jan 2019 17:13:25 -0800 Subject: [PATCH 04/10] Make sure that you can suppress ParseErrors All findings are returned by default, but if you exclude ParseErrors in -Severity we won't emit them. --- Engine/Generic/RuleSeverity.cs | 5 +++++ Engine/ScriptAnalyzer.cs | 10 ++++++---- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/Engine/Generic/RuleSeverity.cs b/Engine/Generic/RuleSeverity.cs index 761d764d2..b579c286d 100644 --- a/Engine/Generic/RuleSeverity.cs +++ b/Engine/Generic/RuleSeverity.cs @@ -22,5 +22,10 @@ public enum RuleSeverity : uint /// 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, }; } diff --git a/Engine/ScriptAnalyzer.cs b/Engine/ScriptAnalyzer.cs index e5adbf1c3..480d1f247 100644 --- a/Engine/ScriptAnalyzer.cs +++ b/Engine/ScriptAnalyzer.cs @@ -1526,8 +1526,9 @@ public IEnumerable AnalyzeScriptDefinition(string scriptDefini var relevantParseErrors = RemoveTypeNotFoundParseErrors(errors, out List diagnosticRecords); - // Add parse errors first! - if ( relevantParseErrors != null ) + int emitParseErrors = severity == null ? 1 : severity.Count(item => item == "ParseError"); + // Add parse errors first if requested! + if ( relevantParseErrors != null && emitParseErrors == 1) { List results = new List(); foreach ( var parseError in relevantParseErrors ) @@ -1878,8 +1879,9 @@ private IEnumerable AnalyzeFile(string filePath) #endif //!PSV3 var relevantParseErrors = RemoveTypeNotFoundParseErrors(errors, out diagnosticRecords); - // First, add all parse errors - if ( relevantParseErrors != null ) + // First, add all parse errors if they've been requested + int emitParseErrors = severity == null ? 1 : severity.Count(item => item == "ParseError"); + if ( relevantParseErrors != null && emitParseErrors == 1 ) { List results = new List(); foreach ( var parseError in relevantParseErrors ) From 4c347a10dd1675258f25da52d0d4e2571d6881d6 Mon Sep 17 00:00:00 2001 From: James Truher Date: Wed, 6 Feb 2019 14:56:05 -0800 Subject: [PATCH 05/10] Fix ordering typo, remove parse errors before counting --- Tests/Rules/AlignAssignmentStatement.tests.ps1 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tests/Rules/AlignAssignmentStatement.tests.ps1 b/Tests/Rules/AlignAssignmentStatement.tests.ps1 index c8e7081e8..72074f91c 100644 --- a/Tests/Rules/AlignAssignmentStatement.tests.ps1 +++ b/Tests/Rules/AlignAssignmentStatement.tests.ps1 @@ -126,8 +126,8 @@ Configuration Sample_ChangeDescriptionAndPermissions # NonExistentModule is not really avaiable to load. Therefore we set erroraction to # SilentlyContinue Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings -ErrorAction SilentlyContinue | - Get-Count | Where-Object { $_.Severity -ne "ParseError" } | + Get-Count | Should -Be 4 } } From b3ef54a9d0a130806d046147df7b3cb11df36ebc Mon Sep 17 00:00:00 2001 From: James Truher Date: Wed, 6 Feb 2019 16:51:07 -0800 Subject: [PATCH 06/10] Change invocation to not ignore errors Force array of output --- Tests/Engine/InvokeScriptAnalyzer.tests.ps1 | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Tests/Engine/InvokeScriptAnalyzer.tests.ps1 b/Tests/Engine/InvokeScriptAnalyzer.tests.ps1 index 0f52ab941..47031a5a1 100644 --- a/Tests/Engine/InvokeScriptAnalyzer.tests.ps1 +++ b/Tests/Engine/InvokeScriptAnalyzer.tests.ps1 @@ -129,9 +129,9 @@ Describe "Test Path" { It "Runs rules on script with more than 10 parser errors" { # this is a script with 12 parse errors - 1..12 | Foreach-Object { ')' } | Out-File "TestDrive:\badfile.ps1" - $moreThanTenErrors = Invoke-ScriptAnalyzer -ErrorAction SilentlyContinue "TestDrive:\badfile.ps1" - $moreThanTenErrors.Count | Should -Be 12 + 1..12 | Foreach-Object { ');' } | Out-File -Encoding ASCII "TestDrive:\badfile.ps1" + $moreThanTenErrors = Invoke-ScriptAnalyzer -Path "TestDrive:\badfile.ps1" + @($moreThanTenErrors).Count | Should -Be 12 } } From 8443abf17ef094a03cbc18b7f72e702032e5cd33 Mon Sep 17 00:00:00 2001 From: James Truher Date: Wed, 6 Feb 2019 17:51:00 -0800 Subject: [PATCH 07/10] Add test to its own context block --- Tests/Engine/InvokeScriptAnalyzer.tests.ps1 | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/Tests/Engine/InvokeScriptAnalyzer.tests.ps1 b/Tests/Engine/InvokeScriptAnalyzer.tests.ps1 index 47031a5a1..4faa17b2a 100644 --- a/Tests/Engine/InvokeScriptAnalyzer.tests.ps1 +++ b/Tests/Engine/InvokeScriptAnalyzer.tests.ps1 @@ -126,11 +126,13 @@ Describe "Test Path" { $withoutPath = Invoke-ScriptAnalyzer -Path $scriptPath $withPath.Count | Should -Be $withoutPath.Count } + } - It "Runs rules on script with more than 10 parser errors" { + Context "When there are more than 10 errors in a file" { + It "All errors are found in a file" { # this is a script with 12 parse errors - 1..12 | Foreach-Object { ');' } | Out-File -Encoding ASCII "TestDrive:\badfile.ps1" - $moreThanTenErrors = Invoke-ScriptAnalyzer -Path "TestDrive:\badfile.ps1" + 1..12 | Foreach-Object { ');' } | Out-File -Encoding ASCII "${TestDrive}\badfile.ps1" + $moreThanTenErrors = Invoke-ScriptAnalyzer -Path "${TestDrive}\badfile.ps1" @($moreThanTenErrors).Count | Should -Be 12 } } From f6f3309a48afc332e536b1117b6fb60e999d6fcf Mon Sep 17 00:00:00 2001 From: James Truher Date: Thu, 7 Feb 2019 11:43:55 -0800 Subject: [PATCH 08/10] Add tests for permutations of Severity which includes the new ParseError severity --- Tests/Engine/InvokeScriptAnalyzer.tests.ps1 | 26 +++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/Tests/Engine/InvokeScriptAnalyzer.tests.ps1 b/Tests/Engine/InvokeScriptAnalyzer.tests.ps1 index 4faa17b2a..788a30c6e 100644 --- a/Tests/Engine/InvokeScriptAnalyzer.tests.ps1 +++ b/Tests/Engine/InvokeScriptAnalyzer.tests.ps1 @@ -313,6 +313,32 @@ Describe "Test Exclude And Include" {1 } Describe "Test Severity" { + Context "Each severity can be chosen in any combination" { + BeforeAll { + $Severities = "ParseError","Error","Warning","Information" + # end space is important + $script = '$a=;ConvertTo-SecureString -Force -AsPlainText "bad practice" ' + $testcases = @{ Severity = "ParseError" }, @{ Severity = "Error" }, + @{ Severity = "Warning" }, @{ Severity = "Information" }, + @{ Severity = "ParseError", "Error" }, @{ Severity = "ParseError","Information" }, + @{ Severity = "Information", "Warning", "Error" } + } + + It "Can retrieve specific severity " -testcase $testcases { + param ( $severity ) + $result = Invoke-ScriptAnalyzer -ScriptDefinition $script -Severity $severity + if ( $severity -is [array] ) { + @($result).Count | Should -Be @($severity).Count + foreach ( $sev in $severity ) { + $result.Severity | Should -Contain $sev + } + } + else { + $result.Severity | Should -Be $severity + } + } + } + Context "When used correctly" { It "works with one argument" { $errors = Invoke-ScriptAnalyzer $directory\TestScript.ps1 -Severity Information From d34b8cb519e11944500bcffd0cbed2b1ffbeeeee Mon Sep 17 00:00:00 2001 From: James Truher Date: Thu, 7 Feb 2019 12:18:21 -0800 Subject: [PATCH 09/10] Add 'ParseError' to severity parameter for proxy cmdlet --- Tests/Engine/LibraryUsage.tests.ps1 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tests/Engine/LibraryUsage.tests.ps1 b/Tests/Engine/LibraryUsage.tests.ps1 index 47930561f..dc0c88daa 100644 --- a/Tests/Engine/LibraryUsage.tests.ps1 +++ b/Tests/Engine/LibraryUsage.tests.ps1 @@ -36,7 +36,7 @@ function Invoke-ScriptAnalyzer { [Parameter(Mandatory = $false)] [string[]] $IncludeRule = $null, - [ValidateSet("Warning", "Error", "Information", IgnoreCase = $true)] + [ValidateSet("Warning", "Error", "Information", "ParseError", IgnoreCase = $true)] [Parameter(Mandatory = $false)] [string[]] $Severity = $null, From 03748517444e1badaf5e124c0e23ae8523a35519 Mon Sep 17 00:00:00 2001 From: James Truher Date: Fri, 8 Feb 2019 12:51:16 -0800 Subject: [PATCH 10/10] Change check for help file to use static regex Update documentation to include new behavior for ParseErrors and fix up logic to be a bit cleaner when emiting ParseErrors --- Engine/ScriptAnalyzer.cs | 19 +++++++++---------- README.md | 39 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 10 deletions(-) diff --git a/Engine/ScriptAnalyzer.cs b/Engine/ScriptAnalyzer.cs index 480d1f247..5b5ccd890 100644 --- a/Engine/ScriptAnalyzer.cs +++ b/Engine/ScriptAnalyzer.cs @@ -31,6 +31,7 @@ public sealed class ScriptAnalyzer private IOutputWriter outputWriter; private Dictionary settings; + private readonly Regex s_aboutHelpRegex = new Regex("^about_.*help\\.txt$", RegexOptions.IgnoreCase | RegexOptions.Compiled); #if !CORECLR private CompositionContainer container; #endif // !CORECLR @@ -1526,12 +1527,11 @@ public IEnumerable AnalyzeScriptDefinition(string scriptDefini var relevantParseErrors = RemoveTypeNotFoundParseErrors(errors, out List diagnosticRecords); - int emitParseErrors = severity == null ? 1 : severity.Count(item => item == "ParseError"); // Add parse errors first if requested! - if ( relevantParseErrors != null && emitParseErrors == 1) + if ( relevantParseErrors != null && (severity == null || severity.Contains("ParseError", StringComparer.OrdinalIgnoreCase))) { - List results = new List(); - foreach ( var parseError in relevantParseErrors ) + var results = new List(); + foreach ( ParseError parseError in relevantParseErrors ) { string parseErrorMessage = String.Format(CultureInfo.CurrentCulture, Strings.ParseErrorFormatForScriptDefinition, parseError.Message.TrimEnd('.'), parseError.Extent.StartLineNumber, parseError.Extent.StartColumnNumber); results.Add(new DiagnosticRecord( @@ -1539,7 +1539,7 @@ public IEnumerable AnalyzeScriptDefinition(string scriptDefini parseError.Extent, parseError.ErrorId.ToString(), DiagnosticSeverity.ParseError, - "" // no script file + String.Empty // no script file ) ); } @@ -1855,7 +1855,7 @@ private IEnumerable AnalyzeFile(string filePath) // short-circuited processing for a help file // no parsing can really be done, but there are other rules to run (specifically encoding). - if ( Regex.Matches(Path.GetFileName(filePath), @"^about_.*help.txt$", RegexOptions.IgnoreCase).Count != 0) + if ( s_aboutHelpRegex.IsMatch(Path.GetFileName(filePath)) ) { return diagnosticRecords.Concat(this.AnalyzeSyntaxTree(scriptAst, scriptTokens, filePath)); } @@ -1877,14 +1877,13 @@ private IEnumerable AnalyzeFile(string filePath) scriptAst = Parser.ParseFile(filePath, out scriptTokens, out errors); } #endif //!PSV3 - var relevantParseErrors = RemoveTypeNotFoundParseErrors(errors, out diagnosticRecords); + IEnumerable relevantParseErrors = RemoveTypeNotFoundParseErrors(errors, out diagnosticRecords); // First, add all parse errors if they've been requested - int emitParseErrors = severity == null ? 1 : severity.Count(item => item == "ParseError"); - if ( relevantParseErrors != null && emitParseErrors == 1 ) + if ( relevantParseErrors != null && (severity == null || severity.Contains("ParseError", StringComparer.OrdinalIgnoreCase))) { List results = new List(); - foreach ( var parseError in relevantParseErrors ) + foreach ( ParseError parseError in relevantParseErrors ) { string parseErrorMessage = String.Format(CultureInfo.CurrentCulture, Strings.ParseErrorFormatForScriptDefinition, parseError.Message.TrimEnd('.'), parseError.Extent.StartLineNumber, parseError.Extent.StartColumnNumber); results.Add(new DiagnosticRecord( diff --git a/README.md b/README.md index 1d8d4677a..dc9f5e617 100644 --- a/README.md +++ b/README.md @@ -187,6 +187,43 @@ Get-TestFailures [Back to ToC](#table-of-contents) +Parser Errors +============= + +In prior versions of ScriptAnalyer, errors found during parsing were reported as errors and diagnostic records were not created. +ScriptAnalyzer now emits parser errors as diagnostic records in the output stream with other diagnostic records. + +```powershell +PS> Invoke-ScriptAnalyzer -ScriptDefinition '"b" = "b"; function eliminate-file () { }' + +RuleName Severity ScriptName Line Message +-------- -------- ---------- ---- ------- +InvalidLeftHandSide ParseError 1 The assignment expression is not + valid. The input to an + assignment operator must be an + object that is able to accept + assignments, such as a variable + or a property. +PSUseApprovedVerbs Warning 1 The cmdlet 'eliminate-file' uses an + unapproved verb. +``` + +The RuleName is set to the `ErrorId` of the parser error. + +If ParseErrors would like to be suppressed, do not include it as a value in the `-Severity` parameter. + +```powershell +PS> Invoke-ScriptAnalyzer -ScriptDefinition '"b" = "b"; function eliminate-file () { }' -Severity Warning + +RuleName Severity ScriptName Line Message +-------- -------- ---------- ---- ------- +PSUseApprovedVerbs Warning 1 The cmdlet 'eliminate-file' uses an + unapproved verb. +``` + + + + Suppressing Rules ================= @@ -272,6 +309,8 @@ Param() **Note**: Rule suppression is currently supported only for built-in rules. +**Note**: Parser Errors cannot be suppressed via the `SuppressMessageAttribute` + [Back to ToC](#table-of-contents) Settings Support in ScriptAnalyzer