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: #42010
Reviewed-By: Shingo Inoue <leko.noor@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
  • Loading branch information
aduh95 authored and danielleadams committed Apr 24, 2022
1 parent 9ae6982 commit c229889
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 c229889

Please sign in to comment.