From ec59ec09c8d001b8c04f9edc09994e2b0d0af0f9 Mon Sep 17 00:00:00 2001 From: Pig Fang Date: Sat, 2 Mar 2019 09:50:57 +0800 Subject: [PATCH] New: add rule "prefer-named-capture-group" (fixes #11381) (#11392) * 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 --- docs/rules/prefer-named-capture-group.md | 43 ++++++ lib/built-in-rules-index.js | 1 + lib/rules/prefer-named-capture-group.js | 123 ++++++++++++++++++ tests/lib/rules/prefer-named-capture-group.js | 95 ++++++++++++++ tools/rule-types.json | 3 +- 5 files changed, 264 insertions(+), 1 deletion(-) create mode 100644 docs/rules/prefer-named-capture-group.md create mode 100644 lib/rules/prefer-named-capture-group.js create mode 100644 tests/lib/rules/prefer-named-capture-group.js diff --git a/docs/rules/prefer-named-capture-group.md b/docs/rules/prefer-named-capture-group.md new file mode 100644 index 00000000000..ff8790c836d --- /dev/null +++ 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 = /(?[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 = /(?ba[rz])/; +const bar = new RegExp('(?ba[rz])'); +const baz = RegExp('(?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 index aaf2f06eccd..d75fbbc698d 100644 --- 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 new file mode 100644 index 00000000000..07e69f022c7 --- /dev/null +++ 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 + */ + +"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 new file mode 100644 index 00000000000..88047d7c9fa --- /dev/null +++ b/tests/lib/rules/prefer-named-capture-group.js @@ -0,0 +1,95 @@ +/** + * @fileoverview Tests for prefer-named-capture-group rule. + * @author Pig Fang + */ + +"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})/", + "/(?[0-9]{4})/", + "/\\u{1F680}/u", + "new RegExp()", + "new RegExp(foo)", + "new RegExp('')", + "new RegExp('(?[0-9]{4})')", + "RegExp()", + "RegExp(foo)", + "RegExp('')", + "RegExp('(?[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 index 44c1d48abcd..1239e5f3ffa 100644 --- 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" -} \ No newline at end of file +}