Updated no-useless-non-greedy rule (#143)

README.md

@@ -110,7 +110,7 @@ The rules with the following star :star: are included in the `plugin:regexp/reco
 | [regexp/no-useless-escape](https://ota-meshi.github.io/eslint-plugin-regexp/rules/no-useless-escape.html) | disallow unnecessary escape characters in RegExp |  |
 | [regexp/no-useless-exactly-quantifier](https://ota-meshi.github.io/eslint-plugin-regexp/rules/no-useless-exactly-quantifier.html) | disallow unnecessary exactly quantifier | :star: |
 | [regexp/no-useless-non-capturing-group](https://ota-meshi.github.io/eslint-plugin-regexp/rules/no-useless-non-capturing-group.html) | disallow unnecessary Non-capturing group | :wrench: |
-| [regexp/no-useless-non-greedy](https://ota-meshi.github.io/eslint-plugin-regexp/rules/no-useless-non-greedy.html) | disallow unnecessary quantifier non-greedy (`?`) | :wrench: |
+| [regexp/no-useless-non-greedy](https://ota-meshi.github.io/eslint-plugin-regexp/rules/no-useless-non-greedy.html) | disallow unnecessarily non-greedy quantifiers | :wrench: |
 | [regexp/no-useless-range](https://ota-meshi.github.io/eslint-plugin-regexp/rules/no-useless-range.html) | disallow unnecessary range of characters by using a hyphen | :wrench: |
 | [regexp/no-useless-two-nums-quantifier](https://ota-meshi.github.io/eslint-plugin-regexp/rules/no-useless-two-nums-quantifier.html) | disallow unnecessary `{n,m}` quantifier | :star::wrench: |
 | [regexp/optimal-lookaround-quantifier](https://ota-meshi.github.io/eslint-plugin-regexp/rules/optimal-lookaround-quantifier.html) | disallow the alternatives of lookarounds that end with a non-constant quantifier |  |

docs/rules/README.md

@@ -38,7 +38,7 @@ The rules with the following star :star: are included in the `plugin:regexp/reco
 | [regexp/no-useless-escape](./no-useless-escape.md) | disallow unnecessary escape characters in RegExp |  |
 | [regexp/no-useless-exactly-quantifier](./no-useless-exactly-quantifier.md) | disallow unnecessary exactly quantifier | :star: |
 | [regexp/no-useless-non-capturing-group](./no-useless-non-capturing-group.md) | disallow unnecessary Non-capturing group | :wrench: |
-| [regexp/no-useless-non-greedy](./no-useless-non-greedy.md) | disallow unnecessary quantifier non-greedy (`?`) | :wrench: |
+| [regexp/no-useless-non-greedy](./no-useless-non-greedy.md) | disallow unnecessarily non-greedy quantifiers | :wrench: |
 | [regexp/no-useless-range](./no-useless-range.md) | disallow unnecessary range of characters by using a hyphen | :wrench: |
 | [regexp/no-useless-two-nums-quantifier](./no-useless-two-nums-quantifier.md) | disallow unnecessary `{n,m}` quantifier | :star::wrench: |
 | [regexp/optimal-lookaround-quantifier](./optimal-lookaround-quantifier.md) | disallow the alternatives of lookarounds that end with a non-constant quantifier |  |

docs/rules/no-useless-non-greedy.md

@@ -2,18 +2,30 @@
 pageClass: "rule-details"
 sidebarDepth: 0
 title: "regexp/no-useless-non-greedy"
-description: "disallow unnecessary quantifier non-greedy (`?`)"
+description: "disallow unnecessarily non-greedy quantifiers"
 since: "v0.3.0"
 ---
 # regexp/no-useless-non-greedy
 
-> disallow unnecessary quantifier non-greedy (`?`)
+> disallow unnecessarily non-greedy quantifiers
 
 - :wrench: The `--fix` option on the [command line](https://eslint.org/docs/user-guide/command-line-interface#fixing-problems) can automatically fix some of the problems reported by this rule.
 
 ## :book: Rule Details
 
-This rule reports unnecessary quantifier non-greedy (`?`).
+This rule reports lazy quantifiers that don't need to by lazy.
+
+There are two reasons why a lazy quantifier doesn't have to lazy:
+
+1. It's a constant quantifier (e.g. `a{3}?`).
+
+2. The quantifier is effectively possessive (e.g. `a+?b`).
+
+   Whether a quantifier (let's call it _q_) is effectively possessive depends on the expression after it (let's call it _e_). _q_ is effectively possessive if _q_ cannot accept the character accepted by _e_ and _e_ cannot accept the characters accepted by _q_.
+
+   In the example above, the character `a` and the character `b` do not overlap. Therefore the quantifier `a+` is possessive.
+
+   Since an effectively possessive quantifier cannot give up characters to the expression after it, it doesn't matter whether the quantifier greedy or lazy. However, greedy quantifiers should be preferred because they require less characters to write and are easier to visually parse.
 
 <eslint-code-block fix>
 
@@ -25,11 +37,13 @@ var foo = /a*?/;
 var foo = /a+?/;
 var foo = /a{4,}?/;
 var foo = /a{2,4}?/;
+var foo = /a[\s\S]*?bar/;
 
 /* ✗ BAD */
 var foo = /a{1}?/;
 var foo = /a{4}?/;
 var foo = /a{2,2}?/;
+var foo = /ab+?c/;
 ```
 
 </eslint-code-block>

lib/rules/no-useless-non-greedy.ts

@@ -1,24 +1,63 @@
-import type { Expression } from "estree"
+import type { Rule, SourceCode } from "eslint"
+import type { Expression, SourceLocation } from "estree"
+import {
+    getMatchingDirection,
+    getFirstConsumedChar,
+    getFirstCharAfter,
+} from "regexp-ast-analysis"
+import type { Quantifier } from "regexpp/ast"
 import type { RegExpVisitor } from "regexpp/visitor"
 import {
     createRule,
     defineRegexpVisitor,
     getRegexpLocation,
     getRegexpRange,
+    parseFlags,
 } from "../utils"
 
+/**
+ * Returns a fix that makes the given quantifier greedy.
+ */
+function makeGreedy(
+    sourceCode: SourceCode,
+    node: Expression,
+    qNode: Quantifier,
+) {
+    return (fixer: Rule.RuleFixer): Rule.Fix | null => {
+        const range = getRegexpRange(sourceCode, node, qNode)
+        if (range == null) {
+            return null
+        }
+        return fixer.removeRange([range[1] - 1, range[1]])
+    }
+}
+
+/**
+ * Returns the source location of the lazy modifier of the given quantifier.
+ */
+function getLazyLoc(
+    sourceCode: SourceCode,
+    node: Expression,
+    qNode: Quantifier,
+): SourceLocation {
+    const offset = qNode.raw.length - 1
+    return getRegexpLocation(sourceCode, node, qNode, [offset, offset + 1])
+}
+
 export default createRule("no-useless-non-greedy", {
     meta: {
         docs: {
-            description: "disallow unnecessary quantifier non-greedy (`?`)",
+            description: "disallow unnecessarily non-greedy quantifiers",
             // TODO In the major version
             // recommended: true,
             recommended: false,
         },
         fixable: "code",
         schema: [],
         messages: {
-            unexpected: "Unexpected quantifier non-greedy.",
+            constant: "Unexpected non-greedy constant quantifier.",
+            possessive:
+                "Unexpected non-greedy constant quantifier. The quantifier is effectively possessive, so it doesn't matter whether it is greedy or not.",
         },
         type: "suggestion", // "problem",
     },
@@ -29,33 +68,61 @@ export default createRule("no-useless-non-greedy", {
          * Create visitor
          * @param node
          */
-        function createVisitor(node: Expression): RegExpVisitor.Handlers {
+        function createVisitor(
+            node: Expression,
+            _pattern: string,
+            flagsStr: string,
+        ): RegExpVisitor.Handlers {
+            const flags = parseFlags(flagsStr)
+
             return {
                 onQuantifierEnter(qNode) {
-                    if (qNode.greedy === false && qNode.min === qNode.max) {
-                        const offset = qNode.raw.length - 1
+                    if (qNode.greedy) {
+                        return
+                    }
+
+                    if (qNode.min === qNode.max) {
+                        // a constant lazy quantifier (e.g. /a{2}?/)
+
                         context.report({
                             node,
-                            loc: getRegexpLocation(sourceCode, node, qNode, [
-                                offset,
-                                offset + 1,
-                            ]),
-                            messageId: "unexpected",
-                            fix(fixer) {
-                                const range = getRegexpRange(
-                                    sourceCode,
-                                    node,
-                                    qNode,
-                                )
-                                if (range == null) {
-                                    return null
-                                }
-                                return fixer.removeRange([
-                                    range[1] - 1,
-                                    range[1],
-                                ])
-                            },
+                            loc: getLazyLoc(sourceCode, node, qNode),
+                            messageId: "constant",
+                            fix: makeGreedy(sourceCode, node, qNode),
                         })
+                        return
+                    }
+
+                    // This is more tricky.
+                    // The basic idea here is that if the first character of the
+                    // quantified element and the first character of whatever
+                    // comes after the quantifier are always different, then the
+                    // lazy modifier doesn't matter.
+                    // E.g. /a+?b+/ == /a+b+/
+
+                    const matchingDir = getMatchingDirection(qNode)
+                    const firstChar = getFirstConsumedChar(
+                        qNode,
+                        matchingDir,
+                        flags,
+                    )
+                    if (!firstChar.empty) {
+                        const after = getFirstCharAfter(
+                            qNode,
+                            matchingDir,
+                            flags,
+                        )
+                        if (
+                            !after.edge &&
+                            firstChar.char.isDisjointWith(after.char)
+                        ) {
+                            context.report({
+                                node,
+                                loc: getLazyLoc(sourceCode, node, qNode),
+                                messageId: "possessive",
+                                fix: makeGreedy(sourceCode, node, qNode),
+                            })
+                        }
                     }
                 },
             }

tests/lib/rules/no-useless-non-greedy.ts

@@ -9,14 +9,24 @@ const tester = new RuleTester({
 })
 
 tester.run("no-useless-non-greedy", rule as any, {
-    valid: [`/a*?/`, `/a+?/`, `/a{4,}?/`, `/a{2,4}?/`, `/a{2,2}/`, `/a{3}/`],
+    valid: [
+        `/a*?/`,
+        `/a+?/`,
+        `/a{4,}?/`,
+        `/a{2,4}?/`,
+        `/a{2,2}/`,
+        `/a{3}/`,
+        `/a+?b*/`,
+        `/[\\s\\S]+?bar/`,
+        `/a??a?/`,
+    ],
     invalid: [
         {
             code: `/a{1}?/`,
             output: `/a{1}/`,
             errors: [
                 {
-                    message: "Unexpected quantifier non-greedy.",
+                    messageId: "constant",
                     line: 1,
                     column: 6,
                     endLine: 1,
@@ -29,7 +39,7 @@ tester.run("no-useless-non-greedy", rule as any, {
             output: `/a{4}/`,
             errors: [
                 {
-                    message: "Unexpected quantifier non-greedy.",
+                    messageId: "constant",
                     line: 1,
                     column: 6,
                     endLine: 1,
@@ -40,20 +50,31 @@ tester.run("no-useless-non-greedy", rule as any, {
         {
             code: `/a{2,2}?/`,
             output: `/a{2,2}/`,
-            errors: ["Unexpected quantifier non-greedy."],
+            errors: [{ messageId: "constant" }],
         },
         {
             code: String.raw`const s = "\\d{1}?"
             new RegExp(s)`,
             output: String.raw`const s = "\\d{1}"
             new RegExp(s)`,
-            errors: ["Unexpected quantifier non-greedy."],
+            errors: [{ messageId: "constant" }],
         },
         {
             code: String.raw`const s = "\\d"+"{1}?"
             new RegExp(s)`,
             output: null,
-            errors: ["Unexpected quantifier non-greedy."],
+            errors: [{ messageId: "constant" }],
+        },
+
+        {
+            code: `/a+?b+/`,
+            output: `/a+b+/`,
+            errors: [{ messageId: "possessive" }],
+        },
+        {
+            code: `/(?:a|cd)+?(?:b+|zzz)/`,
+            output: `/(?:a|cd)+(?:b+|zzz)/`,
+            errors: [{ messageId: "possessive" }],
         },
     ],
 })