-
-
Notifications
You must be signed in to change notification settings - Fork 22
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
Feature: Passing options through the context to named analyzers #8
Comments
I am experimenting with implementing an F# linter. This is an absolute must-have for me if I am to implement it using FSharp.Analyzers.SDK (a decision that also depends on some other aspects). I think that ideally, configs should be searched for in the current analyzed file's directory and all parent directories, with deeper levels overriding settings from higher levels. Perhaps Note that depending on how an analyzer library implements analyzers, the analyzers may need to be namespaced using e.g. the analyzer assembly name in the config and elsewhere, to avoid conflicts. As a side note, I would also love if YAML was supported as I find that easier to write and read, but this may of course require more logic (if nothing else, at least an initial translation step from YAML to JSON using some library). |
Note that some things should be clarified regarding analyzer names. For example, I'm thinking that my linter, Flinter, would have one analyzer per rule. It could also end up with very many rules/analyzers. In order to avoid collisions, should the names be e.g. Note that if using this prefix as a config key with {
"Flinter:FunctionParameterNaming": {
"Rule": "camelCase"
}
} {
"Flinter": {
"FunctionParameterNaming": {
"Rule": "camelCase"
}
}
} Which is nice. |
Note that according to the "Setting config values" section in #47 (comment), which describes overriding config values per scope/file, analyzers must pass a |
For prior art I'd look at Myriad and how it handles options, but this all sound reasonable enough. I would suggest adding a comment about analyzer configuration to the RFC for analyzers (https://github.com/fsharp/fslang-design/blob/main/tooling/FST-1033-analyzers.md), since that's not currently taken into account at all. Also consider using .editorconfig - the existing C# analyzer system controls all analyzer configuration through that file (which is already hierarchical), and having the same control mechanism would be easier for mixed solutions. |
I added a comment to the RFC discussion: fsharp/fslang-design#508 (comment) Regarding editorconfig: Good point. Fantomas uses it. But would it be flexible enough? It can only set a flat list of keys and values. Each analyzer would have several standard keys that should be handled by the SDK for enabling/disabling and setting the level (info/warning/error), in addition to whatever it needs itself. I'm sure we could come up with a naming scheme that handles this automatically, but I'm not convinced it's the best solution. (Not saying it isn't, though.) |
I will look more into the In any case, I thought of something more that configs should ideally support: The ability to enable a predefined set of analyzers. For example, in Flinter, many rules would apply to all F# projects and would be enabled by default, but there would be many that only apply to F# libraries, and still more that only apply to libraries intended to be consumed from other .NET languages. Ideally, users should be able to enable (per project/folder, e.g. using |
For completeness, an alternative to categories (which, by the way, Roslyn analyzers also seem to support), would be to split the analyzers into several NuGet packages. So one package for generally applicable rules, one for rules that are applicable to F# libraries, and so on. These could then be installed into different projects. (This assumes that analyzers only apply to the projects they are installed into, as mentioned in #32 (comment).) I'm not sure I like that approach; it seems a bit heavy-weight and rigid. Also, it prevents one rule from belonging to multiple categories, which might be useful (though I don't have a specific use-case in mind right now). |
For the record, Roslyn uses [*.cs]
dotnet_diagnostic.CA1822.severity = error
dotnet_analyzer_diagnostic.category-performance.severity = warning
dotnet_analyzer_diagnostic.severity = suggestion
Just like Roslyn analyzers, we should support marking files as generated code and suppressing all messages for those files: [*.MyGenerated.cs]
generated_code = true Note that configuration common to all analyzers, such as severity and generated code, should be handled by the SDK, not the individual analyzers. On another note, I think that the SDK, if possible, should respect I notice that Roslyn analyzers do this, and have a separate Regarding what I said in a previous comment:
You must of course also pass a full path to a file, not just a range (unless the Furthermore, this is also needed for any kind of non-global configuration like |
Is your feature request related to a problem? Please describe.
The end user does not have any way to control the behavior the used analyzer within a project. Just like with code linters, it would great if the user could provide options for the analyzers on a per-project basis. One use-case would be to control the strictness of an analyzer and changing the severity of certain errors from compile errors into warning.
Describe the solution you'd like
For this to work, the SDK must have a way to identify the used analyzers. One way to do that is by giving analyzers an optional name from the
Analyzer
attribute:[<Analyzer "MyAnalyzerName">]
Then with the help of ionide, the SDK would pick up a configuration file called
fsharp-analyzers.json
that has the following structure:The SDK can then read this JSON file and parse the options as
Map<string, string>
per analyzer where thisMap<string, string>
would be available through the context input of the analyzer. For example, for the "MyAnalyzerName", the options would beDescribe alternatives you've considered
N/A
Additional context
N/A
The text was updated successfully, but these errors were encountered: