Syntax check only mode, with trailing comma detection #808

Closed
p opened this Issue Jan 8, 2013 · 23 comments

Projects

None yet

6 participants

p commented Jan 8, 2013

I have been looking for a tool to check syntactic validity of javascript. When I tried running jshint on some test files it produced some errors that could not be disabled, e.g.:

core.js: line 428, col 36, Use '===' to compare with '0'.
core.js: line 484, col 18, 'sign' is already defined.
core.js: line 502, col 26, 'sign' used out of scope.

Now, these all probably should be fixed, but it cannot happen right away. In the mean time I would like to check that my js does not have any syntax errors.

Researching options for syntax checking only I came to the conclusion that "syntax check" is typically thought of as "give the js file to some js runtime and see if it complains". In particular, I tried uglify-js and esprima. And therein lied a problem:

The infamous trailing comma.

Both uglify-js and esprima accept it. But this is the one thing I would like the syntax checker to complain of. Interestingly when I was trying to find a tool that would remove the trailing commas from source some time ago, I found that compilers tended to reject trailing commas. Times seem to have changed, but there is a moral to this story: I would like to have predictable handling of trailing commas.

Therefore my request is to have a way to use jshint to check for syntactic validity of js only, and either allow or reject trailing commas according to configuration.

+1; in IE8 trailing commas cause page load errors.

pbomb commented Apr 24, 2013

+1; IE does not like trailing commas

Owner
valueof commented Apr 24, 2013

If the browser you support can't parse trailing commas then it doesn't support ES5. So don't enable es5 option and you're golden (in future releases, 2.0 and above you will have to enable es3 option cause ES5 mode will be turned on by default).

@valueof valueof closed this Apr 24, 2013
Owner
valueof commented Apr 24, 2013

Also syntax check only doesn't make sense because trailing comma is valid JavaScript (ES5 was published in 2009 and is supported by all modern JavaScript platforms).

that sounds perfect; how does one disable es5 and tell it to do es3? are there docs on that somewhere that I should see?

Owner
valueof commented Apr 24, 2013

With the current version ES3 is the default mode. So all you need to do is not to enable es5 option. In future versions there is a es3 option.

with the current version I am absolutely not specifying es5 and it absolutely does not warn me when commas are missing. What might I be doing wrong?

Contributor
guyzmo commented Apr 24, 2013

As anton said, in 1.1 (that you install using npm) you just nees to check es5 is not set, or set to false. In head of repository (future 2.0) you'll have to set es3 to true.

I will have to see if I can find out what I'm doing wrong, then, because I just updated to 1.1.0 (I was on 1.0.0) and it still doesn't complain when I have trailing commas. I am very skeptical because this just stopped working several versions back but I'll keep trying in case maybe I'm doing something wrong and report back.

Owner
valueof commented Apr 24, 2013

@taxilian check if you have a .jshintrc somewhere that enables ES5 mode or if you use node option (Node mode implies ES5). It works for me:

jshint $ jshint demo.js
demo.js: line 1, col 13, Extra comma. (it breaks older versions of IE)

1 error
jshint $ cat demo.js
var a = [1,2,];
jshint $ jshint -v
jshint v1.1.0

Hmm. Yep, I was using the node option. I was not aware that that implied es5; that fixes the issue. Thanks! This is wonderful to have working again. Sorry for the inconvenience

Repeating the question, are there docs for this somewhere that I should have found that would tell me this so I didn't have to bother you?

Owner
valueof commented Apr 24, 2013

Not really for your issue. We're always open to documentation patches (see jshint/site repo)!

I'll have to see if I can find some time to help with that! Thanks for taking time to set me in the right direction; this is wonderful to have it working again.

p commented Apr 25, 2013

The other part of this issue was to allow all syntactically valid javascript (see examples in original description).

This shouldn't be closed as it is still not implemented (unless it has been deemed wont-fix).

As far as I can tell, there is no way to instruct jshint to ONLY warn about syntax errors.

Owner
valueof commented Jul 10, 2013

Yeah, it's a wont-fix. You can use any parser out there to check for syntax errors.

@antonkovalyov -- true, but extra commas aren't syntax errors, so they're hard to detect without jshint.. for now I'm using jshint blah.js | grep "extra comma" Unfortunately it's quite slow.

p commented Jul 15, 2013

Once again, which parser will reject trailing commas and accept ALL OTHER syntactically valid javascript?

Owner
valueof commented Jul 15, 2013

There won't be a mode where JSHint checks only for trailing commas. There won't be a mode where JSHint checks only for valid syntax (use any other parser to do this). There might be an additional check for trailing commas including all other things JSHint checks for.

That's it. Period.

p commented Jul 17, 2013

You keep referring to some "other parser" but such a thing does not exist. All sane parsers accept trailing commas. So while it is true that jshint cannot be used for the task outlined in the initial description in this ticket, your implied assertion that there is another tool that can be used for that task is not true.

Owner
valueof commented Jul 17, 2013

Please read what I said. If you want to check for valid syntax only you can use any other parser. But if you want to have additional rules (and prohibiting trailing commas will be an additional rule since its valid JavaScript) then you will have to use JSHint which won't have syntax-only mode.

p commented Jul 18, 2013

I read everything you said. Perhaps you do not understand the use case I described in this issue? Because your replies continue to be not helpful/irrelevant to this use case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment