Update configure severity code fix for IDE code style diagnostics to use dotnet_diagnostic entries #47052
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…use dotnet_diagnostic entries Implements changes to configure severity code fix as per dotnet#44201. The code fix now does the following for IDE code style diagnostics: 1. Updates the last applicable existing code style option value based entries, i.e. `"%option_name% = %option_value%:%severity%`, to use the new severity. If no such entries are found, we do not add new code style option value based entries. 2. Updates the last applicable existing severity based entries, i.e. `dotnet_diagnostic.<%DiagnosticId%>.severity = %severity%`, to use the new severity. If no such entries are found, we add a new dotnet_diagnostic based entry with new severity. This ensures couple of things: 1. Allow build enforcement of code style diagnostics as compiler only understands dotnet_diagnostic entries. In absence of these entries, compiler will not even execute the code style analyzers on build as these are hidden diagnostics by default. 2. Consistency in severity configuration entries with third party analyzer diagnostics, which can only use dotnet_diagnostic entries. **Before**  **After**  **Existing entry**  We have follow-up work planned to have some UX to allow viewing and configuring code style option values and severities, so the additional dotnet_diagnostic severity entries in the editorconfig file themselves will not be bothersome for majority of the users.
mavasani
requested review from
jasonmalinowski,
sharwell,
jmarolf,
allisonchou,
mikadumont and
a team
mavasani
commented
Aug 21, 2020
| @@ -355,1652 +358,17 @@ public Customer() | |||
| } | |||
| } | |||
| </Document> | |||
| <AnalyzerConfigDocument FilePath=""z:\\.editorconfig"">[*.cs] | |||
There was a problem hiding this comment.
There were quite a few redundant tests in this file as @jasonmalinowski has observed the last time he touched this file. I deleted some of these duplicates but github diff seems to have royally messed it up and shows me adding large number of new tests and deleting much larger set then were actually deleted. Maybe codeflow has a better diff here?
mavasani
commented
Aug 21, 2020
| } | ||
| } | ||
|
|
||
| public class SilentConfigurationTests : CodeStyleOptionBasedSeverityConfigurationTests |
There was a problem hiding this comment.
This class was not deleted, I just trimmed the number tests inside this class to a much smaller number :-(
mavasani
commented
Aug 21, 2020
| @@ -68,6 +68,7 @@ private enum ConfigurationKind | |||
| private readonly bool _isPerLanguage; | |||
| private readonly Project _project; | |||
| private readonly CancellationToken _cancellationToken; | |||
| private readonly bool _addNewEntryIfNoExistingEntryFound; | |||
There was a problem hiding this comment.
This is the only product file changed in this PR.
There was a problem hiding this comment.
@allisonchou I would really appreciate your review on this file. Thanks.
allisonchou
approved these changes
Aug 24, 2020
|
Thank you @allisonchou! |
ghost
modified the milestones:
mavasani
added a commit
to mavasani/roslyn
that referenced
this pull request
Oct 14, 2020
Contributes towards dotnet#44201 - All code style option editorconfig entries can now be specified as `option_name = option_value` without `:severity` suffix - `Configure code style option` code fix will no longer add add `:severity` suffix for new entries. For fix that changes existing entries which have `:severity` suffix, it will retain the existing severity suffix. Note that `Configure severity` code fix was already switched to use `dotnet_diagnostic.RuleID.severity` entries for code style diagnostics in dotnet#47052. Once we have the editorconfig UX to change code style option value and severity, we can safely deprecate the `:severity` suffix in option value syntax. All our UI gestures will use the `option_name = option_value` syntax for code style value changes and `dotnet_diagnostic.RuleID.severity = severity` syntax for severity changes and users do not need to deal with these manually editing or understanind these different syntaxes or mapping rule IDs to option names and vice versa for code styles.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Implements changes to configure severity code fix as per #44201. The code fix now does the following for IDE code style diagnostics:
"%option_name% = %option_value%:%severity%, to use the new severity. If no such entries are found, we do not add new code style option value based entries.dotnet_diagnostic.<%DiagnosticId%>.severity = %severity%, to use the new severity. If no such entries are found, we add a new dotnet_diagnostic based entry with new severity.This ensures couple of things:
Before
After (no existing entry)
After (with existing entry)
We have follow-up work planned to have some UX to allow viewing and configuring code style option values and severities, so the additional dotnet_diagnostic severity entries in the editorconfig file themselves will not be bothersome for majority of the users.