JSLint Errors and Options Described

In the A Quick Tour of Douglas Crockford’s JSLint article, I ran some of my own code through Douglas Crockford’s JSLint, an online code quality tool for JavaScript. The analysis revealed quite a number of deviations from established JavaScript best practices in my code. In that article, I only described a few error messages and their solutions. Today’s article will present several other coding no-nos that developers commonly perpetrate. I think that you’ll find it to be a very worthwhile and enlightening exercise.

If Else Errors

There are a couple of best practice coding standards for If Else statements that are enforced by JSLint. The first is that empty blocks are considered to be bad form. Thus, If Else statements should be rewritten to remove the empty block. Here is one such issue in my script, the resulting error message, and the fix:

if (evt.currentTarget.checked) {
  //select.disabled = true;
}
else {
  select.disabled = false;
  if (select.selectedIndex == 0) select.selectedIndex = 2;
}

//JSLint Error message: "Empty block."

//should be rewritten as:
if (!evt.currentTarget.checked /* or evt.currentTarget.checked === false */) {
  select.disabled = false;
  if (select.selectedIndex == 0) select.selectedIndex = 2;
}

The other bad practice in If Else statements is to omit the curly braces in single line blocks.

if      (criteria1) doThis();
else if (criteria2) doThat();
else                doSomethingElseEntirely(); 

While it’s true that JavaScript allows you to leave out curly braces if the executed statement is only one line, doing so leaves you open to bugs should you ever append additional lines later and forget to add the braces at that time. Here is the line in question from the above snippet, the error message, along with the solution:

  if (select.selectedIndex == 0) select.selectedIndex = 2;
  
  //JSLint Error message: "Expected '{' and instead saw 'select'."
  
  //should be rewritten as:
  if (select.selectedIndex == 0) { select.selectedIndex = 2; }

Identity vs. Equality

JSLint enforces the use of identity === and !== operators instead of their equality == and != counterparts. This helps avoid issues with automatic type conversions that equality operators perform under the covers before testing for equality. Here are just a few of the surprising results that you might obtain using the equality and inequality operators:

1 == true  //true
0 == false //true
10 != "10" //false
new Object() != "[object Object]" //false.  Inequality operator calls object's toString() method

When JSLint spots a comparison test that it thinks should use an identity operator, it reports the following error:

Expected '===' and instead saw '=='.

So, in addition to causing the “Empty block.” message, the same line generated the identity vs. equality error as well.

The fix is simple: Change the ‘==’ equality operator into the ‘===’ identity operator:

if (select.selectedIndex === 0) { select.selectedIndex = 2; }

This forces us to consider the types that we are comparing. In this case, the selectedIndex property is indeed an integer, just like the zero.

If you wish, you can tell JSLint to use equality operators by default by setting the ‘== and !=’ option to true.

Insecure Regular Expressions

In Regular Expressions, the ‘^’ character matches any ASCII character other than the ones listed within square braces. For example, The expression [^ABC] would match any character other than A, B, or C. Since JavaScript strings are in Unicode, that includes not just ASCII characters, but every possible character within the Unicode character set. In effect, you’re allowing just much anything through, and that creates the potential for security and/or validation bypassing issues. Instead of excluding characters, you should allow only characters that are valid. To illustrate, here is a RegEx that validates a URL string:

/bhttps?://[-w+&@#/%?=~|$!:,.;]*[w+&@#/%=~|$]/g

With regards to my sample code, I used a RegEx for parsing parameter names from a function signature. Here’s the offending code, the resulting error, and my solution:

  return funcStr.slice(funcStr.indexOf('(')+1, funcStr.indexOf(')')).match(/([^s,]+)/g); 
  
  //JSLint Error message: "Insecure '^'."
  
  //should be rewritten something like:
         //remove code between parentheses (...)
  return funcStr.slice(funcStr.indexOf('(')+1, funcStr.indexOf(')'))
           //remove whitespace
           .replace( ' ', '')
             //split on comma
             .split(',');

My first instinct was to identify the parameter names themselves, rather than the spaces and coma delimiters. Unfortunately, this would prove to be just about impossible, as valid JS identifiers may include any Unicode character. Considering that an ASCII-only regular expression would be 11,236 characters long, I went back to the delimiters, which was only comprised of two characters. The replace() function removes the spaces, while split() is used to then parse out the parameters.

If you insist on using the ‘^’ character in your regular expressions, you can tell JSLint to ignore them by turning on the “. and [^…] in /RegExp/” option.

Conclusion

Having gone through exercise of improving my code according to JSLint’s recommendations, I can honestly say that doing so has fortified my code and made it less susceptible to a variety of bugs…and I’m not done yet. There are still more standards that can be applied to the code to make it even better. We’ll be looking at those shortly…