Linters and style checkers: how to save hours of painful debugging

I'm al­ways im­pressed with the con­sis­ten­cy of screw-ups caused by the un­will­ing­ness of pro­gram­mers to fol­low ba­sic guide­lines. To be more spe­cif­ic, I'm talk­ing about my own screw-ups here.

I've been con­vinced a long time ago that:

• Style check­ers and lin­ters should be used con­sis­tent­ly on every pro­ject,
• Style-wise, non-com­pli­ant code has no place in the source repos­i­to­ry,
• Lin­ters which are too slow for pre-com­mit hooks should be run by CI serv­er,
• Such check­ers should be set at high­est strict­ness lev­el, which makes the choice be­tween the js­Lint and the jsHint very easy, since the last one is hap­py to let you in­tro­duce bugs, since even the ar­ti­cle ex­plain­ing why jsHint is bet­ter con­tains bug­gy code.
• Any­thing re­vealed by those check­ers should be qual­i­fied as a er­ror, not a warn­ing, that is a sec­ond-class cit­i­zen which has no ac­tu­al mean­ing in de­vel­op­ment.

Fol­low­ing rules en­forced by style check­ers and lin­ters is not that dif­fi­cult. It is amaz­ing to see how eas­i­ly those check­es pre­vent us from in­tro­duc­ing sub­tle, dif­fi­cult to find bugs. For ex­am­ple, I'm ab­solute­ly in­ca­pable of find­ing a bug in this piece of JavaScript code by just glanc­ing at it if I omit style con­ven­tions:

var buildInit = function () {
    // Do something here.
    return
    {
        shell: true,
        acceptRange: null,
        threshold: findThreshold()
    };
};

And still, here it is, the ugly bug which could be found in less than a mil­lisec­ond by a lin­ter, and which makes the func­tion re­turn undefined.

But I'm not here to talk about JavaScript au­to­mat­ic semi­colon in­ser­tion. I'm here to talk about C#, and more pre­cise­ly about a par­tic­u­lar as­pect of it. If you have used C#, you have prob­a­bly no­ticed the ArgumentException/ArgumentNullException in­con­sis­ten­cy. Here are their re­spec­tive sig­na­tures:

public ArgumentException(string message, string paramName)
public ArgumentNullException(string paramName, string message)

Doesn't it look a lot like PHP's haystack-nee­dle mess? Well, there is a good rea­son for this or­der of ar­gu­ments. In the case of ArgumentException, one has to spec­i­fy a mes­sage, be­cause .NET Frame­work doesn't know what is ex­act­ly wrong with the giv­en ar­gu­ment. On the oth­er hand, there is no need to spec­i­fy a mes­sage in ArgumentNullException, be­cause .NET Frame­work is ca­pa­ble of gen­er­at­ing it on its own, at a con­di­tion to have the pa­ra­me­ter name, so it's more of­ten used this way:

public ArgumentNullException(string paramName)

Still, I was nev­er par­tic­u­lar­ly hap­py with code which looks like this:

if (summary == null)
    throw new ArgumentNullException("summary");

if (!summary.IsValid)
    throw new ArgumentException("The summary should be marked as [...]", "summary");

if (price == null)
    throw new ArgumentNullException("price");

It doesn't feel right. Thanks to named ar­gu­ments, we can make it bet­ter:

if (summary == null)
    throw new ArgumentNullException(paramName: "summary");

if (!summary.IsValid)
    throw new ArgumentException(paramName: "summary", message: "The summary [...]");

if (price == null)
    throw new ArgumentNullException(paramName: "price");

This piece of code is clear and much less risky. The same tech­nique was suc­cess­ful­ly used in dif­fer­ent forks of Or­seis to mit­i­gate the risk of this type of bugs.

But I wasn't con­sis­tent across pro­jects. I wasn't us­ing the same tech­nique every­where, which lead to the fol­low­ing piece of code in my lat­est pro­ject:

Pair = new Pair<IAssembly>(
    new Assembly(this.FindAssemblyPathOf<WebClient>());
    new Assembly(this.FindAssemblyPathOf(typeof(WebClientProxy))));

I have com­plete­ly missed the fact that Pair<T>—which was an­oth­er class from the same pro­ject—was ex­pect­ing two ar­gu­ments: a proxy as­sem­bly, and only than a tar­get as­sem­bly. In the piece of code be­low, I was pass­ing the tar­get first, and only then the proxy.

The re­sult was weird. I spent two and a half hours try­ing to un­der­stand what is hap­pen­ing, with no hint what­so­ev­er: code from unit tests seemed to work, while this code of the sys­tem test which is not very dif­fer­ent just fails with un­help­ful er­ror mes­sage.

I fi­nal­ly found the is­sue. The cor­rect­ed ver­sion of the code looks like this:

Pair = new Pair<IAssembly>(
    proxy: new Assembly(this.FindAssemblyPathOf(typeof(WebClientProxy))),
    target: new Assembly(this.FindAssemblyPathOf<WebClient>()));

If only I had a sta­t­ic check­er which could force me to use named ar­gu­ments when­ev­er the method ac­cepts the pa­ra­me­ters of the same type, those two and a half hours could be short­ened to a few mil­lisec­onds. As sim­ple as that.

The morale is that while it might seem cum­ber­some to use style check­ers and lin­ters, they ac­tu­al­ly save hours of de­bug­ging. Not fol­low­ing ba­sic guide­lines be­cause of lazi­ness like I did is just un­pro­fes­sion­al.