Add regexp/no-optional-assertion rule (#135)

ota-meshi · web-flow · commit 51027f74c918 · 2021-04-16T16:14:58.000+02:00
diff --git a/README.md b/README.md
@@ -99,6 +99,7 @@ The rules with the following star :star: are included in the `plugin:regexp/reco
 | [regexp/no-legacy-features](https://ota-meshi.github.io/eslint-plugin-regexp/rules/no-legacy-features.html) | disallow legacy RegExp features |  |
 | [regexp/no-obscure-range](https://ota-meshi.github.io/eslint-plugin-regexp/rules/no-obscure-range.html) | disallow obscure character ranges |  |
 | [regexp/no-octal](https://ota-meshi.github.io/eslint-plugin-regexp/rules/no-octal.html) | disallow octal escape sequence | :star: |
+| [regexp/no-optional-assertion](https://ota-meshi.github.io/eslint-plugin-regexp/rules/no-optional-assertion.html) | disallow optional assertions |  |
 | [regexp/no-potentially-useless-backreference](https://ota-meshi.github.io/eslint-plugin-regexp/rules/no-potentially-useless-backreference.html) | disallow backreferences that reference a group that might not be matched |  |
 | [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 |  |
diff --git a/docs/rules/README.md b/docs/rules/README.md
@@ -27,6 +27,7 @@ The rules with the following star :star: are included in the `plugin:regexp/reco
 | [regexp/no-legacy-features](./no-legacy-features.md) | disallow legacy RegExp features |  |
 | [regexp/no-obscure-range](./no-obscure-range.md) | disallow obscure character ranges |  |
 | [regexp/no-octal](./no-octal.md) | disallow octal escape sequence | :star: |
+| [regexp/no-optional-assertion](./no-optional-assertion.md) | disallow optional assertions |  |
 | [regexp/no-potentially-useless-backreference](./no-potentially-useless-backreference.md) | disallow backreferences that reference a group that might not be matched |  |
 | [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 |  |
diff --git a/docs/rules/no-obscure-range.md b/docs/rules/no-obscure-range.md
@@ -59,7 +59,7 @@ It allows all values that the [allowedCharacterRanges] setting allows.
 
 ### `"allowed": "alphanumeric"`
 
-<eslint-code-block fix>
+<eslint-code-block>
 
 ```js
 /* eslint regexp/no-obscure-range: ["error", { "allowed": "alphanumeric" }] */
@@ -80,7 +80,7 @@ var foo = /[😀-😄]/u;
 
 ### `"allowed": "all"`
 
-<eslint-code-block fix>
+<eslint-code-block>
 
 ```js
 /* eslint regexp/no-obscure-range: ["error", { "allowed": "all" }] */
@@ -101,7 +101,7 @@ var foo = /[\41-\x45]/;
 
 ### `"allowed": [ "alphanumeric", "😀-😏" ]`
 
-<eslint-code-block fix>
+<eslint-code-block>
 
 ```js
 /* eslint regexp/no-obscure-range: ["error", { "allowed": [ "alphanumeric", "😀-😏" ] }] */
diff --git a/docs/rules/no-optional-assertion.md b/docs/rules/no-optional-assertion.md
@@ -0,0 +1,57 @@
+---
+pageClass: "rule-details"
+sidebarDepth: 0
+title: "regexp/no-optional-assertion"
+description: "disallow optional assertions"
+---
+# regexp/no-optional-assertion
+
+> disallow optional assertions
+
+- :exclamation: <badge text="This rule has not been released yet." vertical="middle" type="error"> ***This rule has not been released yet.*** </badge>
+
+## :book: Rule Details
+
+Assertions that are quantified (directly or indirectly) can be considered optional if the quantifier has a minimum of zero.
+
+A simple example is the following pattern: `/a(?:$)*b/`. The `$` assertion will reject but if that happens, it will simply be ignored because of the `*` quantifier. The assertion is optional, serving no function whatsoever.
+
+More generally, an assertion is optional, if there exists a parent quantifier with a minimum of zero such that all possible paths of the quantified element that contain the assertion do not consume characters.
+
+Here's an example: `a(?:foo|(?<!-)(?:-|\b))*b`. The `\b` is optional. However, the lookbehind `(?<!-)` is not optional because the group `(?:-|\b)` right after it can consume a character.
+
+Optional assertions don't affect the pattern in any way. They are essentially dead code.
+
+<eslint-code-block>
+
+```js
+/* eslint regexp/no-optional-assertion: "error" */
+
+/* ✓ GOOD */
+var foo = /\w+(?::|\b)/;
+
+/* ✗ BAD */
+var foo = /a(?:$)*b/;
+var foo = /a(?:foo|(?<!-)(?:-|\b))*b/; // The `\b` is optional.
+var foo = /(?:^)?\w+/;   // warns about `^`
+var foo = /\w+(?::|$)?/; // warns about `$`
+```
+
+</eslint-code-block>
+
+## :wrench: Options
+
+Nothing.
+
+## :heart: Compatibility
+
+This rule was taken from [eslint-plugin-clean-regex].  
+This rule is compatible with [clean-regex/no-optional-assertion] rule.
+
+[eslint-plugin-clean-regex]: https://github.com/RunDevelopment/eslint-plugin-clean-regex
+[clean-regex/no-optional-assertion]: https://github.com/RunDevelopment/eslint-plugin-clean-regex/blob/master/docs/rules/no-optional-assertion.md
+
+## :mag: Implementation
+
+- [Rule source](https://github.com/ota-meshi/eslint-plugin-regexp/blob/master/lib/rules/no-optional-assertion.ts)
+- [Test source](https://github.com/ota-meshi/eslint-plugin-regexp/blob/master/tests/lib/rules/no-optional-assertion.ts)
diff --git a/lib/rules/no-optional-assertion.ts b/lib/rules/no-optional-assertion.ts
@@ -0,0 +1,127 @@
+import type { Expression } from "estree"
+import type { RegExpVisitor } from "regexpp/visitor"
+import type {
+    Alternative,
+    Assertion,
+    CapturingGroup,
+    Group,
+    Quantifier,
+} from "regexpp/ast"
+import { createRule, defineRegexpVisitor, getRegexpLocation } from "../utils"
+import { isZeroLength } from "regexp-ast-analysis"
+
+type ZeroQuantifier = Quantifier & { min: 0 }
+
+/**
+ * Checks whether the given quantifier is quantifier with a minimum of 0.
+ */
+function isZeroQuantifier(node: Quantifier): node is ZeroQuantifier {
+    return node.min === 0
+}
+
+/**
+ * Returns whether the given assertion is optional in regard to the given quantifier with a minimum of 0.
+ *
+ * Optional means that all paths in the element if the quantifier which contain the given assertion also have do not
+ * consume characters. For more information and examples on optional assertions, see the documentation page of this
+ * rule.
+ */
+function isOptional(assertion: Assertion, quantifier: ZeroQuantifier): boolean {
+    let element: Assertion | Quantifier | Group | CapturingGroup = assertion
+    while (element.parent !== quantifier) {
+        const parent: Quantifier | Alternative = element.parent
+        if (parent.type === "Alternative") {
+            // make sure that all element before and after are zero length
+            for (const e of parent.elements) {
+                if (e === element) {
+                    continue // we will ignore this element.
+                }
+
+                if (!isZeroLength(e)) {
+                    // Some element around our target element can possibly consume characters.
+                    // This means, we found a path from or to the assertion which can consume characters.
+                    return false
+                }
+            }
+
+            if (parent.parent.type === "Pattern") {
+                throw new Error(
+                    "The given assertion is not a descendant of the given quantifier.",
+                )
+            }
+            element = parent.parent
+        } else {
+            // parent.type === "Quantifier"
+            if (parent.max > 1 && !isZeroLength(parent)) {
+                // If an ascendant quantifier of the element has maximum of 2 or more, we have to check whether
+                // the quantifier itself has zero length.
+                // E.g. in /(?:a|(\b|-){2})?/ the \b is not optional
+                return false
+            }
+
+            element = parent
+        }
+    }
+    // We reached the top.
+    // If we made it this far, we could not disprove that the assertion is optional, so it has to optional.
+    return true
+}
+
+export default createRule("no-optional-assertion", {
+    meta: {
+        docs: {
+            description: "disallow optional assertions",
+            // TODO Switch to recommended in the major version.
+            // recommended: true,
+            recommended: false,
+        },
+        schema: [],
+        messages: {
+            optionalAssertion:
+                "This assertion effectively optional and does not change the pattern. Either remove the assertion or change the parent quantifier '{{quantifier}}'.",
+        },
+        type: "problem",
+    },
+    create(context) {
+        const sourceCode = context.getSourceCode()
+
+        /**
+         * Create visitor
+         * @param node
+         */
+        function createVisitor(node: Expression): RegExpVisitor.Handlers {
+            // The closest quantifier with a minimum of 0 is stored at index = 0.
+            const zeroQuantifierStack: ZeroQuantifier[] = []
+            return {
+                onQuantifierEnter(q) {
+                    if (isZeroQuantifier(q)) {
+                        zeroQuantifierStack.unshift(q)
+                    }
+                },
+                onQuantifierLeave(q) {
+                    if (zeroQuantifierStack[0] === q) {
+                        zeroQuantifierStack.shift()
+                    }
+                },
+                onAssertionEnter(assertion) {
+                    const q = zeroQuantifierStack[0]
+
+                    if (q && isOptional(assertion, q)) {
+                        context.report({
+                            node,
+                            loc: getRegexpLocation(sourceCode, node, assertion),
+                            messageId: "optionalAssertion",
+                            data: {
+                                quantifier: q.raw.substr(q.element.raw.length),
+                            },
+                        })
+                    }
+                },
+            }
+        }
+
+        return defineRegexpVisitor(context, {
+            createVisitor,
+        })
+    },
+})
diff --git a/lib/utils/rules.ts b/lib/utils/rules.ts
@@ -15,6 +15,7 @@ import noLazyEnds from "../rules/no-lazy-ends"
 import noLegacyFeatures from "../rules/no-legacy-features"
 import noObscureRange from "../rules/no-obscure-range"
 import noOctal from "../rules/no-octal"
+import noOptionalAssertion from "../rules/no-optional-assertion"
 import noPotentiallyUselessBackreference from "../rules/no-potentially-useless-backreference"
 import noTriviallyNestedAssertion from "../rules/no-trivially-nested-assertion"
 import noUnusedCapturingGroup from "../rules/no-unused-capturing-group"
@@ -60,6 +61,7 @@ export const rules = [
     noLegacyFeatures,
     noObscureRange,
     noOctal,
+    noOptionalAssertion,
     noPotentiallyUselessBackreference,
     noTriviallyNestedAssertion,
     noUnusedCapturingGroup,
diff --git a/tests/lib/rules/no-optional-assertion.ts b/tests/lib/rules/no-optional-assertion.ts
@@ -0,0 +1,93 @@
+import { RuleTester } from "eslint"
+import rule from "../../../lib/rules/no-optional-assertion"
+
+const tester = new RuleTester({
+    parserOptions: {
+        ecmaVersion: 2020,
+        sourceType: "module",
+    },
+})
+
+tester.run("no-optional-assertion", rule as any, {
+    valid: [
+        String.raw`/fo(?:o\b)?/`,
+        String.raw`/(?:a|(\b|-){2})?/`,
+        String.raw`/(?:a|(?:\b|a)+)?/`,
+        String.raw`/fo(?:o\b)/`,
+        String.raw`/fo(?:o\b){1}/`,
+    ],
+    invalid: [
+        {
+            code: String.raw`/(?:\b|(?=a))?/`,
+            errors: [
+                {
+                    message:
+                        "This assertion effectively optional and does not change the pattern. Either remove the assertion or change the parent quantifier '?'.",
+                    line: 1,
+                    column: 5,
+                },
+                {
+                    message:
+                        "This assertion effectively optional and does not change the pattern. Either remove the assertion or change the parent quantifier '?'.",
+                    line: 1,
+                    column: 8,
+                },
+            ],
+        },
+        {
+            code: String.raw`/(?:\b|a)?/`,
+            errors: [
+                {
+                    message:
+                        "This assertion effectively optional and does not change the pattern. Either remove the assertion or change the parent quantifier '?'.",
+                    line: 1,
+                    column: 5,
+                },
+            ],
+        },
+        {
+            code: String.raw`/(?:^|a)*/`,
+            errors: [
+                {
+                    message:
+                        "This assertion effectively optional and does not change the pattern. Either remove the assertion or change the parent quantifier '*'.",
+                    line: 1,
+                    column: 5,
+                },
+            ],
+        },
+        {
+            code: String.raw`/(?:((?:(\b|a)))|b)?/`,
+            errors: [
+                {
+                    message:
+                        "This assertion effectively optional and does not change the pattern. Either remove the assertion or change the parent quantifier '?'.",
+                    line: 1,
+                    column: 10,
+                },
+            ],
+        },
+        {
+            code: String.raw`/(?:((?:(\b|a)))|b)*/`,
+            errors: [
+                {
+                    message:
+                        "This assertion effectively optional and does not change the pattern. Either remove the assertion or change the parent quantifier '*'.",
+                    line: 1,
+                    column: 10,
+                },
+            ],
+        },
+        {
+            code: String.raw`/((\b)+){0,}/`,
+            errors: [
+                {
+                    message:
+                        "This assertion effectively optional and does not change the pattern. Either remove the assertion or change the parent quantifier '{0,}'.",
+                    line: 1,
+                    column: 4,
+                },
+            ],
+        },
+    ],
+})
