Menu Search
Jump to the content X X
Smashing Conf New York

You know, we use ad-blockers as well. We gotta keep those servers running though. Did you know that we publish useful books and run friendly conferences — crafted for pros like yourself? E.g. our upcoming SmashingConf New York, dedicated to smart front-end techniques and design patterns.

Terrible JavaScript Mistakes To Avoid With A Static Code Analyzer

Hardly any line of my code comes out perfect the first time I write it. Well, most of the time… Some of the time… Um, hardly ever. The truth is that I spend more time chasing down my own stupid programming errors than I’d like to admit. That’s why I use static analyzers in every JavaScript file I write.

Static analyzers look at code and find problems before you run it. They do simple checks, like enforcing syntax (for example, tabs instead of spaces), and more holistic checks, like making sure your functions aren’t too complex. Static analyzers also find errors that you can’t find with testing, like instances of == when you meant ===.

Further Reading on SmashingMag:

In large projects and on big teams, you’ll be happy to have a little help finding those “simple” bugs that turn out to be a lot less simple than they looked.

JSLint, JSHint And Closure Compiler Link

You have three main choices for static analyzers in the JavaScript world: JSLint4, JSHint5 and Closure Compiler6.

JSLint Link

JSLint was the first static analyzer for JavaScript. You can run it on the official website7 or use one of the wrappers8 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++) {

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) {

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

JSHint Link

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

You can run JSHint from the website13, but most of the time you would install JSHint as a local command-line tool14 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 Link

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 Rhino15 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 Web16, but most of the time you’ll want to download Closure Compiler17 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 Link

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 === Link

JavaScript is a dynamically typed18 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 Link

Let’s start with some simple code:

function test() {
  var myVar = 'Hello, World';

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';

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 hoisted19 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 Link

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

function incrementCount(counter) {
  if (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 Link

if (false)

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) {

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 Link

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 Link

Cyclomatic complexity20 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 Link

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 Chua21.

(al, ml, da)

Footnotes Link

  1. 1
  2. 2
  3. 3
  4. 4
  5. 5
  6. 6
  7. 7
  8. 8
  9. 9
  10. 10
  11. 11
  12. 12
  13. 13
  14. 14
  15. 15
  16. 16
  17. 17
  18. 18
  19. 19
  20. 20
  21. 21

↑ Back to top Tweet itShare on Facebook

Zack Grossbart is an engineer, designer, and author. He's an Architecting Engineer and Human Factors Specialist at Micro Focus where he focuses on enterprise identity and security. Zack began loading DOS from a floppy disk when he was five years old. He's been getting paid to code since he was 15 and started his first software company when he was 16. Zack lives in Cambridge, Massachusetts with his wife and daughter.

  1. 1

    Worth also mentioning JavaScript type (and inferred type) checkers: Flow, TypeScript and others, for example.

    • 2

      Zack Grossbart

      February 18, 2015 4:43 pm

      TypeScript is a language instead of a checker, right? It’s more like CoffeScript than a tool.

      • 3

        Well, it’s a language, yes, but it’s a bit like Closure Compiler in that it’s also a static analysis tool. The only difference between TypeScript and the others is that you actually have to write your code in TypeScript (that is, with type hints) to get the benefit of static analysis.

        • 4

          Actually the typescript compiler accepts all valid Javascript but will warn you about the use of bad practices or when the code might lead to unexpected behaviour. This makes getting started with Typescript really easy. You just start with your existing Javascript knowledge and you slowly start to take advantage of Typescript by adding it’s syntax where you need it.

    • 5

      or Haxe, even better :)

  2. 6

    Good Article! But I’m a little bit confused here, when you talk about global scope pollution:

    function test() {
    myVar = ‘Hello, World’;

    “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.”

    I think that the problem here, is that you have created a Global Implied. That means, that myVar, is really a window object property rather than a global variable. And, it wasn´t hoisted, it’s available because it’s part of the window obj, you can test it easily: window.myVar
    I’m also seeing that JSHint looks for a variable, and due that it cant find it, it’s throwing an error, and it sound reasonable.

    Please let me know your thoughts,

    Thanks so much!


    • 7

      Zack Grossbart

      February 18, 2015 4:59 pm

      Hi Guillermo,

      In JavaScript making something available on the global scope and making something a window variable are the same thing. The real issue is that you probably meant the myVar variable to exist just in the scope of this function. My skipping the var keyword in the declaration you’re adding it to the Window where it can be change or accessed accidentally by other code in your application.

      • 8

        Variable and object property are not the same thing. Guillermo is correct. You can test it very easily … you can “delete” object properties, but you can’t do it with variables. So:

        myGlobalPolutingPseudoVariable = 1;
        delete myGlobalPolutingPseudoVariable; => true

        var myVariable = 1;
        delete myVariable; => false

      • 9

        cool, thanks!

  3. 10

    Can we recap; javascript is so loose, you can access variables that doesn’t exist yet. You can write valid codes which mixes scopes. You can use easily change a variables type. This causes bad performance apps when doing it too much wrong. This frustrates a lot.

    Personally I’m a big fan of compiled languages, who work the other way around. You have source code, when that compiles, you get valid code that performs good. The compiler is aware of types, scope, knows if variables are defined before used, detects ambiguous code patterns. I think fixing it at forehand is nicer than afterwards.

    I use Haxe (an optimizing compiler that also works for multiple platforms) for a few years now, I’m glad I don’t have to worry about such code checks and continue building the stuff I want. Everything that is within compiled context feels much safer than writing normal Javascript.

    • 11

      They way you phrase it sounds like lexical scope and loose typing are a language error, which they are clearly not. Furthermore most performance issues arise when dealing with the DOM, not when calculating prime numbers in JS.

      • 12

        No not a language error in particular, I’m aware it can be very powerful and rich, but both can be “tricky”, isnt that right? If you want to optimize code for the runtime (when creating games/highend apps), there are rules that can be applied where the stricter typing will lead to better performance. But my point is not only about performance, mostly about solving potential issues at forehand, and I wanted to point out a (nice) different approach than using static analyzers.

    • 13

      Actually JS is a big PITA, that´s why you need dozens of tools to prevent mass front dev suicides :D

  4. 14

    Most of that stuff should be known by a decent frontenddev.

  5. 16

    For your “countries” example, taking in account I’m no expert javascripter, couldn’t you limit the cyclomatic complexity to 7, for seven days, or even less and have all the countries listed in an array with entries like this: USA -> Sunday? Just a thought rather than a giant if statement.

    • 17

      Zack Grossbart

      February 18, 2015 5:15 pm

      You’re correct. Building a map is a common way to avoid high cyclomatic complexity while doing lists like this. It’s another good option here.

      • 18

        It was my immediate thought too when I read the example. Add all sinday countries to an array and you’re down to a simple boolean. Makes adding / removing countries a breeze too. Something a bit more complex? Map it up :)

      • 19

        Taylor Marks

        March 1, 2015 4:55 pm

        I disagree with saying “It’s another good option here.”, not because it’s not a good option, but because it’s the only good option thus far presented.

        Both of the options mentioned in the article are bad. One is so obviously bad that jshint can tell you about it. The other is probably even worse, but you’ve obfuscated what’s happening to the point that jshint can’t even tell what’s happening.

  6. 20

    Johan Alkstål

    February 18, 2015 6:27 pm

    Allow me to recommend ESLint among the analysing tools.

  7. 21

    Trevor Brindle

    February 18, 2015 6:59 pm

    For those of us who use SublimeText, you can build these code analyzers right into your editor.

    There is an on-demand option, where you invoke the linter via a keyboard command or menu option:

    But there is also a continuous linting option that I favor:

    This way each line is analyzed as you write it. You can enable multiple analyzers at a time as well.

  8. 22

    I like to use JSHint in conjunction with Uglify in Grunt. Grunt is a JavaScript task runner that introduces a lot of automation into your development work flow. Anytime I save designated JavaScript files, it runs JSHint on the files and them compresses them into one file. I’ve been using this work flow for a couple of months now and I wouldn’t go back.

    • 23

      Exactly the same as my work process.

      I always think that finding a problem 10 seconds after writing it is better than trying to find it 10 minutes or hours later.

  9. 24

    Curious about the cyclomatic complexity example. My way of dealing with laundry lists has always been to create an array/object with possible values as keys.

    I’d do something like:
    var countries = {USA: ‘Sunday’, France: ‘Monday’,…}
    if (typeof countries[country] === ‘string’){
    var firstday = countries[country];
    } else {
    var firstday = ‘[default value]’;

    • 25

      I completely agree with Greg’s comment. A function with a CCN of 50 should be refactored. Even though the code may begin very readable, it’s import to think about the long term maintenance. If somebody slightly tweaks just one branch/conditional, the entire function becomes extremely difficult to reason about. Furthermore, a high CCN usually means there is a lot of copy pasting which can lead to bugs. Keeping it DRY will avoid these issues.

      • 26

        Honestly even a switch statement would be cleaner.. they do exist in JavaScript. Greg’s solution is a much more maintainable solution.

    • 27

      You can go even further by calling methods this way, which is even cooler :)

      All people introduced in coding on whatever language, go through – if/else first, afterwards there is the switch(case) and do/while constructions.

    • 28

      ‘firstday’ is already defined

  10. 29

    I don’t understand what’s the problem with the following, it seems perfectly valid to me:

    function test() {
    var myVar = ‘Hello, World’;

    • 30

      The declaration of myVar has an uppercase V and the reference to it has a lowercase V. myVar vs. myvar.

  11. 31

    Your linter was right to complain: `getFirstDay()` should be a map, instead:

    firstDay = firstDays[‘USA’]; // ‘Sunday’

    Complexity: 1

  12. 32

    JSLInt does not require you to write “i = i + 1”; you can use the much more elegant “i += 1”. Plus plus is discouraged because programmers are prone to abuse its side effect nature. e.g. Consider that s.charAt(++i) is different to s.charAt(i++) you can easily see how it can become a source of bugs. (That said, there is a JSLint flag to permit plus plus.)

    As for the var declaration, that’s not JSLint requiring you to do that, that’s how JavaScript works. There’s no concept of block scope in pre-ES6 JavaScript. Your “i” gets hoisted to the top of the function whether you like it or not. There’s no point pretending otherwise.

    I suggest taking the time to understand why Crockford instituted the rules he did before discarding JSLint.

  13. 34

    And don’t forget to put ‘use strict’ at the beginning of your functions.

    Read more about this here

  14. 35

    What? myVar vs myvar .. use IntelliJ for feck sake.

  15. 36

    I can never understand why people have a problem with

    it all makes perfect sense. Are people really having trouble comprehending how these work?

    • 37

      It may be a sarcastic comment and would work as well, it is all about how prone to error are people… even though they understand how they work…
      also it can be replaced by i += 1 (more generic operator) and not the lame i = i + 1 as author deceptively suggest :)

  16. 38

    Another tool can be ESLint sich is plugable.

  17. 39

    I would also recommend checking out ESLint and JSCS!

  18. 40

    “Static analyzers also find errors that you can’t find with testing”

    If there is no test that you could perform that would reveal the “error”, is it really an error? Doesn’t that mean that the code does, in fact, have the desired behaviour?

    “like instances of == when you meant ===”

    Well yes, those operators behave differently. But in quite a few circumstances == will have the desired result. And again, if your test cases can’t tell the differences then either the test cases are insufficient or it genuinely doesn’t matter whether you use == or === for a particular expression.

  19. 41

    Mikhail Bartashevich

    February 23, 2015 4:33 pm

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

    incrementCount({count : 4})

    Actually increments count from the initial object. But don’t why.
    I’ve played with this example and noticed that it works like it is described in article only when you assign some value to the inner variable.

    function incrementCount(counter) {
    var counter = 1;
    console.log(counter); //1
    if (counter.count) {

    function incrementCount(counter) {
    var counter
    console.log(counter); //Object {count: 4}
    if (counter.count) {

  20. 42

    function incrementCount(counter) {
    var counter
    console.log(counter); //Object {count: 4}
    if (counter.count) {

    local counter variable is ignored and parameter counter variable is used which has a reference to the outer counter object.

    function incrementCount(counter) {
    var counter = 1;
    console.log(counter); //1
    if (counter.count) {

    local counter variable is used and shadows the parameter counter variable. local counter and parameter counter/outer counter are different objects.

  21. 43

    One additional option on your `getFirstDay(country)` function is to reduce the Cyclomatic complexity to 1 by assigning the value of the weekday to a variable and at the end of the function just return that variable.

    “`function getFirstDay(country) {
    var weekday = ”;
    if (country === ‘USA’) {
    weekday = ‘Sunday’;
    } else if (country === ‘France’) {
    weekday = ‘Monday’;
    } else if…

    return weekday;

    I guess you already know that, but some other new developer might not.

  22. 44

    Solid read, but I have a couple of quibbles. First, I’ve never had a problem with mixing quote types in string concatenations. This *might* cause a problem if you aren’t using a editor that has syntax highlighting…but who does that? No one I know. I generally prefer single quotes, but if I’m writing English sentences (error messages and the like), I use double quotes instead because of apostrophized words (“Don’t tread on me!”).

    Secondly, I will fight to the death for the ability to write control flow statements with single-line bodies WITHOUT using curly brackets. I don’t know why a linter (JSHint would be my preference) hasn’t added a “softer” version of this warning, where the following do not produce errors:

    if(true) console.log(“Woot!”);
    console.log(“Woot woot”);

    But the following does:

    console.log(“Woot woot!”);

  23. 45

    You left out the latest of the jslint + jshint lineage: eslint. More config options, differentiates between warnings and errors, and you can easily add your own rules. Highly recommended.


↑ Back to top