Is it a good practice to treat warnings as errors?

Making Visual Studio treat warnings the same way as errors

.NET projects have an interesting property that we can set. It’s named TreatWarningsAsErrors. We can set it in *.csproj files:

<PropertyGroup>
    <TreatWarningsAsErrors>true</TreatWarningsAsErrors>
<PropertyGroup>
Code language: HTML, XML (xml)

For years, I would say that this is great! The more strict code checking rules, the better, right? Code style, warnings, conventions. And in theory, there are nice benefits to that.

If we have strict automated rules to check everything, we save hours on code review discussions about tabs, spaces, spaces around braces etc. Automated reviewer will just reject that. No one needs to waste their attention on pointing out breaking conventions. We don’t have to annoy people by picking on minor issues.

Additionally, we get a super-fast feedback loop. With modern Visual Studio and its analyzers, we can see the squiggly red line indicating error a second after we wrote the code. We are incentivized by that red line to instantly fix the issue, because we’re trained to keep the code error-free as we work. Fast feedback loop is, in general, a great thing.

But then I landed in a project where I saw such strict rules applied. And I hated it. And I think I can articulate good arguments for not being too strict, and particularly, not treating warnings as errors.

Why I think that compiler should not treat warnings the same as errors

I’ll start with an example. This is a kind of error I’ve been seeing dozens times a day now:

Let’s assume that I agree that this code style rule is useful and that the team should not commit such lines to the repository. Should it be an error then? I think it should not, and here are my arguments:

• Build of the whole application will fail if there are any errors like that. It’s hard to test your code if it doesn’t compile. You can’t run unit tests. You can’t launch application and see if it works.
  The nature of writing code is that before you commit your final solution, you add code. You move it, remove it, change it as you discover problems and so on. Maybe we gain immediate feedback on code style issues, but we loose quick feedback that comes from executing the code frequently and with minimum friction.
• The IDE will constantly draw your attention to minor issues like “unused using statement”, or “two empty lines after each other”. If the editor shows the same kind of feedback (a red, squiggly line and an entry in the Errors window) for all issues, regardless of severity, you cannot focus on important issues, because all of them become equally important if they prevent the compilation.
• It’s just a waste of time to fix code style violations in a code that you will likely delete/rewrite in the coming hours.

Should we then just ignore all warnings and code style violations?

But if we don’t prevent developers from introducing new warnings and code style violations as early as possible, how to avoid having their number grow and accumulate as a technical debt? Let my try enumerate our options:

• On the one side of the spectrum, we can prevent code with minor issues from compiling instantly, right in the IDE like Visual Studio. We can do it with <TreatWarningsAsErrors>true</TreatWarningsAsErrors>, or by increasing severity of analyzer rules to ‘Error’. I already explained why I don’t like this idea.
• On the other side of the spectrum, we can only check for code issues at the last moment, before we merge code to an integration branch. We can do it in Pull Request’s validation pipeline. Theoretically, you can do it even later, but it would be a really bad quality process.
  I worked in such setups. I found them pretty annoying. Everything in your code seems to be fine in the IDE. Then you create a Pull Request and wait few minutes or few hours for the pipeline to complete. Only then you get feedback that you forgotten to add a comment to a public method. The issue here is a long feedback time between making a mistake and getting feedback about it.
• Both of those extremes approaches seem to be inefficient. So I think it’s best to find something in between.

In reality, there don’t seem to be that many options in between to hook ourselves with the code validation. This is how I see our choices:

From that perspective, we could consider a git hook validating the code before each git push operation a good place to stop the bad code from going further. But from my experience, it has drawbacks. It’s hard to enforce setting up git hooks on the team. It might be inconvenient to try build solution on each commit to find out errors. The feedback might not be in a nice form. Maybe there are ways to do it, but I haven’t seen that working well yet.

How to best deal with warnings

I don’t have a definite answer, but taking the above into consideration, I would strive to make a setup where:

• Errors are errors, and warnings remain warnings.
• Errors are issues that make it impossible for compiler to produce a binary (library or executable)
• I would consider selectively increasing severity of some warnings to errors. I think of warnings that normally would not prevent compilation to succeed, but would very likely lead to runtime errors (e.g., conflicts of dependencies versions within the project)
• Warnings should remain warnings in the IDE. I’d be ok with a zero-warning policy enforced later, in the validation pipeline. Developer would still see orange squiggly line for warnings in the IDE, but would be able to ignore them until she’s ready to publish the Pull Request.
• Minor issues that don’t break the compilation, introduce no risks to runtime and don’t dramatically decrease other coders’ experience, should have lower severity (e.g. suggestion, hint). I’d allow the team to merged them to the integration branch. Therefore I’d consciously allow them to accumulate over time in the code base. I’d not say that few years ago, but I think this is the rational way to do.

Did this essay triggered some interesting thoughts in you? Please share in the comments whether you agree with me or not 🙂