Repository: ota-meshi/eslint-plugin-regexp
Pull request #660 (2023-10-20 12:37:10)

Fixed crash for v flag in `regexp/optimal-quantifier-concatenation`
related to #659

changeset-bot[bot]: ###  ⚠️  No Changeset found

Latest commit: feb0f425731cfdea541e8ef1d7bbb55d332db460

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. **If these changes should result in a version bump, you need to add a changeset.**

<details><summary>This PR includes no changesets</summary>

  When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

</details>

[Click here to learn what changesets are, and how to add one](https://github.com/changesets/changesets/blob/main/docs/adding-a-changeset.md).

[Click here if you're a maintainer who wants to add a changeset to this PR](https://github.com/ota-meshi/eslint-plugin-regexp/new/optimal-quantifier-concatenation-v-flag?filename=.changeset/chilly-pumpkins-raise.md&value=---%0A%22eslint-plugin-regexp%22%3A%20patch%0A---%0A%0AFixed%20crash%20for%20v%20flag%20in%20%60regexp%2Foptimal-quantifier-concatenation%60%0A)



Patch:
diff --git a/.changeset/chilly-pumpkins-raise.md b/.changeset/chilly-pumpkins-raise.md
new file mode 100644
index 0000000..7973257
--- /dev/null
+++ b/.changeset/chilly-pumpkins-raise.md
@@ -0,0 +1,5 @@
+---
+"eslint-plugin-regexp": patch
+---
+
+Fixed crash for v flag in `regexp/optimal-quantifier-concatenation`
diff --git a/.changeset/nine-pets-tan.md b/.changeset/nine-pets-tan.md
new file mode 100644
index 0000000..e3622ff
--- /dev/null
+++ b/.changeset/nine-pets-tan.md
@@ -0,0 +1,5 @@
+---
+"eslint-plugin-regexp": patch
+---
+
+Fixed minor mistake in message of `no-misleading-unicode-character`
diff --git a/.changeset/odd-oranges-matter.md b/.changeset/odd-oranges-matter.md
new file mode 100644
index 0000000..2c3d260
--- /dev/null
+++ b/.changeset/odd-oranges-matter.md
@@ -0,0 +1,5 @@
+---
+"eslint-plugin-regexp": minor
+---
+
+Improve detection of useless assertions for `regexp/no-useless-assertion`
diff --git a/lib/rules/no-misleading-unicode-character.ts b/lib/rules/no-misleading-unicode-character.ts
index 8ecf87f..5f50aa0 100644
--- a/lib/rules/no-misleading-unicode-character.ts
+++ b/lib/rules/no-misleading-unicode-character.ts
@@ -276,7 +276,10 @@ export default createRule("no-misleading-unicode-character", {
                         messageId: "characterClass",
                         data: {
                             graphemes,
-                            unit: flags.unicode ? "code points" : "char codes",
+                            unit:
+                                flags.unicode || flags.unicodeSets
+                                    ? "code points"
+                                    : "char codes",
                             uFlag: uFlag ? " Use the `u` flag." : "",
                         },
                         ...makeFix(
diff --git a/lib/rules/no-useless-assertions.ts b/lib/rules/no-useless-assertions.ts
index 422fdcb..808b147 100644
--- a/lib/rules/no-useless-assertions.ts
+++ b/lib/rules/no-useless-assertions.ts
@@ -4,6 +4,7 @@ import type {
     EdgeAssertion,
     Element,
     LookaroundAssertion,
+    Node,
     WordBoundaryAssertion,
 } from "@eslint-community/regexpp/ast"
 import type { RegExpContext } from "../utils"
@@ -23,10 +24,106 @@ import {
     isPotentiallyEmpty,
     isZeroLength,
     FirstConsumedChars,
+    invertMatchingDirection,
 } from "regexp-ast-analysis"
 import { mention } from "../utils/mention"
 import { assertNever } from "../utils/util"
 
+function containsAssertion(n: Node): boolean {
+    return hasSomeDescendant(n, (d) => d.type === "Assertion")
+}
+
+/**
+ * Returns whether the given lookaround asserts exactly one character in the given direction.
+ */
+function isSingleCharacterAssertion(
+    assertion: Assertion,
+    direction: MatchingDirection,
+    flags: ReadonlyFlags,
+): boolean {
+    switch (assertion.kind) {
+        case "word":
+            // \b and \B assert one character in BOTH directions
+            return false
+
+        case "start":
+            return direction === "rtl"
+        case "end":
+            return direction === "ltr"
+
+        default:
+            break
+    }
+
+    if (getMatchingDirectionFromAssertionKind(assertion.kind) !== direction) {
+        return false
+    }
+
+    return assertion.alternatives.every((alt) => {
+        if (!containsAssertion(alt)) {
+            // if we don't contains assertions, then we can only need to check
+            // the length range of the consumed characters
+            const range = getLengthRange(alt, flags)
+            return range.min === 1 && range.max === 1
+        }
+
+        // now it gets tricky
+        // e.g. (?=(?=[a-z])\w(?<=[a-g])) is a single character assertion
+
+        let consumed = false
+        let asserted = false
+        const elements =
+            direction === "ltr" ? alt.elements : [...alt.elements].reverse()
+        for (const e of elements) {
+            if (!consumed) {
+                // before we consumed our first (and only) character, we can
+                // have single character assertions
+
+                if (
+                    e.type === "Assertion" &&
+                    isSingleCharacterAssertion(e, direction, flags)
+                ) {
+                    asserted = true
+                    continue
+                }
+
+                if (containsAssertion(e)) {
+                    // too complex to reason about, so we just give up
+                    return false
+                }
+
+                const range = getLengthRange(e, flags)
+                if (range.max === 0) {
+                    // we haven't consumed anything, so onto the next one
+                    continue
+                } else if (range.min === 1 && range.max === 1) {
+                    // finally, a character
+                    consumed = true
+                } else {
+                    // it's not exactly a single character
+                    return false
+                }
+            } else {
+                // since we already consumed our first (and only) character, we
+                // can at most have assertions in the direction of the
+                // first character
+
+                const otherDir = invertMatchingDirection(direction)
+                if (
+                    e.type === "Assertion" &&
+                    isSingleCharacterAssertion(e, otherDir, flags)
+                ) {
+                    continue
+                }
+
+                return false
+            }
+        }
+
+        return consumed || asserted
+    })
+}
+
 /**
  * Combines 2 look chars such that the result is equivalent to 2 adjacent
  * assertions `(?=a)(?=b)`.
@@ -356,40 +453,31 @@ export default createRule("no-useless-assertions", {
                     )
                 }
 
-                if (after.edge) {
-                    return
-                }
-
-                // accept is harder because that can't generally be decided by the first character
-
-                // if this contains another assertion then that might reject. It's out of our control
+                // We can only decide the accept case for exact single-character assertions.
+                // We want the character after the assertion to be a subset of the asserted characters. For this to be
+                // correct, the set of assertion characters needs to be exact. We also have to consider edges. Edges
+                // can be thought of as a special character, so the same subset requirement applies.
+                const edgeSubset = firstOf.edge || !after.edge
                 if (
-                    !hasSomeDescendant(
+                    firstOf.exact &&
+                    edgeSubset &&
+                    after.char.isSubsetOf(firstOf.char) &&
+                    isSingleCharacterAssertion(
                         assertion,
-                        (d) => d !== assertion && d.type === "Assertion",
+                        getMatchingDirectionFromAssertionKind(assertion.kind),
+                        flags,
                     )
                 ) {
-                    const range = getLengthRange(assertion.alternatives, flags)
-                    // we only check the first character, so it's only correct if the assertion requires only one
-                    // character
-                    if (range.max === 1) {
-                        // require exactness
-                        if (
-                            firstOf.exact &&
-                            after.char.isSubsetOf(firstOf.char)
-                        ) {
-                            report(
-                                assertion,
-                                assertion.negate
-                                    ? "alwaysForNegativeLookaround"
-                                    : "alwaysForLookaround",
-                                {
-                                    kind: assertion.kind,
-                                    acceptOrReject: accept,
-                                },
-                            )
-                        }
-                    }
+                    report(
+                        assertion,
+                        assertion.negate
+                            ? "alwaysForNegativeLookaround"
+                            : "alwaysForLookaround",
+                        {
+                            kind: assertion.kind,
+                            acceptOrReject: accept,
+                        },
+                    )
                 }
             }
 
diff --git a/lib/rules/optimal-quantifier-concatenation.ts b/lib/rules/optimal-quantifier-concatenation.ts
index a1b8b6d..9679076 100644
--- a/lib/rules/optimal-quantifier-concatenation.ts
+++ b/lib/rules/optimal-quantifier-concatenation.ts
@@ -64,7 +64,8 @@ function getSingleConsumedChar(
     element: Element | Alternative,
     flags: ReadonlyFlags,
 ): SingleConsumedChar {
-    const empty = flags.unicode ? EMPTY_UNICODE : EMPTY_UTF16
+    const empty =
+        flags.unicode || flags.unicodeSets ? EMPTY_UNICODE : EMPTY_UTF16
 
     switch (element.type) {
         case "Alternative":


Test patch:
diff --git a/tests/lib/rules/no-misleading-unicode-character.ts b/tests/lib/rules/no-misleading-unicode-character.ts
index 0fe3ada..1f2a7da 100644
--- a/tests/lib/rules/no-misleading-unicode-character.ts
+++ b/tests/lib/rules/no-misleading-unicode-character.ts
@@ -292,6 +292,14 @@ tester.run("no-misleading-unicode-character", rule as any, {
             options: [{ fixable: true }],
             errors: [{ messageId: "characterClass" }],
         },
+        {
+            code: String.raw`/[👶🏻👨‍👩‍👦]/v`,
+            output: String.raw`/[\q{👶🏻|👨‍👩‍👦}]/v`,
+            options: [{ fixable: true }],
+            errors: [
+                "The character(s) '👶🏻', '👨‍👩‍👦' are all represented using multiple code points.",
+            ],
+        },
         {
             code: String.raw`/[👶🏻&👨‍👩‍👦]/v`,
             output: String.raw`/[\q{👶🏻|👨‍👩‍👦}&]/v`,
diff --git a/tests/lib/rules/no-useless-assertions.ts b/tests/lib/rules/no-useless-assertions.ts
index 5dedfa4..b7c755f 100644
--- a/tests/lib/rules/no-useless-assertions.ts
+++ b/tests/lib/rules/no-useless-assertions.ts
@@ -433,5 +433,26 @@ tester.run("no-useless-assertions", rule as any, {
                 "'\\B' will always accept because it is preceded by a non-word character and followed by a non-word character.",
             ],
         },
+
+        {
+            code: String.raw`/(?!$)$/`,
+            errors: ["The negative lookahead '(?!$)' will always reject."],
+        },
+        {
+            code: String.raw`/(?=a|$)a/`,
+            errors: ["The lookahead '(?=a|$)' will always accept."],
+        },
+        {
+            code: String.raw`/(?=(?=[a-f])(?=a|A)[\w%])a/`,
+            errors: [
+                "The lookahead '(?=(?=[a-f])(?=a|A)[\\w%])' will always accept.",
+            ],
+        },
+        {
+            code: String.raw`/(?=(?=[a-f])(?=[aA])\w(?<=[aA])(?<=[a-f]))a/`,
+            errors: [
+                "The lookahead '(?=(?=[a-f])(?=[aA])\\w(?<=[aA])(?<=[a-f]))' will always accept.",
+            ],
+        },
     ],
 })
diff --git a/tests/lib/rules/optimal-quantifier-concatenation.ts b/tests/lib/rules/optimal-quantifier-concatenation.ts
index aa75171..20580ce 100644
--- a/tests/lib/rules/optimal-quantifier-concatenation.ts
+++ b/tests/lib/rules/optimal-quantifier-concatenation.ts
@@ -25,6 +25,7 @@ tester.run("optimal-quantifier-concatenation", rule as any, {
         },
         String.raw`/^(?:\[\[\[?.+?\]?\]\]|<<.+?>>)$/`,
         String.raw`/a\s*?(?=\r?\n|\r)/`,
+        String.raw`/(?:a|b)?c/v`,
     ],
     invalid: [
         {