Skip to content

Commit

Permalink
Fix regexp/negation false positives (#275)
Browse files Browse the repository at this point in the history
* Fix `regexp/negation` false positives

* Fixed cache bug
toCharSet is cached and asumes that given elements are not changed.
I previously broke that assumption.
  • Loading branch information
RunDevelopment authored and ota-meshi committed Jul 20, 2021
1 parent 448d349 commit ce599ff
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 9 deletions.
44 changes: 36 additions & 8 deletions lib/rules/negation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,23 +31,51 @@ export default createRule("negation", {
node,
getRegexpLocation,
fixReplaceNode,
toCharSet,
flags,
}: RegExpContext): RegExpVisitor.Handlers {
return {
onCharacterClassEnter(ccNode) {
if (!ccNode.negate || ccNode.elements.length !== 1) {
return
}

const element = ccNode.elements[0]
if (element.type === "CharacterSet") {
const negatedCharSet = getNegationText(element)
context.report({
node,
loc: getRegexpLocation(ccNode),
messageId: "unexpected",
data: { negatedCharSet },
fix: fixReplaceNode(ccNode, negatedCharSet),
if (element.type !== "CharacterSet") {
return
}

if (flags.ignoreCase && element.kind === "property") {
// The ignore case canonicalization affects negated
// Unicode property escapes in a weird way. In short,
// /\p{Foo}/i is not the same as /[^\P{Foo}]/i if
// \p{Foo} contains case-varying characters.
//
// Note: This only affects Unicode property escapes.
// All other character sets are either case-invariant
// (/./, /\s/, /\d/) or inconsistent (/\w/).

const ccSet = toCharSet(ccNode)

const negatedElementSet = toCharSet({
...element,
negate: !element.negate,
})

if (!ccSet.equals(negatedElementSet)) {
// We cannot remove the negative
return
}
}

const negatedCharSet = getNegationText(element)
context.report({
node,
loc: getRegexpLocation(ccNode),
messageId: "unexpected",
data: { negatedCharSet },
fix: fixReplaceNode(ccNode, negatedCharSet),
})
},
}
}
Expand Down
14 changes: 13 additions & 1 deletion tests/lib/rules/negation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,12 @@ const tester = new RuleTester({
})

tester.run("negation", rule as any, {
valid: [String.raw`/[\d]/`, String.raw`/[^\d\s]/`],
valid: [
String.raw`/[\d]/`,
String.raw`/[^\d\s]/`,
String.raw`/[^\p{ASCII}]/iu`,
String.raw`/[^\P{Ll}]/iu`,
],
invalid: [
{
code: String.raw`/[^\d]/`,
Expand Down Expand Up @@ -94,6 +99,13 @@ tester.run("negation", rule as any, {
"Unexpected negated character class. Use '\\p{Ll}' instead.",
],
},
{
code: String.raw`/[^\P{White_Space}]/iu;`,
output: String.raw`/\p{White_Space}/iu;`,
errors: [
"Unexpected negated character class. Use '\\p{White_Space}' instead.",
],
},
{
code: String.raw`const s ="[^\\w]"
new RegExp(s)`,
Expand Down

0 comments on commit ce599ff

Please sign in to comment.