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

fix: do not report global references in id-match rule #15420

Merged
merged 6 commits into from Dec 15, 2021
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
24 changes: 23 additions & 1 deletion lib/rules/id-match.js
Expand Up @@ -78,7 +78,21 @@ module.exports = {
const IMPORT_TYPES = new Set(["ImportSpecifier", "ImportNamespaceSpecifier", "ImportDefaultSpecifier"]);

/**
* Checks if a string matches the provided pattern
* Checks whether the given node represents a reference to a global variable that is not declared in the source code.
* These identifiers will be allowed, as it is assumed that user has no control over the names of external global variables.
* @param {ASTNode} node `Identifier` node to check.
* @returns {boolean} `true` if the node is a reference to a global variable.
*/
function isReferenceToGlobalVariable(node) {
const scope = context.getScope();
snitin315 marked this conversation as resolved.
Show resolved Hide resolved
const variable = scope.set.get(node.name);

return variable && variable.defs.length === 0 &&
variable.references.some(ref => ref.identifier === node);
}

/**
* Checks if a string matches the provided pattern and is not a global reference
snitin315 marked this conversation as resolved.
Show resolved Hide resolved
* @param {string} name The string to check.
* @returns {boolean} if the string is a match
* @private
Expand Down Expand Up @@ -160,6 +174,10 @@ module.exports = {
parent = node.parent,
effectiveParent = (parent.type === "MemberExpression") ? parent.parent : parent;

if (isReferenceToGlobalVariable(node)) {
return;
}

if (parent.type === "MemberExpression") {

if (!checkProperties) {
Expand Down Expand Up @@ -249,6 +267,10 @@ module.exports = {

const isClassField = node.parent.type === "PropertyDefinition";

if (isReferenceToGlobalVariable(node)) {
return;
}
snitin315 marked this conversation as resolved.
Show resolved Hide resolved

if (isClassField && !checkClassFields) {
return;
}
Expand Down
81 changes: 81 additions & 0 deletions tests/lib/rules/id-match.js
Expand Up @@ -185,6 +185,17 @@ ruleTester.run("id-match", rule, {
}]
},

// Should not report for global references - https://github.com/eslint/eslint/issues/15395
{
code: `
var foo = Object.keys(bar);
var a = Array.from(b);
`,
options: ["^\\$?[a-z]+([A-Z0-9][a-z0-9]+)*$", {
properties: true
}]
},

// Class Methods
{
code: "class x { foo() {} }",
Expand Down Expand Up @@ -641,6 +652,76 @@ ruleTester.run("id-match", rule, {
]
},

// https://github.com/eslint/eslint/issues/15395
{
code: `
const foo_variable = 1;
class MyClass {
}
let a = new MyClass();
let b = {id: 1};
let c = Object.keys(b);
let d = Array.from(b);
let e = (Object) => Object.keys(obj, prop); // not global Object
let f = (Array) => Array.from(obj, prop); // not global Array
foo.Array = 5; // not global Array
`,
options: ["^\\$?[a-z]+([A-Z0-9][a-z0-9]+)*$", {
properties: true
}],
parserOptions: { ecmaVersion: 6 },
errors: [
{
message: "Identifier 'foo_variable' does not match the pattern '^\\$?[a-z]+([A-Z0-9][a-z0-9]+)*$'.",
type: "Identifier",
line: 2,
column: 19
},
{
message: "Identifier 'MyClass' does not match the pattern '^\\$?[a-z]+([A-Z0-9][a-z0-9]+)*$'.",
type: "Identifier",
line: 3,
column: 19
},

// let e = (Object) => Object.keys(obj, prop)
{
message: "Identifier 'Object' does not match the pattern '^\\$?[a-z]+([A-Z0-9][a-z0-9]+)*$'.",
type: "Identifier",
line: 9,
column: 22
},
{
message: "Identifier 'Object' does not match the pattern '^\\$?[a-z]+([A-Z0-9][a-z0-9]+)*$'.",
type: "Identifier",
line: 9,
column: 33
},

// let f =(Array) => Array.from(obj, prop);
{
message: "Identifier 'Array' does not match the pattern '^\\$?[a-z]+([A-Z0-9][a-z0-9]+)*$'.",
type: "Identifier",
line: 10,
column: 22
},
{
message: "Identifier 'Array' does not match the pattern '^\\$?[a-z]+([A-Z0-9][a-z0-9]+)*$'.",
type: "Identifier",
line: 10,
column: 32
},

// foo.Array = 5;
{
message: "Identifier 'Array' does not match the pattern '^\\$?[a-z]+([A-Z0-9][a-z0-9]+)*$'.",
type: "Identifier",
line: 11,
column: 17
}
]
},

// Class Methods
{
code: "class x { _foo() {} }",
Expand Down