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.

Sign up for GitHub

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

Jump to bottom

feat: false negative with onlyDeclarations + properties in id-match #15431

Merged
merged 5 commits into from Dec 21, 2021
Merged

feat: false negative with onlyDeclarations + properties in id-match #15431

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
10215e6
fix: false negative with `onlyDeclarations` in `id-match`
snitin315 Dec 16, 2021
8dad653
test: add more cases
snitin315 Dec 16, 2021
bbe507e
fix: false positive with computed properties
snitin315 Dec 18, 2021
c5a813d
refactor: code
snitin315 Dec 21, 2021
88f192c
test: remove invalid test case
snitin315 Dec 21, 2021
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
11 changes: 11 additions & 0 deletions lib/rules/id-match.js
View file
Open in desktop
Expand Up @@ -238,6 +238,17 @@ module.exports = {
}
}

// For https://github.com/eslint/eslint/issues/15123
if (parent.parent && parent.parent.type === "ObjectExpression") {
if (
(checkProperties && onlyDeclarations && !parent.computed) &&
parent.key === node &&
isInvalid(name)
) {
report(node);
}
}
snitin315 marked this conversation as resolved.
Show resolved Hide resolved

// never check properties or always ignore destructuring
if (!checkProperties || (ignoreDestructuring && isInsideObjectPattern(node))) {
return;
Expand Down
133 changes: 132 additions & 1 deletion tests/lib/rules/id-match.js
View file
Open in desktop
Expand Up @@ -197,6 +197,70 @@ ruleTester.run("id-match", rule, {
}],
parserOptions: { ecmaVersion: 2022 }
},
{
code: `
const foo = {
foo_one: 1,
bar_one: 2,
fooBar: 3
};
`,
options: ["^[^_]+$", {
properties: false
}],
parserOptions: { ecmaVersion: 2022 }
},
{
code: `
const foo = {
foo_one: 1,
bar_one: 2,
fooBar: 3
};
`,
options: ["^[^_]+$", {
onlyDeclarations: true
}],
parserOptions: { ecmaVersion: 2022 }
},
{
code: `
const foo = {
foo_one: 1,
bar_one: 2,
fooBar: 3
};
`,
options: ["^[^_]+$", {
properties: false,
onlyDeclarations: false
}],
parserOptions: { ecmaVersion: 2022 }
},
{
code: `
const foo = {
[a]: 1,
};
`,
options: ["^[^a]", {
properties: false,
onlyDeclarations: false
}],
parserOptions: { ecmaVersion: 2022 }
},
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be invalid, but that's another bug. It might be best to just remove this test case for now, and revisit this rule at some point. In particular, L219-L249 looks broken.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should create a separate issue to track it.

{
code: `
const foo = {
[a]: 1,
};
`,
options: ["^[^a]", {
properties: true,
onlyDeclarations: true
}],
parserOptions: { ecmaVersion: 2022 }
},

// Class Methods
{
Expand Down Expand Up @@ -774,7 +838,74 @@ ruleTester.run("id-match", rule, {
type: "PrivateIdentifier"
}
]
}
},

// https://github.com/eslint/eslint/issues/15123
{
code: `
const foo = {
foo_one: 1,
bar_one: 2,
fooBar: 3
};
`,
options: ["^[^_]+$", {
properties: true,
onlyDeclarations: true
}],
parserOptions: { ecmaVersion: 2022 },
errors: [
{
message: "Identifier 'foo_one' does not match the pattern '^[^_]+$'.",
type: "Identifier"
},
{
message: "Identifier 'bar_one' does not match the pattern '^[^_]+$'.",
type: "Identifier"
}
]
},
{
code: `
const foo = {
foo_one: 1,
bar_one: 2,
fooBar: 3
};
`,
options: ["^[^_]+$", {
properties: true,
onlyDeclarations: false
}],
parserOptions: { ecmaVersion: 2022 },
errors: [
{
message: "Identifier 'foo_one' does not match the pattern '^[^_]+$'.",
type: "Identifier"
},
{
message: "Identifier 'bar_one' does not match the pattern '^[^_]+$'.",
type: "Identifier"
}
]
},
{
code: `
const foo = {
[a]: 1,
};
`,
options: ["^[^a]", {
properties: true,
onlyDeclarations: false
}],
parserOptions: { ecmaVersion: 2022 },
errors: [
{
message: "Identifier 'a' does not match the pattern '^[^a]'.",
type: "Identifier"
}
]
}
]
});