Skip to content

Commit

Permalink
tools: fix bugs in prefer-primordials linter rule
Browse files Browse the repository at this point in the history
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: nodejs#42010
Reviewed-By: Shingo Inoue <leko.noor@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
  • Loading branch information
aduh95 authored and bengl committed Feb 21, 2022
1 parent 6bf4367 commit b910890
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 2 deletions.
28 changes: 28 additions & 0 deletions test/parallel/test-eslint-prefer-primordials.js
Expand Up @@ -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: [
{
Expand Down
19 changes: 17 additions & 2 deletions tools/eslint-rules/prefer-primordials.js
Expand Up @@ -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: {
Expand Down Expand Up @@ -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;
}
Expand Down

0 comments on commit b910890

Please sign in to comment.