Skip to content

TypeNotFound suppressions #1041

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
avalery opened this issue Jul 19, 2018 · 9 comments · Fixed by #1130
Closed

TypeNotFound suppressions #1041

avalery opened this issue Jul 19, 2018 · 9 comments · Fixed by #1130

Comments

@avalery
Copy link

avalery commented Jul 19, 2018

Hello,

Since version 1.17.1, TypeNotFound errors are not thrown but returned as warnings (#957), which is very nice.
But is there a way to suppress them?

I tried adding a SuppressMessage attribute:

[Diagnostics.CodeAnalysis.SuppressMessageAttribute("TypeNotFound", "", Justification = "")]

And also excluding this rule in the settings file:

ExcludeRules = @('TypeNotFound')

But it's not working.

Thank you

@bergmeister
Copy link
Collaborator

bergmeister commented Jul 19, 2018

Initially, I wanted to just log a warning but during the PR it was suggested/decided to rather return an object of type DiagnosticRecord to still alert the user about it in some way. It is therefore a bit unfortunate/misleading because it is not a rule per se and can therefore not be suppressed. However, because it is PowerShell you can filter out those DiagnosticRecords from the list of returned results. I myself am not 100% happy with this solution but this is how it works at the moment, sorry that it is a bit confusing.

@avalery
Copy link
Author

avalery commented Jul 19, 2018

Ok that's what I though but I wasn't completely sure.
I agree that ideally we should be able to treat it like any other rule violation.
But it's ok, I'll filter it afterwards like you mentioned.

Thank you for your quick answer!

@thomasrayner
Copy link
Contributor

It's hugely frustrating that it overrides whatever settings or suppression statements the user indicates. Even something such as this, would still see these "informational" items.

Invoke-ScriptAnalyzer -Path $file -Severity Warning

Obviously when you specify you only want warnings, and you see info records returned too, it is hugely detrimental to one's confidence in the product. Not everyone is going to come hunt through Github issues and learn that this is on purpose.

If I want to use PSSA in a CI/CD pipeline, and stop it on violations, am I just not supposed to use PSSA on files that have classes? Is the expectation that I'll filter these out myself? This is the wrong user experience.

@bergmeister
Copy link
Collaborator

Ok, gotcha, I guess it is better to convert them to PowerShell warnings instead of returning an object, I wasn't a fan of that approach either when we did the PR but the behaviour in 1.16 was worse because the cmdlet re-threw the parser exceptions.

@thomasrayner
Copy link
Contributor

That'd be way better, @bergmeister . The diagnostic record makes it look like PSSA is reporting the error and that leads a user to think they can use PSSA to address it. Instead they're left with this record they can't do anything about. If there's something I can upvote to change the behavior, please do let me know. :)

@bergmeister
Copy link
Collaborator

@thomasrayner Upvoting helps with prioritising a bit but in general, one just needs to open a PR if change is wanted. I opened PR 1126 for that since this is an easy fix that I had in the back of my mind anyway (but there's just so many other things)

@thomasrayner
Copy link
Contributor

Thanks, @bergmeister ! I may take a look if I can make time for it, if that's not overstepping.

@bergmeister
Copy link
Collaborator

@thomasrayner Don't worry, PR 1126 is ready now with a fix for it, it is just pending a CI problem on Linux due to an AppVeyor image update.

@bergmeister
Copy link
Collaborator

Just an update: PR #1126 had to be abandoned as the approach of just outputting a warning was not acceptable. New efforts using a different approach are happening in PR #1130 now

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