New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

function syntax checks inconsistent with Google Closure Linter #811

Closed
idearat opened this Issue Jan 10, 2013 · 6 comments

Comments

Projects
None yet
5 participants
@idearat

idearat commented Jan 10, 2013

You cannot run the Google Closure Linter and JSHint successfully on the same code and get them both to run cleanly without setting white: false (even in RC3). Unfortunately setting white: false means JSHint will not warn for things like foo=bar. This seems to imply that a new option be created specific to function() vs. function () syntax. JSLint has such an option, anon. I have a pull request which adds this feature which I propose submitting so that the specific check for a space after functions be independently configurable.

@valueof

This comment has been minimized.

Show comment
Hide comment
@valueof

valueof Jan 10, 2013

Member

This issue is too minor for its own option. To clarify, does JSHint fail on code that GCL passes or vice versa?

Member

valueof commented Jan 10, 2013

This issue is too minor for its own option. To clarify, does JSHint fail on code that GCL passes or vice versa?

@idearat

This comment has been minimized.

Show comment
Hide comment
@idearat

idearat Jan 10, 2013

JSHint fails if you leave the space out, GCL fails unless you put it in. If you turn on white: false in JSHint you can run both, but then JSHint is also ignoring a bunch of other checks, which I feel is inappropriate as a workaround since it encourages sloppier code formatting in general. The only inconsistency between the two linters that I've been able to find is this one issue. That's why Crock added anon to jslint for me. I was hoping to match that flag in JSHint via the associated pull request.

idearat commented Jan 10, 2013

JSHint fails if you leave the space out, GCL fails unless you put it in. If you turn on white: false in JSHint you can run both, but then JSHint is also ignoring a bunch of other checks, which I feel is inappropriate as a workaround since it encourages sloppier code formatting in general. The only inconsistency between the two linters that I've been able to find is this one issue. That's why Crock added anon to jslint for me. I was hoping to match that flag in JSHint via the associated pull request.

@alexgraul

This comment has been minimized.

Show comment
Hide comment
@alexgraul

alexgraul Jan 14, 2013

I'd love to see this is a relaxer option, we have a codebase I'd love to turn white=true on for but the "function ()" requirement that comes with it is a non-starter.

alexgraul commented Jan 14, 2013

I'd love to see this is a relaxer option, we have a codebase I'd love to turn white=true on for but the "function ()" requirement that comes with it is a non-starter.

@joshkehn

This comment has been minimized.

Show comment
Hide comment
@joshkehn

joshkehn Feb 19, 2013

I was going to open an issue about the whitespace complaining for function name () (fails) vs function name() (passes).

Really this should be under a generalized whitespace option, or ignored like it is with for and other control structures. for(.., for (.., if (.., if(.. all pass. Curiously enough, console.log (.. passes as well though I've never seen someone add a space for an invocation.

joshkehn commented Feb 19, 2013

I was going to open an issue about the whitespace complaining for function name () (fails) vs function name() (passes).

Really this should be under a generalized whitespace option, or ignored like it is with for and other control structures. for(.., for (.., if (.., if(.. all pass. Curiously enough, console.log (.. passes as well though I've never seen someone add a space for an invocation.

@idearat

This comment has been minimized.

Show comment
Hide comment
@idearat

idearat Feb 21, 2013

While I appreciate that there's some sense that this is "too minor for it's own option" I think that misses the larger point. The entire point of having a linter is to improve code quality. Being able to run the Google Closure Linter as a secondary linter provides developers with the ability to not only check their code quality but also confirm that they have commented their code properly and in conformance with the JSDoc standard. I can't see how one could argue that leaving out a feature which can so directly impact overall code quality can be in the best interest of the JS community.

idearat commented Feb 21, 2013

While I appreciate that there's some sense that this is "too minor for it's own option" I think that misses the larger point. The entire point of having a linter is to improve code quality. Being able to run the Google Closure Linter as a secondary linter provides developers with the ability to not only check their code quality but also confirm that they have commented their code properly and in conformance with the JSDoc standard. I can't see how one could argue that leaving out a feature which can so directly impact overall code quality can be in the best interest of the JS community.

@bedney

This comment has been minimized.

Show comment
Hide comment
@bedney

bedney Feb 21, 2013

I would agree that having an option would be optimal. I've been in situations where we need to run both tools and, before the 'anon' option was added to JSLint, that was impossible.

Also, Scott is being modest here -- Crock is very reluctant to deviate from his prescribed method of doing things (hence the reason for the whole JSHint project, no?) and he thought this worthy enough to add another whole flag to JSLint based on Scott's argument.

I think JSHint should consider doing the same.

bedney commented Feb 21, 2013

I would agree that having an option would be optimal. I've been in situations where we need to run both tools and, before the 'anon' option was added to JSLint, that was impossible.

Also, Scott is being modest here -- Crock is very reluctant to deviate from his prescribed method of doing things (hence the reason for the whole JSHint project, no?) and he thought this worthy enough to add another whole flag to JSLint based on Scott's argument.

I think JSHint should consider doing the same.

@valueof valueof closed this in 5623f08 Feb 28, 2013

jugglinmike added a commit to jugglinmike/jshint that referenced this issue Oct 21, 2014

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