Roslyn: Deprecate 'severity' field for IDE code style editorconfig syntax

Created on 13 May 2020  路  9Comments  路  Source: dotnet/roslyn

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.

Area-IDE Concept-Design Debt IDE-CodeStyle

Most helpful comment

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.

All 9 comments

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:

  1. 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.
  2. 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:

  1. Switch all the rules in the CodeStyle NuGet package to be warnings by default OR
  2. Recommend users who do not want to use dotnet_diagnostic.RuleId.severity based 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?

Was this page helpful?
0 / 5 - 0 ratings