JSLint puked on my javascript!

Recently, I used JSLint to try to find the source of a nasty IE-only bug in my code. I’d never used JSLint directly before. What a trip.

Here’s a close facsimile of a tiny snippet of the javascript that it puked on. (I can’t paste the actual code since it doesn’t belong to me.)

Brackets and shoes required

JSLint’s main complaints were with the four if statements. As you can see, none of them have curly brackets ( { } ). My philosophy was that the { } make the code less readable and were only added to the javascript specs to help group multiple statements.

My background is heavily laden with pascal, and in pascal this is exactly the case. The if statement is defined as: if [conditions] then [statement]. (Note the singular [statement].) To group multiple statements, you need to use a grouping statement such as the begin statement which is declared as this: begin [statements...] end.

I was happily snubbing my nose at all those developers who thought there was some golden rule that brackets { } were stipulated.

Little did I know they were right!

I decide to check my javascript bible. It’s a copy of the javascript 1.5 specs I downloaded from Mozilla just after Netscape was bought by AOL. I refer to it now and then. Like when I can’t remember the order of the Array#splice parameters, or when I need to make sure my regex will run in all browsers.

Anyways, I decided to check the specs. I think it’s the first time I actually looked at the if statement. At least it’s the first time I really looked at it. And there it was in front of me: the specs mandate the use of the { }!!!

Syntax
if (condition) {
   statements1
}
[else {
   statements2
}]

It’s also obvious that the if (and else) blocks are grouping statements.

I’ve been totally foiled!

But I still think it’s more readable without the { } under some circumstances so I’ll probably continue to omit them when it feels right (and my client isn’t running my code through jslint). So there!

A perfect storm of anti-patterns

My next biggest trangression was with the following foible:

if (!already)
    var itemInit = ctrlr._getItemInit(which);
if (itemInit)
    // some lines snipped
    itemInit.call(ctrlr, root, doneMap);

The exact complaint was:

Problem at line 16 character 21: 'itemInit' used out of scope.

This is patently untrue. itemInit is definitely in scope. If you can’t see that, you’re either a dumbass noob or you’ve been blinded by java. In javascript, scope is determined by function blocks, not by statement blocks or any other type of block (except the with statement, but let’s not go there).

Therefore, this code is 100% legit. It doesn’t break in any javascript environment. Just because the var statement is in an if block, does not change the fact that it is in scope throughout the function block. It doesn’t matter where the variable is declared in the function: the scope of the variable is always the entire function.

In fact, the following code, despite being totally mental, will work flawlessly and won’t leak the today variable into the global scope as JSLint insists it will:

(function beerOfTheDay () {
    today = new Date();
    return 'Old Rasputin Imperial Stout ' + (today.getDay() == 5 ? 'is' : 'isn't') + ' the beer of the day.'
    var today;
})();

But does that make it right? Well….. no.

When I wrote the original code for my client, there was a perfect storm of anti-patterns that took over my psyche. (That’s my story and I’m sticking to it.)

The first anti-pattern:

This project is getting bulkier than I had planned. I better start conserving bytes. Placing the gimped var itemInit; outside the if block will add a whopping 9 bytes! Ouch! Every little bit counts!

The second anti-pattern (to help justify the first):

This is a core function. I better squeeze every last CPU cycle out of this. If I were to use var itemInit = !already && ctrlr._getItemInit(which);, the javascript interpreter is gonna have to write to the heap and that’s ssslllloooowwwww. bad. bad. bad!

The final (and kicker) anti-pattern:

This is pretty obvious. I mean, look. The var statement is sitting just above the lines in which the variable is used. Nobody with half a brain is gonna miss that!

The first two anti-patterns aren’t always anti-patterns. If you’re writing code for mobile devices, you may need to skimp on bytes just so your code can sneak into the device’s cache (ahem, iPhone). Or if you’re writing code that may need to execute hundreds of times per second, avoiding a variable assignment might be a good thing.

Of course, neither of these were the case here. (In my defense, I was working on a parallel project in which both of these were the case!)

The third anti-pattern is antithetical because code is always changing. Today it may look fine, but tomorrow — under somebody else’s gaze or after a few iterations of feature requests — it may look completely different.

This third anti-pattern is the one that bothered me the most. I strive to always write code that’s easy for another developer to understand and manipulate. This, clearly, was a minor transgression and I’ll need to refocus my efforts in this regard.

So, in one respect, JSLint helped me out today. But that doesn’t mean I like it any more than I did when I cursed at it earlier today. It’s still a stupid tool that doesn’t know shit about good code as far as I’m concerned.

For instance, don’t effin tell me to always use obj === null. Some times I really do want to use obj == null! I know the difference, and will likely leave a comment so the next noob who comes along doesn’t screw it up.

Stupid friggin tool. 😉

Be Sociable, Share!

10 thoughts on “JSLint puked on my javascript!

  1. Well, looks like someone’s feelings got hurt. 🙂

    Joking aside, though, you can configure it so it only spits out errors and warnings that you do care about, which is what I respect about JSLint; not forcing itself upon the user with self-righteous demands. While I do agree with some of the points you’ve raised (especially using clauses like the ‘==’ you mentioned earlier), this tool is draconic for a reason – there’s a myriad of developers who come from all sorts of backgrounds, and JSLint helps with the most common eyesores and no-nos.

    While I agree, this tends to align itself with the way Apple treats its users (lock ’em out since they don’t know what they’re doing), but the fact that with a little configuration you can get only “your” good parts out of the parser, which makes it an excellent tool.

    • Hey Krof, thanks for the info about the configuration options. I used the online version at http://jslint.com/. It’s got only a few configuration options. I imagine if I download and run it locally, it might have options for some of the other items I encountered.

      For instance, it also didn’t like that I used unescaped curly braces in my regular expressions or that I used escaped line endings in my strings. I imagine those might break some of the dumber minification tools.

      I didn’t see any options for those on the site. I think they should put some text in the options to let users know. Something like “Want more options to configure jslint? Download it for full control.” Or something like that.

  2. I always had trouble with minification tools whenever I skipped the curly braces in IF, so I started insisting my team use them. Maybe the tools have gotten smarter since then? The side benefit, of course, was that grouping/nesting problems (which plagued one colleague in particular) magically went away. heh.

    PS Old Rasputin FTW!

  3. Pingback: Tweets that mention JSLint puked on my javascript! | Unscriptable.com -- Topsy.com

  4. I’m not sure what specs you were looking at but ECMAScript defines the following syntax:

    if ( Expression ) Statement else Statement
    if ( Expression ) Statement

    {…} (a block) is technically a statement in itself (a statement that contains statements). I tend to use blocks because, oddly, I find them more readable.

    With the variable declaring issue, I always opt for declaring ALL variables at the top of the function — you may as well, considering that they’re all going to be “hoisted” there anyway…

    I’ve never had to use obj == null … most of the time this is exactly the same as just “obj” in a boolean context. (i.e. if(obj){…} or !!obj).

    • Hey James,

      Heh, the old Netscape specs are kinda loose. In a recent blog post, Andrea Giammarchi reminded me that if (x == null) is the same as writing if (x == null || x == undefined) (and also removes your responsibility of ensuring that undefined is really undefined.) Once in a while I’ll use that if I don’t care about whether the next developer understands Javascript’s many coercion nuances.

      You’re certainly right about just using if (obj) some times. If the docmentation clearly states that we’re expecting an object for a parameter (or something else that won’t coerce ambiguously), I’ll use that. Until ES5, it’s by far the fastest and lightest way to detect missing args — and, imho, the easiest to read. 🙂

  5. When you intend to do that things that JSLint thinks are wrong, you can indicate it in the code and the tool will not warn you:

    var test = 0;
    /*jslint: eqeqeq: false */
    if (test == false) { /*jslint: eqeqeq: true */
    doStuff();
    }
    /*jslint plusplus: false */
    for (test = 0; test < 5; ++test) { /*jslint plusplus: true */
    doMoreStuff()
    }

    Of course, there doesn't seem to be an option to allow == null – since null and undefined are distinct values, "typeof(test) === 'undefined' || test === null" indicates to the next programmer (with fewer bytes than a comment) that you know precisely what your variable should contain.

    Definitely agree with #3 – if you post your issue on http://tech.groups.yahoo.com/group/jslint_com/ Douglas usually responds very quickly.

  6. Hey John, Nice post.
    You might have observed during the time we interacted – I too write my code with braces, whenever possible. I still feel it is easier to read code that way.
    BTW, you did nail the IE-only bug right?

    • Ok, so like 95% of the world insists on braces. Sheesh. (Including at least one minifier — although I haven’t encountered it.)

      So now I’m trying to use braces for one-liners. Maybe I’ll get used to it.

      Eventually. 😛

      (@Pankaj, yah, I found the bug the old-fashioned way: alert() statements!)

Comments are closed.