-
-
Notifications
You must be signed in to change notification settings - Fork 9
Add IGNORE: IONIDE-XXX to allow for single line analyzer ignores
#160
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
Conversation
|
Idle aside: as you do here, I've been hand-rolling these per analyzer; it would be nice if the SDK itself could handle that somehow. |
💯 It would be nice if we could reuse the work Martin put into the scoped nowarn |
|
Hmm looks like the nowarn/warnon stuff is still really based on rules that are soley for the compiler usage =\ Probably would be advantageous for us to align with roslyn. Using editorconfig and pragma |
|
We should breathe some life into ionide/FSharp.Analyzers.SDK#47 |
|
@TheAngryByrd @Smaug123 I made an attempt at at least reducing work for every analyzer by bringing functionality to the SDK. If a form of this gets implemented, analyzers should just be able to do a code based lookup to see if they are being ignored i.e. |
|
Awesome work! Is it possible to not even need to analyzers to do anything? I think it be preferable if the SDK was able to filter out without an analyzer caring about what ranges are turned on/off. I could see an analyzers being expensive to compute and may be advantageous to allow them to not need to do work for performance but thats separate from completing filtering. |
|
Echoing @TheAngryByrd, this is awesome. Looking forward, would it be possible to integrate/combine similar configuration from the following sources:
|
to @TheAngryByrdSo I believe the analyzers will always need to do something since the SDK doesn't have the knowledge of the code constructs each analyzer is looking at. However, I could be wrong in my understanding. The different levels of ignore we are currently supporting in the SDK are:
I believe a performance improvement made with the SDK changes would be that each analyzer does not have to iterate over the code comments on the same file and can rather refer to the ignore map created for each file. Granted this is not as expensive as walking the trees I'm sure. to @baronfelWhen discussing with Jimmy the other day we were discussing that pragmas are available in the syntax tree but can't be used arbitrarily so I'm unsure of the mechanism if we were to try and utilize them. As for the .editorconfig, are you thinking we'd do something along the lines of below in order to configure analyzer ignores at the file scope level? I imagine the MSBuild would be similar, just using all the build property magic to convey similar information. |
|
At a risk of adding more complexity (not sure how complicated this is compared to trying to use pragmas etc) - Roslyn analyzers can be disbled using the SuppressMessage attribute as well as with pragmas and .editoconfig, which I sometimes find useful for disabling an analyser for a single class or function without having to turn the analyzer back on again as you need to with a pragma. |
Well since we're already getting the ignore ranges every time, we could add an additional filter step after Running since a message will have a range. I guess an analyzer could report a range outside but that feels like a bug for the analyzer. |
|
Yeah unfortunately pramgas are quite limited in how they can be used in F# syntax tree. They can't seem to be used at any arbitrary range. You get messages like @baronfel will dotnet/fsharp#13786 still be an issue here? |
|
UGH my nemesis! Yes this will likely be a problem still. |
|
Closing this PR since we accomplished the intent over in ionide/FSharp.Analyzers.SDK#254. Thanks for the input & assistance from everyone! |
This PR introduces a mechanism to allow users to ignore specific analyzer messages on a line-by-line basis using special comments. This provides developers with fine-grained control over when analyzer rules should be suppressed without having to disable entire analyzers on larger scales.