We should stop using build warnings

I almost titled this "Build Warnings Considered Harmful", but that seemed too silly. (Also, honestly, I kind of assume someone has already written that article. It's probably better than this one, realistically.) But it's pretty much an accurate description of how I feel about build warnings - they're well-intentioned, but on reflection, are just a bad solution to the problem they're trying to solve. So without further ado, let's get into it.

Build Errors

Well, okay, a little ado. As a point of contrast, I just want to briefly talk about build errors.

(Also, I'd like to note - 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)

I see two main use cases for build errors. The first is the sort of obvious one - anything that's so fundamentally wrong it's not really possible for your build system to proceed. This is things like syntax errors or referencing nonexistent files. Build errors can also be used to enforce rules you or your team has decided on. Maybe the language you're using allows for implicit casting between double and float, but as a team you've decided you want to disallow this so your code is clearer. Build errors are great for this - make it an error to have an implicit cast, and now it becomes impossible for anyone to build if they have an implicit cast.

Our goal is: prevent people from writing certain types of code. And the method of enforcement works very well: 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!

Build Warnings

So, build warnings. The main reason I see people use build warnings is essentially to draw attention to "suspicious" code. These are things which your team feels are often issues, or are associated with issues, but are not guaranteed to be a problem. 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 or style violation is likely to occur. For example, you might want to avoid dereferencing null values, so you consider making it an error to dereference a value that the compiler thinks could be null. However, there are lots of cases where an object can't actually be null, but the compiler isn't smart enough to figure that you. So, you make it a warning instead of an error - drawing attention to the potential issue, but without requiring a change so your code isn't littered with tons of null checks. The same principle applies to enforcing style rules.

The goal here is fine, I think - 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 is a false positive - it 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 has to notify everyone who builds about it every single time.

Sometimes this is framed as useful - you can use the build warnings as sort of an issue tracker, thinking of them as things that should be looked into at some point, but aren't urgent. But even for this, they seem like an iffy solution. As your codebase grows, there will be more and more warnings. And realistically, 99% of the time someone builds, the warnings will be completely useless. Some of them will be from code that user has no familiarity with. Some of them will be from code they're familiar with, but even so, it's likely code that's unrelated to their current task. If they're lucky, maybe one of the warnings relates to the thing they're in the middle of working on - but because there are so many other warnings, there's a solid chance they just won't notice! If you want to use them as a sort of issue tracker... well, okay, but that doesn't seem like good reason to show the list of issues to every single person who ever builds in a way that could drown out genuine problems that the user might be in a good place to fix.

Better Solutions

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 individually suppressed.

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 ~200 warnings if I run a build, which is actually what prompted me to write this post. So, realistically, we would probably end up getting kicked out of said cool club. You can't have 'em all.)