Skip to content
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

var in for stops parsing. #92

Closed
baryluk opened this issue Dec 28, 2011 · 10 comments
Closed

var in for stops parsing. #92

baryluk opened this issue Dec 28, 2011 · 10 comments

Comments

@baryluk
Copy link

baryluk commented Dec 28, 2011

when linting thing like this:

/*jslint white: true, maxerr: 100 */
function f() {
 for (var i = 0; i < 2; i++) {
  return i;
 }
}

kah dkjash kajshdk jahkd as d
and other non-senses

jslint will stop on for loop:

Problem at line 2 character 2: Missing 'use strict' statement.

 for (var i = 0; i < 2; i++) {

Problem at line 2 character 7: Move 'var' declarations to the top of the function.

 for (var i = 0; i < 2; i++) {

Problem at line 2 character 7: Stopping. (22% scanned).

No, option combination prevents it from stopping stopping, or not emiting this varning.

@douglascrockford
Copy link
Collaborator

Did you read the directions?

@baryluk
Copy link
Author

baryluk commented Dec 28, 2011

I do not understand you. You mean I actually need to "Move 'var' declarations to the top of the function." ?

But I have exactly same error even if I use "vars: true"!

And I personally disagree, that for (var i = ...; ) is bad, or worse that declaring same var i; at beginning of function. It is more readable, and if problematic, it should be an warning not error.

So my bug report is basically about 2 things:

  • making jslin, display this error, but CONTINUE parsing document (seeing just one error in big source code, forces developer fix it one by one, not allowing him choicing what is important to fix first and what not).
  • making "vars: true" actually not produce this error.

Please reopen.

@douglascrockford
Copy link
Collaborator

Fix your code, then you won't have a problem.

Here's a tip: grep for "for (var", then you can fix them all at once.

@RoryO
Copy link

RoryO commented Dec 29, 2011

@baryluk you are incorrect on how Javascript works. Javascript only has function scope, not block scope, and it also hoists all variable declarations to the top of the function to ensure that the functions is deterministic. Doing for (var i) is incorrect because the variable i is hoisted to the top of the function at runtime. JSLint is telling you exactly this.

@baryluk
Copy link
Author

baryluk commented Dec 29, 2011

In what way it is incorrect? I know how scopeing works in JS (and I know it is completly different than in things like C or Java), this is exact reason why I created this bug report.

Doing for (var i) is incorrect because the variable i is hoisted to the top of the function at runtime.

It is not incorrect in any way. It is perfectly valid code, and will work without problem.

My main problem is not arguing if for (var i) is good or not (I can find reasons it can be considered bad practice), but about jslint stopping processing, despite explicty telling it to continue showing more subsequent errors/warnings, as well no possibility of disabling this "error".

Please read ECMA SCRIPT 242, 5th edition, chapter 12.6.3 "The for statment" (page 90). There is EXPLICIT grammar rule for "for (var ...; ....; ....). so just stop telling me this is incorrect code. You will also find same gramar rule in 3th edition, on page 65.

@baryluk
Copy link
Author

baryluk commented Dec 29, 2011

It looks that this problem was already reported in issue #29, and was closed without good example and explanation. Also this bug report pointed that it was fatal error, and not allowed continuing analysis, exact same issue I have with it.

If you are saying it is bad, because even experienced programmers can miss some errors, than why have all this jslint options at all? Just enforce everything if you know better, without any configuration possible. Or perform some better code analysis, like only giving error when such variable is used outside a block it was declared, because it for sure means he is doing something wrong, and then should move it to the top of function. But it is not used outside this for loop, then what is problem? it is simple to detect, and forcing all people to move all variables to the top is just artificially making coding in JS harder than necessary. I will be perfectly happy with error message, which will indicate me and other developers need to fix this code, but I often have other priorities, and would like to concentrate first on other (probably more important) possible bugs in the code. THere is no way currently to do this in JSLint.

@baryluk
Copy link
Author

baryluk commented Dec 29, 2011

And again it was mentioned in issue #17. Everyone is saying that it makes tool less usefull than it could be. What is wrong?

@RoryO
Copy link

RoryO commented Dec 29, 2011

'Everyone'? The only thing I'm seeing with all the evidence you're pointing to is people that have written code that can be highly inconsistent according to the scoping rules of the language, and said people complaining, yourself included, that they're too lazy to fix their code. You're making a ton of noise on something that you've proven you don't understand.

Read the spec you posted. It says:

  1. Evaluate VariableDeclarationListNoIn

A JS parser will evaluate any statement in VariableDeclarationListNoIn with precisely the same rules as the rest of the language, which is to create the variable at the top of the function scope. This is the same class of errors as if you used var halfway down a function. There is no difference according to the specification of the language.
Oh, and since you're trying to rules lawyer on specifications, there's a great deal in Javascript that is technically correct by the specification but no sane person would ever advocate for. According to ECMAScript 7.9, semicolons are completely optional in Javascript. Are you also advocating that JSLint should also be able to tolerate the lack of semicolons?

@baryluk
Copy link
Author

baryluk commented Dec 29, 2011

A JS parser will evaluate any statement in VariableDeclarationListNoIn with precisely the same rules as the rest of the language, which is to create the variable at the top of the function scope.

And where I said it would not? I was just saying, that your statement that this code is incorrect, is actually false (I proved this giving you actual specification reference). It is correct JS. Maybe you mean something different, I do not know.

Essentially you still doesn't understand what I have said.

I'm not complaining about it is bad or not, I'm complaining about fact that JSLint STOPS PROCESSING file when it detects even single such error. It makes it very hard when applying to big projects.

For example, you mentions semicolons are optional in JSS. But when you put anything with lacking semicolons into JSLint, it reports errors (which is good), but CONTINUES with analysis (which is also good). So in fact JSLint is ALREADY "tolerating" semicolons.

@douglascrockford
Copy link
Collaborator

Once again, fix your code, then you won't have a problem.

Fixing your code would have taken significantly less time than posting all of the stuff you put here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants