Monday, September 6, 2010

First impressions of gjslint

I saw a blog post from the Closure Tools Blog recently and it piqued my interest enough to give it a try. I've been running jslint on a codebase for the past 6 months or so and I've been a little frustrated with some of the 'errors' reported by it (I don't think I'm the only one). jslint is not without its warts, but it has definitely forced me to take a second look at my code and made me rethink one or two coding decisions. Some of the 'issues' my source code continuously bumps into with jslint include:

1. Not using the new operator:
Lint at line 89 character 53: Do not use 'new' for side effects.

2. Misuse of Ternary operators:
Lint at line 145 character 112: Expected an assignment or function call and instead saw an expression.
!disabled ? dojo.removeClass(el, 'disabledButton') : dojo.addClass(el, 'disabledButton');

3. Unexpected use of >> << & |
return (transferMode >> 0) & 3;

4. Use of === vs. ==
if (range.text == '') {

Anyways, I'm always interested in seeing some static analysis of my code, so I downloaded and installed gjslint. Running it is pretty simple - just point it at a directory full of .js files:
gjslint -r path/to/my/directory

Oddly, none of the issues which kept on showing up in the regular jslint checker appeared in the results of gjslint. I guess this is because gjslint is more focused on closure's code formatting rules more so than strictly correct (at least in the eyes of jslint) JavaScript?

After running gjslint on my codebase, I saw a a _lot_ of these types of errors:
Line 54, E:0200: Invalid JsDoc tag: name
Line 55, E:0200: Invalid JsDoc tag: function
Line 56, E:0200: Invalid JsDoc tag: memberOf

Apparently, the gjslint checker only allows a certain subset of certain JsDoc rules. I tried using the --nojsdoc flag in the hope that all these 'errors' would be suppressed, but no dice.

Also - 80 characters seems to be the limit that gjslint allows for JavaScript files. Would be nice if this rule could be suppressible or at least configurable. Not even sure why its a rule in the first place.

For kicks, I tried passing the --strict flag to see the output. I seem to get a lot of indentation errors:
Line 342, E:0006: (New error) Wrong indentation: expected any of {4, 16} but got 8
Line 343, E:0006: (New error) Wrong indentation: expected any of {4, 16} but got 8
Line 344, E:0006: (New error) Wrong indentation: expected any of {4, 16} but got 8

I've never been a huge fan of being overtly strict when it comes to the number of spaces which represents an indentation. I use 4 for each indent - gjslint advises you to use 2 - no big deal in my books.

Included in this release of gjslint, there's a handy tool called fixjsstyle which will help you automatically fix any styling issues in your code. One point of caution when running this tool though - make sure your code is under source control before you run it. Stepping through your code after running fixjsstyle reveals some more gjslint rules. For example, running fixjsstyle on a code base will convert all double quotes to single quotes. Wonder why that's in the style guide? Actually, it'd be great to see some explanations behind some of the rules.

Overall - I've always felt that static code analysis of code is useful to a point. It'll definitely keep you keen when it comes to coding standards and it is useful in that it'll catch any errors that any self-respecting compiler would catch, but it'll only get you so far. I've always found gathering and understanding runtime analysis of a codebase more difficult to achieve (I still have a lot to learn in this field), but more rewarding in the end.

No comments:

Post a Comment