Skip to content
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

New Editorconfig UX does not use the correct syntax for updating severity of IDE code style #52720

Open
mavasani opened this issue Apr 19, 2021 · 3 comments

Comments

@mavasani
Copy link
Contributor

mavasani commented Apr 19, 2021

Version Used:

Steps to Reproduce:

Change the severity of any code style option, say null propagation in the new editorconfig UX (say bump it to a warning)

Expected Behavior:
Generated editorconfig uses dotnet_diagnostic syntax for severity specification:

dotnet_diagnostic.IDE0031.severity = warning

Actual Behavior:

dotnet_style_null_propagation=true:warning

Generated syntax above is deprecated for severity specification, as the severity suffix is not respected on command line builds. Please see the Severity section under https://docs.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/language-rules#option-format and he discussion in #44201 for details.

@jmarolf As mentioned in #44201 (comment), the editorconfig UX should use the severity specification syntax that works for both in IDE and command line, but the current behavior only works in the IDE. ConfigurationUpdater component that is used in Configure Severity light bulb action already does this the correct way, and also adds a useful comment header:
image

I was really hoping we re-used the same ConfigurationUpdater component for severity/option changes in the editorconfig file from the editorconfig UX to avoid such inconsistencies. Did we end up rewriting a completely new component for updating editorconfig in the editorconfig UX?

@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged Issues and PRs which have not yet been triaged by a lead label Apr 19, 2021
@mavasani
Copy link
Contributor Author

Tagging @vatsalyaagrawal - I believe this a very critical issue and should be addressed asap. Editorconfig UX and Configure Severity lightbulb action now diverge significantly in how editorconfig gets updated and this adds confusion for the users, plus also causes different IDE gestures to configure editorconfig severity to generate different syntax which has differing behavior on command line. We need to fix this before the deprecated severity syntax starts spreading through more editorconfigs.

@Youssef1313
Copy link
Member

Duplicate of #52694

@mavasani
Copy link
Contributor Author

Thanks @Youssef1313. Agreed that technically it is duplicate. However, I still want to retain this issue as it seems we added a duplicate severity/options configuration component for Editorconfig UX, instead of re-using the one used for Configure Severity lightbulb action. Regardless of how the syntax changes are done, we should only have a single shared component between these two UI gestures to change/update editorconfig.

@jmarolf jmarolf added this to the 16.10 milestone Apr 19, 2021
@jinujoseph jinujoseph added Urgency-Soon and removed untriaged Issues and PRs which have not yet been triaged by a lead labels Apr 19, 2021
jmarolf added a commit to jmarolf/roslyn that referenced this issue Apr 26, 2021
@jinujoseph jinujoseph modified the milestones: 16.10, 16.11 Jun 1, 2021
@jinujoseph jinujoseph modified the milestones: 16.11, 17.0 Jul 16, 2021
@jmarolf jmarolf modified the milestones: 17.0, 17.1 Oct 6, 2021
mavasani added a commit to mavasani/roslyn that referenced this issue Dec 22, 2021
…ation in editorconfig

Underlying issue: dotnet#55541
Corresponding batch compiler fix: dotnet#58460
This change fixes the code that computes effective severity for Analyzers node. We also need to update the code that computes effective severity for EditorConfig UX, I will file a separate issue for the same.

NOTE: We have dotnet#52720 to track avoiding code duplication for computing effective severity between EditorConfig UX and other parts of the IDE
@jmarolf jmarolf modified the milestones: 17.1, 17.2 Jan 10, 2022
@jmarolf jmarolf modified the milestones: 17.2, 17.3 Mar 18, 2022
@arkalyanms arkalyanms modified the milestones: 17.3, 17.6 P2 Jan 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants