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

"Use of future reserved word in strict mode" false positive with eslint-env es6 #2134

Closed
cvrebert opened this issue Mar 23, 2015 · 10 comments
Closed
Assignees
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion bug ESLint is working incorrectly core Relates to ESLint's core APIs and features

Comments

@cvrebert
Copy link
Contributor

ESLint version: 0.17.1 (and 67936e0 gives the same results)

foo.js:

/*eslint-env es6 */
let x = 42;

.eslintrc:

{
    "ecmaFeatures": { "modules": true }
}

The false positive:

$ ./node_modules/.bin/eslint foo.js 

foo.js
  2:1  error  Use of future reserved word in strict mode

✖ 1 problem (1 error, 0 warnings)
@nzakas nzakas added bug ESLint is working incorrectly core Relates to ESLint's core APIs and features accepted There is consensus among the team that this change meets the criteria for inclusion labels Mar 23, 2015
@nzakas
Copy link
Member

nzakas commented Mar 23, 2015

Most likely we're not apply ecmaFeatures for inline environments correctly. As a workaround, you can try adding env: { es6: true } to your config file.

@satya164
Copy link

satya164 commented May 8, 2015

Facing the same issue. It's not optimal to have it in the config file in an ES5 and ES6 mixed environment.

@nzakas
Copy link
Member

nzakas commented May 8, 2015

You can toggle it on and off in different config files es6: true in one and es6: false in another.

@satya164
Copy link

satya164 commented May 9, 2015

If I use 2 different config files, won't I have to duplicate all the configs, and they'll get out of sync?

I use the extension .es6 for my ES6 files. May be there could be config option to enable certain options for certain file extensions? May be I'm thinking too far.

@ilyavolodin
Copy link
Member

Ok, I started working on this (I think it's actually an easy fix), but run into a bunch of things that I can't test and explain. Here's the code that currently applies inline environments: https://github.com/eslint/eslint/blob/master/lib/eslint.js#L306
From what I can tell, that code is broken and currently doesn't do anything at all for rules (verified) as it tries to merge in true value into existing object.
This is the only code I can find that deals with eslint-env comment. However, somehow, global variables are being set correctly when enabling environments that way. I can't find that code anywhere.
I also don't see any way to verify that after I set eslint-env rules will be enabled in config and ecmaFeatures will get copied. eslint doesn't expose any of those options unfortunately. So I don't think it's possible to unittest my fix.

@satya164
Copy link

satya164 commented May 9, 2015

I tested the fix and it seems to be working. Hope to see it in the stable version soon.

Thanks a lot \o/

@nzakas
Copy link
Member

nzakas commented May 15, 2015

Closed by mistake.

We still have an issue where ecmaFeatures aren't applied because we don't read comments until after parsing has already happened.

@ilyavolodin
Copy link
Member

I'm not really sure if it's worth keeping this issue open. I don't really see a way to fix this without two-stage parsing.

@satya164
Copy link

It's such a basic functionality. It needs fixing.

@nzakas
Copy link
Member

nzakas commented May 15, 2015

@ilyavolodin we can't leave this. Either we remove the es6 environment or we figure out a way to make this work.

We could, in theory, just search for /*eslint-env and see if there's es6:true before parsing.

@eslint-deprecated eslint-deprecated bot locked and limited conversation to collaborators Feb 7, 2018
@eslint-deprecated eslint-deprecated bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Feb 7, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion bug ESLint is working incorrectly core Relates to ESLint's core APIs and features
Projects
None yet
Development

No branches or pull requests

5 participants