Every so often an exasperated teammate recommends enabling -Werror in all our repos. The reasoning is sound:
- Compiler warnings are bad.
- Having lots of them is demoralizing.
- If we had prevented them from ever occurring we wouldn’t be in this mess.
This is a particularly good policy to prevent warnings in new code. If my compiler warns me right as I do something bad, this is the perfect time to fix it. Like when I write this function:
override fun toString(): String {
return buildString {
append(name)
append('=')
append(value)
if (httpOnly) {
append("; httponly")
}
return toString()
}
}If you paste the above function into IntelliJ, it’ll show you an immediate Unreachable Code warning for the return statement on line 2. This naughty code needlessly returns two ways, and removing the return statement on line 11 is the best fix.
This example isn’t theoretical; this problem has been in OkHttp’s Cookie.kt since I originally wrote it in May 2019.
Why check in unreachable code?
This function has been problematic since it was written. Why did I ignore the warning in the first place?
Well I didn’t get a warning! This unused code detector was added sometime between May 2019’s Kotlin 1.3.40 and today’s Kotlin 1.6.10. Each time we upgrade Kotlin we get smarter warnings that detect more problems than before.
We aught to stay on top of these warnings, particularly after upgrading third party libraries, build tools, IDEs, and language versions. We should take advantage of their smarts and proactively fix things.
Fix warnings when they’re introduced?
It’s tempting to couple two complementary changes:
- Upgrading a dependency
- Fixing the warnings introduced by that upgrade
With -Werror, we make these two steps are inseparable. You can’t check in code that has warnings! But I don’t like -Werror because it breaks incrementalism.
Simple fixes may have unexpected consequences! I want to make and review such changes independently, perhaps adding tests to check before & after behavior.
Doing both dependency upgrades and warnings fixes in a single commit is an unforced risk. If a dependency upgrade introduces new (helpful!) warnings, -Werror may have the side-effect of discouraging or delaying the upgrade.
Yuck, deprecated things
Deprecations are the biggest source of warnings in the codebases I contribute to. Fixing deprecations is tedious work, and may even cascade into further deprecations!
Unfortunately Kotlin has no mechanism to keep deprecations as warnings while promoting everything else to errors. Vote for this issue if you’d like that!
Compiler Warnings Encourage Incrementalism
Compiler warnings are like the Check Engine warning light on the dashboard of your car. There’s a problem that needs attention! But should it illuminate while you’re traveling 100 km/h on the expressway, do not slam on the brakes and block traffic while you fix it. Non-urgent warnings do not warrant urgent attention.
Do attend to your Check Engine light. Do attend to your compiler warnings. But if you block progress until the warning is cleared, you might be making things worse.