Add option to allow only partial use to the allowTop option of regexp/no-useless-non-capturing-group rule. (#215)

Author: ota-meshi

docs/rules/no-useless-non-capturing-group.md

@@ -21,15 +21,15 @@ This rule reports unnecessary non-capturing group
 /* eslint regexp/no-useless-non-capturing-group: "error" */
 
 /* ✓ GOOD */
-var foo = /(?:abcd)?/
-var foo = /a(?:ab|cd)/
+var foo = /(?:abcd)?/.test(str)
+var foo = /a(?:ab|cd)/.test(str)
 
 /* ✗ BAD */
-var foo = /(?:ab|cd)/
-var foo = /(?:abcd)/
-var foo = /(?:[a-d])/
-var foo = /(?:[a-d])|e/
-var foo = /(?:a|(?:b|c)|d)/
+var foo = /(?:ab|cd)/.test(str)
+var foo = /(?:abcd)/.test(str)
+var foo = /(?:[a-d])/.test(str)
+var foo = /(?:[a-d])|e/.test(str)
+var foo = /(?:a|(?:b|c)|d)/.test(str)
 ```
 
 </eslint-code-block>
@@ -39,33 +39,78 @@ var foo = /(?:a|(?:b|c)|d)/
 ```json5
 {
   "regexp/no-useless-non-capturing-group": ["error", {
-    "allowTop": true
+    "allowTop": "partial" // or "always" or "never"
   }]
 }
 ```
 
 - `"allowTop"`:
-  Whether a top-level non-capturing group is allowed. Defaults to `false`.
+  Whether a top-level non-capturing group is allowed. Defaults to `"partial"`.
 
   Sometimes it's useful to wrap a whole pattern into a non-capturing group (e.g. when the pattern is used as a building block to construct more complex patterns). Use this option to allow top-level non-capturing groups.
+  - `"partial"`:
+    Allows top-level non-capturing groups of patterns used as strings via `.source`.
 
-<eslint-code-block fix>
+    <eslint-code-block fix>
 
-```js
-/* eslint regexp/no-useless-non-capturing-group: ["error", { allowTop: true }] */
+    ```js
+    /* eslint regexp/no-useless-non-capturing-group: ["error", { allowTop: "partial" }] */
 
-/* ✓ GOOD */
-var foo = /(?:abcd)/
-var foo = /(?:ab|cd)/
-var foo = /(?:abcd)/
-var foo = /(?:[a-d])/
+    /* ✓ GOOD */
+    var foo = /(?:ab|cd)/;
+    var bar = /(?:ab|cd)/; // We still don't know how it will be used.
 
-/* ✗ BAD */
-var foo = /(?:[a-d])|e/
-var foo = /(?:a|(?:b|c)|d)/
-```
+    /* ✗ BAD */
+    /(?:ab|cd)/.test(str);
 
-</eslint-code-block>
+    /*-------*/
+    var baz = new RexExp(foo.source + 'e');
+    baz.test(str);
+    ```
+
+    </eslint-code-block>
+
+  - `"always"`:
+    Always allow top-level non-capturing groups.
+
+    <eslint-code-block fix>
+
+    ```js
+    /* eslint regexp/no-useless-non-capturing-group: ["error", { allowTop: "always" }] */
+
+    /* ✓ GOOD */
+    var foo = /(?:abcd)/.test(str)
+    var foo = /(?:ab|cd)/.test(str)
+    var foo = /(?:abcd)/.test(str)
+    var foo = /(?:[a-d])/.test(str)
+
+    /* ✗ BAD */
+    var foo = /(?:[a-d])|e/.test(str)
+    var foo = /(?:a|(?:b|c)|d)/.test(str)
+    ```
+
+    </eslint-code-block>
+
+  - `"never"`:
+    Never allow top-level non-capturing groups.
+
+    <eslint-code-block fix>
+
+    ```js
+    /* eslint regexp/no-useless-non-capturing-group: ["error", { allowTop: "never" }] */
+
+    /* ✗ BAD */
+    var foo = /(?:ab|cd)/;
+    var bar = /(?:ab|cd)/;
+
+    /(?:ab|cd)/.test(str);
+
+    /*-------*/
+    var baz = new RexExp(foo.source + 'e');
+    baz.test(str);
+    ```
+
+    </eslint-code-block>
 
 ## :rocket: Version
 

lib/rules/no-useless-non-capturing-group.ts

@@ -2,6 +2,7 @@ import type { Group } from "regexpp/ast"
 import type { RegExpVisitor } from "regexpp/visitor"
 import type { RegExpContext } from "../utils"
 import { canUnwrapped, createRule, defineRegexpVisitor } from "../utils"
+import { UsageOfPattern } from "../utils/get-usage-of-pattern"
 
 /**
  * Returns whether the given group is the top-level group of its pattern.
@@ -36,7 +37,15 @@ export default createRule("no-useless-non-capturing-group", {
             {
                 type: "object",
                 properties: {
-                    allowTop: { type: "boolean" },
+                    allowTop: {
+                        anyOf: [
+                            {
+                                // backward compatibility
+                                type: "boolean",
+                            },
+                            { enum: ["always", "never", "partial"] },
+                        ],
+                    },
                 },
                 additionalProperties: false,
             },
@@ -47,7 +56,12 @@ export default createRule("no-useless-non-capturing-group", {
         type: "suggestion", // "problem",
     },
     create(context) {
-        const allowTop = context.options[0]?.allowTop ?? false
+        const allowTop: "always" | "never" | "partial" =
+            context.options[0]?.allowTop === true
+                ? "always"
+                : context.options[0]?.allowTop === false
+                ? "never"
+                : context.options[0]?.allowTop ?? "partial"
 
         /**
          * Create visitor
@@ -56,10 +70,25 @@ export default createRule("no-useless-non-capturing-group", {
             node,
             getRegexpLocation,
             fixReplaceNode,
+            getUsageOfPattern,
         }: RegExpContext): RegExpVisitor.Handlers {
+            let isIgnored: (gNode: Group) => boolean
+            if (allowTop === "always") {
+                isIgnored = isTopLevel
+            } else if (allowTop === "partial") {
+                if (getUsageOfPattern() !== UsageOfPattern.whole) {
+                    isIgnored = isTopLevel
+                } else {
+                    isIgnored = () => false
+                }
+            } else {
+                // allowTop === "never"
+                isIgnored = () => false
+            }
+
             return {
                 onGroupEnter(gNode) {
-                    if (allowTop && isTopLevel(gNode)) {
+                    if (isIgnored(gNode)) {
                         return
                     }
 
@@ -98,7 +127,18 @@ export default createRule("no-useless-non-capturing-group", {
                         node,
                         loc: getRegexpLocation(gNode),
                         messageId: "unexpected",
-                        fix: fixReplaceNode(gNode, gNode.raw.slice(3, -1)),
+                        fix: fixReplaceNode(gNode, () => {
+                            if (
+                                allowTop === "never" &&
+                                isTopLevel(gNode) &&
+                                getUsageOfPattern() !== UsageOfPattern.whole
+                            ) {
+                                // Top-level group and potentially partially used patterns
+                                // do not autofix because they can cause side effects.
+                                return null
+                            }
+                            return gNode.raw.slice(3, -1)
+                        }),
                     })
                 },
             }