Add a "treat warnings as errors" option

[Porges]

I would like an option to be able to treat all warnings as errors, when running from the command line.

[AndyGerlicher]

Thanks for the suggestion, sounds like a potentially useful feature. This doesn't meet the bar for our roadmap at this time however.

[jbjoshi]

Is there a Roadmap defined somewhere?

[eatdrinksleepcode]

If it is a potentially useful feature, why is the issue closed? Someone from the community may want to pick it up.

[chandramouleswaran]

As a work around, you should be (I think) able to add TreatWarningsAsErrors property in a targets file - set it to true and invoke that target as a part of the msbuild command line argument along with your default targets. Wouldn't that work?

[Porges]

@chandramouleswaran: that's for the compiler warnings, not for warnings from MSBuild itself.

I want to be able to make things like missing files be errors, not warnings like they are currently.

[chandramouleswaran]

@Porges - Good point :+1:

[rainersigwald]

Reopening and marking as up-for-grabs. This isn't high on our priority list but it's a good feature to have.

[iouri-s]

We are getting a bunch of these warnings as well. The code accumulated dozens of warnings such as importing a target file several times, even though we have TreatWarningsAsErrors=true.

[D3-LucaPiombino]

We are in the same situation.

Use case

Misconfigured Platform across a Solution

Having some dependency that require a specific platform (e.g. x64) but that when build from msbuild using the solution configuration matrix produce:

C:\Program Files (x86)\MSBuild\14.0\bin\Microsoft.Common.CurrentVersion.targets (1820, 5)
There was a mismatch between the processor architecture of the project being built "MSIL" and the processor architecture of the reference "Microsoft.ServiceFabric.Internal", "AMD64". This mismatch may cause runtime failures. Please consider changing the targeted processor architecture of your project through the Configuration Manager so as to align the processor architectures between your project and references, or take a dependency on references with a processor architecture that matches the targeted processor architecture of your project.

Since this is plugged in a CI/CD workflow, it would be nice to be able to block the release process in this case.

[dotMorten]

We want the appveyor builds to fail if a PR introduces new warnings. However we don't want the dev to deal with this permanently as they are working towards a PR. Essentially we want gated checkins that also prevents users from introducing new warnings. Simply being able to do this through a commandline would be the easiest way.

[jeffkl]

I'm working on this for MSBuild 15.

Design

A new command-line argument /WarnAsError to enable the functionality. When a warning is treated as an error, the execution of the target will continue but the overall build will be marked as failed.

All Warnings as Errors

Specifying just /WarnAsError will log all warnings as errors and the build will fail.

Example

msbuild.exe myproject.proj /warnaserror

Specifying Which Warnings Should Be Errors

Specifying a comma-delimited list of warning codes will treat just that set of warnings as errors. The value can include duplicates and only that set will be logged as errors and the build will fail.

Example

msbuild.exe myproject.proj /warnaserror:MSB3000,MSB4000,CA1000,FOO,123

This will also work within response files.

Suppressing Warnings

Although we don't recommend it, there are cases when it might be necessary to suppress warnings. To do this, you'll be able to specify a comma-delimited list of warning codes to suppress. You must specify a list of warnings to suppress as we will not be just suppressing everything.

Example

msbuild.exe myproject.proj /nowarn:MSB1000,MSB2000,FOO,123

Open Questions

1. The values cannot be MSBuild properties so they are passed in as command-line arguments or contained in response files. Should there be environment variables as well?
2. The build will fail if a warning is treated as an error but tasks won't stop executing. The target execution model will still treat it as a warning and tasks will continue executing. This is because warnings are typically non-critical. Is this okay?
3. Is it acceptable that attached loggers will only receive the mutated warning as an error and will not be aware that it was supposed to be a warning?

Please send me any feedback on this design.

[iouri-s]

This design would satisfy our needs. Replacing "can not" with "cannot" in the first open question would make it easier to understand.

[dotMorten]

1. The values can not be MSBuild properties so they are passed in as command-line arguments or contained in response files

Does this mean we can't enable this in .csproj? That would suck ☹

[jeffkl]

@dotMorten The problem is that project properties are declared too late. MSBuild logs warnings during project evaluation which would mean they could not be turned into errors. For example:

<Project xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
  <PropertyGroup Condition="true or true and true">
    <MSBuildWarningsAsErrors>MSB4130</MSBuildWarningsAsErrors>
  </PropertyGroup>
  <Target Name="Build" />
</Project>

Results in the MSB4130 warning:

warning MSB4130: The condition "true or true and true" may have been evaluated incorrectly in an
earlier version of MSBuild. Please verify that the order of the AND and OR clauses is written
 as intended.  To avoid this warning, add parentheses to make the evaluation order explicit.

The warning is logged before the MSBuildWarningsAsErrors property is declared. The TreatWarningsAsErrors works as a property because the compiler is called after the property is declared. But I don't know of any property you can declare in your project that will affect MSBuild itself, just the targets that are executed. There's also the problem of project-to-project references where MSBuild is building a massive tree of hundreds of projects in a solution, if the setting was a property in one of the projects, should it apply to everything in the build graph?

It might be possible to add a new property to the <MSBuild /> task which would let you use a property to control it. But I felt that in order to really nail this scenario, it needed to be a global control knob. Since it can only be enabled as a command-line parameter it will only really be helpful in the hosted build scenario for things like CI environments. So local builds in Visual Studio and command-line buiilds wthout the switch would still emit warnings but branch owners would be able to keep the warnings from making their way into the code base.

You had original stated:

Simply being able to do this through a commandline would be the easiest way.

And this design should provide this capability. Does this seem reasonable?

[danmoseley]

We used to recommend that all tasks return return !Log.HasLoggedErrors; from Execute() -- the idea being that all well behaved tasks log one or more errors if and only if they are fail -- really Execute() should have been void and the engine could have enforced this. Anyway -- what happens to tasks that log a warning which is upgraded to an error. I didn't look carefully at the change but I'm guessing TaskLoggingHelper (aka Log) is unaware so the task will still succeed in this case and build will continue. Presumably that's what desired.

[edited to make sense]

[jeffkl]

The logic will be if you specify /warnaserror than the overall build result will fail if any errors are logged. Individual targets can still show as "Success". So if I build pragmatically via Build(), you would get back a BuildResult with an OverallResult of BuildResultCode.Failure. But in the ResultsByTarget collection, all of the target results could show as success.

If all of the tasks return !Log.HasLoggedErrors than the target would show as fail like you said so the logic is a catch-all. It would be a little harder to fail the task/target so I figured it was good enough that at least the overall build fails. Do you agree?

[iouri-s]

@jeffkl - I am not sure I am convinced on the command line switch vs. MSBuild property argument. For most cases, specifying the property early enough would solve the corner case you show. It is expected in a project file that an msbuild property does not take effect before it is declared. The behavior of a switch can be achieved with the existing /p command-line switch even if it is a property. Project-to-project references are not a problem either. Typically, a (compiler) warnings as errors policy is team-wide, and teams already have a process in place for ensuring that the property gets into all the necessary projects, by creating a common targets file, for example. Having inconsistent behavior between hosts will create further confusion and frustration.

[jeffkl]

@iouri-s The main concern is that if you set a property to suppress a warning and there is ever a case where that warning wouldn't be treated as an error, it defeats the purpose of this functionality. My example above is not really a corner case in my opinion.

Do you think an environment variable would good enough?

I believe that the local build experience will still show warnings unless people manually specify to treat them as errors. But the /warnaserror switch would at least let hosted build environments like CI to fail if a warning is introduced. This means that pull request validation builds would fail and users would know they need to fix the warning.

I would love to come up with a way to have the warning list be an MSBuild property but I just don't see a way to ensure that what is specified will actually be treated as an error.

[davkean]

@jeffkl Hey Jeff, have you talked to anyone inside Visual Studio to get an option inside VS to turn this on?

[Porges]

I never noticed this was closed. Thanks @jeffkl! 👍

[jeffkl]

@davkean Not yet, any recommendations on who to talk to?

@Porges You're very welcome, I've been wanting the feature myself for a long time. Spread the word!

[memark]

Any update on this?

[jeffkl]

@memark the command-line arguments shipped in MSBuild 15.1 and the properties shipped in MSBuild 15.3 via #1928

[solvingj]

@jeffkl this is a great improvement, thank you!

This was closed quite a while ago. Since then, have you worked with the VS team to get this added as a VS option as @davkean mentioned? Until that exists, it's my understanding that there is simply no way to control this behavior for any Visual-Studio-Driven workflows.

In our specific case, we would want to add a property to Directory.Build.Props. As @iouri-s mentioned, the property would work for a majority of cases. At least it would be a start, and could come with the disclaimer that users would have to ensure the order it was processed.

Of note, for native C/C++ projects, we have ForceImportBeforeCppTargets flag for MSBuild. However that feature is implemented in MSBuild seems like it could be generalized to ForceImportBeforeAnythingElse for this very specific case of "bootstrapping MSBuild settings".

[rainersigwald]

@solvingj <MSBuildTreatWarningsAsErrors>true</MSBuildTreatWarningsAsErrors> has been supported since MSBuild 15.3:

the command-line arguments shipped in MSBuild 15.1 and the properties shipped in MSBuild 15.3 via #1928

[solvingj]

Thank you for that @rainersigwald . I was actually more specifically needing to solve this related issue of suppressing specific warnings, in my case MSB4011: https://github.com/Microsoft/msbuild/issues/910

I'm about to look at that link you provided to see if there's a mechanism for this as well.

[solvingj]

It looks like this has no effect, likely due to the limitation explained in the start of the PR.
<MSBuildWarningsAsMessages>MSB4011</MSBuildWarningsAsMessages>

I guess the remaining questions is, how is it possible that there is no way for the user to append/alter or otherwise affect the MSBuild flags when using Visual Studio? If that existed, we could have easily passed /WarnAsMessage (and any other future flag) from visual studio without even needing this property, and without any caveats.

[MattBussing]

@rainersigwald , if I understand this comment correctly, Visual Studio should support MSBuildWarningsAsErrors. It isn't working for me. Here is my Directory.Build.props.

<Project>
    <PropertyGroup>
        <MSBuildWarningsAsErrors>MSB3276;MSB3247;MSB3277;NU1605;MSB3245;MSB3243</MSBuildWarningsAsErrors>
        <WarningsAsErrors>MSB3276;MSB3247;MSB3277;NU1605;MSB3245;MSB3243</WarningsAsErrors>
    </PropertyGroup>
</Project>

I am using Visual Studio 2019. When I build my solution it doesn't fail the build or output the warnings as errors to the error pane. It just outputs them in the warning pane like before I set the MSBuildWarningsAsErrors setting. Also, I did this after deleting the .vs folder and clearing the bin and obj folders. On the other hand, Rider and MSBuild will fail and show the errors.

What do you or anyone else recommend we do to get this fixed?

p.s. I know I put the same warnings in both MSBuildWarningsAsErrors and WarningsAsErrors. I wasn't sure what the difference was, so I used both.

[rainersigwald]

@MattBussing I don't reproduce your problem. Can you file a new issue with more details (ideally a repro project) please?

p.s. I know I put the same warnings in both MSBuildWarningsAsErrors and WarningsAsErrors. I wasn't sure what the difference was, so I used both.

Great news! As of #5774 you can just use WarningsAsErrors.

In the past, WarningsAsErrors was passed explicitly to some places (notably the C# compiler) but never applied to MSBxxxx messages, and many tasks didn't use it. MSBuildWarningsAsErrors is respected by MSBuild itself, so it can apply to any* warning that has a code.

* This may actually be the problem you're seeing; some of MSBuild's warnings are emitted before we can read this property and thus can't be changed by the property. But please let us know the specific problem you're seeing.

[MattBussing]

@rainersigwald , thank you so much for the quick response! Thanks for your patience while I filed another issue. Here is the issue with a project that reproduces the error. https://github.com/dotnet/msbuild/issues/6473

So what you are saying for the second part is that WarningsAsErrors is the same as MSBuildWarningsAsErrors now?

[rainersigwald]

So what you are saying for the second part is that WarningsAsErrors is the same as MSBuildWarningsAsErrors now?

Correct, in MSBuild 16.9 and higher.