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

Make all diagnostic severities configurable #1036

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

walterlv
Copy link

@walterlv walterlv commented Aug 30, 2019

Fixes #47 , #524 , #543 .

What I am doing?

Visual Studio analyzer extensions can be installed as a vsix extension and a NuGet package. Anyone can config the diagnostic severities for both the vsix extension and the NuGet package but he has to add a custom .EditorConfig file or a custom ruleset file into the project repository.

Some of the repositories are not ourselves' so we can't add any preference files into the public repositories. But if we want to config the severities for the code-cracker analyzers, what can we do? I find that we can do nothing to solve such an issue. Only we can do is to make the code-cracker analyzers configurable globally.

So I make this pull request to extract all the diagnostic severities into one manager class and read the user's ruleset file from a global path.

Current progress

  • Create a configuration class to manage all the severities for all the analyzers.
  • Read the user's custom ruleset file to apply the configured rulesets.

Problems

I'm stuck because I don't know how to read a file from a global file path in a portable project. I only know how to do it in a .NET Standard project.

@carloscds
Copy link
Contributor

@walterlv Can you explain benefits of this big change ?

@walterlv
Copy link
Author

walterlv commented Sep 3, 2019

@carloscds I've extracted all the Diagnostic Severities into one configuration class in this pull request. Then we can load a custom ruleset file later to use the user's preference. This makes the custom diagnostic severities work well as a Visual Studio extension not only as NuGet packages.

@sharwell
Copy link
Contributor

sharwell commented Sep 3, 2019

I'm super confused. Unless you are using WellKnownDiagnosticTags.NotConfigurable (which the analyzers here do not appear to be doing), the severities are already configurable without this change.

@sharwell
Copy link
Contributor

sharwell commented Sep 3, 2019

@carloscds I don't think this change is needed. If you reach a point where you do not agree with this, please reach out to me so we can make sure it's not just a failure on our part to document the analyzer behaviors. 😄

@walterlv
Copy link
Author

walterlv commented Sep 3, 2019

@sharwell As a NuGet package, the code-cracker works well and the MSBuild automatically use the user's ruleset file to configure our diagnostic severities. But as a Visual Studio extension, the MSBuild does nothing for these analyzers so we need to configure the severities ourselves. I change these codes only for the configuration when we use it as a Visual Studio extension.

@carloscds
Copy link
Contributor

@sharwell I agree with you, code is very complex after this change.

@sharwell
Copy link
Contributor

sharwell commented Sep 3, 2019

@walterlv After this change, exactly how would one configure the severity inside Visual Studio? Currently ruleset files work with VSIX-installed analyzers, and starting with 16.3 you can use .editorconfig to set the severity for VSIX-installed analyzers. Neither of these approaches requires changes to the implementation code.

@walterlv
Copy link
Author

walterlv commented Sep 3, 2019

@sharwell you still don't get the main reason why I should do this! How can I add .EditorConfig or ruleset for all projects? Some of them are even not mine.

I can't add a code-cracker only ruleset for Microsoft GitHub projects, or for other team's projects. So there are actually no available custom ruleset for vsix-installed analyzers.

If I can’t configure the severities globally, I have to ruin all projects with my custom ruleset files. Then no pull requests will be merged in any team's GitHub projects because no one will accept a private preference file that they don't even know what it is!

@walterlv
Copy link
Author

walterlv commented Sep 3, 2019

By the way, I can't find the description that the global ruleset will work in 16.3 preview.

Visual Studio 2019 Preview Release Notes

@walterlv
Copy link
Author

walterlv commented Sep 3, 2019

I should add the ruleset file loading code for this pull request so that the user can change the severities globally.

@walterlv
Copy link
Author

walterlv commented Sep 4, 2019

@sharwell The core project is a portable project so I can't generate a configuration file path. How can I read the custom user's ruleset in codes only for this analyzer extension if I can't find the configuration path?

@sharwell
Copy link
Contributor

sharwell commented Sep 5, 2019

@walterlv From your explanation, it makes sense why you can't use .editorconfig or ruleset files. The part which is still not clear to me is which alternative you are planning to use if this is merged.

@walterlv
Copy link
Author

walterlv commented Sep 6, 2019

@sharwell Thanks for your reply. From the description of this pull request, you can find that I plan to read a custom ruleset file from file system but I don't know how to do it in a portable project. Could you give me some suggestions?

@jwooley
Copy link
Contributor

jwooley commented Oct 2, 2019

@sharwell I see that 16.3 includes support for configuring the rules via .editorconfig. Is there design documentation available on how this works that are public? If it can pick up an .editorconfig from the solution root, when installing the tools as a vsix? would that solve the issue that @walterlv is trying to accomplish. I would be hesitant to make this large of a change if the tooling is moving towards embracing .editorconfig more fully.

@sharwell
Copy link
Contributor

sharwell commented Oct 2, 2019

Is there design documentation available on how this works that are public?

https://github.com/dotnet/roslyn/blob/master/docs/compilers/analyzer-config.md

If it can pick up an .editorconfig from the solution root, when installing the tools as a vsix?

Yes, it would pick them up like this in 16.3.

@walterlv
Copy link
Author

walterlv commented Oct 4, 2019

@jwooley I've posted an opinion above that .editorconfig is not a right solution:

If I can’t configure the severities globally, I have to ruin all projects with my custom ruleset files. Then no pull requests will be merged in any team's GitHub projects because no one will accept a private preference file that they don't even know what it is!

This means that I can't put an .editorconfig file on every solution repositories.

@sharwell
Copy link
Contributor

sharwell commented Oct 4, 2019

@walterlv you can place it in the parent folder of the solution (outside/above the source control directory) and it will still get picked up.

@walterlv
Copy link
Author

@sharwell Thanks.

I've tried multiple .editorconfig files both inside and outside the git repository. I find that both of them work. So I can prepare a nearly global .editorconfig file to my drive partition root directory.

This helps me instead of config it inside a settings UI like below:

image

@walterlv
Copy link
Author

I've recommended Code Cracker via my blog post:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow user to download custom combination of analyzers and refactorings
4 participants