Warnings-as-errors friction by Mark Seemann
TDD friction. Surely that's a bad thing(?)
Paul Wilson recently wrote on Mastodon:
Software development opinion (warnings as errors)
Just seen this via Elixir Radar, https://curiosum.com/til/warnings-as-errors-elixir-mix-compile on on treating warnings as errors, and yeah don't integrate code with warnings. But ....
Having worked on projects with this switched on in dev, it's an annoying bit of friction when Test Driving code. Yes, have it switched on in CI, but don't make me fix all the warnings before I can run my failing test.
(Using an env variable for the switch is a good compromise here, imo).
This made me reflect on similar experiences I've had. I thought perhaps I should write them down.
To be clear, this article is not an attack on Paul Wilson. He's right, but since he got me thinking, I only find it honest and respectful to acknowledge that.
The remark does, I think, invite more reflection.
Test friction example #
An example would be handy right about now.
As I was writing the example code base for Code That Fits in Your Head, I was following the advice of the book:
- Turn on Nullable reference types (only relevant for C#)
- Turn on static code analysis or linters
- Treat warnings as errors. Yes, also the warnings produced by the two above steps
As Paul Wilson points out, this tends to create friction with test-driven development (TDD). When I started the code base, this was the first TDD test I wrote:
[Fact] public async Task PostValidReservation() { var response = await PostReservation(new { date = "2023-03-10 19:00", email = "katinka@example.com", name = "Katinka Ingabogovinanana", quantity = 2 }); Assert.True( response.IsSuccessStatusCode, $"Actual status code: {response.StatusCode}."); }
Looks good so far, doesn't it? Are any of the warnings-as-errors settings causing friction? Not directly, but now regard the PostReservation helper method:
[SuppressMessage( "Usage", "CA2234:Pass system uri objects instead of strings", Justification = "URL isn't passed as variable, but as literal.")] private async Task<HttpResponseMessage> PostReservation( object reservation) { using var factory = new WebApplicationFactory<Startup>(); var client = factory.CreateClient(); string json = JsonSerializer.Serialize(reservation); using var content = new StringContent(json); content.Headers.ContentType.MediaType = "application/json"; return await client.PostAsync("reservations", content); }
Notice the [SuppressMessage] attribute. Without it, the compiler emits this error:
error CA2234: Modify 'ReservationsTests.PostReservation(object)' to call 'HttpClient.PostAsync(Uri, HttpContent)' instead of 'HttpClient.PostAsync(string, HttpContent)'.
That's an example of friction in TDD. I could have fixed the problem by changing the last line to:
return await client.PostAsync(new Uri("reservations", UriKind.Relative), content);
This makes the actual code more obscure, which is the reason I didn't like that option. Instead, I chose to add the [SuppressMessage] attribute and write a Justification. It is, perhaps, not much of an explanation, but my position is that, in general, I consider CA2234 a good and proper rule. It's a specific example of favouring stronger types over stringly typed code. I'm all for it.
If you grok the motivation for the rule (which, evidently, the documentation code-example writer didn't) you also know when to safely ignore it. Types are useful because they enable you to encapsulate knowledge and guarantees about data in a way that strings and ints typically don't. Indeed, if you are passing URLs around, pass them around as Uri objects rather than strings. This prevents simple bugs, such as accidentally swapping the place of two variables because they're both strings.
In the above example, however, a URL isn't being passed around as a variable. The value is hard-coded right there in the code. Wrapping it in a Uri object doesn't change that.
But I digress...
This is an example of friction in TDD. Instead of being able to just plough through, I had to stop and deal with a Code Analysis rule.
SUT friction example #
But wait! There's more.
To pass the test, I had to add this class:
[Route("[controller]")] public class ReservationsController { #pragma warning disable CA1822 // Mark members as static public void Post() { } #pragma warning restore CA1822 // Mark members as static }
I had to suppress CA1822 as well, because it generated this error:
error CA1822: Member Post does not access instance data and can be marked as static (Shared in VisualBasic)
Keep in mind that because of my settings, it's an error. The code doesn't compile.
You can try to fix it by making the method static, but this then triggers another error:
error CA1052: Type 'ReservationsController' is a static holder type but is neither static nor NotInheritable
In other words, the class should be static as well:
[Route("[controller]")] public static class ReservationsController { public static void Post() { } }
This compiles. What's not to like? Those Code Analysis rules are there for a reason, aren't they? Yes, but they are general rules that can't predict every corner case. While the code compiles, the test fails.
Out of the box, that's just not how that version of ASP.NET works. The MVC model of ASP.NET expects action methods to be instance members.
(I'm sure that there's a way to tweak ASP.NET so that it allows static HTTP handlers as well, but I wasn't interested in researching that option. After all, the above code only represents an interim stage during a longer TDD session. Subsequent tests would prompt me to give the Post method some proper behaviour that would make it an instance method anyway.)
So I kept the method as an instance method and suppressed the Code Analysis rule.
Friction? Demonstrably.
Opt in #
Is there a way to avoid the friction? Paul Wilson mentions a couple of options: Using an environment variable, or only turning warnings into errors in your deployment pipeline. A variation on using an environment variable is to only turn on errors for Release builds (for languages where that distinction exists).
In general, if you have a useful tool that unfortunately takes a long time to run, making it a scheduled or opt-in tool may be the way to go. A mutation testing tool like Stryker can easily run for hours, so it's not something you want to do for every change you make.
Another example is dependency analysis. One of my recent clients had a tool that scanned their code dependencies (NuGet, npm) for versions with known vulnerabilities. This tool would also take its time before delivering a verdict.
Making tools opt-in is definitely an option.
You may be concerned that this requires discipline that perhaps not all developers have. If a tool is opt-in, will anyone remember to run it?
As I also describe in Code That Fits in Your Head, you could address that issue with a checklist.
Yeah, but do we then need a checklist to remind us to look at the checklist? Right, quis custodiet ipsos custodes? Is it going to be turtles all the way down?
Well, if no-one in your organisation can be trusted to follow any commonly-agreed-on rules on a regular basis, you're in trouble anyway.
Good friction? #
So far, I've spent some time describing the problem. When encountering resistance your natural reaction is to find it disagreeable. You want to accomplish something, and then this rule/technique/tool gets in the way!
Despite this, is it possible that this particular kind of friction is beneficial?
By (subconsciously, I'm sure) picking a word like 'friction', you've already chosen sides. That word, in general, has a negative connotation. Is it the only word that describes the situation? What if we talked about it instead in terms of safety, assistance, or predictability?
Ironically, friction was a main complaint about TDD when it was first introduced.
"What do you mean? I have to write a test before I write the implementation? That's going to slow me down!"
The TDD and agile movement developed a whole set of standard responses to such objections. Brakes enable you to go faster. If it hurts, do it more often.
Try those on for size, only now applied to warnings as errors. Friction is what makes brakes work.
Additive mindset #
As I age, I'm becoming increasingly aware of a tendency in the software industry. Let's call it the additive mindset.
It's a reflex to consider addition a good thing. An API with a wide array of options is better than a narrow API. Software with more features is better than software with few features. More log data provides better insight.
More code is better than less code.
Obviously, that's not true, but we. keep. behaving as though it is. Just look at the recent hubbub about ChatGPT, or GitHub Copilot, which I recently wrote about. Everyone reflexively view them as productivity tools because the can help us produce more code faster.
I had a cup of coffee with my wife as I took a break from writing this article, and I told her about it. Her immediate reaction when told about friction is that it's a benefit. She's a doctor, and naturally view procedure, practice, regulation, etcetera as occasionally annoying, but essential to the practice of medicine. Without procedures, patients would die from preventable mistakes and doctors would prescribe morphine to themselves. Checking boxes and signing off on decisions slow you down, and that's half the point. Making you slow down can give you the opportunity to realise that you're about to do something stupid.
Worried that TDD will slow down your programmers? Don't. They probably need slowing down.
But if TDD is already being touted as a process to make us slow down and think, is it a good idea, then, to slow down TDD with warnings as errors? Are we not interfering with a beneficial and essential process?
Alternatives to TDD #
I don't have a confident answer to that question. What follows is tentative. I've been doing TDD since 2003 and while I was also an early critic, it's still central to how I write code.
When I began doing TDD with all the errors dialled to 11 I was concerned about the friction, too. While I also believe in linters, the two seem to work at cross purposes. The rule about static members in the above example seems clearly counterproductive. After all, a few commits later I'd written enough code for the Post method that it had to be an instance method after all. The degenerate state was temporary, an artefact of the TDD process, but the rule triggered anyway.
What should I think of that?
I don't like having to deal with such false positives. The question is whether treating warnings as errors is a net positive or a net negative?
It may help to recall why TDD is a useful practice. A major reason is that it provides rapid feedback. There are, however, other ways to produce rapid feedback. Static types, compiler warnings, and static code analysis are other ways.
I don't think of these as alternatives to TDD, but rather as complementary. Tests can produce feedback about some implementation details. Constructive data is another option. Compiler warnings and linters enter that mix as well.
Here I again speak with some hesitation, but it looks to me as though the TDD practice originated in dynamically typed tradition (Smalltalk), and even though some Java programmers were early adopters as well, from my perspective it's always looked stronger among the dynamic languages than the compiled languages. The unadulterated TDD tradition still seems to largely ignore the existence of other forms of feedback. Everything must be tested.
At the risk of repeating myself, I find TDD invaluable, but I'm happy to receive rapid feedback from heterogeneous sources: Tests, type checkers, compilers, linters, fellow ensemble programmers.
This suggests that TDD isn't the only game in town. This may also imply that the friction to TDD caused by treating warnings as errors may not be as costly as first perceived. After all, slowing down something that you rely on 75% of the time isn't quite as bad as slowing down something you rely on 100% of the time.
While it's a cost, perhaps it went down...
Simplicity #
As always, circumstances matter. Is it always a good idea to treat warnings as errors?
Not really. To be honest, treating warnings as errors is another case of treating a symptom. The reason I recommend it is that I've seen enough code bases where compiler warnings (not errors) have accumulated. In a setting where that happens, treating (new) warnings as errors can help get the situation under control.
When I work alone, I don't allow warnings to build up. I rarely tell the compiler to treat warnings as errors in my personal code bases. There's no need. I have zero tolerance for compiler warnings, and I do spot them.
If you have a team that never allows compiler warnings to accumulate, is there any reason to treat them as errors? Probably not.
This underlines an important point about productivity: A good team without strict process can outperform a poor team with a clearly defined process. Mindset beats tooling. Sometimes.
Which mindset is that? Not the additive mindset. Rather, I believe in focusing on simplicity. The alternative to adding things isn't to blindly remove things. You can't add features to a program only by deleting code. Rather, add code, but keep it simple. Decouple to delete.
perfection is attained not when there is nothing more to add, but when there is nothing more to remove.
Simple code. Simple tests. Be warned, however, that code simplicity does not imply naive code understandable by everyone. I'll refer you to Rich Hickey's wonderful talk Simple Made Easy and remind you that this was the line of thinking that lead to Clojure.
Along the same lines, I tend to consider Haskell to be a vehicle for expressing my thoughts in a simpler way than I can do in F#, which again enables simplicity not available in C#. Simpler, not easier.
Conclusion #
Does treating warnings as errors imply TDD friction? It certainly looks that way.
Is it worth it, nonetheless? Possibly. It depends on why you need to turn warnings into errors in the first place. In some settings, the benefits of treating warnings as errors may be greater than the cost. If that's the only way you can keep compiler warnings down, then do treat warnings as errors. Such a situation, however, is likely to be a symptom of a more fundamental mindset problem.
This almost sounds like a moral judgement, I realise, but that's not my intent. Mindset is shaped by personal preference, but also by organisational and peer pressure, as well as knowledge. If you only know of one way to achieve a goal, you have no choice. Only if you know of more than one way can you choose.
Choose the way that leaves the code simpler than the other.
