no-useless-backreference: Made compatible with clean-regex/no-empty-backreference (#111)

RunDevelopment · web-flow · commit 17f9d5a3b647 · 2021-04-16T09:26:34.000+09:00
* `no-useless-backreference`: Made compatible with `clean-regex/no-empty-alternative`

* More efficient implementation

* Removed `isEmptyBackreference` call

* Updated docs

* Update

* Fixed editor

* Added more docs
diff --git a/README.md b/README.md
@@ -100,7 +100,7 @@ The rules with the following star :star: are included in the `plugin:regexp/reco
 | [regexp/no-octal](https://ota-meshi.github.io/eslint-plugin-regexp/rules/no-octal.html) | disallow octal escape sequence | :star: |
 | [regexp/no-trivially-nested-assertion](https://ota-meshi.github.io/eslint-plugin-regexp/rules/no-trivially-nested-assertion.html) | disallow trivially nested assertions | :wrench: |
 | [regexp/no-unused-capturing-group](https://ota-meshi.github.io/eslint-plugin-regexp/rules/no-unused-capturing-group.html) | disallow unused capturing group |  |
-| [regexp/no-useless-backreference](https://ota-meshi.github.io/eslint-plugin-regexp/rules/no-useless-backreference.html) | disallow useless backreferences in regular expressions |  |
+| [regexp/no-useless-backreference](https://ota-meshi.github.io/eslint-plugin-regexp/rules/no-useless-backreference.html) | disallow useless backreferences in regular expressions | :star: |
 | [regexp/no-useless-character-class](https://ota-meshi.github.io/eslint-plugin-regexp/rules/no-useless-character-class.html) | disallow character class with one character | :wrench: |
 | [regexp/no-useless-dollar-replacements](https://ota-meshi.github.io/eslint-plugin-regexp/rules/no-useless-dollar-replacements.html) | disallow useless `$` replacements in replacement string |  |
 | [regexp/no-useless-escape](https://ota-meshi.github.io/eslint-plugin-regexp/rules/no-useless-escape.html) | disallow unnecessary escape characters in RegExp |  |
diff --git a/docs/rules/README.md b/docs/rules/README.md
@@ -28,7 +28,7 @@ The rules with the following star :star: are included in the `plugin:regexp/reco
 | [regexp/no-octal](./no-octal.md) | disallow octal escape sequence | :star: |
 | [regexp/no-trivially-nested-assertion](./no-trivially-nested-assertion.md) | disallow trivially nested assertions | :wrench: |
 | [regexp/no-unused-capturing-group](./no-unused-capturing-group.md) | disallow unused capturing group |  |
-| [regexp/no-useless-backreference](./no-useless-backreference.md) | disallow useless backreferences in regular expressions |  |
+| [regexp/no-useless-backreference](./no-useless-backreference.md) | disallow useless backreferences in regular expressions | :star: |
 | [regexp/no-useless-character-class](./no-useless-character-class.md) | disallow character class with one character | :wrench: |
 | [regexp/no-useless-dollar-replacements](./no-useless-dollar-replacements.md) | disallow useless `$` replacements in replacement string |  |
 | [regexp/no-useless-escape](./no-useless-escape.md) | disallow unnecessary escape characters in RegExp |  |
diff --git a/docs/rules/no-useless-backreference.md b/docs/rules/no-useless-backreference.md
@@ -9,15 +9,80 @@ since: "v0.1.0"
 
 > disallow useless backreferences in regular expressions
 
+- :gear: This rule is included in `"plugin:regexp/recommended"`.
+
 ## :book: Rule Details
 
-This rule is a copy of the ESLint core [no-useless-backreference] rule.  
-The [no-useless-backreference] rule was added in ESLint 7.x, but this plugin supports ESLint 6.x.
-Copied to this plugin to allow the same [no-useless-backreference] rules to be used in ESLint 6.x.
+Backreferences that will always trivially accept serve no function and can be removed.
+
+This rule is a based on the ESLint core [no-useless-backreference] rule. It reports all the ESLint core rule reports and some more.
+
+### Causes
+
+Backreferences can be useless for multiple reasons.
+
+#### Empty capturing groups
+
+The is the easiest reason. The references capturing group does not consume any characters (e.g. `/(\b)a\1/`). Since the capturing group can only capture the empty string, the backreference is guaranteed to accept any input.
+
+#### Nested backreferences
+
+If a backreference is inside the group it references (e.g. `/(a\1)/`), then it is guaranteed to trivially accept.
+
+This is because the regex engine only sets the text of a capturing group **after** the group has been matched. Since the regex engine is still in the process of matching the group, its capture text is undefined.
+
+#### Different alternatives
+
+If a backreference and its referenced capturing group are in different alternatives (e.g. `/(a)|\1/`), then the backreference will always trivially accept because the captured text of the referenced group is undefined.
+
+#### Forward references and backward references
+
+Backreferences are supposed to be matched **after** their referenced capturing group. Since regular expressions are matched from left to right, backreferences usually appear to the right of to their referenced capturing groups (e.g. `/(a)\1/`). However, backreferences can also be placed before (to the left of) their referenced capturing group (e.g. `/\1(a)/`). These backreferences are to trivially accept because the captured text of their referenced groups is undefined. We call these backreferences _forward references_.
+
+Inside **lookbehind assertions**, regular expressions are matched from right to left and not from left to right. This means that only backreferences now have to appear to the left of their respective capturing group to be matched after them (e.g. `/(?<=\1(a))/`). Backreferences placed to before (to the right of) their referenced capturing group inside lookbehinds are guaranteed to trivially accept. We call these backreferences _backward references_.
+
+#### Negated lookaround assertions
+
+If the referenced capturing group of a backreference is inside a negated lookaround assertion the backreference is also part of, then the backreference will be guaranteed to trivially accept.
+
+To understand why this is the case, let's look at the example `/(?!(a))\w\1/y`.
+
+1. Let's assume the input string is `ab`. <br>
+   Since `(a)` accepts the character `a`, `(?!(a))` will reject. So the input is reject before the backreference `\1` can be reached.
+
+   The result of `/(?!(a))\w\1/y.exec("ab")` is `null`.
+2. Let's assume the input string is `bc`. <br>
+   Since `(a)` rejects the character `b`, its captured text will be undefined and `(?!(a))` will accept. Then `\w` will accept and consume the character `b`. Since the captured text of `(a)` is undefined, the backreference `\1` will trivially accept without consuming characters.
+
+   The result of `/(?!(a))\w\1/y.exec("bc")` is `[ 'b', undefined, index: 0, input: 'bc' ]`.
+
+Note that this is only a problem if the backreference is not part of the negated lookaround assertion. I.e. `/(?!(a)\1)\w/` is okay.
+
+<eslint-code-block>
+
+```js
+/* eslint regexp/no-useless-backreference: "error" */
+
+/* ✓ GOOD */
+var foo = /(a)b\1/;
+var foo = /(a?)b\1/;
+var foo = /(\b|a)+b\1/;
+var foo = /(a)?(?:a|\1)/;
+
+/* ✗ BAD */
+var foo = /\1(a)/;
+var foo = /(a\1)/;
+var foo = /(a)|\1/;
+var foo = /(?:(a)|\1)+/;
+var foo = /(?<=(a)\1)/;
+var foo = /(\b)a\1/;
+```
+
+</eslint-code-block>
 
 ## :wrench: Options
 
-See [no-useless-backreference] document.
+Nothing.
 
 ## :books: Further reading
 
diff --git a/lib/configs/recommended.ts b/lib/configs/recommended.ts
@@ -1,5 +1,3 @@
-import eslint from "eslint"
-
 export = {
     plugins: ["regexp"],
     rules: {
@@ -9,10 +7,8 @@ export = {
         "no-misleading-character-class": "error",
         "no-regex-spaces": "error",
         "prefer-regex-literals": "error",
-        // If ESLint is 7 or higher, use core rule. If it is 6 or less, use the copied rule.
-        [parseInt(eslint.Linter?.version?.[0] ?? "6", 10) >= 7
-            ? "no-useless-backreference"
-            : "regexp/no-useless-backreference"]: "error",
+        // The ESLint rule will report fewer cases than our rule
+        "no-useless-backreference": "off",
 
         // eslint-plugin-regexp rules
         "regexp/match-any": "error",
@@ -23,6 +19,7 @@ export = {
         "regexp/no-escape-backspace": "error",
         "regexp/no-invisible-character": "error",
         "regexp/no-octal": "error",
+        "regexp/no-useless-backreference": "error",
         "regexp/no-useless-exactly-quantifier": "error",
         "regexp/no-useless-two-nums-quantifier": "error",
         "regexp/prefer-d": "error",
diff --git a/lib/rules/no-useless-backreference.ts b/lib/rules/no-useless-backreference.ts
@@ -1,63 +1,89 @@
 import type { Expression } from "estree"
 import type { RegExpVisitor } from "regexpp/visitor"
-import type { Node as RegExpNode, LookaroundAssertion } from "regexpp/ast"
-import { createRule, defineRegexpVisitor } from "../utils"
+import type {
+    Node as RegExpNode,
+    Backreference,
+    Alternative,
+    CapturingGroup,
+} from "regexpp/ast"
+import { createRule, defineRegexpVisitor, getRegexpLocation } from "../utils"
+import {
+    getClosestAncestor,
+    getMatchingDirection,
+    isZeroLength,
+} from "regexp-ast-analysis"
 
-/* istanbul ignore file */
 /**
- * Finds the path from the given `regexpp` AST node to the root node.
- * @param {regexpp.Node} node Node.
- * @returns {regexpp.Node[]} Array that starts with the given node and ends with the root node.
+ * Returns whether the list of ancestors from `from` to `to` contains a negated
+ * lookaround.
  */
-function getPathToRoot(node: RegExpNode) {
-    const path = []
-    let current = node
-
-    while (current) {
-        path.push(current)
-        if (!current.parent) {
-            break
+function hasNegatedLookaroundInBetween(
+    from: CapturingGroup,
+    to: Alternative,
+): boolean {
+    for (let p: RegExpNode | null = from.parent; p && p !== to; p = p.parent) {
+        if (
+            p.type === "Assertion" &&
+            (p.kind === "lookahead" || p.kind === "lookbehind") &&
+            p.negate
+        ) {
+            return true
         }
-        current = current.parent
     }
-
-    return path
+    return false
 }
 
 /**
- * Determines whether the given `regexpp` AST node is a lookaround node.
- * @param {regexpp.Node} node Node.
- * @returns {boolean} `true` if it is a lookaround node.
+ * Returns the message id specifying the reason why the backreference is
+ * useless.
  */
-function isLookaround(node: RegExpNode): node is LookaroundAssertion {
-    return (
-        node.type === "Assertion" &&
-        (node.kind === "lookahead" || node.kind === "lookbehind")
-    )
-}
+function getUselessMessageId(backRef: Backreference): string | null {
+    const group = backRef.resolved
 
-/**
- * Determines whether the given `regexpp` AST node is a negative lookaround node.
- * @param {regexpp.Node} node Node.
- * @returns {boolean} `true` if it is a negative lookaround node.
- */
-function isNegativeLookaround(node: RegExpNode) {
-    return isLookaround(node) && node.negate
-}
+    const closestAncestor = getClosestAncestor(backRef, group)
 
-/**
- * Get last element
- */
-function last<T>(arr: T[]): T {
-    return arr[arr.length - 1]
+    if (closestAncestor === group) {
+        return "nested"
+    } else if (closestAncestor.type !== "Alternative") {
+        // if the closest common ancestor isn't an alternative => they're disjunctive.
+        return "disjunctive"
+    }
+
+    if (hasNegatedLookaroundInBetween(group, closestAncestor)) {
+        // if there are negated lookarounds between the group and the closest ancestor
+        // => group has already failed when backRef starts to match.
+        // e.g. `/(?!(a))\w\1/`
+        return "intoNegativeLookaround"
+    }
+
+    const matchingDir = getMatchingDirection(closestAncestor)
+
+    if (matchingDir === "ltr" && backRef.end <= group.start) {
+        // backRef is left, group is right ('forward reference')
+        // => group hasn't matched yet when backRef starts to match.
+        return "forward"
+    } else if (matchingDir === "rtl" && group.end <= backRef.start) {
+        // the opposite of the previous when the regex is matching backwards
+        // in a lookbehind context.
+        return "backward"
+    }
+
+    if (isZeroLength(group)) {
+        // if the referenced group does not consume characters, then any
+        // backreference will trivially be replaced with the empty string
+        return "empty"
+    }
+
+    // not useless
+    return null
 }
 
 export default createRule("no-useless-backreference", {
     meta: {
         docs: {
             description:
                 "disallow useless backreferences in regular expressions",
-            recommended: false,
+            recommended: true,
         },
         schema: [],
         messages: {
@@ -71,75 +97,31 @@ export default createRule("no-useless-backreference", {
                 "Backreference '{{ bref }}' will be ignored. It references group '{{ group }}' which is in another alternative.",
             intoNegativeLookaround:
                 "Backreference '{{ bref }}' will be ignored. It references group '{{ group }}' which is in a negative lookaround.",
+            empty:
+                "Backreference '{{ bref }}' will be ignored. It references group '{{ group }}' which always captures zero characters.",
         },
         type: "suggestion", // "problem",
     },
     create(context) {
+        const sourceCode = context.getSourceCode()
+
         /**
          * Create visitor
          * @param node
          */
         function createVisitor(node: Expression): RegExpVisitor.Handlers {
             return {
-                onBackreferenceEnter(bref) {
-                    const group = bref.resolved
-                    const brefPath = getPathToRoot(bref)
-                    const groupPath = getPathToRoot(group)
-                    let messageId = null
-
-                    if (brefPath.includes(group)) {
-                        // group is bref's ancestor => bref is nested ('nested reference') => group hasn't matched yet when bref starts to match.
-                        messageId = "nested"
-                    } else {
-                        // Start from the root to find the lowest common ancestor.
-                        let i = brefPath.length - 1
-                        let j = groupPath.length - 1
-
-                        do {
-                            i--
-                            j--
-                        } while (brefPath[i] === groupPath[j])
-
-                        const indexOfLowestCommonAncestor = j + 1
-                        const groupCut = groupPath.slice(
-                            0,
-                            indexOfLowestCommonAncestor,
-                        )
-                        const commonPath = groupPath.slice(
-                            indexOfLowestCommonAncestor,
-                        )
-                        const lowestCommonLookaround = commonPath.find(
-                            isLookaround,
-                        )
-                        const isMatchingBackward =
-                            lowestCommonLookaround &&
-                            lowestCommonLookaround.kind === "lookbehind"
-
-                        if (!isMatchingBackward && bref.end <= group.start) {
-                            // bref is left, group is right ('forward reference') => group hasn't matched yet when bref starts to match.
-                            messageId = "forward"
-                        } else if (
-                            isMatchingBackward &&
-                            group.end <= bref.start
-                        ) {
-                            // the opposite of the previous when the regex is matching backward in a lookbehind context.
-                            messageId = "backward"
-                        } else if (last(groupCut).type === "Alternative") {
-                            // group's and bref's ancestor nodes below the lowest common ancestor are sibling alternatives => they're disjunctive.
-                            messageId = "disjunctive"
-                        } else if (groupCut.some(isNegativeLookaround)) {
-                            // group is in a negative lookaround which isn't bref's ancestor => group has already failed when bref starts to match.
-                            messageId = "intoNegativeLookaround"
-                        }
-                    }
+                onBackreferenceEnter(backRef) {
+                    const messageId = getUselessMessageId(backRef)
 
                     if (messageId) {
                         context.report({
                             node,
+                            loc: getRegexpLocation(sourceCode, node, backRef),
                             messageId,
                             data: {
-                                bref: bref.raw,
-                                group: group.raw,
+                                bref: backRef.raw,
+                                group: backRef.resolved.raw,
                             },
                         })
                     }
diff --git a/tests/lib/rules/no-useless-backreference.ts b/tests/lib/rules/no-useless-backreference.ts
diff --git a/tools/update-rulesets.ts b/tools/update-rulesets.ts
