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

no-undef misleading example #11963

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 documentation Relates to ESLint's documentation rule Relates to ESLint's core rules

Comments

@mdjermanovic
Copy link
Member

The version of ESLint you are using.

master

The problem you want to solve.

From no-undef docs:

Examples of correct code for this rule with global declaration:

/*global someFunction b:writable*/
/*eslint no-undef: "error"*/

var a = someFunction();
b = 10;

The b:writable syntax in /*global */ indicates that assignment to b is correct.

Examples of incorrect code for this rule with global declaration:

/*global b*/
/*eslint no-undef: "error"*/

b = 10;

By default, variables declared in /*global */ are read-only, therefore assignment is incorrect.

'Incorrect' examples should be those with warnings, but ESLint does not report any warnings for the second example.

The problem with this is that the second example is actually correct, because no-undef rule does not check assignments to readonly global variables, that's what no-global-assign does.

Your take on the correct solution to problem.

I'm not sure. Probably modify the first example and remove the second. Maybe reference no-global-assign. Perhaps also add more info to no-global-assign.

Are you willing to submit a pull request to implement this change?
Yes.

@mdjermanovic mdjermanovic added core Relates to ESLint's core APIs and features enhancement This change enhances an existing feature of ESLint triage An ESLint team member will look at this issue soon labels Jul 7, 2019
@platinumazure platinumazure added accepted There is consensus among the team that this change meets the criteria for inclusion documentation Relates to ESLint's documentation rule Relates to ESLint's core rules and removed core Relates to ESLint's core APIs and features enhancement This change enhances an existing feature of ESLint triage An ESLint team member will look at this issue soon labels Jul 7, 2019
@platinumazure
Copy link
Member

Hi @mdjermanovic, thanks for the issue.

I think no-undef used to handle this case in the past, so it makes sense that the documentation might now incorrectly claim that the rule will check global assignments. We would be grateful for a PR that fixes this.

@mdjermanovic
Copy link
Member Author

mdjermanovic commented Jul 12, 2019

The interesting thing here is that no-implicit-globals also reports this, albeit in a different context.

/* eslint no-implicit-globals:"error" */
/* eslint no-global-assign:"error" */
/* global a:readonly */

a = 1;
  5:1  error  Implicit global variable, assign as global property instead  no-implicit-globals
  5:1  error  Read-only global 'a' should not be modified                  no-global-assign

None of the rules will report anything if the global variable is set to writeable.

Is this the intended behavior of no-implicit-globals?

@mysticatea
Copy link
Member

I guess that it's intended because there are tests.

But it sounds a bug to me. In the original issue #4542, they talked about declarations.

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