Treat Warnings As Errors. I Mean It. You Need to Do This

We here on the SubMain blog love to talk about programming best practices.?Sometimes we’ll cover topics that are specific to C# or .NET as a whole. Other times, however, we write about concepts that are universal. Today’s post falls into the latter category. And I’ll start out by making a strong claim: you absolutely should treat warnings as errors when developing in .NET. Well, to be honest, that applies to virtually any platform.

The examples in this post, as usual, will be in the C# language.?But as we’ve said, the concept applies pretty much everywhere.

First Things First: What Are Warnings? And Errors?

Up until this point, I’ve been writing assuming that everyone reading this will know perfectly well what warnings are and how they differ from errors. I now realize that that’s an unsupported assumption and it might well be the case that many readers aren’t familiar?with this terminology.

If that’s not you, no problem. Feel free to skip this section. On the other hand, if it’s not clear to you what these terms mean, then read on.

Compiler Errors

Let’s start our explanation with errors. When we talk about errors in the context of this post, we’re talking about compilation errors, which drastically narrows down the universe of programming languages to which this post applies. Consider the following code snippet:

int x;
x = "ten";

Even a very novice C# coder can see in a spot that this code won’t work. But what matters for us here is the details of how it won’t work. If I open up my Visual Studio 2017, create a new console application and paste that code inside the “main” method, this is what I get:

If I hover over that “ten” string, Visual Studio helpfully tells me “Cannot implicitly convert type ‘string’ to ‘int’,” which is exactly the same error message I’d see if I pressed F6 in order to try and build the solution. That happens, obviously, because I’m trying to assign a string literal to a variable of type int, which isn’t allowed since those types are completely different?from one another and there is no implicit conversion declared.

So, that’s pretty much what a compiler error boils down to. The compiler is letting me know that the code I’ve written makes no sense. It tries to do something that’s completely at odds with the language’s syntax and/or semantics.

Compiler Warnings

I like to imagine a compiler error as if the compiler was a cranky professor yelling at me. It’s silly, I know, but it just makes me chuckle. Please indulge me with this silly metaphor.

So, if a compiler error is my cranky old college professor yelling at me at the top of his lungs because I did something stupid…then a compiler warning would be the same cranky professor yelling at me. But this time, he’s being gentler because what I did was slightly less stupid. At least in theory.

Let’s abandon the world of silly imagery and get down to code again. Consider the following example:

static void PrintMonthName(int month)
{
    Console.WriteLine(new CultureInfo("en-US").DateTimeFormat.GetMonthName(month));
}

Nothing too surprising or even interesting about the code above, right? It’s just a static method that will print the name of a month, in English, according to the specified number.

Now let’s assume one of your colleagues, fearing a null reference exception, decided to make the code more robust, adding a null-check:

static void PrintMonthName(int month)
{
    if (month != null)
        Console.WriteLine(new CultureInfo("en-US").DateTimeFormat.GetMonthName(month));
}

If you actually go and type or paste this code in your Visual Studio, you’ll see something like the following image:

There’s clearly something amiss, but the visual signs seem to indicate the problem is not as serious this time. And indeed they aren’t since Visual Studio will gladly build my application if I press F6:

What’s the green underline about then? If I hover over it, this is the message I get:

The result of the expression is always ‘true’ since a value of type ‘int’ is never equal to ‘null’ of type ‘int?’

The warning message is telling me that the “if” instruction is unnecessary since a value of type int can never be null.?In other words, the code above, again, makes no sense but in a more benign way than before.

Let’s keep developing our previous example:

static void PrintMonthName(int month)
{
    string culture = "en-US";
    Console.WriteLine(new CultureInfo("en-US").DateTimeFormat.GetMonthName(month));
}

In this version, the “if” statement is gone. However, there’s a new kid on the block: the “culture” variable. It receives the “en-US” ISO language code. But what happens is that, after being declared and assigned, the variable isn’t used anywhere in the code. It’s completely useless, and our always-vigilant friend the compiler tells us so, with a warning, as indicated by the green underline:

The message I get when hovering over the “culture” variable is anything but surprising:

The variable ‘culture’ is assigned but its value is never used

That’s pretty much it for warnings. They are ways in which the compiler tells you that you screwed up, but you didn’t screw up badly. You wrote some piece of wrong code, but it’s not as not as harmful as a full-blown error.

Or isn’t it?

Not All Warnings Are Equal

In the silly metaphor that I employed a while back, I said that the compiler warning equates to my professor yelling at me, but not as loud as before because this time I was slightly less stupid. In more concrete terms, warnings generally indicate less serious problems?and sometimes not even problems but rather things that might be problems. (Have you ever heard the phrase “code smell”?)

Having said that, I’m suggesting we all get back to the old, cranky professor who’s always screaming at the top of his lungs. I’m suggesting we treat every warning?or most warnings, anyway?as an error. Why is that?

Up until now, we’ve been working under the assumption that warnings are mostly harmless, or they’re at least not as harmful as errors. In other words, we’re operating under the assumption that a warning indicates issues that aren’t as severe as the ones indicated by errors.

I don’t really believe that assumption, though.

Sure, sometimes a warning indicates problems that are harmless. For instance, the first example in this post showed a warning triggered by code?performing a null check on a variable of type int. This check is in itself harmless, I’ll give you that (even though I could argue that it shows a lack of knowledge about the difference between value and reference types, which could cause future problems).

Other warnings, though, shed light on issues that, if left untreated, could lead to bugs. For instance, take a look at the code below:

public BookService(IBookRepository repository)
{
    repository = repository;
}

What the author of the code above probably meant to do was to initialize a private field in the “BookService” class. Instead, they just assigned the parameter’s value to the same variable. That triggers compiler warning CS1717, with the following message:

Assignment made to same variable; did you mean to assign something else?

Since the author of the snippet was me, I can answer confidently: yes, I did mean something else. This would be the correct code:

public BookService(IBookRepository repository)
{
    this.repository = repository;
}

This might not seem like that big of a deal, but it’s definitely a bug. Left untreated, this would cause the “private” field to remain with its default value null. Down the road, the code would fail at runtime with a null reference exception, when time had come to use the field’s value.

That was just an example, but it’s not the only one. Off the top of my head, I could cite the following warnings:

• The similar warning CS1718, triggered by a comparison made to the same variable.
• CS0659, which warns you about overriding Equals(object o) but not GetHashCode() on a given class. This can lead to comparison problems (for example, when using the custom object as a key on a dictionary).
• CS0251, triggered by accessing an array with a negative index.

The Cranky Professor Is Your Friend

OK, there are some warnings that indicate potentially serious problems. So what? Why not keep them as warnings?

For one simple reason: people tend to completely ignore warnings.?Well, they might not ignore them in the beginning. But as time goes on and pressure starts to creep in, even the more disciplined software team eventually gives in. Before you have time to blink, you now have 4,000 warnings when you build the solution.

You might be wondering: isn’t it a tad excessive to treat all warnings as errors? After all, as we’ve just said in this post, not all warnings are created equal. They come in all shapes, sizes, and severity levels.

Yes, even once in a while you will encounter the occasional warning you’d be better off leaving as a warning or even disabling completely. Those should be the exception to the rule, though. Here’s some practical advise: at the start of a new project, start treating all warnings as errors, by default. As time goes by and you face some warning that doesn’t quite fit your reality, you can then decided what to about them on a per case basis.

Tools at Your Disposal

SubMain’s offering CodeIt.Right actually has a rule that encourages you to treat compilers warnings as errors. Similarly to the way .NET projects are configured, it’s opt-in. That means it doesn’t come switched on by default. But by all means, switch it on yourself.

Treat warnings as errors and your compiler will become a powerful ally in the eternal quest for code quality. Download and install CodeIt.Right, which will then be your second line of defense against all kinds of mistakes and bad practices that threaten your project’s health.

Learn more how CodeIt.Right can help you automate code reviews and improve the quality of your code.

About the author

Carlos Schults

Contributing Author

Carlos Schults is a .NET software developer with experience in both desktop and web development, and he?s now trying his hand at mobile. He has a passion for writing clean and concise code, and he?s interested in practices that help you improve app health, such as code review, automated testing, and continuous build. You can read more from Carlos at carlosschults.net.

This author does not have any more posts.