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

Upgrade to new Severity Syntax for VS 16.9 #46

Open
RehanSaeed opened this issue Jan 18, 2021 · 13 comments
Open

Upgrade to new Severity Syntax for VS 16.9 #46

RehanSaeed opened this issue Jan 18, 2021 · 13 comments
Labels
enhancement Issues describing an enhancement or pull requests adding an enhancement.

Comments

@RehanSaeed
Copy link
Owner

RehanSaeed commented Jan 18, 2021

See dotnet/roslyn#44201 (comment)

We currently use both the old [rule] = [value]:[severity] syntax and the new [rule].[RuleId].[severity] syntax side by side. After 16.9 the old syntax should not be needed anymore but we should still probably keep it around for backward compatability.

@RehanSaeed RehanSaeed added the enhancement Issues describing an enhancement or pull requests adding an enhancement. label Jan 18, 2021
@maxild
Copy link

maxild commented Jan 19, 2021

I have been working/following the code analysis .editorconfig/.globalconfig based changes in the .NET 5 SDK where code styles and code quality have been built into the sdk to be more cross-plat, and found your repo.

I am sure you already know, but just to be precise...

I guess what you mean is the following

# We currently use both the old
#       csharp_prefer_braces = when_multiline:warning
# syntax and the new
#       csharp_prefer_braces = when_multiline
#       dotnet_diagnostic.IDE0011.severity = warning
# syntax (side-by-side, mixed, ??) as in
#       csharp_prefer_braces = when_multiline:warning
#       dotnet_diagnostic.IDE0011.severity = warning
# After 16.9 the old syntax should not be needed anymore
# but we should still probably keep it around for backward compatability.
# See also https://github.com/dotnet/roslyn/issues/44201#issuecomment-761672157

This should probably be put into the .editorconfig, such that people hand-editing the file will know.

@RehanSaeed
Copy link
Owner Author

RehanSaeed commented Jan 19, 2021

Yes that's correct.

We should probably add a comment to that effect. I was hoping that we could transition to the new syntax by now so we wouldn't need to but that seems to have been delayed to VS 16.9. I'm also trying to understand when the old syntax will be deprecated but don't have an answer to that yet.

@lonix1
Copy link

lonix1 commented Jan 19, 2021

Are the severities in your editorconfig based on your personal preference, or an official MS source?

They publish a "default" set of severities for the code analysis rules, but I haven't found one for the code style rules.

So I'm wondering how you decided on the severities for your config?

@RehanSaeed
Copy link
Owner Author

RehanSaeed commented Jan 19, 2021

We default to warning in all cases except very few where making it a hard requirement may make the code worse. We should probably review them. Using warnings has the advantage of giving developers the ability to use TreatWarningsAsErrors.

@lonix1
Copy link

lonix1 commented Jan 19, 2021

Yeah I thought as much. It's unfortunate they don't publish an "official" set like they do for code analysis rules.

Defaulting to warning probably makes sense. Having a GUI tool like you mentioned in that other thread, makes even more sense. Something like those online gitignore tools would be cool. I'd do that myself if I had the time 😞

@maxild
Copy link

maxild commented Jan 19, 2021

They publish a "default" set of severities for the code analysis rules, but I haven't found one for the code style rules.

The SDK uses global analyzer config files also distributed in the nupkg

dotnet/roslyn#49250 (comment)

@maxild
Copy link

maxild commented Jan 19, 2021

You have to use microsoft.codeanalysis.csharp.codestyle.3.9.0-xxxxxx preview builds to get those files for explicit/recommended code style (IDExxxx) severity settings from there CI-feed...It is experimental, and will probably first come in .NET 6 SDK?

I would argue that it is very difficult to enforce any styles for all projects. But with AnalysisMode/AnalysisLevel the SDK is trying. Therefore any editorconfig (also https://github.com/RehanSaeed/EditorConfig/blob/main/.editorconfig) is just a starting point.

Wish dotnet was more like GO (gofmt) wrt code styles.

@maxild
Copy link

maxild commented Jan 19, 2021

Using warnings has the advantage of giving developers the ability to use TreatWarningsAsErrors.

This will only work probably when 16.9 ship dotnet/roslyn#43051 (comment)

@lonix1
Copy link

lonix1 commented Jan 20, 2021

@maxild

You have to use microsoft.codeanalysis.csharp.codestyle.3.9.0-xxxxxx preview builds to get those files for explicit/recommended code style (IDExxxx) severity settings from there CI-feed

I checked this nupkg but it only has code quality rules. There aren't code style rules in there.

Am I looking at the right package?

Wish dotnet was more like GO (gofmt) wrt code styles

YES!!! Code style is such a waste of time, gofmt does it right.

@maxild
Copy link

maxild commented Jan 22, 2021

@RehanSaeed
Copy link
Owner Author

RehanSaeed commented Aug 9, 2021

Tested migrating from [rule] = [value]:[severity] syntax to [rule].[RuleId].[severity] in VS 16.10 and it still doesn't seem to work completely. At some point the old syntax will be deprecated (See dotnet/roslyn#44201). Not sure when that will be.

Raised #57 to fix this. Won't merge until issues are resolved.

@RehanSaeed
Copy link
Owner Author

Raised dotnet/roslyn#55542

@RehanSaeed
Copy link
Owner Author

Fixed in #75

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Issues describing an enhancement or pull requests adding an enhancement.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants