Skip to content

PSSA doesn't respect the config I give it. #1060

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
webtroter opened this issue Aug 28, 2018 · 6 comments
Closed

PSSA doesn't respect the config I give it. #1060

webtroter opened this issue Aug 28, 2018 · 6 comments

Comments

@webtroter
Copy link

webtroter commented Aug 28, 2018

This is how I call PSSA :

$scriptAnalyzerParams = @{
    Path = $ModulePath
    Severity = @('Error', 'Warning')
    ExcludeRule=@('PSUseBOMForUnicodeEncodedFile','TypeNotFound')
    Recurse = $true
    Verbose = $false
}

$saResults = Invoke-ScriptAnalyzer @scriptAnalyzerParams

code_2018-08-28_15-12-06

This is the file failing.
https://gist.github.com/webtroter/d44a5725849ad4cd30bbadd1efb69bd2

@bergmeister
Copy link
Collaborator

bergmeister commented Aug 28, 2018

@webtroter This is a duplicate of #1041 because TypeNotFound is not a rule per se but emitted to still warn the user that the AST could only be partially analysed. I discussed at the time being if it might be confusing to return TypeNotFound as a DiagnosticRecord, which might be confusing but it was decided to rather return an object than emit a warning (which cannot be seen in editors such as VSCode. cc @JamesWTruher
@tylerl0706 @rjmholt Would it be possible for PSES/VSCode to display warnings from PSSA as well so that we can fix those issues by returning a PowerShell warning instead of a DiagnosticRecord object in PSSA?

@webtroter
Copy link
Author

webtroter commented Aug 28, 2018

Thank you @bergmeister.

How would your proceed to ignore that error? It's blocking my build process.

I just read your post on #1041, and I'm not sure how to filter out those DiagnosticRecords from the list of returned results.

Thank you.

@rjmholt
Copy link
Contributor

rjmholt commented Aug 28, 2018

@bergmeister I'm not sure I follow you there on the PowerShell warning side of things.

There are probably a few points to address here:

  1. TypeNotFound is a legitmate parse error - PowerShell itself will not run a script that results in the emission of this error. See Type checking in PowerShell class method bodies is not needed PowerShell#6722, Using external types / classes in Custom PowerShell Classes (v5) results in Parsing Error PowerShell#2074 and Defining a PowerShell class in a script causes a parser error if the class references external types that aren't currently loaded PowerShell#3641. Converting the parse error to an information record may be ergonomic for users, but it masks a real issue in the script -- that the required types need to be imported ahead of time before that script/module is parsed. (This is a PowerShell issue I've been working to rectify in my spare time)
  2. PSSA is a linter/diagnostic tool, and I think it should behave the way the parser does with respect to errors -- by making a clear distinction between diagnosing an error in the script its analysing and showing an actual error in itself. Writing to the error stream conventionally means there is a problem in execution, but PSSA currently writes errors diagnosed in the script under analysis as errors, rather than diagnostics. My opinion is that PSSA should write to the error stream only if there has been an error in PSSA. PSSA probably needs a new diagnostic category for parse errors and the ability to suppress any given parse error by FullyQualifiedErrorId. Writing diagnosed errors to the error stream makes it harder for PSES and other PSSA-embedding tools to handle PSSA's output, especially if some parse errors are treated differently.
  3. The other problem with the way PSSA currently treats parse errors is that it strips out important information, such as where the error occurs (see ScriptAnalyzer needs to emit Diagnostic records for AST parsing errors #264 (comment)). So in fact we have to get our parse error diagnostics in PSES elsewhere (by parsing the script ourselves). If a TypeNotFound error were emitted as a warning like this and information were similarly stripped out, we would not be able to display it.

@webtroter I think this whole discussion might be tangential to what you're trying to achieve. You should be able to filter your Invoke-ScriptAnalyzer to weed out TypeNotFound diagnostics. Something like this:

$saResults = Invoke-ScriptAnalyzer @scriptAnalysisParams | Where-Object { $_.RuleName -ne "TypeNotFound" }

@JamesWTruher this is pretty relevant to a discussion we had the other day

@rjmholt
Copy link
Contributor

rjmholt commented Aug 29, 2018

Anyway, I understand that all the things I'm suggesting are breaking changes. But they are ones that I think will improve the ease of use and API consistency for PSSA for everyone, especially anyone trying to embed PSSA in a hosted scenario. I was trying to write a PSSA GitHub bot the other week and the inability to extract an extent from the parse errors it emits was frustrating to say the least.

@webtroter
Copy link
Author

Thank you @rjmholt .
I went ahead and changed it to ignore my specific Type, instead of the whole "TypeNotFound" rule.

I'm using

$saResults = Invoke-ScriptAnalyzer @scriptAnalyzerParams | Where-Object { $_.Extent.Text -ne "Microsoft.Online.Administration.User" }

@bergmeister
Copy link
Collaborator

bergmeister commented Aug 29, 2018

I'm glad you could sort it out @webtroter
P.S.: a slightly more generic way would be to filter like this:

Invoke-ScriptAnalyzer @scriptAnalyzerParams | Where-Object { $_.RuleName -ne 'TypeNotFound' }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants