Terrible JavaScript Mistakes To Avoid With A Static Code Analyzer

JSLint, JSHint And Closure Compiler

You have three main choices for static analyzers in the JavaScript world: JSLint, JSHint and Closure Compiler.

JSLint

JSLint was the first static analyzer for JavaScript. You can run it on the official website or use one of the wrappers to run it on your local files. JSLint finds a lot of useful errors, but it’s very rigid. Here’s a good example:

var s = 'mystring';
for (var i = 0; i < s.length; i++) {
  console.log(s.charAt(i));
}

JSLint will show two errors for this code:

Unexpected '++'.
Move 'var' declarations to the top of the function.

The first problem is the declaration of the variable i at the top of the loop. JSLint also doesn’t like the ++ operator at the end of the loop declaration. It wants the code to look like this:

var s = 'mystring';
var i;
for (i = 0; i < s.length; i = i + 1) {
  console.log(s.charAt(i));
}

I appreciate where JSLint is coming from, but it’s just too strict for me. It was too rigid for Anton Kovalyov as well, so he created JSHint.

JSHint

JSHint works similarly to JSLint, but it’s written on top of Node.js and it’s much more flexible. JSHint has a long list of options, making it possible to create custom checks by writing your own reporter.

You can run JSHint from the website, but most of the time you would install JSHint as a local command-line tool using Node.js. Once JSHint is installed, you can run it against your files with a command like this:

jshint test.js

JSHint also has plugins for popular text editors, so you can run JSHint while you’re coding.

Closure Compiler

Closure Compiler, from Google, is a different breed. As the name suggests, it’s a compiler as well as a checker. It’s written in Java and based on the Rhino parser from Mozilla. Closure Compiler has a simple mode to do basic code checking, but it also has more advanced modes to do extra checking and enforce special type declarations.

Closure Compiler reports errors in JavaScript code, but it also creates minimized versions of JavaScript. The compiler removes white space, comments and unused variables and simplifies long statements to make a script as small as possible.

Google makes a simple version of its compiler available on the Web, but most of the time you’ll want to download Closure Compiler and run it locally.

Closure Compiler will output a list of files into a single minimized file after checking their code. You can run it like that after you’ve downloaded the compiler.jar file.

java -jar compiler.jar --js_output_file compress.js --js test1.js --js test2.js

Choosing the Right Checker

In my projects, I combine Closure Compiler with JSHint. Closure Compiler does the minimization and basic checking, while JSHint handles the more complex code analysis. The two work well together, and each covers some areas that the other doesn’t. In addition, I can use the extension capabilities of JSHint to write custom checkers. One common checker I write checks for particular functions that I don’t want, like calling functions that I don’t want to allow in my project.

Now that we’ve looked at a few checkers, let’s look at some bad code. All of these six examples are code you should never write and are spots where code checkers would keep you out of trouble.

This article uses JSHint for most examples, but Closure Compiler would produce similar warnings.

== Versus ===

JavaScript is a dynamically typed language. You don’t have to declare types when you’re coding, but they exist at runtime. JavaScript offers two compare operators to handle these dynamic types: == and ===. Let’s look at an example.

var n = 123;
var s = '123';

if (n == s) {
  alert('The variables were equal');
}

if (n === s) {
  alert('The variables were identical');
}

The == operator compares the values of the two objects. It converts the objects and compares them separately from their types. The === operator compares the object types and the values. In this case, the first if block will pop up an alert, and the second if block won’t — because n and s have the same value but not the same type.

The == comparator is a relic from the C language roots of JavaScript. Using it is almost always a mistake: Comparing values separate from types is rarely what the developer means to do. In reality, the number “one hundred twenty-three” is different from the string “one two three.” These operators are easy to mistype and even easier to misread.

Check this code with JSHint and you’ll get this:

test.js: line 9, col 12, Expected '===' and instead saw '=='.

Undefined Variables And Late Definitions

Let’s start with some simple code:

function test() {
  var myVar = 'Hello, World';
  console.log(myvar);
}

See the bug? I make this mistake all the time. Run this code and you’ll get an error:

ReferenceError: myvar is not defined

Let’s make the problem a little more difficult to spot:

function test() {
  myVar = 'Hello, World';
  console.log(myVar);
}

Run this and you’ll get:

Hello, World

This second example works, but it has some very unexpected side effects. The rules for declaring JavaScript variables and the scopes they end up in are confusing at best.

In the first case, JSHint will tell you this:

test.js: line 3, col 17, 'myvar' is not defined.

In the second case, it will tell you this:

test.js: line 2, col 5, 'myVar' is not defined.
test.js: line 3, col 17, 'myVar' is not defined.

The first case saves you from a runtime bug. You don’t have to test your app — JSHint will find the error for you. The second case is worse because testing won’t find the bug.

The problem with the second case is insidiously subtle and complex. The variable myVar has now escaped from its function scope and been hoisted into the global scope for the whole page. This means that it will exist and have a value of Hello, World after the test function has run. This is called “global scope pollution.”

The myVar variable will exist for every other function that runs after the test function. Run the following code after you’ve run the test function:

console.log('myVar: ' + myVar);

You’ll still get Hello, World. The myVar variable will hang around your code like mold, causing tricky bugs you won’t find until 3:00 am the night before you release, all because you forgot to type var.

Variable Reuse

Redefining variables is allowed in JavaScript, but it’s almost always an accident. Take a look:

function incrementCount(counter) {
  if (counter.count) {
    counter.count++;
  } else {
    var counter = 1;
    counter.count = counter;
  }
}

In this function we’re incrementing the count property on the object that was passed in, but we need to add the property if it doesn’t already exist. See the bug?

This function will never add or increment a counter on anything. The else statement will always be called, and it will redefine the function argument counter. Basically this function creates a new object, assigns a property to it and then loses the object when the function returns. It will never change the object that was passed in.

This simple typo will make the code run without any errors but will produce a very strange result.

JSHint will tell you this:

test.js: line 21, col 21, 'counter' is already defined.

Curly Braces In Blocks, Loops And Conditionals

if (false)
  doSomethingElse();
  doSomething();

Will this code doSomething or doSomethingElse? At first glance, I always think it won’t doSomething or doSomethingElse. That’s the way it works in Python, but not in JavaScript. JavaScript will treat the one line after the if statement merely as part of the block; the indenting doesn’t matter.

This issue is simply about code readability. If you can’t understand what the code will do, then you’ll write bugs.

Python and CoffeeScript like to skip the curly braces. That might work fine in languages that guarantee to format white space well, but JavaScript is looser than that. JavaScript allows a lot of strange syntax, and curly braces will keep you out of trouble.

if (false) {
  doSomethingElse();
  doSomething();
}

Add the braces and you’ll always make code more readable. Skip them and JSHint will tell you this:

test.js: line 27, col 5, Expected '{' and instead saw 'doSomething'.

Single And Double Quotes

console.log("This is a string. It's OK.");
console.log('This string is OK too.');
console.log("This string " + 'is legal, but' + "really not OK.");

JavaScript allows you to define a string with single or double quotes. It’s nice to have the flexibility, like when you’re defining HTML, but the added flexibility can lead to some very inconsistent code.

Google has a code style guide that always uses single quotes for strings, so that they don’t have to escape double quotes in HTML. I can’t argue that single quotes are better than double quotes, but I can argue for consistency. Keeping everything consistent makes code more readable.

JSHint will warn you about mixed quotes like this:

test.js: line 31, col 27, Mixed double and single quotes.

Copying and pasting or mistyping a quote is easy. Once you have one bad quote, others will follow, especially if a lot of people are editing the file. Static analyzers will help keep the quotes consistent and prevent a big cleanup in the future.

Cyclomatic Complexity

Cyclomatic complexity is the measure of how complex a given block of code is. Look at the code and count the number of paths that could possibly run: That number is its cyclomatic complexity.

For example, this code has a cyclomatic complexity of 1:

function main() {
  return 'Hello, World!';
}

You can follow only one path through this code.

Let’s add a little conditional logic:

function main() {
  if (true) {
    return 'Hello, World!';
  } else {
    return 'Hello, unWorld!';
  }
}

The cyclomatic complexity has jumped to 2.

Ideal code is easy to read and understand. The higher the cyclomatic complexity, the more difficult the code will be to understand. Everyone agrees that high cyclomatic complexity is bad, but no one agrees on a limit; 5 is fine, and 100 is too high — but there’s a lot of gray area in the middle.

If the cyclomatic complexity gets to the predefined limit, then JSHint will let you know.

test.js: line 35, col 24, This function's cyclomatic complexity is too high. (17)

JSHint is the only one of the three checkers that looks at cyclomatic complexity. It also allows you to set the limit. Go above the maxcomplexity number that you’ve set and JSHint will warn you. I like to set the limit to 14, but I’ll go a little higher in projects in which I do a lot of parsing or when I have other reasons to need many code paths.

The real reason the complexity number is important is that it tells you when to refactor your code. The first time you write a long function, it always makes sense. But if you wait six months and then come back to fix bugs, you’ll be glad that you took the time to make it easier to read.

Cyclomatic complexity usually breaks down with laundry lists. For example, I created a calendar, and I wanted to get the correct first day of the week for each country. I had a function that looked something like this:

function getFirstDay(country) {
  if (country === 'USA') {
    return 'Sunday';
  } else if (country === 'France') {
    return 'Monday';
  } else if…
}

I supported a lot of countries, so the cyclomatic complexity quickly grew to over 50. Although the code was very easy to read, the number of paths was high, so my code analyzer complained. In the end, I split up the function to get the complexity below my maximum. It was a hack for this particular case, but it’s a small price to pay for cleaner code overall.

Check Everything That You’ll Ever Edit More Than Once

Static checkers find the bugs that you wouldn’t come across with simple testing. They also find bugs at compile time, as opposed to runtime — those middle-of-the-night bugs that only creep in when a dozen people are all trying to do the same thing. Finding all of those subtle bugs is a long and painful process without code checking.

I began this article by claiming that I always use a code analyzer, but I don’t in one case: with throwaway code. I like using quick prototypes to show interactive ideas and to help my team come together on how something should work. Those prototypes are write-once code; I never need to fix bugs in them because I’ll be throwing away the prototypes a few weeks later. This throwaway code exists solely for the quick demos, and I don’t care if it has subtle bugs. Everything I care about, though, gets analyzed.

Fixing these types of bugs at the beginning of a project is easy; finding them on the night before you release will drive you nuts. Code analyzers have saved my butt many times, and they’ll also save yours.

Image on front page created by Ruiwen Chua.