From 5fb3917ca58d5032c705ae3d3d26c6351a8998b8 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Fri, 18 Feb 2022 19:42:02 +0100 Subject: [PATCH] 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. PR-URL: https://github.com/nodejs/node/pull/42010 Reviewed-By: Shingo Inoue Reviewed-By: Rich Trott --- .../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..d2531556de225d 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 identifiers that refer to other references, not the ones + // 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; }