Repository: ota-meshi/eslint-plugin-regexp (ts)

Issue:
`optimal-quantifier-concatenation` behaves inconsistently around `?`s
![image](https://user-images.githubusercontent.com/20878432/122076567-4438f180-cdfb-11eb-9a7d-0c1c413b8f84.png)

This is because of a condition I added to all `aa?`. I forgot to consider `a?a`.

This might not look like a problem but it's just really strange when you see it happen in practice:

![image](https://user-images.githubusercontent.com/20878432/122077006-a5f95b80-cdfb-11eb-94b6-c1146d301004.png)


Pull request description:
Make `optimal-quantifier-concatenation` consistent
This fixes #252.

Patch:
diff --git a/docs/rules/optimal-quantifier-concatenation.md b/docs/rules/optimal-quantifier-concatenation.md
index b9e6168..c5ad2b9 100644
--- a/docs/rules/optimal-quantifier-concatenation.md
+++ b/docs/rules/optimal-quantifier-concatenation.md
@@ -34,7 +34,6 @@ var foo = /a\w*/;
 /* ✗ BAD */
 var foo = /\w+\d+/;
 var foo = /\w+\d?/;
-var foo = /\w?\w/;
 var foo = /[ab]*(?:a|b)/;
 var foo = /\w+(?:(a)|b)*/;
 ```
diff --git a/lib/rules/optimal-quantifier-concatenation.ts b/lib/rules/optimal-quantifier-concatenation.ts
index 55d528c..ccf30d1 100644
--- a/lib/rules/optimal-quantifier-concatenation.ts
+++ b/lib/rules/optimal-quantifier-concatenation.ts
@@ -293,6 +293,41 @@ function getQuantifierRepeatedElementReplacement(
     return { messageId: "combine", raw }
 }
 
+/**
+ * Whether the computed replacement is to be ignored.
+ */
+function ignoreReplacement(
+    left: Element,
+    right: Element,
+    result: Replacement,
+): boolean {
+    // There is a relatively common case for which we want to make
+    // an exception: `aa?`
+    // We will only suggest the replacement if the new raw is
+    // shorter than the current one.
+    if (left.type === "Quantifier") {
+        if (
+            left.raw.length + right.raw.length <= result.raw.length &&
+            isGroupOrCharacter(right) &&
+            left.min === 0 &&
+            left.max === 1
+        ) {
+            return true
+        }
+    }
+    if (right.type === "Quantifier") {
+        if (
+            left.raw.length + right.raw.length <= result.raw.length &&
+            isGroupOrCharacter(left) &&
+            right.min === 0 &&
+            right.max === 1
+        ) {
+            return true
+        }
+    }
+    return false
+}
+
 /**
  * Returns the replacement for the two adjacent elements.
  */
@@ -303,7 +338,7 @@ function getReplacement(
 ): Replacement | null {
     if (left.type === "Quantifier" && right.type === "Quantifier") {
         const result = getQuantifiersReplacement(left, right, context)
-        if (result) return result
+        if (result && !ignoreReplacement(left, right, result)) return result
     }
 
     if (left.type === "Quantifier") {
@@ -313,7 +348,7 @@ function getReplacement(
                 [left, rightRep],
                 context,
             )
-            if (result) return result
+            if (result && !ignoreReplacement(left, right, result)) return result
         }
     }
 
@@ -324,22 +359,7 @@ function getReplacement(
                 [leftRep, right],
                 context,
             )
-            if (result) {
-                // There is a relatively common case for which we want to make
-                // an exception: `aa?`
-                // We will only suggest the replacement if the new raw is
-                // shorter than the current one.
-                if (
-                    isGroupOrCharacter(left) &&
-                    right.min === 0 &&
-                    right.max === 1 &&
-                    left.raw.length + right.raw.length <= result.raw.length
-                ) {
-                    return null
-                }
-
-                return result
-            }
+            if (result && !ignoreReplacement(left, right, result)) return result
         }
     }
 


Test patch:
diff --git a/tests/lib/rules/optimal-quantifier-concatenation.ts b/tests/lib/rules/optimal-quantifier-concatenation.ts
index a9cdb88..546a0ca 100644
--- a/tests/lib/rules/optimal-quantifier-concatenation.ts
+++ b/tests/lib/rules/optimal-quantifier-concatenation.ts
@@ -17,6 +17,7 @@ tester.run("optimal-quantifier-concatenation", rule as any, {
         String.raw`/a+b+c+d+[abc]+/`,
         String.raw`/(?:a|::)?\w+/`,
         String.raw`/aa?/`,
+        String.raw`/\w?\w/`,
     ],
     invalid: [
         {
@@ -103,13 +104,6 @@ tester.run("optimal-quantifier-concatenation", rule as any, {
                 "'\\w*' and '\\w' can be combined into one quantifier '\\w+'.",
             ],
         },
-        {
-            code: String.raw`/\w?\w/`,
-            output: String.raw`/\w{1,2}/`,
-            errors: [
-                "'\\w?' and '\\w' can be combined into one quantifier '\\w{1,2}'.",
-            ],
-        },
         {
             code: String.raw`/\w+\w/`,
             output: String.raw`/\w{2,}/`,