ScriptAnalyzer needs to emit Diagnostic records for AST parsing errors

[raghushantha]

In current implementation, parsing errors in a given script are treated as non-terminating PowerShell ErrorRecords.

Proposal is to convert these to diagnostic records and create a new rule for parsing issues.

Helps to solve following:

• In a service model, all results will be in the form of diagnostic records. Single stream for user to go over the results.
• For users refactoring their code from v2/v3 to higher versions, they can use the parsing rule to validate the changes.

Let us know if the community has any opinions about this proposal

Thanks.
Raghu

[JennDahm]

I think we should be careful not to call catching parsing errors a "rule." Calling it a rule comes with some implications that don't fit what this does. To give a couple examples:

• Rules are only run after the script is properly parsed, but parsing errors happen during parsing, before rules are run.
• Rules can be excluded. How would excluding parsing errors work? You can't exactly tell PowerShell to ignore syntax errors.

I think we should be explicit in calling these parsing errors, and not ScriptAnalyzer Rules.

[rjmholt]

Just want to add that the ParseErrors emitted by PSScriptAnalyzer don't currently seem to have an associated extent, which makes them very hard to use.

I feel like an analyzer/linter should emit parse errors to stdout (i.e. not to an error stream), as diagnostic records like all the others, with an extent, so that automated tooling can indicate where the error occurs.

Currently PSScriptAnalyzer seems to roll its own ParseErrors from the ones generated by the parser and leaves out useful information:

PSScriptAnalyzer/Engine/ScriptAnalyzer.cs

Lines 1531 to 1535 in db2aabf

	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));
	}

This basically means applications embedding PSScriptAnalyzer have to call the parser twice on the same script in order to get the actual ParseErrors out and report where they are.

This is sort of related to #1056, insofar as being able to pass in a pre-parsed AST would mitigate the cost of parsing the script twice (the operation itself is less expensive, but it does have to allocate quite a few objects).

[bergmeister]

This has been implemented by PR #1130