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

Relation between ruleset, editorconfig rules and VS Code Style #19618

Open
GerjanOnline opened this issue May 18, 2017 · 23 comments
Open

Relation between ruleset, editorconfig rules and VS Code Style #19618

GerjanOnline opened this issue May 18, 2017 · 23 comments

Comments

@GerjanOnline
Copy link

GerjanOnline commented May 18, 2017

Can somebody explain me what the vision/direction is regarding rulesets, editorconfig rules and VS Code Styles for C# projects?

  1. Rulesets
    image

  2. Editor config
    image

  3. VS Code Style
    image

Some questions about this:

  1. How do they relate to each other?
  2. Which one will override the other one?
  3. Can they exist next to eachother (union set)?
  4. Do they update each other (if I update a VS Code Style, will it update my ruleset?)
  5. Where are VS Code Styles stored?

I have read but i could not find anything
https://blogs.msdn.microsoft.com/dotnet/2016/12/15/code-style-configuration-in-the-vs2017-rc-update/

Personally, I find editorconfig rules still very buggy and limited, but if MS says it will be expanded then maybe I should pick this.

I am now in a position to start some new big projects but I am not sure which one I should pick, can somebody advice me in this?
I want a file that I can share (Git) with all my team members for code style and conventions.

@sharwell
Copy link
Member

The information I'm adding here is based on the designs we're planning to implement, but some items may not be working right now. It is also subject to change based on user feedback / problems / etc.

How do they relate to each other?

They basically don't, since none of the three can directly reference the existence of any other of the three.

Which one will override the other one?

For any given setting, it is taken from the first of the following in which it is available:

  1. A rule set file
  2. An EditorConfig file
  3. Visual Studio options (ignored when running a build)
  4. Default values

Can they exist next to eachother (union set)?

Yes, with the caveat being Visual Studio settings only take effect when working in the IDE, and not counting builds within the IDE.

Do they update each other (if I update a VS Code Style, will it update my ruleset?)

No. I strongly encourage you to try out whatever the future experience is as soon as you hear about it, think about ways in which you would want this to work, and get a bug filed. In one way or another I would like to see it made easy to go from Visual Studio options to shared options (EditorConfig).

Where are VS Code Styles stored?

"Magic happens" (aka I'm not exactly sure and don't want to give you bad info)

Personally, I find editorconfig rules still very buggy and limited, but if MS says it will be expanded then maybe I should pick this.

I am now in a position to start some new big projects but I am not sure which one I should pick, can somebody advice me in this?
I want a file that I can share (Git) with all my team members for code style and conventions.

EditorConfig is the solution we are driving for. We're actively working to make it a solution across the board (inside the IDE, during IDE and command line builds, and even for third-party analyzers). It doesn't do everything yet, and there are known bugs (and bug fixes), but it's unlikely for this to be anything other than our primary focus moving forward for options.

@Pilchie
Copy link
Member

Pilchie commented May 18, 2017

Tagging @kuhlenh too.

@Pilchie Pilchie added this to the 15.later milestone May 18, 2017
@jmarolf
Copy link
Contributor

jmarolf commented May 23, 2017

@GerjanOnline

Where are VS Code Styles stored?

All settings that are set in the IDE are serialized out as XML whenever an option is changed. This XML file is saved to %USERPROFILE%\Documents\Visual Studio 2017\Settings
If you go to Tools -> Import and Export Settings in Visual Studio you can save the XML file representing your current VS settings to the location of your choice. You can also import the XML file with these setting if you want.
In roslyn this serialization happens in the ambiguously named AutomationObject class.

@jinujoseph jinujoseph modified the milestones: 15.6, Unknown Nov 3, 2017
@nvankesteren
Copy link

nvankesteren commented Feb 9, 2018

I do not understand why having an explicit rule in editorconfig to turn a code style check ON be completely overruled by a ruleset that has the same code style check set to NONE. I expect the latter rule to be DISABLED from checking anything (so the editorconfig setting is 'the one that checks the rule'), but that is not the case. So, having a rule in a ruleset set to NONE seems to mean 'suppress this check' instead of 'skip this check'.

Having editorconfig, rulesets and R# code style settings cancel each other out is not a great user experience...

@sharwell
Copy link
Member

sharwell commented Feb 9, 2018

I do not understand why having an explicit rule in editorconfig to turn a code style check ON be completely overruled by a ruleset that has the same code style check set to NONE.

Something needed to come first. At the time the decision needed to be made, only .ruleset files were considered in the lowest layer of the compiler. Placing .ruleset files with the highest priority means the IDE behavior and the compiler behavior are consistent.

Normally if someone wants to configure a rule with .editorconfig, they omit that rule from the .ruleset file. Is there a reason why this strategy will not work for you?

So, having a rule in a ruleset set to NONE seems to mean 'suppress this check' instead of 'skip this check'.

I'm not sure I understand this part. Can you clarify?

@nvankesteren
Copy link

Yes. So I have this rule in my editorconfig
dotnet_style_qualification_for_method = true:warning
which is being used across editors and across different projects.

I also have a ruleset that applies to my current project, containing -for example- the StyleCop.Analyzers ruleset. Since the current state of the code throws lots of warnings, I decide to disable some part of the rules in the ruleset, thinking that if I turn a rule OFF (by using the checkbox) or setting it to Action = None the rule is not checked within the context of the code analysis tool I use. Turning off a lot of rules (including SA1101) without looking, I'm assuming the editorconfig covers the "basics" (4 spaces instead of tabs, and some c# specific rules) and only the active rules of the ruleset (Info, Warning, Error) cover the rest.

That assumption is incorrect. You have to manually go through all the rules in the ruleset and activate those rules if you have them turned on in the editorconfig as well, otherwise they are not shown. So having an editorconfig file with c# rules is useless when you also happen to have a ruleset with the same check. It will always take the latter, no matter how that rule in the ruleset is set.

I don't know, it breaks the principle of least astonishment for me.

@sharwell
Copy link
Member

sharwell commented Feb 9, 2018

Ah, the issue with SA1101 (Prefix local calls with this) is it appears to align with the .editorconfig settings dotnet_style_qualification_for_*, but in reality the two are unrelated features. The latter settings are actually used to configure an analyzer that produces IDE0009.

At first glance, it's strange to have two rules that do the same thing. However, there is a subtle difference here that we're working hard to resolve. Since you installed StyleCop Analyzers via a NuGet package, SA1101 works both inside the IDE and during your builds. The IDE analyzers, on the other hand, are shipped with Visual Studio in the same form as a Visual Studio extension - we refer to these as "VSIX-installed analyzers" even though you didn't manually install anything. VSIX-installed analyzers run inside the IDE, but have no impact on the build.

Combining all of the above, you ended up with this situation when you disabled SA1101:

  • The StyleCop Analyzers enforcement of this. did not work because SA1101 was disabled in the rule set file
  • The IDE0009 enforcement of this. did not work because as a VSIX-installed analyzer (with no NuGet option available), it doesn't support builds at this time

@nvankesteren
Copy link

Thanks for the additional info, that does help a bit! I'm using ReSharper as well, that does make things more confusing when some team members have it and some do not. "Where does this warning come from and why do you not see it at all?" ;-)

But I do have to wonder why the active ruleset editor has to be so confusing. What's with the rules with a 'WithoutSuggestion' postfix? When I have a clean ruleset file (only <RuleSet ...></RuleSet> with no rules), why are some rules still active when I open this active ruleset in the ruleset editor? ;-)

@sharwell
Copy link
Member

sharwell commented Feb 9, 2018

What's with the rules with a 'WithoutSuggestion' postfix?

Can you file a separate issue for this? Better chance it can be improved. 😄

@prlcutting
Copy link

@sharwell

About 18 months ago you made the following statement:

EditorConfig is the solution we are driving for. We're actively working to make it a solution across the board (inside the IDE, during IDE and command line builds, and even for third-party analyzers). It doesn't do everything yet, and there are known bugs (and bug fixes), but it's unlikely for this to be anything other than our primary focus moving forward for options.

I think moving to EditorConfig makes a lot of sense and supports a flexible and powerful scheme of inherited and overridden settings that can be version controlled and easily shared across team members. However, not enforcing settings defined in EditorConfig files during a CI build is a deal breaker for us. The following notes was from the referenced link:

Conventions that are set in an EditorConfig file cannot currently be enforced in a CI/CD pipeline as build errors or warnings. Any style deviations appear only in the Visual Studio editor and Error List.

Can you please provide a status update and estimated timeframe when EditorConfig files can be used to define our StyleCop settings for use both in the Visual Studio IDE and CI builds in Azure DevOps? Thanks.

@sharwell
Copy link
Member

sharwell commented Nov 15, 2018

@prlcutting We're in the process of testing FormattingAnalyzer, which is the first of the .editorconfig IDE analyzers moved down to a NuGet package for use in CI. However, keep in mind that the IDE analyzers are not intended to provide exactly the same set of rules or features as StyleCop. If you are specifically looking for StyleCop enforcement, I would recommend looking at StyleCop Analyzers which already works in CI builds.

@prlcutting
Copy link

@sharwell Thanks for the feedback. We actually are already using StyleCop Analyzers for our IDE and CI build validation. However, these are based on ruleset files as I understand it rather than EditorConfig files, which means we can't have a hierarchical/inherited/overridden group of settings like we could with EditorConfig. Apologies if there's something fundamental I'm missing here.

Can you please elaborate on (or point me to some information) explaining why the IDE analyzers and StyleCop should be different? I would think we would want them to be the same. My thinking is that we want developers to catch things during local development and build, with the CI build as a backstop if they weren't caught locally.

Thanks for your time and help.

@dobryanskyy
Copy link

dobryanskyy commented Dec 29, 2018

@sharwell This question is bothering me as well. I am currently in progress of creating common stylyguide for my company and am a bit confused on how to enforce these rules. Should I use editorconfig or StyleCop Analyzers? Especially if I want to see this solution working in the future. And of course I want this to be working on CI/CD(at least in the future, if not now)

@sharwell
Copy link
Member

sharwell commented Jan 2, 2019

@dobryanskyy StyleCop Analyzers is an independent project. Like any other open source project developed by unpaid volunteers, part of determining its suitability for use will involve deciding if its ongoing maintenance meets your organizations expectations.

I recommend that all teams working with Visual Studio 2017+ include a .editorconfig file in their repository defining at least the code formatting options used by the repository, since this ensures the Format Document command in Visual Studio will work the same way for all users, even if some users have customized formatting in Tools → Options. In the past, many users create their initial .editorconfig file by copying the one in dotnet/corefx or dotnet/roslyn and customizing it as necessary. Starting with Visual Studio 2019, it is possible to create a .editorconfig file directly from the Code Style page in Tools → Options.

@dobryanskyy
Copy link

Thanks a lot. I will probably share the editorconfig file between different projects/solutions via nuget package. I intend to this the same way as here. You think it's a good idea?

@sharwell
Copy link
Member

sharwell commented Jan 2, 2019

@dobryanskyy As far as I know, .editorconfig configurations cannot be shared via NuGet packages. #19028 is a feature request to add support for this in some way, but I don't believe work has started on that yet.

@dobryanskyy
Copy link

@sharwell - I know. The solution I proposed is kind of hand-made.

@Bartolomeus-649
Copy link

This is a mess!
Documentation is all over the place, contradicting and extremely confusing.

  1. Where is the valid, correct, consistent documentation on how you are supposed to use Analyzers in brand new Visual Studio Team Services .NET Core Solution with multiple C#-based projects and a team of developers who should all have the same settings and where the Analyzers run when building locally and on the Team Services build servers?

  2. What method should you use to find out which Microsoft C# Analysers exist, and which of them should be used with which types of projects?

  3. Where can you find documentation on which analyzers are recommended, and best practice for using them for .NET Core in Visual Studio 2019 with Team Foundation Services?

  4. Which method should be used to figure out, assert and verify which analyzers are actually used and where they "are"?

  5. Which method should be used to figure out the exact and unambiguous source of an error, warning or info message that show up either in the build output log, the Error List Window in Visual Studio, or anywhere else in VS for that matter?

  6. Which method should be used to figure out the exact and unambiguous source of any refactoring suggestion and where to configure it?

  7. What method should be used to figure out what type an analyzer is? Like if it is a roslyn analyzer, a static analyzer or something else? For example, what is a FxCop, the documentation say "... FxCop static analysis ..." and ".. [Static code analysis is not supported for .NET Core](Static code analysis is not supported for .NET Core ) ..." while the NuGet package description say "Microsoft FxCop rules implemented as analyzers using the .NET Compiler Platform (Roslyn)." and you don't get any build errors with it in a .NET core project.


@Bartolomeus-649
Copy link

@sharwell , @jmarolf , @Pilchie , This is still a mess....any progress?

@JadaVonRuth
Copy link

Unfortunately editorconfig settings are being ignored when there is ruleset defined. Still true for Visual Studio 2019 16.4.3 and project in .NET 4.7.2, in .NET core it seems to be working fine

@sharwell
Copy link
Member

@JadaVonRuth The order of evaluation is given in #19618 (comment). If the severity of a diagnostic is set in a rule set file, that value will override the severity (if any) which is set in the .editorconfig file.

@JadaVonRuth
Copy link

JadaVonRuth commented Jan 17, 2020

Today I have just submitted a bug report to visual studio community with recording where it shows editorconfig is completely ignored despite there is no ruleset defined. Only thing that was able to change analyzer rules default severity was the ruleset. I believe editorconfig should have higher priority since it can cover much more - all coding styles and naming conventions for all kinds of formats and ruleset only analyzer rules.

@MrFoxPro
Copy link

MrFoxPro commented May 2, 2020

This is really weird thing. Lack of one settings configuration causes messing all configurations. This casues problems in other ides. I think the best solution for this is to create one convention for configuration and UI for creating rules, converting old rulesets to new-style config. (last one could be created and maintained by community)

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

No branches or pull requests