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/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 01b4ef3cd..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,23 +1527,26 @@ public IEnumerable AnalyzeScriptDefinition(string scriptDefini var relevantParseErrors = RemoveTypeNotFoundParseErrors(errors, out List diagnosticRecords); - if (relevantParseErrors != null && relevantParseErrors.Count > 0) + // Add parse errors first if requested! + if ( relevantParseErrors != null && (severity == null || severity.Contains("ParseError", StringComparer.OrdinalIgnoreCase))) { - 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); - 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, + String.Empty // 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 +1843,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 +1853,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 ( s_aboutHelpRegex.IsMatch(Path.GetFileName(filePath)) ) + { + 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 + IEnumerable relevantParseErrors = RemoveTypeNotFoundParseErrors(errors, out diagnosticRecords); + + // First, add all parse errors if they've been requested + if ( relevantParseErrors != null && (severity == null || severity.Contains("ParseError", StringComparer.OrdinalIgnoreCase))) + { + List 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( + parseError.Message, + parseError.Extent, + parseError.ErrorId.ToString(), + DiagnosticSeverity.ParseError, + filePath) + ); + } + diagnosticRecords.AddRange(results); + } + return diagnosticRecords.Concat(this.AnalyzeSyntaxTree(scriptAst, scriptTokens, filePath)); } 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 diff --git a/Tests/Engine/InvokeScriptAnalyzer.tests.ps1 b/Tests/Engine/InvokeScriptAnalyzer.tests.ps1 index c43cac665..788a30c6e 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 } } } @@ -119,14 +121,19 @@ 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 "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 + 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" + @($moreThanTenErrors).Count | Should -Be 12 } } @@ -306,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 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, 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..72074f91c 100644 --- a/Tests/Rules/AlignAssignmentStatement.tests.ps1 +++ b/Tests/Rules/AlignAssignmentStatement.tests.ps1 @@ -126,6 +126,7 @@ Configuration Sample_ChangeDescriptionAndPermissions # NonExistentModule is not really avaiable to load. Therefore we set erroraction to # SilentlyContinue Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings -ErrorAction SilentlyContinue | + Where-Object { $_.Severity -ne "ParseError" } | Get-Count | Should -Be 4 } 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 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 }