JavaScript Static Code Analysis, part 2

In the previous post, I talked about static code analysis and how it can be useful in enforcing and maintaining a consistent coding style, but also in catching subtle bugs and code smells. The focus was on two tools, jscs and jshint. What other tools and techniques can you use?

Another problem you can encounter is inconsistent file names. Just like code elements, there are many ways to name a file. Developers with different backgrounds have different habits, so before you know it, you can end up with a file structure like this:

- lib
  |-- Utilities.js
  |-- factory.js
- Controllers
  |-- home_controller.js
  |-- Products.js
  |-- pricesController.js

 

That’s not pretty to look at and it’s definitely not fun to work with. Luckily, there are tools out there to save the day. Our build system at work is based on grunt, so it was easy to add grunt-filenames in the pipeline. That tool allows you to specify regular expression rules and overrides tailored to your needs. In the example above, you can specify for example:

  • all folders must be lowercase
  • all files must be PascalCase
  • all files inside the Controllers folder must end in Controller.js

and any violation will make the build break. No need to check for these things during code reviews any more.

Back to the content of the files. What can you do beyond jscs and jshint?

A simple approach is to search for a text pattern that you don’t want people to use in the code. As a use case, consider unit tests. We use chai at work as an assertion library. Chai has two flavors: expect and assert. If you decide to use one, it’s best to avoid using the other (again because of readability and maintainability of the code base). You can use a tool like grunt-search and break the build if your unit tests contain the patterns “assert(” or “assert.”. It’s a bit of a blunt knife, but it can do the trick and it can search in any type of text file.

When it comes to JavaScript though, you can try to build something more sophisticated. One frequent comment I leave during code reviews is that all unit test names must start with the word ‘should’. Let’s see it in code:

// valid
it('should calculate the sum of two numbers', function() {});

// invalid
it('calculates the sum of two numbers', function() {});

// invalid (uppercase Should)
it('Should calculate the sum of two numbers', function() {});

You might’ve guessed why: readability and maintainability. When you get back the full report of the unit tests that failed, you’ll find it easier to read when they’re written in the same style.

For only a fraction of the time I spend in code reviews, I developed two prototypes to automatically detect this issue. They both work with tools that parse the JavaScript code and provide you the syntax tree to play with. You should be hearing the voice in your head: “Power! Unlimited power!”

The first one is built with esprima and it’s here. The second one is built as a jscs plugin and you can see it here.

We rolled out the jscs plugin solution at work yesterday and it works fine. Why did we go for the jscs plugin approach? Because it’s always best to stand on the shoulders of giants.

jscs will call your custom plugin. Guess who else calls jscs? Your text editors. I tried it out in Atom, Sublime Text, IntelliJ. I have to say, it was very satisfying to see all these editors highlight the code as invalid with my error message. And as far as fail fast feedback goes, there’s nothing faster than seeing it fail as you type.

jscs-custom-rule

You don’t have to change anything else in your build pipeline either. Your plugin is part of jscs, which makes it part of your build process.

Finally, you always want to be able to tell the machine that you’re still in control (well, for now). In other words, you need to be able to disable the rule if you have to. When you build your custom validation as a jscs plugin, you can override it with inline comments, just like you would do with any jscs rule.

One last tip: consider prefixing your custom rules’ names to avoid any clashes. Depending on how psyched you are with all this, you may want to write a custom rule that validates your custom rules are prefixed!

With this simple custom rule in place, a door has been opened. The obvious change is that we no longer have to worry about this convention in the code reviews. The less obvious change remains to be seen: will this small new feature tempt other team members to build on top of it and further change their mindset, from manual towards automated? Let’s see.

Advertisements

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s