No More Build Warnings, Please

Build warnings are an almost universal constant, present in (I believe) every single build system I have ever used or seen. And I think because of that, we pretty much take them as given. But if you actually sit down to think about them - as I have recently - I think it's hard to ignore the fact that build warnings are actually fairly poorly motivated, and not really that great a solution to any problem. Why? Well, allow me to explain.

First, as a point of contrast, I want to first just go over the reasoning for using build errors. (And also, as a sidenote, I'm only going to be talking about building code in this post for simplicity, but everything I say should apply more generally to any sort of build system.)

The obvious use case for build errors is situations which are so fundamentally wrong that it's really not possible to proceed further - such as syntax errors, or referencing a nonexistent file. This is sort of a necessity, so I don't have much to say about it. But build errors can also be used to enforce rules, even if violations of these rules don't break anything catastrophically (or even break them at all). For example, you might decide as a team that implicitly casing from double to float is likely to lead to bugs, so you configure the compiler to error on these casts, requiring people to do it explicitly.

So, our goal is to prevent people from writing certain types of code. And build errors work very well for this: if you write code that violates the rules, the compiler prevents you from finishing your work until you update your code to follow the rules. Great!

Okay, so what about build warnings?

Build warnings' primary use case is, I think, to draw attention to "suspicious" code. Perhaps it's been noted that there's a coding pattern or language feature which often results in bugs, but for whatever reason, it's not worth making it an error to use it. In some sense the fact that we have warnings at all is because compilers can't be perfect - the ideal compiler would have no warnings, and a single build error: "Code contains a bug", and that would be that. But compilers can't actually detect all bugs, so you have to use simple rules to try and detect places where a bug is likely to occur.

A classic example, might be dereferences of possibly null values. Compilers can detect to some degree when a value might be null, but they aren't super smart about it, so making it a full-on error to dereference any potentially null value is too annoying. So, you make it a warning instead of an error - drawing attention to the potential issue, but without requiring a change, so you don't have to garnish every function in your codebase with a dozen null checks.

So - the goal here is totally reasonable: compilers aren't perfect, and having something which flags suspicious code for manual review makes sense. But the method of enforcement is... questionable. In the ideal case, the warning identifies a genuine issue and the programmer fixes the issue - great! But if the warning doesn't identify a genuine issue, then it just sticks around forever. The compiler can't know who is responsible for a warning or when exactly it was introduced, so it notifies everyone who builds about the issue every single time. As your codebase grows, there will be more and more warnings, until eventually no one is paying any attention to them because there are so many, and cruicially, because *99% of the time the warning output is completely useless to the task at hand*.

I think some people make the argument that this is actually good - warnings essentially function as a list of things which *should* be dealt with, but aren't urgent. And I think this at least makes a little sense. But I still think it doesn't really hold up. For one thing, as we discussed above, warnings kind of by definition only identify things which are *not necessarily* issues. Or at least, if they don't do that - that is, they identify things which are always issues - it seems like they would be better off as errors. But moreover, while having a list of "things to look into eventually" makes sense, it doesn't feel like your compiler's build output is the right place for that, really. It's just going to clutter up the build output, which has a bunch of other potentially useful information. And in particular - if there are any warnings which are more important, they're going to end up getting ignored! I have had this happen a few times at my job - someone will come to me about something not working, and after tracking down the issue, we realize that the compiler was actually warning about it the whole time. But of course, the one extra warning was buried under dozens of other completely irrelevant warnings.

So, okay, build warnings are a bad solution to this problem. But the problem, I think, is still valid - you want to flag suspicious code, encouraging people to take a second look at it. So what should you do? I can think of two better solutions:

Solution 1: Upgrade all of your warnings into errors which can be manually suppressed on a case-by-case basis.

If you have a situation where you want to flag something for manual review, making it an error requires the person who introduced it to manually review it. But if they review it and decide actually, this isn't a problem, then they're allowed to suppress that specific instance of the warning (most compilers give you a way to do this with special commands written in the code. Though if they don't, this solution might not really be feasible). This means the warning is still useful to the person writing the code, but once they've dealt with it, it doesn't bother anyone else. Since you're explicitly suppressing the warnings with commands that are written in the code, it's clear when reading it that someone has decided some suspicious part is okay (maybe you even require the writer to tag themselves and write a comment with their explanation). If you change your mind about a warning, or are just worried about issues hidden behind warning suppressions, it should be easy to run a search through the codebase for all the places where a particular warning has been suppressed. Great!

Solution 2: Put warning-worthy situations in a separate report, not generated during normal build.

In some cases, solution 1 may be too restrictive or annoying (or literally not possible). Maybe you don't want to force everyone to constantly be dealing with suppressing warnings, or you don't want your code littered with warning suppression commands. I'll note that if that's the case, you might consider just having fewer warnings (and I'll expand on that in a sec). But that aside, I think the best solution is to disable your warnings in local builds (or whatever it is people use when they're actively developing), but have some separate process which detects warnings and flags them for review. Maybe when a user submits a pull request, your build system does a special build with warnings enabled, figures out which ones are new, and then they're reviewed as part of the code review. Maybe you have a nightly process which builds with warnings enabled and sends out a message to relevant people to review it. By making warnings generation into a separate, non-default process, you can insert it wherever you want to, without needing to spam every builder of your code with useless messages.

With this option specifically, I want to preempt what I think is a common objection to this solution (I'm guessing you probably thought it as soon as you read the previous paragraph). You're worried that if you do this, all the warnings will be just be ignored. I feel the same impulse, but you should think hard about whether that's really the issue here.

For one thing, as I already talked about, a lot of your warnings probably are being ignored anyway just because people get in the habit of ignoring them. But also, if you're concered people will ignore the warnings because there are a lot of them, you should consider having fewer warnings! Having lots of warnings lying around means they're generating a fair amount of false positives, and the more false positives a warning generates, the less useful it becomes and the more likely people are to just completely ignore it. There's no magic right number here, but it's important to remember that there are tons of bugs and issues that are impossible to catch with warnings, which is what things like code review, unit tests, QA, and just "hiring good programmers" are for. The number of bugs in your code is probably a lot more closely related to those factors than it is to the number of warnings you have. Not to say that they're completely useless, but it's important to remember that they aren't everything.

But most importantly - if you're worried no one will look at the warnings report regardless of how many warnings there are - at the end of the day, your entire company is made up of people (crazy, I know), and if they all decided to be super lazy, they could ignore literally every warning, solve every error with the quickest possible solution, write useless unit tests, and rubber stamp their code reviews. If they ARE doing that, uh, you have bigger problems than people ignoring warnings. But my guess is that they are in fact following most of the rules and generally writing non-awful code, and so it seems reasonable to expect them to also look at some warnings reports. If they're currently looking at your build warnings and acting on them, clearly they're willing to address these problems. (And again, if they aren't doing that, you have nowhere to go but up!) Communicating it in the right way is important - maybe no one reads emails, so an email report is no good - but there is probably a way you can get the information to them in a way that's convenient.

So: no more warnings, please! As it happens, the codebase I work in at my job has 0 warnings, because we have chosen solution 1 (along with just having a pretty small number of "warnings upgraded to errors" overall) and it works fine. You, too, can be part of our cool club! (Though amusingly, our custom build system for the whole game does have warnings, and as of writing I get around ~50 warnings if I run a build, which is actually what prompted me to write this post. If you're one of my coworkers and you're reading this, sorry for calling you out :P)