TreatWarningsAsErrors ignored when severity set in EditorConfig

Workaround

Use dotnet build -warnaserror instead of dotnet build -p:TreatWarningsAsErrors=true in step 5 of the below repros.

Repro 1 (Compiler diagnostics)

Repro steps

dotnet new console
Edit the source code to:

class Program
{
    static void Main(string[] args)
    {
        // warning CS0219: The variable 'unused' is assigned but its value is never used
        int unused = 0;
    }
}

dotnet build -p:TreatWarningsAsErrors=true
Add .editorconfig with:

root = true

[*.cs]
dotnet_diagnostic.CS0219.severity = warning

dotnet build -p:TreatWarningsAsErrors=true

Expected behavior

I expect the build output in steps 3 and 5 to be the same, i.e. the build in both cases should fail due to CS0219 being promoted to an error.

Actual behavior

Build fails in step 3, but not in step 5. dotnet_diagnostic.CS0219.severity = warning takes precedence over /warnaserror+ passed to csc.exe from -p:TreatWarningsAsErrors=true

Repro 2 (Analyzer diagnostics)

Analyzer package

Microsoft.CodeAnalysis.FxCopAnalyzers

Package Version

v2.9.8 (Latest)

Diagnostic ID

N/A

$ dotnet --info
.NET Core SDK (reflecting any global.json):
 Version:   3.1.200
 Commit:    c5123d973b

Runtime Environment:
 OS Name:     ubuntu
 OS Version:  18.04
 OS Platform: Linux
 RID:         ubuntu.18.04-x64
 Base Path:   /usr/share/dotnet/sdk/3.1.200/

Host (useful for support):
  Version: 3.1.2
  Commit:  916b5cba26

.NET Core SDKs installed:
  3.1.200 [/usr/share/dotnet/sdk]

.NET Core runtimes installed:
  Microsoft.AspNetCore.App 3.1.2 [/usr/share/dotnet/shared/Microsoft.AspNetCore.App]
  Microsoft.NETCore.App 3.1.2 [/usr/share/dotnet/shared/Microsoft.NETCore.App]

To install additional .NET Core runtimes or SDKs:
  https://aka.ms/dotnet-download

Repro steps

dotnet new console
dotnet add package Microsoft.CodeAnalysis.FxCopAnalyzers -v 2.9.8
dotnet build -p:TreatWarningsAsErrors=true
Add .editorconfig with:

root = true

[*.cs]
dotnet_diagnostic.CA1303.severity = default
dotnet_diagnostic.CA1801.severity = warning

dotnet build -p:TreatWarningsAsErrors=true

Expected behavior

I expect the build output in steps 3 and 5 to be the same, i.e. the build in both cases should fail due to two build errors: CA1303, CA1801.

Actual behavior

The output from step 3 shows build failure due to two errors: CA1303, CA1801. This is correct and expected:

$ dotnet build -p:TreatWarningsAsErrors=true
Microsoft (R) Build Engine version 16.5.0+d4cbfca49 for .NET Core
Copyright (C) Microsoft Corporation. All rights reserved.

  Restore completed in 35.32 ms for /home/a-x-d/dev/fx1/fx1.csproj.
Program.cs(9,31): error CA1303: Method 'void Program.Main(string[] args)' passes a literal string as parameter 'value' of a call to 'void Console.WriteLine(string value)'. Retrieve the following string(s) from a resource table instead: "Hello World!". [/home/a-x-d/dev/fx1/fx1.csproj]
Program.cs(7,35): error CA1801: Parameter args of method Main is never used. Remove the parameter or use it in the method body. [/home/a-x-d/dev/fx1/fx1.csproj]

Build FAILED.

Program.cs(9,31): error CA1303: Method 'void Program.Main(string[] args)' passes a literal string as parameter 'value' of a call to 'void Console.WriteLine(string value)'. Retrieve the following string(s) from a resource table instead: "Hello World!". [/home/a-x-d/dev/fx1/fx1.csproj]
Program.cs(7,35): error CA1801: Parameter args of method Main is never used. Remove the parameter or use it in the method body. [/home/a-x-d/dev/fx1/fx1.csproj]
    0 Warning(s)
    2 Error(s)

Time Elapsed 00:00:00.98

The build output from step 5 shows build passing with two warnings for CA1303, CA1801. Somehow editor config configuration overrides TreatWarningsAsErrors property. This is not expected and seems incorrect:

$ dotnet build -p:TreatWarningsAsErrors=true
Microsoft (R) Build Engine version 16.5.0+d4cbfca49 for .NET Core
Copyright (C) Microsoft Corporation. All rights reserved.

  Restore completed in 216.45 ms for /home/a-x-d/dev/fx1/fx1.csproj.
Program.cs(9,31): warning CA1303: Method 'void Program.Main(string[] args)' passes a literal string as parameter 'value' of a call to 'void Co
nsole.WriteLine(string value)'. Retrieve the following string(s) from a resource table instead: "Hello World!". [/home/a-x-d/dev/fx1/fx1.cspro
j]
Program.cs(7,35): warning CA1801: Parameter args of method Main is never used. Remove the parameter or use it in the method body. [/home/a-x-d
/dev/fx1/fx1.csproj]
  fx1 -> /home/a-x-d/dev/fx1/bin/Debug/netcoreapp3.1/fx1.dll

Build succeeded.

Program.cs(9,31): warning CA1303: Method 'void Program.Main(string[] args)' passes a literal string as parameter 'value' of a call to 'void Co
nsole.WriteLine(string value)'. Retrieve the following string(s) from a resource table instead: "Hello World!". [/home/a-x-d/dev/fx1/fx1.cspro
j]
Program.cs(7,35): warning CA1801: Parameter args of method Main is never used. Remove the parameter or use it in the method body. [/home/a-x-d/dev/fx1/fx1.csproj]
    2 Warning(s)
    0 Error(s)

Time Elapsed 00:00:01.35

I migrated from rulesets to editorconfig for all analyzer configuration recently and our team is now manually checking CI build results for warnings. With rulesets we could easily make all analyzer diagnostics warnings locally, but errors in CI builds thanks to TreatWarningsAsErrors.

I'm looking into a workaround until a solution has been found - any ideas? It looks like for now I can just make our CI inject the following at the start of the editorconfig file:

dotnet_analyzer_diagnostic.severity = error

I don't really like this approach as we're aiming for the ability to control this behavior easily in our dev environments. For that we import some kind of User.props that allows any team member to basically turn on "build as CI" with a single MSBuild property.

Is there any other MSBuild property that controls this behavior already, or does it need a fix to make TreatWarningsAsErrors work for these, too?

@agocke

Tagging @agocke. ~~I believe this behavior is by design. Editorconfig based severity settings take precedence over settings from ruleset file and command line arguments.~~ See next comment.

Actually, I think my last comment is not true. All our repos (roslyn, roslyn-analyzers, etc.) use editorconfig based configuration, we install many analyzer packages that configure bunch of code style rules as warnings for local development (for example, IDE0055 "Fix formatting") and then specify /p:TreatWarningsAsErrors = true in CI on msbuild command line and these get bumped to errors and break build.

@mavasani

@mavasani there are two ways to treat warnings as errors in command line builds. One is an MSBuild flag (/warnAsError), which raises the severity of all messages coming from individual tools, and the other is a CSC flag (set via /p:TreatWarningsAsErrors=true), which instructs the C# compiler to increase the severity of diagnostics. Our builds set both of these at different times, so it would be good to verify that the test is specific to cases where only the latter is set.

I would expect these flags to have the highest priority.

@sharwell

@sharwell - thanks that clears things for me. If the original repro steps use /warnaserror instead of /p:TreatWarningsAsErrors=true then the analyzer diagnostics are indeed promoted to errors and break build. So at least there is a workaround now.

I also verified that /p:TreatWarningsAsErrors=true promotes compiler warnings to errors. So the issue seems specific to analyzer diagnostics. I am going to mark this as "Area-Analyzers" and investigate further.

I also verified that /p:TreatWarningsAsErrors=true promotes compiler warnings to errors. So the issue seems specific to analyzer diagnostics. I am going to mark this as "Area-Analyzers" and investigate further.

This turned out to be a red-herring. The only reason why compiler warning got promoted to an error was that there was no explicit entry in the editorconfig. Let me add a Repro 2 in the original issue description to clarify this is not specific to analyzer diagnostics, but the fact that diagnostic severity settings coming from an .editorconfig file are overriding /warnaserror from command line/MSBuild file.

Are there any documentation of how to best write configs considering all the options for both a split in file types (editorconf=file-based vs globalconf="virtual/flat"-based) and the different ways to specify syntax for an ID above

Realistically, there should be no split. You should ideally choose global config for folder agnostic project based option/severity specifications AND you choose .editorconfig for folder based option/severity specification. Generally the latter should suffice, but there might be some cases you need the former.

I guess code styles (language, naming, format/whitespace etc) should all go in .editorconfig. And only severity configuration should go into global analyzer files.

No, this is not true. All option/severity configurations for analyzers can go either in an editorconfig or globalconfig. All non-analyzer related settings should go into editorconfig.

Maybe other IDEs (vscode, rider) will not be able to work with option/severity config from .globalconfig? Haven't tested it yet, but will soon!

Based on your answers, it seems like the following conclusions can be made (about end-user config):

Use core (https://editorconfig.org/) options from original .editorconfig (everybody knows that...not an interesting point)
Do not configure option and severity on the same line (Deprecate 'severity' field for IDE code style editorconfig syntax #44201) (I have a few questions about that later)
i. Use Code style syntax for specifying option value (without severity)
ii. Use the diagnostic-ID based syntax for specifying severity
Add any (dotnet/csharp) 'code styles' and/or 'code quality' option and/or severity configuration to whatever file you like:
i. editorconfig if you need globbing and file-system based precedence for the option or severity.
ii. global analyzer config (named .globalconfig) if you need file-system agnostic precedence for the option or severity, and have any conflicts under control(!?!?!). (that is until Support global analyzer config levels: #49834, you should always use a single .globalconfig in the repo root....you should probably always have a single file anyway, unless you need to conditionally bring it in via msbuild control flow....I still guess that multiple files will come from nuget/msbuild tooling)
iii. But have a preference for .editorconfig file(s) (they are probably sufficient and are better known and more idiomatic in the community).

Questions:

a. Why do roslyn and roslyn-analyzers use this soon deprecated syntax?

b. How do I translate dotnet_style_qualification_for_field into some RuleID (in this case ID0003 and ID0009 found here) ? (this seems like a manual/tedious process)

@jmarolf

Your conclusions above all seem to be valid

Why do roslyn and roslyn-analyzers use this soon deprecated syntax?

Just haven't got to cleaning it up, and we still have devs that are used to the old syntax, so it will hopefully die down with time.

How do I translate dotnet_style_qualification_for_field into some RuleID (in this case ID0003 and ID0009 found here) ? (this seems like a manual/tedious process)

Yes, unfortunately, it is quite a manual process right now. You can reference this index to help you: https://docs.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/#index. Our long term goal is to have an editorconfig/globalconfig UX that allows you to select a specific code style and context menus to configure the style and severity, such that the tooling will automatically update the config file and you will not have to manually deciper/update these files. @jmarolf is actively working on such a UX.

@mavasani

@mavasani Thanks for the link.

Our long term goal is to have an editorconfig/globalconfig UX that allows you to select a specific code style and context menus to configure the style and severity, such that the tooling will automatically update the config file and you will not have to manually deciper/update these files. @jmarolf is actively working on such a UX.

Are there any public specs for such UX, or is it to early for that ?

(In 16.8 I can configure severity from a multitude of places: error list, references->analyzers in solution explorer, alt+enter tips in editor etc), and I have allready analyzed the AnalysisMode/AnalysisLevel/Category based editorconfig-severity files from the CodeStyle/CodeQuality nupkgs (`build/config' subfolder).

I guess the UX will be part of vs2019 16.9+. Right now I have installed win10/vs2019 in vmware player, just to work with editorconfig-based config on Linux (The UX in vs2019 16.8.x is way better than in rider/vscode). I could actually dogfood 16.9 previews, but right now my main milestone is getting to work on 16.8 (i.e. sdk-5.0.102) accross workstations running win/macos/linux).

For anyone interested in working on 16.8.x bits (5.0.1nn, equal to this sdk patch-line I guess?) you have to duplicate all your severity config in .editorconfig and .globalconfig, and then define all options in the .editorconfig. This way both live analysis and IDE/CLI build work wrt errors.

@mavasani

@mavasani I still have to use dotnet msbuild -t:Rebuild -warnaserror instead of dotnet msbuild -t:Rebuild -p:TreatWarningsAsErrors=true. As a workaround I used the undocumented dotnet msbuild -t:Rebuild -p:MSBuildTreatWarningsAsErrors=true, because I needed msbuild property (not msbuild switch/option CLI argument).

I am on SDK version 5.0.202

@maxild

@maxild What version of the compiler are you using? You can check this by adding this code to a source file:

#error version

This issue is fixed starting in the 3.9 compiler.

@sharwell

$ dotnet /usr/share/dotnet/sdk/5.0.202/Roslyn/bincore/csc.dll -version
3.9.0-6.21160.10 (59eedc33)

BTW @sharwell I wish dotnet --info was a bit more informative wrt csc, nuget, msbuild versions. See dotnet/sdk#15496. Would make issue handling (cross-platform, cross-arch) much easier without me having to write script myself, or remembering undocumented preprocessor directives #version

@sharwell

@sharwell Maybe I'm missing something but this still appears to be an issue.

dotnet --info
5.0.301 [C:\Program Files\dotnet\sdk]

Given a .globalconfig file or .editorconfig with the following:

dotnet_analyzer_diagnostic.severity = warning

And property in the .csproj

<TreatWarningsAsErrors>true</TreatWarningsAsErrors>

The build will succeed despite StyleCop analyzer config warnings.

Removing that explicit rule makes things work as expected.

I discovered this when raising an issue in a popular .editorconfig template repo.

RehanSaeed/EditorConfig#56 (comment)

I experimented with

<WarningsAsErrors />

As set via Properties UI in Visual Studio but that didn't seem to work at all.

@mavasani

@mavasani @sharwell This is still an issue as per @JimBobSquarePants post above. Can this be re-opened?

@JimBobSquarePants

Hi @JimBobSquarePants,

Can you follow the steps here to determine the compiler version used in the build?
#43051 (comment)

Note that the version of CSC used in the build may or may not be the one included in the dotnet SDK (it can vary by project). The #error version line is the most reliable way to get the exact compiler version used for a given build.

Thanks,
Sam

@sharwell

@sharwell Following your instructions I get this.

version: '3.10.0-4.21329.37 (246ce641)'. Language version: 9.0.

I can confirm that the sample project listed here RehanSaeed/EditorConfig#56 still demonstrates an issue.

@JimBobSquarePants

@JimBobSquarePants @RehanSaeed Thank you for the additional details. I was able to verify that the original bug above has been fixed (i.e. the dotnet_diagnostic.ID.severity syntax works correctly with TreatWarningsAsErrors). Can you file a new bug for the behavior of dotnet_analyzer_diagnostic.severity and dotnet_analyzer_diagnostic.category-Category.severity (a different setting)?

@sharwell

@sharwell I've raised #55541.

Navigation Menu

Repository navigation

TreatWarningsAsErrors ignored when severity set in EditorConfig #43051

Workaround

Repro 1 (Compiler diagnostics)

Repro steps

Expected behavior

Actual behavior

Repro 2 (Analyzer diagnostics)

Analyzer package

Package Version

Diagnostic ID

Repro steps

Expected behavior

Actual behavior

27 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

TreatWarningsAsErrors ignored when severity set in EditorConfig #43051

Description

Workaround

Repro 1 (Compiler diagnostics)

Repro steps

Expected behavior

Actual behavior

Repro 2 (Analyzer diagnostics)

Analyzer package

Package Version

Diagnostic ID

Repro steps

Expected behavior

Actual behavior

Activity

mavasani commented on Apr 3, 2020

bddckr commented on May 18, 2020

mavasani commented on May 18, 2020

mavasani commented on May 18, 2020

sharwell commented on May 18, 2020

mavasani commented on May 18, 2020

mavasani commented on May 18, 2020

27 remaining items

mavasani commented on Jan 14, 2021

mavasani commented on Jan 14, 2021

maxild commented on Jan 14, 2021

mavasani commented on Jan 14, 2021

maxild commented on Jan 15, 2021

maxild commented on Jan 15, 2021

maxild commented on Apr 22, 2021

sharwell commented on Apr 22, 2021

maxild commented on Apr 23, 2021

JimBobSquarePants commented on Jun 15, 2021

RehanSaeed commented on Aug 9, 2021

sharwell commented on Aug 9, 2021

JimBobSquarePants commented on Aug 10, 2021

sharwell commented on Aug 10, 2021

RehanSaeed commented on Aug 11, 2021

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions