Proj0039: Treat all warnings as errors is considered a bad practice

MSBuild can be configured to treat all warnings as errors. How tempting this may feel (ideally a solution should not contain any warnings), there are some objections to consider. Because such a policy might be counterproductive.

1. We’re not developing in a perfect world

When a warning pops up, the developer facing it, is supposed to solve the issue. That is why we have static code analysis enabled after all. The fact that this is not happening hints that we do not live in a perfect world. Changing the severity to error for all rules will not make a difference.

A developer that was not willing to resolve the issue as a warning is most likely also not willing to resolve it as an error beyond the bare minimum: that means by suppressing the issue, disabling the rule, or by adjusting the code just to satisfy the rule. None of these resolutions is guaranteed to be an improvement.

Developers that are not capable to solve a warning (correctly) are still not able to do so once it becomes an error. If they did find the support to solve it correctly, what would make they would do this now? Or will they come up with the solutions equal to the unwilling developer?

2. Deprecated code

A specific, but common scenario is when a new version of a package is released, suddenly some code has been deprecated. Obviously the code has to be adjusted at some point, but it is not always the case that the moment you need the new version of the package, is also the moment to refactor all code that relies on this now deprecated code.

3. New rules

The introduction of new rules (introduced by an existing analyser or by a newly added analyser) will potentially report a lot of issues. They might be valid points, but you don’t necessarily want to halt the release of all features until all issues are solved. By (temporarily) disabling such rules, newly added code will not yet benefit from this rule.

4. Warnings are not errors

It is possible to create static code analysis with an error severity. The reason that this is not done by the maker of the rule is with a reason. It might be inefficient, deprecated or unreadable code, but not necessarily wrong.

5. Conflicting IDE extensions

Some extensions in Visual Studio (like VS Spell Checker or SonarQube IDE) add warnings to the build output as they are de facto analysers. As they are not configured by the solution but by the environment of the developer, those developers can not build the solution.

6. False positives

Although analyser rule builders - like us - try their best to come up with robust and correct analysers, it turns out that now and then a rule reports a false positive. In such cases it is better to not suppress the issue, or adjust the code just to satisfy the rule, but to file a bug report and wait for an update that fixes the FP.

In the end, warnings are an indication of technical debt, and should be handled a such: with care. Using tools like Qodana and SonarQube can create reports on pull requests to reduce the introduction of new warnings (ideally to zero).

On a regular basis, the team working on the project should allocate time to address outstanding issues that should be fixed, or decide that a certain rule is not worth the effort for the project. The goal should be to get as close to zero warnings as possible.

Non-compliant

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <TargetFramework>net8.0</TargetFramework>
    <TreatWarningsAsErrors>true</TreatWarningsAsErrors>
  </PropertyGroup>

</Project>

Compliant

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <TargetFramework>net8.0</TargetFramework>
  </PropertyGroup>

</Project>