jakejs / jake

JavaScript build tool, similar to Make or Rake. Built to work with Node.js.
http://jakejs.com
Apache License 2.0
1.97k stars 190 forks source link

Clearing away JSHint warnings #192

Closed mcandre closed 11 years ago

mcandre commented 11 years ago

I applied JSHint to the code to look for potential sources of bugs. Originally, jshint . output 516 warnings, but I was able to trim them down to 39 by adapting .jshintrc to your coding style. Now JSHint outputs:

$ jshint .
lib\api.js: line 19, col 11, Weird construction. Is 'new' unnecessary?

lib\file_list.js: line 174, col 8, Missing semicolon.
lib\file_list.js: line 118, col 22, Weird construction. Is 'new' unnecessary?

lib\jake.js: line 78, col 31, Missing semicolon.
lib\jake.js: line 134, col 16, 'p' is already defined.
lib\jake.js: line 145, col 31, Missing semicolon.
lib\jake.js: line 163, col 26, Missing semicolon.
lib\jake.js: line 176, col 25, Missing semicolon.
lib\jake.js: line 52, col 19, Weird construction. Is 'new' unnecessary?

lib\namespace.js: line 11, col 23, Weird construction. Is 'new' unnecessary?

lib\npm_publish_task.js: line 111, col 41, Missing semicolon.
lib\npm_publish_task.js: line 142, col 29, ['package'] is better written in dot notation.
lib\npm_publish_task.js: line 174, col 32, ['clobber'] is better written in dot notation.
lib\npm_publish_task.js: line 31, col 28, Weird construction. Is 'new' unnecessary?

lib\package_task.js: line 298, col 10, Don't make functions within a loop.
lib\package_task.js: line 186, col 25, Weird construction. Is 'new' unnecessary?

lib\parseargs.js: line 55, col 53, Use '===' to compare with '0'.
lib\parseargs.js: line 79, col 28, Use '===' to compare with '0'.
lib\parseargs.js: line 48, col 30, Weird construction. Is 'new' unnecessary?
lib\parseargs.js: line 132, col 1, Missing '()' invoking a constructor.

lib\program.js: line 221, col 25, ['__root__'] is better written in dot notation.
lib\program.js: line 118, col 21, Weird construction. Is 'new' unnecessary?

lib\rule.js: line 7, col 11, Weird construction. Is 'new' unnecessary?
lib\rule.js: line 253, col 6, Missing semicolon.
lib\rule.js: line 198, col 18, Weird construction. Is 'new' unnecessary?

lib\task\file_task.js: line 8, col 12, Weird construction. Is 'new' unnecessary?

lib\task\task.js: line 193, col 19, Missing semicolon.
lib\task\task.js: line 38, col 12, Weird construction. Is 'new' unnecessary?

lib\utils\index.js: line 76, col 6, Missing semicolon.
lib\utils\index.js: line 83, col 20, Weird construction. Is 'new' unnecessary?
lib\utils\index.js: line 208, col 22, Use '!==' to compare with '0'.
lib\utils\index.js: line 213, col 22, Use '===' to compare with '0'.
lib\utils\index.js: line 161, col 29, Weird construction. Is 'new' unnecessary?

lib\utils\logger.js: line 3, col 14, Weird construction. Is 'new' unnecessary?

test\exec.js: line 34, col 8, Don't make functions within a loop.

test\file_task.js: line 9, col 25, Missing semicolon.

test\helpers.js: line 3, col 15, Weird construction. Is 'new' unnecessary?

test\parseargs.js: line 122, col 69, Duplicate key 'test --trace does not expect a value, -f does (throw howdy away)'.

test\rule.js: line 11, col 25, Missing semicolon.

39 errors

I'm not familiar enough with the code base to resolve these, just thought I should mention. Feel free to tweak .jshintrc to cull even more warnings.

mde commented 11 years ago

Hey, this is pretty awesome! I'm guessing all the 'weird construction' warnings are about the throwaway-constructor syntax that I like to use. JSLint doesn't like it either (which is why I hacked up the JSLint source to do what I want: https://github.com/mde/dotfiles/blob/master/fulljslint.js).

Do you know if JSHint can be configured to understand these inline, throwaway constructors?

mde commented 11 years ago

Yep, it's the 'supernew' relax-option. Now we're left with only valid problems. Thanks for this!