Deprecate 'severity' field for IDE code style editorconfig syntax
Current IDE code style options support the following editorconfig syntax:
option_name = value:severity
For example:
[*.cs]
dotnet_style_qualification_for_field = true:warning
Command line compiler supports configuring severity of a specific diagnostic ID with the following editorconfig syntax:
dotnet_diagnostic.RuleID.severity = severity
This former syntax (option_name = value:severity) allows configuring both code style option value and severity. However, the severity setting from this syntax is only respected when executing the corresponding analyzer in the IDE live analysis. This syntax is not recognized by the command line compiler and/or the analyzer driver, so the severity setting above is redundant when executing from an explicit command line build. Additionally, with the command line compiler optimization implemented in https://github.com/dotnet/roslyn/pull/43546, we no longer execute hidden/suggestion analyzers in command line builds, unless the user has explicitly specified the analyzer diagnostic ID as a warning or error with the dotnet_diagnostic.RuleID.severity = severity syntax. This essentially means that the only way to enforce an IDE code style rule on command line build from our CodeStyle NuGet package is by using the latter compiler recognized syntax. Given this behavior, we should deprecate the former syntax and only allow option_name = value in the code style option syntax, and require the latter syntax for severity configuration.
Tag @mikadumont @sharwell
Design Review
- No object to the proposal
- Suggestion to enhance MadsK extension to generate the new form and have code fix to migrate existing configuration to the new form.
I'm a bit confused with this change:
There's no documentation about diagnostic ids and corresponding rules now.
Even if there is one, splitting rule preference and severity into two places is not friendly.
I cannot work properly now without MadsK's extension. Please consider to enhance first class IDE support for editorconfig.
Ideally, I want a similar experience with IntelliSense of project files: it shows available diagnostic ids and their explanations.
Further more, you may create a GUI of it (reusing the parts from VS options). JetBrains Rider is already doing so.
Tagging @mikadumont - yes, we understand the concerns, but allowing controlling severity for each option, and then mapping it to severities of specific diagnostic ID(s) is difficult to implement in the compiler.
As you mentioned, I think the best option is to provide a GUI for editorconfig to allow users to get the experience of controlling severity for each code style without having to be affected by how these are represented in the underlying editorconfig, which can still have separate entries for severity and code style value.
Have I understood correctly that the proposed solution is to go with a separate setting for the severity, while using a rule ID to refer to it? The rule ID is opaque and requires either manual lookup in documentation or a UI to do the lookup for you. That seems to be a regression in usability and authoring of .editorconfig files. It also makes the .editorconfig a lot more verbose and less terse.
Current Solution
[*.cs]
dotnet_style_qualification_for_field = true:warning
Proposed Solution
[*.cs]
dotnet_style_qualification_for_field = true
dotnet_diagnostic.RuleID.severity = warning
@RehanSaeed The proposed solution is:
- Accept the fact that it is not possible or scalable for any person to remember either the code style names, IDE diagnostic rule IDs and/or the exact .editorconfig syntax for code style or severity configuration entries.
- Invest in an Editorconfig UI for users to do any style or severity configuration settings, on the same lines as the UI available in Tools/Options for VS wide per-user code style settings.
Actually, I think we might have couple of ways to get around not deprecating the code style syntax for severity setting:
- Switch all the rules in the CodeStyle NuGet package to be warnings by default OR
- Recommend users who do not want to use
dotnet_diagnostic.RuleId.severitybased severity configuration to follow the suggestion at https://github.com/dotnet/roslyn/issues/33558#issuecomment-660102320 to default all rules in the package to be warnings by default.
In both these cases, all rules get turned on for build as warnings and once that is done, both the syntaxes for severity setting should work fine to change the severity or disable rules with subsequent editorconfig entries.
https://developercommunity.visualstudio.com/content/problem/1116079/editorconfig-improvement.html shows an example for a user who contradicts the claim made by @RehanSaeed that configuring the severity of code style options by code style value name is easier to remember as compared to using diagnostic IDs. They prefer the other way.
I think this shows that @jmarolf's work towards a UI to help users view and configure styles and severities without worrying about the underlying syntax is extremely important.
Even though I have set dotnet_analyzer_diagnostic.severity = warning in VS 16.8, I seem to have lost some warnings which I can only get back by setting a specific severity using the old style:
csharp_using_directive_placement = inside_namespace:warning
Am I correct in thinking that I need to wait for VS 16.9 to completely switch over?
I can confirm that using vs2019 16.8.4 with sdk 5.0.102 the (soon...but how soon?) deprecated syntax often is necessary to see warnings (and avoid analyzer exceptions, in some rare cases: Microsoft.CodeAnalysis.CSharp.Diagnostics.AddBraces.CSharpAddBracesDiagnosticAnalyzer threw an exception of type System.ArgumentNullException with message 'Value cannot be null):
csharp_prefer_braces = when_multiline:warning
So when should the syntax be switched over?
@maxild Starting 16.9, all code style options can be written without severity suffix.
For backward compatability I have been using both forms for setting severity. What version of VS will the older [rule] = [value]:[severity] syntax no longer work? When do you recommend to make the switch?
@maxild Starting 16.9, all code style options can be written without severity suffix.
@mavasani With 16.9 being released today, I have tried to use the simpler syntax, and it appears that it does not work. Did this change not make it into 16.9 after all?
IDE1006 takes care of all naming rules. Unfortunately you have to specify all properties (including severity) of a naming rule to activate a rule. You are now forced to set severity for each naming rule to default to get the configured behavior of IDE1006. This confused me, but now it's working.
How will this work when multiple code style options map to 1 rule? E.g. if I have:
csharp_style_var_for_built_in_types = false:suggestion
csharp_style_var_when_type_is_apparent = false:none
csharp_style_var_elsewhere = false:warning
My understanding is that false here maps to IDE0008 ("Use explicit type instead of var") but with setting severity on the rule I can no longer have different severity based on the conditions that the code style options was targeting?
Good question @vikekh! I really hope that we won't loose those separate severity settings.
@mavasani What is the latest on this? Which version of VS can we expect to use the new syntax? Also, how will this https://github.com/dotnet/roslyn/issues/44201#issuecomment-1333773598 work?
Given the customer feedback, we decided to not deprecate the :severity syntax, but instead respect it now. See https://github.com/dotnet/roslyn/issues/52991.
Closing this issue as Won't fix.