Skip to content

Commit 18c43e6

Browse files
JamesWTruherJamesWTruher
committed
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.
1 parent 7a9d45d commit 18c43e6

File tree

5 files changed

+89
-68
lines changed

5 files changed

+89
-68
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/ScriptAnalyzer.cs‎

Lines changed: 60 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -1526,23 +1526,26 @@ public IEnumerable<DiagnosticRecord> AnalyzeScriptDefinition(string scriptDefini
15261526

15271527
var relevantParseErrors = RemoveTypeNotFoundParseErrors(errors, out List<DiagnosticRecord> diagnosticRecords);
15281528

1529-
if (relevantParseErrors != null && relevantParseErrors.Count > 0)
1529+
// Add parse errors first!
1530+
if ( relevantParseErrors != null )
15301531
{
1531-
foreach (var parseError in relevantParseErrors)
1532+
List<DiagnosticRecord> results = new List<DiagnosticRecord>();
1533+
foreach ( var parseError in relevantParseErrors )
15321534
{
15331535
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));
1536+
results.Add(new DiagnosticRecord(
1537+
parseError.Message,
1538+
parseError.Extent,
1539+
parseError.ErrorId.ToString(),
1540+
DiagnosticSeverity.ParseError,
1541+
"" // no script file
1542+
)
1543+
);
15351544
}
1545+
diagnosticRecords.AddRange(results);
15361546
}
15371547

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-
1548+
// now, analyze the script definition
15461549
return diagnosticRecords.Concat(this.AnalyzeSyntaxTree(scriptAst, scriptTokens, String.Empty));
15471550
}
15481551

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

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
1845+
// If the file doesn't exist, return
1846+
if (! File.Exists(filePath))
18851847
{
18861848
this.outputWriter.ThrowTerminatingError(new ErrorRecord(new FileNotFoundException(),
18871849
string.Format(CultureInfo.CurrentCulture, Strings.InvalidPath, filePath),
@@ -1890,6 +1852,50 @@ private IEnumerable<DiagnosticRecord> AnalyzeFile(string filePath)
18901852
return null;
18911853
}
18921854

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

‎Tests/Engine/InvokeScriptAnalyzer.tests.ps1‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -109,9 +109,11 @@ 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
}
@@ -124,9 +126,11 @@ Describe "Test Path" {
124126
$withPath.Count -eq $withoutPath.Count | Should -BeTrue
125127
}
126128

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
129+
It "Runs rules on script with more than 10 parser errors" {
130+
# this is a script with 12 parse errors
131+
1..12 | Foreach-Object { ')' } | Out-File "TestDrive:\badfile.ps1"
132+
$moreThanTenErrors = Invoke-ScriptAnalyzer -ErrorAction SilentlyContinue "TestDrive:\badfile.ps1"
133+
$moreThanTenErrors.Count | Should -Be 12
130134
}
131135
}
132136

‎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)