Leveraging Code Quality Tools

Anton Kovalyov / GothamJS 2011

Who is this guy?

JavaScript, DISQUS, JSHint, Hiro, etc.

Incentive: DISQUS

A Brief History of JavaScript

Creation (1995)

Inglourious Browsers (1995–2003)

A New Hope (2003–)

A New Hope (2003–)

Modern implementations are much faster: (ie.microsoft.com/testdrive/benchmarks/sunspider/default.html)

JavaScript: The Tricky Parts

What you see

var a = 1, b = 2;

function printVars() {
  console.log(a, b); // undefined, 2
  var a = 5;
  console.log(a);    // 5
}

What your JavaScript engine sees

var a;
var b;
a = 1;
b = 2;

function printVars() {
  var a;
  console.log(a, b); // undefined, 2
  a = 5;
  console.log(a);    // 5
}

What you see

function sayHello() {
  return
    "Hello, World";
}

console.log(sayHello()); // undefined

What your JavaScript engine sees

function sayHello() {
  return;
  "Hello, World";
}

console.log(sayHello()); // undefined

What you see

var friend = true;

if (friend)
  function say() { return "xo"; }
else
  function say() { return "fu"; }

console.log(say()); // "fu"

What your JavaScript engine sees

function say() { return "xo"; }
function say() { return "fu"; }

var friend;
friend = true;

if (friend) {}
else {}

console.log(say()); // "fu"

Solving the problem

Step 1: Learn the language

Step 2: Don't trust your gut

Unit Tests

Code Reviews

Review of my tiny change to DISQUS JavaScript code

Lint Tools

lint—a program that flagged suspicious and non-portable constructs in C code

JSLint

JSLint

JSHint

JSHint

JSHint

twitter.com/jeresig/status/38709496792485888

How to use

What is it for?

Simple example

var Slides = { /* implementation */ };

window.addEventListener('message', function (event) {
  if (event.data == 'next')
    slides.nextSlide();
  else if (event.data == 'prev')
    Slides.prevSlide();
  else
    console.log('lolwat?');
});

Output

var Slides = { /* implementation */ };

window.addEventListener('message', function (event) {
  if (event.data == 'next')
    slides.nextSlide();
  else if (event.data == 'prev')
    Slides.prevSlide();
  else
    console.log('lolwat?');
});

// >> Implied globals: window, slides, Slides, console

Enabling more strict variable control

/*jshint undef:true */

var Slides = { /* implementation */ };

window.addEventListener('message', function (event) {
  if (event.data == 'next')
    slides.nextSlide();
  else if (event.data == 'prev')
    Slides.prevSlide();
  else
    console.log('lolwat?');
});

Output

/*jshint undef:true */

var Slides = { /* implementation */ };

window.addEventListener('message', function (event) {
  if (event.data == 'next')
    slides.nextSlide();
  else if (event.data == 'prev')
    Slides.prevSlide();
  else
    console.log('lolwat?');
});

// >> Errors:
// >>   Line 5: window is not defined
// >>   Line 7: slides is not defined
// >>   Line 11: console is not defined

Adding globals

/*jshint undef:true */
/*globals Slides:true, window:false, console:false */

var Slides = { /* implementation */ };

window.addEventListener('message', function (event) {
  if (event.data == 'next')
    slides.nextSlide();
  else if (event.data == 'prev')
    Slides.prevSlide();
  else
    console.log('lolwat?');
});

Output

/*jshint undef:true */
/*globals Slides:true, window:false, console:false */

var Slides = { /* implementation */ };

window.addEventListener('message', function (event) {
  if (event.data == 'next')
    slides.nextSlide();
  else if (event.data == 'prev')
    Slides.prevSlide();
  else
    console.log('lolwat?');
});

// >> Errors:
// >>   Line 8: slides is not defined

Another example

/*jshint jquery:true */

if (jQuery == null) {
  console.log("No jQuery");
}

(function () {
  /*jshint eqnull:true */

  if (jQuery == null) {
    jQuery = {};
  }
}());

Another example

/*jshint jquery:true */

if (jQuery == null) {       // Use '===' to compare with 'null'
  console.log("No jQuery");
}

(function () {
  /*jshint eqnull:true */

  if (jQuery == null) {
    jQuery = {};            // Read only.
  }
}());

JSHint

My commits to DISQUS source code after enabling strict variable checking for all our files

What JSHint it not suitable for

Integration = Enforcement

Integration

Simple Pre-commit Hook

# Source/disqus/.git/hooks/pre-commit

import os

def main():
    "Checks your git commit with PyFlakes and JSHint"

    # [...]

    results = {}
    for file in get_files_changed("*.js"):
        status, output = jshint(file)
        if status:
            results[file] = output

    if results:
        print_errors(results)
        sys.exit(1) # Abort the commit

    sys.exit(0) # All good

Integration

But authors can skip our hooks by using the git-commit -n option

Integration

Integration

Integration

Broken DISQUS builds (no deploy, everyone panics)

Summary

Future of JSHint

How to contribute

How to contribute

You want to contribute but your company doesn't encourage spending time on open source projects?

Come work with us

disqus.com/jobs

Thanks!

The Last Slide