New: add rule "prefer-named-capture-group" (fixes #11381) (#11392)

g-plane · platinumazure · commit ec59ec09c8d0 · 2019-03-01T19:50:57.000-06:00
* New: add rule "require-named-capture-group" (fixes #11381) * Chore: fix linting errors * Update: change rule name * Update: use `ReferenceTracker` * Update: ignore regexp syntax errors * Update: detect "unicode" flag * Update: report group instead of AST node * Docs: improve words * Chore: enable early return * Update: improve the message * Chore: simplify message Co-Authored-By: g-plane <g-plane@hotmail.com>
diff --git a/docs/rules/prefer-named-capture-group.md b/docs/rules/prefer-named-capture-group.md
@@ -0,0 +1,43 @@
+# Suggest using named capture group in regular expression (prefer-named-capture-group)
+
+With the landing of ECMAScript 2018, named capture groups can be used in regular expressions, which can improve their readability.
+
+```js
+const regex = /(?<year>[0-9]{4})/;
+```
+
+## Rule Details
+
+This rule is aimed at using named capture groups instead of numbered capture groups in regular expressions.
+
+Examples of **incorrect** code for this rule:
+
+```js
+/*eslint prefer-named-capture-group: "error"*/
+
+const foo = /(ba[rz])/;
+const bar = new RegExp('(ba[rz])');
+const baz = RegExp('(ba[rz])');
+
+foo.exec('bar')[1]; // Retrieve the group result.
+```
+
+Examples of **correct** code for this rule:
+
+```js
+/*eslint prefer-named-capture-group: "error"*/
+
+const foo = /(?<id>ba[rz])/;
+const bar = new RegExp('(?<id>ba[rz])');
+const baz = RegExp('(?<id>ba[rz])');
+
+foo.exec('bar').groups.id; // Retrieve the group result.
+```
+
+## When Not To Use It
+
+If you are targeting ECMAScript 2017 and/or older environments, you can disable this rule, because this ECMAScript feature is only supported in ECMAScript 2018 and/or newer environments.
+
+## Related Rules
+
+* [no-invalid-regexp](./no-invalid-regexp.md)
diff --git a/lib/built-in-rules-index.js b/lib/built-in-rules-index.js
@@ -232,6 +232,7 @@ module.exports = {
     "prefer-arrow-callback": require("./rules/prefer-arrow-callback"),
     "prefer-const": require("./rules/prefer-const"),
     "prefer-destructuring": require("./rules/prefer-destructuring"),
+    "prefer-named-capture-group": require("./rules/prefer-named-capture-group"),
     "prefer-numeric-literals": require("./rules/prefer-numeric-literals"),
     "prefer-object-spread": require("./rules/prefer-object-spread"),
     "prefer-promise-reject-errors": require("./rules/prefer-promise-reject-errors"),
diff --git a/lib/rules/prefer-named-capture-group.js b/lib/rules/prefer-named-capture-group.js
@@ -0,0 +1,123 @@
+/**
+ * @fileoverview Rule to enforce requiring named capture groups in regular expression.
+ * @author Pig Fang <https://github.com/g-plane>
+ */
+
+"use strict";
+
+//------------------------------------------------------------------------------
+// Requirements
+//------------------------------------------------------------------------------
+
+const {
+    CALL,
+    CONSTRUCT,
+    ReferenceTracker,
+    getStringIfConstant
+} = require("eslint-utils");
+const regexpp = require("regexpp");
+
+//------------------------------------------------------------------------------
+// Helpers
+//------------------------------------------------------------------------------
+
+const parser = new regexpp.RegExpParser();
+
+//------------------------------------------------------------------------------
+// Rule Definition
+//------------------------------------------------------------------------------
+
+module.exports = {
+    meta: {
+        type: "suggestion",
+
+        docs: {
+            description: "enforce using named capture group in regular expression",
+            category: "Best Practices",
+            recommended: false,
+            url: "https://eslint.org/docs/rules/prefer-named-capture-group"
+        },
+
+        schema: [],
+
+        messages: {
+            required: "Capture group '{{group}}' should be converted to a named or non-capturing group."
+        }
+    },
+
+    create(context) {
+
+        /**
+         * Function to check regular expression.
+         *
+         * @param {string} regex The regular expression to be check.
+         * @param {ASTNode} node AST node which contains regular expression.
+         * @param {boolean} uFlag Flag indicates whether unicode mode is enabled or not.
+         * @returns {void}
+         */
+        function checkRegex(regex, node, uFlag) {
+            let ast;
+
+            try {
+                ast = parser.parsePattern(regex, 0, regex.length, uFlag);
+            } catch (_) {
+
+                // ignore regex syntax errors
+                return;
+            }
+
+            regexpp.visitRegExpAST(ast, {
+                onCapturingGroupEnter(group) {
+                    if (!group.name) {
+                        const locNode = node.type === "Literal" ? node : node.arguments[0];
+
+                        context.report({
+                            node,
+                            messageId: "required",
+                            loc: {
+                                start: {
+                                    line: locNode.loc.start.line,
+                                    column: locNode.loc.start.column + group.start + 1
+                                },
+                                end: {
+                                    line: locNode.loc.start.line,
+                                    column: locNode.loc.start.column + group.end + 1
+                                }
+                            },
+                            data: {
+                                group: group.raw
+                            }
+                        });
+                    }
+                }
+            });
+        }
+
+        return {
+            Literal(node) {
+                if (node.regex) {
+                    checkRegex(node.regex.pattern, node, node.regex.flags.includes("u"));
+                }
+            },
+            Program() {
+                const scope = context.getScope();
+                const tracker = new ReferenceTracker(scope);
+                const traceMap = {
+                    RegExp: {
+                        [CALL]: true,
+                        [CONSTRUCT]: true
+                    }
+                };
+
+                for (const { node } of tracker.iterateGlobalReferences(traceMap)) {
+                    const regex = getStringIfConstant(node.arguments[0]);
+                    const flags = getStringIfConstant(node.arguments[1]);
+
+                    if (regex) {
+                        checkRegex(regex, node, flags && flags.includes("u"));
+                    }
+                }
+            }
+        };
+    }
+};
diff --git a/tests/lib/rules/prefer-named-capture-group.js b/tests/lib/rules/prefer-named-capture-group.js
@@ -0,0 +1,95 @@
+/**
+ * @fileoverview Tests for prefer-named-capture-group rule.
+ * @author Pig Fang <https://github.com/g-plane>
+ */
+
+"use strict";
+
+//------------------------------------------------------------------------------
+// Requirements
+//------------------------------------------------------------------------------
+
+const rule = require("../../../lib/rules/prefer-named-capture-group"),
+    RuleTester = require("../../../lib/testers/rule-tester");
+
+//------------------------------------------------------------------------------
+// Tests
+//------------------------------------------------------------------------------
+
+const ruleTester = new RuleTester({ parserOptions: { ecmaVersion: 2018 } });
+
+ruleTester.run("prefer-named-capture-group", rule, {
+    valid: [
+        "/normal_regex/",
+        "/(?:[0-9]{4})/",
+        "/(?<year>[0-9]{4})/",
+        "/\\u{1F680}/u",
+        "new RegExp()",
+        "new RegExp(foo)",
+        "new RegExp('')",
+        "new RegExp('(?<year>[0-9]{4})')",
+        "RegExp()",
+        "RegExp(foo)",
+        "RegExp('')",
+        "RegExp('(?<year>[0-9]{4})')",
+        "RegExp('(')", // invalid regexp should be ignored
+        "RegExp('\\\\u{1F680}', 'u')"
+    ],
+
+    invalid: [
+        {
+            code: "/([0-9]{4})/",
+            errors: [{
+                messageId: "required",
+                type: "Literal",
+                data: { group: "([0-9]{4})" },
+                line: 1,
+                column: 2,
+                endColumn: 12
+            }]
+        },
+        {
+            code: "new RegExp('([0-9]{4})')",
+            errors: [{
+                messageId: "required",
+                type: "NewExpression",
+                data: { group: "([0-9]{4})" },
+                line: 1,
+                column: 13,
+                endColumn: 23
+            }]
+        },
+        {
+            code: "RegExp('([0-9]{4})')",
+            errors: [{
+                messageId: "required",
+                type: "CallExpression",
+                data: { group: "([0-9]{4})" },
+                line: 1,
+                column: 9,
+                endColumn: 19
+            }]
+        },
+        {
+            code: "/([0-9]{4})-(\\w{5})/",
+            errors: [
+                {
+                    messageId: "required",
+                    type: "Literal",
+                    data: { group: "([0-9]{4})" },
+                    line: 1,
+                    column: 2,
+                    endColumn: 12
+                },
+                {
+                    messageId: "required",
+                    type: "Literal",
+                    data: { group: "(\\w{5})" },
+                    line: 1,
+                    column: 13,
+                    endColumn: 20
+                }
+            ]
+        }
+    ]
+});
diff --git a/tools/rule-types.json b/tools/rule-types.json
@@ -222,6 +222,7 @@
     "prefer-arrow-callback": "suggestion",
     "prefer-const": "suggestion",
     "prefer-destructuring": "suggestion",
+    "prefer-named-capture-group": "suggestion",
     "prefer-numeric-literals": "suggestion",
     "prefer-object-spread": "suggestion",
     "prefer-promise-reject-errors": "suggestion",
@@ -264,4 +265,4 @@
     "wrap-regex": "layout",
     "yield-star-spacing": "layout",
     "yoda": "suggestion"
-}
+}
