In #11916 my intention is to force Roslyn analyzers at build time to exclude regressions in code formatting and code style.

Here you add analyzers but not enable/activate them for build or design time (otherwise the build would fail). Also we already have editorconfig file - why do we need the ruleset. I am not sure that we need this in the time.

It would great if you grabbed #11916 and improved it.

@iSazonov yes this PR has a similar intention as #11916, but review should be easier 😛

Please see 6acc529 for proof analyzers are activated, CA1001 ia marked as warning which causes build failure:
https://dev.azure.com/powershell/2972bb5c-f20c-4a60-8bd9-00ffe9987edc/_apis/build/builds/54691/logs/18

The CodeAnalysis.ruleset contains 211 rules, I think it makes sense to keep these seperate from the .editorconfig

@xtqqczze Thanks for clarify! I see the behavior of VS was changed since 16.5. But It is not clear has EditorConfig a priority on CodeAnalysisRuleSet?

@xtqqczze Please have a look at the failing builds.

@xtqqczze Could you please check with CA1001 that EditorConfig has a priority over CodeAnalysisRuleSet?

@iSazonov 2c736cb shows EditorConfig has a priority over CodeAnalysisRuleSet

In the case I suggest to use default ruleset and do all customizations in EditorConfig.

This pull request has been automatically marked as Review Needed because it has been there has not been any activity for 7 days.
Mainainer, Please provide feedback and/or mark it as Waiting on Author

@xtqqczze Can respond to @iSazonov comment

In the case I suggest to use default ruleset and do all customizations in EditorConfig.

In the case I suggest to use default ruleset and do all customizations in EditorConfig.

I think using Ruleset only makes more sense than specifiying default rules in Ruleset and overriding some rules in EditorConfig.

Currently there are issues with using EditorConfig to configure diagnostics, for example TreatWarningsAsErrors is ignored: dotnet/roslyn#43051.

@iSazonov I think we should use a Ruleset until EditorConfig support for diagnostics is mature.

@xtqqczze It seems the dotnet/roslyn#43051 issue consider a specific scenario to have an exclusion list of warnings. It is not our case. We use TreatWarningsAsErrors to force all warning as errors.

This pull request has been automatically marked as Review Needed because it has been there has not been any activity for 7 days.
Maintainer, please provide feedback and/or mark it as Waiting on Author

@xtqqczze I see MSFT updated documentation.
https://docs.microsoft.com/en-us/visualstudio/code-quality/roslyn-analyzers-overview?view=vs-2019#build-errors

Create a .NET 5.0 project which includes analyzers by default in the .NET SDK. Code analysis is enabled, by default, for projects that target .NET 5.0 or later. You can enable code analysis on projects that target earlier .NET versions by setting the EnableNETAnalyzers property to true.

So we already got this.

https://docs.microsoft.com/en-us/visualstudio/code-quality/use-roslyn-analyzers?view=vs-2019#convert-an-existing-ruleset-file-to-editorconfig-file

Starting in Visual Studio 2019 version 16.5, ruleset files are deprecated in favor of the EditorConfig file for analyzer configuration for managed code. Most of the Visual Studio tooling for analyzer rule severity configuration has been updated to work on EditorConfig files instead of ruleset files. EditorConfig files allow you to configure both analyzer rule severities and analyzer options, including Visual Studio IDE code style options. It is highly recommended that you convert your existing ruleset file to an EditorConfig file. It is also recommended that you save the EditorConfig file at the root of your repo or in the solution folder. By using the root of your repo or solution folder, you make sure that the severity settings from this file are automatically applied to the entire repo or solution, respectively.

I think we already got this too and we should do nothing to evolve Roslyn Analyzer. We can check this by forcing some rule in our EditorConfig file. Can you check and confirm?

https://github.com/dotnet/roslyn-analyzers#microsoftcodeanalysisnetanalyzers

Microsoft.CodeAnalysis.NetAnalyzers
This is the primary analyzer package for this repo that contains all the .NET code analysis rules (CAxxxx) that are built into the .NET SDK starting .NET5 release. The documentation for CA rules can be found at docs.microsoft.com/visualstudio/code-quality/code-analysis-for-managed-code-warnings.

So We have no need to reference the package.

Please also see https://docs.microsoft.com/en-us/dotnet/fundamentals/code-analysis/overview#code-style-analysis
about EnforceCodeStyleInBuild. I think we could add this.

Code analysis in .NET

Learn about source code analysis in the .NET SDK.

So We have no need to reference the package.

I have removed the PackageReference, and pushed commit be5c5c3 which proves that a change in the ruleset will break the build.

• In VS2019 16.8, you should be able to workaround this issue by using global analyzer configs as per TreatWarningsAsErrors ignored when severity set in EditorConfig dotnet/roslyn#43051 (comment).
• In VS2019 16.9 Preview1 and later, the original reported issue should be resolved and you can use editorconfig file itself and /warnaserror will escalate diagnostics configured as warnings in editorconfig to be errors.

Originally posted by @mavasani in dotnet/roslyn#43051 (comment)

I have tested EditorConfig configuration with TreatWarningsAsErrors and it is still not working with VS2019 16.8.0 Preview 5.0. The alternative is to use a global AnalyzerConfig (see #13835).

See also: dotnet/roslyn#47707

@xtqqczze TreatWarningsAsErrors issue is about using warning severity only but we should use error severity if we want to force a rule. Yes? I mean why would we use warning severity if we want to stop build with error?

@xtqqczze TreatWarningsAsErrors issue is about using warning severity only but we should use error severity if we want to force a rule. Yes?

That is a goal of this PR, to enforce various rules during build.

I mean why would we use warning severity if we want to stop build with error?

If TreatWarningsAsErrors is enabled then warnings should stop the build.

Since we get a fix for TreatWarningsAsErrors in near future I believe we have no need a workaround today. In EditorConfig we can set severity to error if we want to force a rule.

Since we get a fix for TreatWarningsAsErrors in near future I believe we have no need a workaround today. In EditorConfig we can set severity to error if we want to force a rule.

@iSazonov In that case, I suggest closing this PR in favour of #13835. Can we make a decision please.

We can use .globalconfig file (#13835) instead, closing.

@xtqqczze Why would we need a global config? We can use our .editorconfig file. If we set a severity of a rule to error we will stop the build if the rule violates. Yes?

@xtqqczze Why would we need a global config? We can use our .editorconfig file. If we set a severity of a rule to error we will stop the build if the rule violates. Yes?

Using a global AnalyzerConfig avoids bloating EditorConfig with analyzer configuration. Documentation is at https://docs.microsoft.com/dotnet/fundamentals/code-analysis/configuration-files#naming

Configuration files for code analysis rules - .NET

Learn about different configuration files to configure code analysis rules.

avoids bloating EditorConfig with analyzer configuration

Bloating? Our file has 194 lines and 142 lines of them with analyzer configuration. I don't expect we will add a lot of rules in near time (maybe ~10 to force new syntax). I believe can live with the EditorConfig while. Also global config does not still supported in VS GUI that is bad UX.

avoids bloating EditorConfig with analyzer configuration

Bloating? Our file has 194 lines and 142 lines of them with analyzer configuration. I don't expect we will add a lot of rules in near time (maybe ~10 to force new syntax). I believe can live with the EditorConfig while. Also global config does not still supported in VS GUI that is bad UX.

I think we would want to specify the severity for each rule explicitly (like #13835). There are 246 rules in Microsoft.CodeAnalysis.NetAnalyzers alone, ~738 additional lines.

AnalyzerConfig is a simple text file, I'm not sure what IDE support is needed.

The way I see it different configuration kinds should be in separate files:

• EditorConfig: file-based configuration settings.
• Global AnalyzerConfig: project-level configuration settings. According to documentation, AnalyzerConfig is designed specifically for specifying project-level analyzer configuration options and applies to all the source files in a project, regardless of their file names or file paths.

I think we would want to specify the severity for each rule explicitly (like #13835). There are 246 rules in Microsoft.CodeAnalysis.NetAnalyzers alone, ~738 additional lines.

Currently we use defaults and it is well. Do you expect that we will change defaults and set severity to error for 246 rules?

AnalyzerConfig is a simple text file, I'm not sure what IDE support is needed.

It is mandatory requirement that IDE support Roslyn analyzer configuration. I know VS Code supports EditorConfig but I am not sure it supports GlobalConfig. Can you confirm?

I tried VS Code with GlobalConfig and it does not work at all :-( OmniSharp uses defaults and ignore the global config. I will open the feature request in OmniSharp repo.

@xtqqczze Now I suggest to put all global config content to a end of out EditorConfig file. After we get the feature in OmniSharp we will be able to move the rules back to a global config file.

OmniSharp

@iSazonov Works for me. Do you have omnisharp.enableRoslynAnalyzers set to true in .vscode\settings.json

See also: #11627

@xtqqczze EditorConfig works but GlobalConfig doesn't work.

I tested with

dotnet_diagnostic.CA1507.severity = silent

@iSazonov I tested in VS Code 1.50.1 (Omnisharp 1.37.3) with src\Microsoft.PowerShell.Commands.Management\cimSupport\cmdletization\SessionBasedWrapper.cs open in the editor.

As expected, a diagnostic with code CA1507 is listed in the problems tab.

Then I set dotnet_diagnostic.CA1507.severity = silent in .globalconfig and restarted VS Code and the diagnostic is no longer listed.

In my environment, OmniSharp is using the MSBuild instance provided by Visual Studio Enterprise 2019 16.8.30615.102.

Perhaps you could you try setting omnisharp.loggingLevel to debug and examine the log output?

Visual Studio Enterprise 2019 16.8.30615.102

I updated to latest preview and now it works. Thanks!

It is mandatory requirement that IDE support Roslyn analyzer configuration. I know VS Code supports EditorConfig but I am not sure it supports GlobalConfig. Can you confirm?

Support for .globalconfig is built into roslyn, so it should work in any IDE (including Visual Studio Enterprise 2019 16.8)