From 0cc37b47e32e50d7e83b180150957fa3ee67274b Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Wed, 16 Feb 2022 11:29:26 +0100 Subject: [PATCH 1/2] tools: fix bugs in prefer-primordials linter rule The ESLint rule would repport false positive if code is using an identifier that happens to have the same name as a primordials member. --- .../test-eslint-prefer-primordials.js | 28 +++++++++++++++++++ tools/eslint-rules/prefer-primordials.js | 19 +++++++++++-- 2 files changed, 45 insertions(+), 2 deletions(-) diff --git a/test/parallel/test-eslint-prefer-primordials.js b/test/parallel/test-eslint-prefer-primordials.js index 61b6b6327279cc..30c8cd25355c5a 100644 --- a/test/parallel/test-eslint-prefer-primordials.js +++ b/test/parallel/test-eslint-prefer-primordials.js @@ -99,6 +99,34 @@ new RuleTester({ `, options: [{ name: 'Function' }], }, + { + code: 'function identifier() {}', + options: [{ name: 'identifier' }] + }, + { + code: 'function* identifier() {}', + options: [{ name: 'identifier' }] + }, + { + code: 'class identifier {}', + options: [{ name: 'identifier' }] + }, + { + code: 'new class { identifier(){} }', + options: [{ name: 'identifier' }] + }, + { + code: 'const a = { identifier: \'4\' }', + options: [{ name: 'identifier' }] + }, + { + code: 'identifier:{const a = 4}', + options: [{ name: 'identifier' }] + }, + { + code: 'switch(0){case identifier:}', + options: [{ name: 'identifier' }] + }, ], invalid: [ { diff --git a/tools/eslint-rules/prefer-primordials.js b/tools/eslint-rules/prefer-primordials.js index 7ddff5396409fa..8dcb09a8d544fa 100644 --- a/tools/eslint-rules/prefer-primordials.js +++ b/tools/eslint-rules/prefer-primordials.js @@ -57,8 +57,18 @@ function getDestructuringAssignmentParent(scope, node) { return declaration.defs[0].node.init; } -const identifierSelector = - '[type!=VariableDeclarator][type!=MemberExpression]>Identifier'; +const parentSelectors = [ + // We want to select identifier that refer to another reference, not the one + // that create a new reference. + 'ClassDeclaration', + 'FunctionDeclaration', + 'LabeledStatement', + 'MemberExpression', + 'MethodDefinition', + 'SwitchCase', + 'VariableDeclarator', +]; +const identifierSelector = parentSelectors.map((selector) => `[type!=${selector}]`).join('') + '>Identifier'; module.exports = { meta: { @@ -90,6 +100,11 @@ module.exports = { reported = new Set(); }, [identifierSelector](node) { + if (node.parent.type === 'Property' && node.parent.key === node) { + // If the identifier is the key for this property declaration, it + // can't be referring to a primordials member. + return; + } if (reported.has(node.range[0])) { return; } From 7431ba0f1073ee012911e1768c781ed1a4223523 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Fri, 18 Feb 2022 11:03:11 +0100 Subject: [PATCH 2/2] Update tools/eslint-rules/prefer-primordials.js Co-authored-by: Rich Trott --- tools/eslint-rules/prefer-primordials.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/eslint-rules/prefer-primordials.js b/tools/eslint-rules/prefer-primordials.js index 8dcb09a8d544fa..d2531556de225d 100644 --- a/tools/eslint-rules/prefer-primordials.js +++ b/tools/eslint-rules/prefer-primordials.js @@ -58,7 +58,7 @@ function getDestructuringAssignmentParent(scope, node) { } const parentSelectors = [ - // We want to select identifier that refer to another reference, not the one + // We want to select identifiers that refer to other references, not the ones // that create a new reference. 'ClassDeclaration', 'FunctionDeclaration',