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

Jump to bottom

no-implied-eval doesn't consider environments(env) #12733

Closed
yeonjuan opened this issue Jan 1, 2020 · 1 comment · Fixed by #12757
Closed

no-implied-eval doesn't consider environments(env) #12733

yeonjuan opened this issue Jan 1, 2020 · 1 comment · Fixed by #12757
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 rule Relates to ESLint's core rules

Comments

@yeonjuan
Copy link
Member

yeonjuan commented Jan 1, 2020 •
edited

Tell us about your environment

  • ESLint Version: v6.8.0
  • Node Version: v12.13.0
  • npm Version: v6.12.0

What parser (default, Babel-ESLint, etc.) are you using?

Please show your full configuration:

Configuration 
N/A

What did you do? Please include the actual source code causing the issue, as well as the command that you used to run ESLint.

The rule no-implied-eval doesn't consider the environment configuration(env), while the rule no-eval does.

  • no-implied-eval - demo
/*eslint no-implied-eval: "error"*/
/*eslint-env node*/

window.setTimeout("var a = 0"); // 4:1- Implied eval. Consider passing a function instead of a string(no-implied-eval)
/*eslint no-eval: "error"*/
/*eslint-env node*/

window.eval("var a = 0"); // No Error.

What did you expect to happen?

No error in no-implied-eval, because the environment is configured with 'node'.

What actually happened? Please include the actual, raw output from ESLint.
1 Error

Are you willing to submit a pull request to fix this bug?
Yes, If it is a bug.
@yeonjuan yeonjuan added bug ESLint is working incorrectly triage An ESLint team member will look at this issue soon labels Jan 1, 2020
@kaicataldo
Copy link
Member

kaicataldo commented Jan 6, 2020 •
edited

Definitely would be nice to have matching behavior here. While we're at it, we could probably also update no-implied-eval to account for globalThis (doesn't have to be the same PR, though!)?

@kaicataldo kaicataldo added accepted There is consensus among the team that this change meets the criteria for inclusion rule Relates to ESLint's core rules and removed triage An ESLint team member will look at this issue soon labels Jan 6, 2020
@yeonjuan yeonjuan mentioned this issue Jan 7, 2020
Merged
btmills pushed a commit that referenced this issue Mar 21, 2020
@yeonjuan
Update: consider env in no-implied-eval (fixes #12733) (#12757)
085979f 
* Fix: consider env in no-implied-eval (fixes #12733)

* fix typo, refactor to chaining

* change to check only CallExpression, refactoring

* check defs

* checks callee

* check static string value

* check falsy - first argument

* check empty arg

* fix typo
anikethsaha pushed a commit to anikethsaha/eslint that referenced this issue Mar 23, 2020
@yeonjuan @anikethsaha
Update: consider env in no-implied-eval (fixes eslint#12733) (eslint#…
c0e2a5b 
…12757)

* Fix: consider env in no-implied-eval (fixes eslint#12733)

* fix typo, refactor to chaining

* change to check only CallExpression, refactoring

* check defs

* checks callee

* check static string value

* check falsy - first argument

* check empty arg

* fix typo
@eslint-deprecated eslint-deprecated bot locked and limited conversation to collaborators Sep 19, 2020
@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 Sep 19, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
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 rule Relates to ESLint's core rules
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Update: consider env in no-implied-eval (fixes #12733)
2 participants
@kaicataldo @yeonjuan