Skip to content

Commit c9915b1

Browse files
JamesWTruherJamesWTruher
authored
Merge pull request #1130 from JamesWTruher/ParseErrorDiagnostic
Emit parsing errors as diagnostic records
2 parents 7a9d45d + 0374851 commit c9915b1

11 files changed

+181
-77
lines changed

‎Engine/Commands/InvokeScriptAnalyzerCommand.cs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ public string[] IncludeRule
133133
/// <summary>
134134
/// IncludeRule: Array of the severity types to be enabled.
135135
/// </summary>
136-
[ValidateSet("Warning", "Error", "Information", IgnoreCase = true)]
136+
[ValidateSet("Warning", "Error", "Information", "ParseError", IgnoreCase = true)]
137137
[Parameter(Mandatory = false)]
138138
[SuppressMessage("Microsoft.Performance", "CA1819:PropertiesShouldNotReturnArrays")]
139139
public string[] Severity
@@ -432,6 +432,7 @@ private void WriteToOutput(IEnumerable<DiagnosticRecord> diagnosticRecords)
432432
var errorCount = 0;
433433
var warningCount = 0;
434434
var infoCount = 0;
435+
var parseErrorCount = 0;
435436

436437
foreach (DiagnosticRecord diagnostic in diagnosticRecords)
437438
{
@@ -447,6 +448,9 @@ private void WriteToOutput(IEnumerable<DiagnosticRecord> diagnosticRecords)
447448
case DiagnosticSeverity.Error:
448449
errorCount++;
449450
break;
451+
case DiagnosticSeverity.ParseError:
452+
parseErrorCount++;
453+
break;
450454
default:
451455
throw new ArgumentOutOfRangeException(nameof(diagnostic.Severity), $"Severity '{diagnostic.Severity}' is unknown");
452456
}

‎Engine/Generic/DiagnosticRecord.cs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,5 +142,10 @@ public enum DiagnosticSeverity : uint
142142
/// ERROR: This diagnostic is likely to cause a problem or does not follow PowerShell's required guidelines.
143143
/// </summary>
144144
Error = 2,
145+
146+
/// <summary>
147+
/// ERROR: This diagnostic is caused by an actual parsing error, and is generated only by the engine.
148+
/// </summary>
149+
ParseError = 3,
145150
};
146151
}

‎Engine/Generic/RuleSeverity.cs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,5 +22,10 @@ public enum RuleSeverity : uint
2222
/// ERROR: This warning is likely to cause a problem or does not follow PowerShell's required guidelines.
2323
/// </summary>
2424
Error = 2,
25+
26+
/// <summary>
27+
/// ERROR: This diagnostic is caused by an actual parsing error, and is generated only by the engine.
28+
/// </summary>
29+
ParseError = 3,
2530
};
2631
}

‎Engine/ScriptAnalyzer.cs

Lines changed: 61 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ public sealed class ScriptAnalyzer
3131

3232
private IOutputWriter outputWriter;
3333
private Dictionary<string, object> settings;
34+
private readonly Regex s_aboutHelpRegex = new Regex("^about_.*help\\.txt$", RegexOptions.IgnoreCase | RegexOptions.Compiled);
3435
#if !CORECLR
3536
private CompositionContainer container;
3637
#endif // !CORECLR
@@ -1526,23 +1527,26 @@ public IEnumerable<DiagnosticRecord> AnalyzeScriptDefinition(string scriptDefini
15261527

15271528
var relevantParseErrors = RemoveTypeNotFoundParseErrors(errors, out List<DiagnosticRecord> diagnosticRecords);
15281529

1529-
if (relevantParseErrors != null && relevantParseErrors.Count > 0)
1530+
// Add parse errors first if requested!
1531+
if ( relevantParseErrors != null && (severity == null || severity.Contains("ParseError", StringComparer.OrdinalIgnoreCase)))
15301532
{
1531-
foreach (var parseError in relevantParseErrors)
1533+
var results = new List<DiagnosticRecord>();
1534+
foreach ( ParseError parseError in relevantParseErrors )
15321535
{
15331536
string parseErrorMessage = String.Format(CultureInfo.CurrentCulture, Strings.ParseErrorFormatForScriptDefinition, parseError.Message.TrimEnd('.'), parseError.Extent.StartLineNumber, parseError.Extent.StartColumnNumber);
1534-
this.outputWriter.WriteError(new ErrorRecord(new ParseException(parseErrorMessage), parseErrorMessage, ErrorCategory.ParserError, parseError.ErrorId));
1537+
results.Add(new DiagnosticRecord(
1538+
parseError.Message,
1539+
parseError.Extent,
1540+
parseError.ErrorId.ToString(),
1541+
DiagnosticSeverity.ParseError,
1542+
String.Empty // no script file
1543+
)
1544+
);
15351545
}
1546+
diagnosticRecords.AddRange(results);
15361547
}
15371548

1538-
if (relevantParseErrors != null && relevantParseErrors.Count > 10)
1539-
{
1540-
string manyParseErrorMessage = String.Format(CultureInfo.CurrentCulture, Strings.ParserErrorMessageForScriptDefinition);
1541-
this.outputWriter.WriteError(new ErrorRecord(new ParseException(manyParseErrorMessage), manyParseErrorMessage, ErrorCategory.ParserError, scriptDefinition));
1542-
1543-
return new List<DiagnosticRecord>();
1544-
}
1545-
1549+
// now, analyze the script definition
15461550
return diagnosticRecords.Concat(this.AnalyzeSyntaxTree(scriptAst, scriptTokens, String.Empty));
15471551
}
15481552

@@ -1839,49 +1843,8 @@ private IEnumerable<DiagnosticRecord> AnalyzeFile(string filePath)
18391843
this.outputWriter.WriteVerbose(string.Format(CultureInfo.CurrentCulture, Strings.VerboseFileMessage, filePath));
18401844
var diagnosticRecords = new List<DiagnosticRecord>();
18411845

1842-
//Parse the file
1843-
if (File.Exists(filePath))
1844-
{
1845-
// processing for non help script
1846-
if (!(Path.GetFileName(filePath).ToLower().StartsWith("about_") && Path.GetFileName(filePath).ToLower().EndsWith(".help.txt")))
1847-
{
1848-
try
1849-
{
1850-
scriptAst = Parser.ParseFile(filePath, out scriptTokens, out errors);
1851-
}
1852-
catch (Exception e)
1853-
{
1854-
this.outputWriter.WriteWarning(e.ToString());
1855-
return null;
1856-
}
1857-
#if !PSV3
1858-
//try parsing again
1859-
if (TrySaveModules(errors, scriptAst))
1860-
{
1861-
scriptAst = Parser.ParseFile(filePath, out scriptTokens, out errors);
1862-
}
1863-
#endif //!PSV3
1864-
var relevantParseErrors = RemoveTypeNotFoundParseErrors(errors, out diagnosticRecords);
1865-
1866-
//Runspace.DefaultRunspace = oldDefault;
1867-
if (relevantParseErrors != null && relevantParseErrors.Count > 0)
1868-
{
1869-
foreach (var parseError in relevantParseErrors)
1870-
{
1871-
string parseErrorMessage = String.Format(CultureInfo.CurrentCulture, Strings.ParserErrorFormat, parseError.Extent.File, parseError.Message.TrimEnd('.'), parseError.Extent.StartLineNumber, parseError.Extent.StartColumnNumber);
1872-
this.outputWriter.WriteError(new ErrorRecord(new ParseException(parseErrorMessage), parseErrorMessage, ErrorCategory.ParserError, parseError.ErrorId));
1873-
}
1874-
}
1875-
1876-
if (relevantParseErrors != null && relevantParseErrors.Count > 10)
1877-
{
1878-
string manyParseErrorMessage = String.Format(CultureInfo.CurrentCulture, Strings.ParserErrorMessage, System.IO.Path.GetFileName(filePath));
1879-
this.outputWriter.WriteError(new ErrorRecord(new ParseException(manyParseErrorMessage), manyParseErrorMessage, ErrorCategory.ParserError, filePath));
1880-
return new List<DiagnosticRecord>();
1881-
}
1882-
}
1883-
}
1884-
else
1846+
// If the file doesn't exist, return
1847+
if (! File.Exists(filePath))
18851848
{
18861849
this.outputWriter.ThrowTerminatingError(new ErrorRecord(new FileNotFoundException(),
18871850
string.Format(CultureInfo.CurrentCulture, Strings.InvalidPath, filePath),
@@ -1890,6 +1853,50 @@ private IEnumerable<DiagnosticRecord> AnalyzeFile(string filePath)
18901853
return null;
18911854
}
18921855

1856+
// short-circuited processing for a help file
1857+
// no parsing can really be done, but there are other rules to run (specifically encoding).
1858+
if ( s_aboutHelpRegex.IsMatch(Path.GetFileName(filePath)) )
1859+
{
1860+
return diagnosticRecords.Concat(this.AnalyzeSyntaxTree(scriptAst, scriptTokens, filePath));
1861+
}
1862+
1863+
// Process script
1864+
try
1865+
{
1866+
scriptAst = Parser.ParseFile(filePath, out scriptTokens, out errors);
1867+
}
1868+
catch (Exception e)
1869+
{
1870+
this.outputWriter.WriteWarning(e.ToString());
1871+
return null;
1872+
}
1873+
#if !PSV3
1874+
//try parsing again
1875+
if (TrySaveModules(errors, scriptAst))
1876+
{
1877+
scriptAst = Parser.ParseFile(filePath, out scriptTokens, out errors);
1878+
}
1879+
#endif //!PSV3
1880+
IEnumerable<ParseError> relevantParseErrors = RemoveTypeNotFoundParseErrors(errors, out diagnosticRecords);
1881+
1882+
// First, add all parse errors if they've been requested
1883+
if ( relevantParseErrors != null && (severity == null || severity.Contains("ParseError", StringComparer.OrdinalIgnoreCase)))
1884+
{
1885+
List<DiagnosticRecord> results = new List<DiagnosticRecord>();
1886+
foreach ( ParseError parseError in relevantParseErrors )
1887+
{
1888+
string parseErrorMessage = String.Format(CultureInfo.CurrentCulture, Strings.ParseErrorFormatForScriptDefinition, parseError.Message.TrimEnd('.'), parseError.Extent.StartLineNumber, parseError.Extent.StartColumnNumber);
1889+
results.Add(new DiagnosticRecord(
1890+
parseError.Message,
1891+
parseError.Extent,
1892+
parseError.ErrorId.ToString(),
1893+
DiagnosticSeverity.ParseError,
1894+
filePath)
1895+
);
1896+
}
1897+
diagnosticRecords.AddRange(results);
1898+
}
1899+
18931900
return diagnosticRecords.Concat(this.AnalyzeSyntaxTree(scriptAst, scriptTokens, filePath));
18941901
}
18951902

‎README.md

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,43 @@ Get-TestFailures
187187

188188
[Back to ToC](#table-of-contents)
189189

190+
Parser Errors
191+
=============
192+
193+
In prior versions of ScriptAnalyer, errors found during parsing were reported as errors and diagnostic records were not created.
194+
ScriptAnalyzer now emits parser errors as diagnostic records in the output stream with other diagnostic records.
195+
196+
```powershell
197+
PS> Invoke-ScriptAnalyzer -ScriptDefinition '"b" = "b"; function eliminate-file () { }'
198+
199+
RuleName Severity ScriptName Line Message
200+
-------- -------- ---------- ---- -------
201+
InvalidLeftHandSide ParseError 1 The assignment expression is not
202+
valid. The input to an
203+
assignment operator must be an
204+
object that is able to accept
205+
assignments, such as a variable
206+
or a property.
207+
PSUseApprovedVerbs Warning 1 The cmdlet 'eliminate-file' uses an
208+
unapproved verb.
209+
```
210+
211+
The RuleName is set to the `ErrorId` of the parser error.
212+
213+
If ParseErrors would like to be suppressed, do not include it as a value in the `-Severity` parameter.
214+
215+
```powershell
216+
PS> Invoke-ScriptAnalyzer -ScriptDefinition '"b" = "b"; function eliminate-file () { }' -Severity Warning
217+
218+
RuleName Severity ScriptName Line Message
219+
-------- -------- ---------- ---- -------
220+
PSUseApprovedVerbs Warning 1 The cmdlet 'eliminate-file' uses an
221+
unapproved verb.
222+
```
223+
224+
225+
226+
190227
Suppressing Rules
191228
=================
192229

@@ -272,6 +309,8 @@ Param()
272309

273310
**Note**: Rule suppression is currently supported only for built-in rules.
274311

312+
**Note**: Parser Errors cannot be suppressed via the `SuppressMessageAttribute`
313+
275314
[Back to ToC](#table-of-contents)
276315

277316
Settings Support in ScriptAnalyzer

‎Tests/Engine/InvokeScriptAnalyzer.tests.ps1

Lines changed: 42 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -109,24 +109,31 @@ Describe "Test available parameters" {
109109

110110
Describe "Test ScriptDefinition" {
111111
Context "When given a script definition" {
112-
It "Does not run rules on script with more than 10 parser errors" {
113-
$moreThanTenErrors = Invoke-ScriptAnalyzer -ErrorAction SilentlyContinue -ScriptDefinition (Get-Content -Raw "$directory\CSharp.ps1")
114-
$moreThanTenErrors.Count | Should -Be 0
112+
It "Runs rules on script with more than 10 parser errors" {
113+
# this is a script with 12 parse errors
114+
$script = ');' * 12
115+
$moreThanTenErrors = Invoke-ScriptAnalyzer -ScriptDefinition $script
116+
$moreThanTenErrors.Count | Should -Be 12
115117
}
116118
}
117119
}
118120

119121
Describe "Test Path" {
120122
Context "When given a single file" {
121123
It "Has the same effect as without Path parameter" {
122-
$withPath = Invoke-ScriptAnalyzer $directory\TestScript.ps1
123-
$withoutPath = Invoke-ScriptAnalyzer -Path $directory\TestScript.ps1
124-
$withPath.Count -eq $withoutPath.Count | Should -BeTrue
124+
$scriptPath = Join-Path $directory "TestScript.ps1"
125+
$withPath = Invoke-ScriptAnalyzer $scriptPath
126+
$withoutPath = Invoke-ScriptAnalyzer -Path $scriptPath
127+
$withPath.Count | Should -Be $withoutPath.Count
125128
}
129+
}
126130

127-
It "Does not run rules on script with more than 10 parser errors" {
128-
$moreThanTenErrors = Invoke-ScriptAnalyzer -ErrorAction SilentlyContinue $directory\CSharp.ps1
129-
$moreThanTenErrors.Count | Should -Be 0
131+
Context "When there are more than 10 errors in a file" {
132+
It "All errors are found in a file" {
133+
# this is a script with 12 parse errors
134+
1..12 | Foreach-Object { ');' } | Out-File -Encoding ASCII "${TestDrive}\badfile.ps1"
135+
$moreThanTenErrors = Invoke-ScriptAnalyzer -Path "${TestDrive}\badfile.ps1"
136+
@($moreThanTenErrors).Count | Should -Be 12
130137
}
131138
}
132139

@@ -306,6 +313,32 @@ Describe "Test Exclude And Include" {1
306313
}
307314

308315
Describe "Test Severity" {
316+
Context "Each severity can be chosen in any combination" {
317+
BeforeAll {
318+
$Severities = "ParseError","Error","Warning","Information"
319+
# end space is important
320+
$script = '$a=;ConvertTo-SecureString -Force -AsPlainText "bad practice" '
321+
$testcases = @{ Severity = "ParseError" }, @{ Severity = "Error" },
322+
@{ Severity = "Warning" }, @{ Severity = "Information" },
323+
@{ Severity = "ParseError", "Error" }, @{ Severity = "ParseError","Information" },
324+
@{ Severity = "Information", "Warning", "Error" }
325+
}
326+
327+
It "Can retrieve specific severity <severity>" -testcase $testcases {
328+
param ( $severity )
329+
$result = Invoke-ScriptAnalyzer -ScriptDefinition $script -Severity $severity
330+
if ( $severity -is [array] ) {
331+
@($result).Count | Should -Be @($severity).Count
332+
foreach ( $sev in $severity ) {
333+
$result.Severity | Should -Contain $sev
334+
}
335+
}
336+
else {
337+
$result.Severity | Should -Be $severity
338+
}
339+
}
340+
}
341+
309342
Context "When used correctly" {
310343
It "works with one argument" {
311344
$errors = Invoke-ScriptAnalyzer $directory\TestScript.ps1 -Severity Information

‎Tests/Engine/LibraryUsage.tests.ps1

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ function Invoke-ScriptAnalyzer {
3636
[Parameter(Mandatory = $false)]
3737
[string[]] $IncludeRule = $null,
3838

39-
[ValidateSet("Warning", "Error", "Information", IgnoreCase = $true)]
39+
[ValidateSet("Warning", "Error", "Information", "ParseError", IgnoreCase = $true)]
4040
[Parameter(Mandatory = $false)]
4141
[string[]] $Severity = $null,
4242

‎Tests/Engine/ModuleDependencyHandler.tests.ps1

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -139,8 +139,12 @@ Describe "Resolve DSC Resource Dependency" {
139139

140140
Context "Invoke-ScriptAnalyzer without switch" {
141141
It "Has parse errors" -skip:$skipTest {
142-
$dr = Invoke-ScriptAnalyzer -Path $violationFilePath -ErrorVariable parseErrors -ErrorAction SilentlyContinue
143-
$parseErrors.Count | Should -Be 1
142+
$dr = Invoke-ScriptAnalyzer -Path $violationFilePath -ErrorVariable analyzerErrors -ErrorAction SilentlyContinue
143+
$analyzerErrors.Count | Should -Be 0
144+
145+
$dr |
146+
Where-Object { $_.Severity -eq "ParseError" } |
147+
Get-Count | Should -Be 1
144148
}
145149
}
146150

@@ -182,10 +186,13 @@ Describe "Resolve DSC Resource Dependency" {
182186
Copy-Item -Recurse -Path $modulePath -Destination $tempModulePath
183187
}
184188

185-
It "Doesn't have parse errors" -skip:$skipTest {
189+
It "has a single parse error" -skip:$skipTest {
186190
# invoke script analyzer
187-
$dr = Invoke-ScriptAnalyzer -Path $violationFilePath -ErrorVariable parseErrors -ErrorAction SilentlyContinue
188-
$dr.Count | Should -Be 0
191+
$dr = Invoke-ScriptAnalyzer -Path $violationFilePath -ErrorVariable analyzerErrors -ErrorAction SilentlyContinue
192+
$analyzerErrors.Count | Should -Be 0
193+
$dr |
194+
Where-Object { $_.Severity -eq "ParseError" } |
195+
Get-Count | Should -Be 1
189196
}
190197

191198
It "Keeps PSModulePath unchanged before and after invocation" -skip:$skipTest {

‎Tests/Rules/AlignAssignmentStatement.tests.ps1

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,7 @@ Configuration Sample_ChangeDescriptionAndPermissions
126126
# NonExistentModule is not really avaiable to load. Therefore we set erroraction to
127127
# SilentlyContinue
128128
Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings -ErrorAction SilentlyContinue |
129+
Where-Object { $_.Severity -ne "ParseError" } |
129130
Get-Count |
130131
Should -Be 4
131132
}

‎Tests/Rules/UseUTF8EncodingForHelpFile.tests.ps1

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
1-
$violationMessage = "File about_utf16.help.txt has to use UTF8 instead of System.Text.UTF32Encoding encoding because it is a powershell help file."
2-
$violationName = "PSUseUTF8EncodingForHelpFile"
3-
$directory = Split-Path -Parent $MyInvocation.MyCommand.Path
4-
$violations = Invoke-ScriptAnalyzer $directory\about_utf16.help.txt | Where-Object {$_.RuleName -eq $violationName}
5-
$noViolations = Invoke-ScriptAnalyzer $directory\about_utf8.help.txt | Where-Object {$_.RuleName -eq $violationName}
6-
$notHelpFileViolations = Invoke-ScriptAnalyzer $directory\utf16.txt | Where-Object {$_.RuleName -eq $violationName}
7-
1+
$directory = Split-Path -Parent $MyInvocation.MyCommand.Path
82
Describe "UseUTF8EncodingForHelpFile" {
3+
BeforeAll {
4+
$violationMessage = "File about_utf16.help.txt has to use UTF8 instead of System.Text.UTF32Encoding encoding because it is a powershell help file."
5+
$violationName = "PSUseUTF8EncodingForHelpFile"
6+
$violations = Invoke-ScriptAnalyzer $directory\about_utf16.help.txt | Where-Object {$_.RuleName -eq $violationName}
7+
$noViolations = Invoke-ScriptAnalyzer $directory\about_utf8.help.txt | Where-Object {$_.RuleName -eq $violationName}
8+
$notHelpFileViolations = Invoke-ScriptAnalyzer $directory\utf16.txt | Where-Object {$_.RuleName -eq $violationName}
9+
}
10+
911
Context "When there are violations" {
1012
It "has 1 avoid use utf8 encoding violation" {
1113
$violations.Count | Should -Be 1

0 commit comments

Comments
 (0)