Fix: allow references to external globals in id-blacklist (fixes #12567)

[mdjermanovic]

Prerequisites checklist

• I have read the contributing guidelines.
• The team has reached consensus on the changes proposed in this pull request. If not, I understand that the evaluation process will begin with this pull request and won't be merged until the team has reached consensus.

What is the purpose of this pull request? (put an "X" next to an item)

[X] Bug fix

Fixes #12567

What changes did you make? (Give an overview)

id-blacklist will now ignore identifiers that represent a reference to an external global variable:

/* eslint id-blacklist: ["error", "Number", "undefined"] */

const foo = Number.parseInt(bar); // ok

let baz = undefined; // ok

Global variables declared in the same source code will be still invalid:

/* eslint id-blacklist: ["error", "foo"] */

var foo = 1; // error

foo = 2; // error

Is there anything you'd like reviewers to focus on?

This will be still invalid although it is a reference to a global variable, because it also represents a property name:

/* eslint id-blacklist: ["error", "undefined"] */

var foo = { undefined };

[yeonjuan]

maybe, Is this PR intended to ignoring check an identifier in an assignment that has no declaration?

I tested it and recognized it pass the blacklist identifier with assignment in destructuring.

This is the case which I tested.

({a: parseInt} = a); // no warning in this pr version.

// but it warns with the declaration.
var paseInt; ({a: parseInt} = a);  // warning

And it is warned in v6.8.0 (no declaraion)

• online demo

/*eslint id-blacklist: ["error", "parseInt"] */
({a: parseInt} = foo); // warning

[mdjermanovic]

maybe, Is this PR intended to ignoring check an identifier in an assignment that has no declaration?

Only if it is a global variable that isn't declared in the same source code. The logic is: if it is declared, then we can assume it's a user-controlled name. Otherwise, it's a built-in/env- specific/third-party global so user can't change the name.

It could be also indeed a user-controlled name declared elsewhere, but in that case id-blacklist will warn in the file where it is declared.

This shouldn't be a warning because parseInt is a global variable that isn't declared in this code:

/*eslint id-blacklist: ["error", "parseInt"] */

({a: parseInt} = a);

This should be warning (2 warnings, one for each Identifier node) because parseInt is declared:

/*eslint id-blacklist: ["error", "parseInt"] */

var parseInt; ({a: parseInt} = a);

This should be also a warning because foo isn't defined in the configuration as a global variable:

/*eslint id-blacklist: ["error", "foo"] */

({a: foo} = a);

[yeonjuan]

@mdjermanovic

Only if it is a global variable that isn't declared in the same source code. The logic is: if it is declared, then we can assume it's a user-controlled name. Otherwise, it's a built-in/env- specific/third-party global so user can't change the name.

Thanks for the explanation. 👍 it makes sense to me.

[yeonjuan]

LGTM! Thanks 👍

[kaicataldo]

LGTM! Appreciate all the clear comments in the code and tests 👍

[kaicataldo]

👍