From 49d535e29d9a5ae22a927e404bab91c25ff79f93 Mon Sep 17 00:00:00 2001 From: Pig Fang Date: Thu, 14 Feb 2019 15:19:15 +0800 Subject: [PATCH 01/11] New: add rule "require-named-capture-group" (fixes #11381) --- docs/rules/require-named-capture-group.md | 43 +++++++++ lib/rules/require-named-capture-group.js | 87 +++++++++++++++++++ .../lib/rules/require-named-capture-group.js | 57 ++++++++++++ 3 files changed, 187 insertions(+) create mode 100644 docs/rules/require-named-capture-group.md create mode 100644 lib/rules/require-named-capture-group.js create mode 100644 tests/lib/rules/require-named-capture-group.js diff --git a/docs/rules/require-named-capture-group.md b/docs/rules/require-named-capture-group.md new file mode 100644 index 00000000000..571825e3c1e --- /dev/null +++ b/docs/rules/require-named-capture-group.md @@ -0,0 +1,43 @@ +# Suggest using named capture group in regular expression (require-named-capture-group) + +With the landing of ECMAScript 2018, named capture group can be used in regular expression, which can improve the readability. + +```js +const regex = /(?[0-9]{4})/; +``` + +## Rule Details + +This rule is aimed at using named capture group instead of numbered capture group in regular expression. + +Examples of **incorrect** code for this rule: + +```js +/*eslint require-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 require-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/rules/require-named-capture-group.js b/lib/rules/require-named-capture-group.js new file mode 100644 index 00000000000..3e3d07227d8 --- /dev/null +++ b/lib/rules/require-named-capture-group.js @@ -0,0 +1,87 @@ +/** + * @fileoverview Rule to enforce requiring named capture groups in regular expression. + * @author Pig Fang + */ + +"use strict"; + +//------------------------------------------------------------------------------ +// Requirements +//------------------------------------------------------------------------------ + +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/require-named-capture-group" + }, + + schema: [], + + messages: { + required: "Capture group '{{group}}' in regular expression should be named." + } + }, + + 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. + * @returns {void} + */ + function checkRegex(regex, node) { + const ast = parser.parsePattern(regex); + regexpp.visitRegExpAST(ast, { + onCapturingGroupEnter(group) { + if (!group.name) { + context.report({ + node, + messageId: "required", + data: { + group: group.raw + } + }); + } + } + }); + } + + return { + Literal(node) { + if (node.regex) { + checkRegex(node.regex.pattern, node); + } + }, + "NewExpression, CallExpression"(node) { + const { callee, arguments: [firstArg] } = node; + if ( + callee.type === "Identifier" && + callee.name === "RegExp" && + firstArg && + firstArg.type === "Literal" && + typeof firstArg.value === "string" + ) { + checkRegex(firstArg.value, node); + } + } + } + } +} diff --git a/tests/lib/rules/require-named-capture-group.js b/tests/lib/rules/require-named-capture-group.js new file mode 100644 index 00000000000..7dc91918b4c --- /dev/null +++ b/tests/lib/rules/require-named-capture-group.js @@ -0,0 +1,57 @@ +/** + * @fileoverview Tests for require-named-capture-group rule. + * @author Pig Fang + */ + +"use strict"; + +//------------------------------------------------------------------------------ +// Requirements +//------------------------------------------------------------------------------ + +const rule = require("../../../lib/rules/require-named-capture-group"), + RuleTester = require("../../../lib/testers/rule-tester"); + +//------------------------------------------------------------------------------ +// Tests +//------------------------------------------------------------------------------ + +const ruleTester = new RuleTester({ parserOptions: { ecmaVersion: 2018 } }); + +ruleTester.run("require-named-capture-group", rule, { + valid: [ + "/normal_regex/", + "/(?:[0-9]{4})/", + "/(?[0-9]{4})/", + "new RegExp()", + "new RegExp(foo)", + "new RegExp('')", + "new RegExp('(?[0-9]{4})')", + "RegExp()", + "RegExp(foo)", + "RegExp('')", + "RegExp('(?[0-9]{4})')" + ], + + invalid: [ + { + code: "/([0-9]{4})/", + errors: [{ messageId: "required", type: "Literal", data: { group: '([0-9]{4})' } }] + }, + { + code: "new RegExp('([0-9]{4})')", + errors: [{ messageId: "required", type: "NewExpression", data: { group: '([0-9]{4})' } }] + }, + { + code: "RegExp('([0-9]{4})')", + errors: [{ messageId: "required", type: "CallExpression", data: { group: '([0-9]{4})' } }] + }, + { + code: "/([0-9]{4})-(\\w{5})/", + errors: [ + { messageId: "required", type: "Literal", data: { group: '([0-9]{4})' } }, + { messageId: "required", type: "Literal", data: { group: '(\\w{5})' } }, + ] + } + ] +}); From 8976784a0eb5b2f8835e4c0ecdc115d40ae0e018 Mon Sep 17 00:00:00 2001 From: Pig Fang Date: Thu, 14 Feb 2019 15:42:56 +0800 Subject: [PATCH 02/11] Chore: fix linting errors --- lib/built-in-rules-index.js | 1 + lib/rules/require-named-capture-group.js | 7 +++++-- tests/lib/rules/require-named-capture-group.js | 10 +++++----- tools/rule-types.json | 3 ++- 4 files changed, 13 insertions(+), 8 deletions(-) diff --git a/lib/built-in-rules-index.js b/lib/built-in-rules-index.js index aaf2f06eccd..fbdc97a7817 100644 --- a/lib/built-in-rules-index.js +++ b/lib/built-in-rules-index.js @@ -245,6 +245,7 @@ module.exports = { "require-atomic-updates": require("./rules/require-atomic-updates"), "require-await": require("./rules/require-await"), "require-jsdoc": require("./rules/require-jsdoc"), + "require-named-capture-group": require("./rules/require-named-capture-group"), "require-unicode-regexp": require("./rules/require-unicode-regexp"), "require-yield": require("./rules/require-yield"), "rest-spread-spacing": require("./rules/rest-spread-spacing"), diff --git a/lib/rules/require-named-capture-group.js b/lib/rules/require-named-capture-group.js index 3e3d07227d8..23b9783de09 100644 --- a/lib/rules/require-named-capture-group.js +++ b/lib/rules/require-named-capture-group.js @@ -40,6 +40,7 @@ module.exports = { }, create(context) { + /** * Function to check regular expression. * @@ -49,6 +50,7 @@ module.exports = { */ function checkRegex(regex, node) { const ast = parser.parsePattern(regex); + regexpp.visitRegExpAST(ast, { onCapturingGroupEnter(group) { if (!group.name) { @@ -72,6 +74,7 @@ module.exports = { }, "NewExpression, CallExpression"(node) { const { callee, arguments: [firstArg] } = node; + if ( callee.type === "Identifier" && callee.name === "RegExp" && @@ -82,6 +85,6 @@ module.exports = { checkRegex(firstArg.value, node); } } - } + }; } -} +}; diff --git a/tests/lib/rules/require-named-capture-group.js b/tests/lib/rules/require-named-capture-group.js index 7dc91918b4c..51986b45456 100644 --- a/tests/lib/rules/require-named-capture-group.js +++ b/tests/lib/rules/require-named-capture-group.js @@ -36,21 +36,21 @@ ruleTester.run("require-named-capture-group", rule, { invalid: [ { code: "/([0-9]{4})/", - errors: [{ messageId: "required", type: "Literal", data: { group: '([0-9]{4})' } }] + errors: [{ messageId: "required", type: "Literal", data: { group: "([0-9]{4})" } }] }, { code: "new RegExp('([0-9]{4})')", - errors: [{ messageId: "required", type: "NewExpression", data: { group: '([0-9]{4})' } }] + errors: [{ messageId: "required", type: "NewExpression", data: { group: "([0-9]{4})" } }] }, { code: "RegExp('([0-9]{4})')", - errors: [{ messageId: "required", type: "CallExpression", data: { group: '([0-9]{4})' } }] + errors: [{ messageId: "required", type: "CallExpression", data: { group: "([0-9]{4})" } }] }, { code: "/([0-9]{4})-(\\w{5})/", errors: [ - { messageId: "required", type: "Literal", data: { group: '([0-9]{4})' } }, - { messageId: "required", type: "Literal", data: { group: '(\\w{5})' } }, + { messageId: "required", type: "Literal", data: { group: "([0-9]{4})" } }, + { messageId: "required", type: "Literal", data: { group: "(\\w{5})" } } ] } ] diff --git a/tools/rule-types.json b/tools/rule-types.json index 44c1d48abcd..c4f75c8eaa9 100644 --- a/tools/rule-types.json +++ b/tools/rule-types.json @@ -235,6 +235,7 @@ "require-atomic-updates": "problem", "require-await": "suggestion", "require-jsdoc": "suggestion", + "require-named-capture-group": "suggestion", "require-unicode-regexp": "suggestion", "require-yield": "suggestion", "rest-spread-spacing": "layout", @@ -264,4 +265,4 @@ "wrap-regex": "layout", "yield-star-spacing": "layout", "yoda": "suggestion" -} \ No newline at end of file +} From 0d19bdb1438e479121d0aaae05e43eeba3009296 Mon Sep 17 00:00:00 2001 From: Pig Fang Date: Sat, 16 Feb 2019 10:05:10 +0800 Subject: [PATCH 03/11] Update: change rule name --- ...named-capture-group.md => prefer-named-capture-group.md} | 6 +++--- lib/built-in-rules-index.js | 2 +- ...named-capture-group.js => prefer-named-capture-group.js} | 2 +- ...named-capture-group.js => prefer-named-capture-group.js} | 6 +++--- tools/rule-types.json | 2 +- 5 files changed, 9 insertions(+), 9 deletions(-) rename docs/rules/{require-named-capture-group.md => prefer-named-capture-group.md} (84%) rename lib/rules/{require-named-capture-group.js => prefer-named-capture-group.js} (97%) rename tests/lib/rules/{require-named-capture-group.js => prefer-named-capture-group.js} (89%) diff --git a/docs/rules/require-named-capture-group.md b/docs/rules/prefer-named-capture-group.md similarity index 84% rename from docs/rules/require-named-capture-group.md rename to docs/rules/prefer-named-capture-group.md index 571825e3c1e..58135035b08 100644 --- a/docs/rules/require-named-capture-group.md +++ b/docs/rules/prefer-named-capture-group.md @@ -1,4 +1,4 @@ -# Suggest using named capture group in regular expression (require-named-capture-group) +# Suggest using named capture group in regular expression (prefer-named-capture-group) With the landing of ECMAScript 2018, named capture group can be used in regular expression, which can improve the readability. @@ -13,7 +13,7 @@ This rule is aimed at using named capture group instead of numbered capture grou Examples of **incorrect** code for this rule: ```js -/*eslint require-named-capture-group: "error"*/ +/*eslint prefer-named-capture-group: "error"*/ const foo = /(ba[rz])/; const bar = new RegExp('(ba[rz])'); @@ -25,7 +25,7 @@ foo.exec('bar')[1]; // Retrieve the group result. Examples of **correct** code for this rule: ```js -/*eslint require-named-capture-group: "error"*/ +/*eslint prefer-named-capture-group: "error"*/ const foo = /(?ba[rz])/; const bar = new RegExp('(?ba[rz])'); diff --git a/lib/built-in-rules-index.js b/lib/built-in-rules-index.js index fbdc97a7817..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"), @@ -245,7 +246,6 @@ module.exports = { "require-atomic-updates": require("./rules/require-atomic-updates"), "require-await": require("./rules/require-await"), "require-jsdoc": require("./rules/require-jsdoc"), - "require-named-capture-group": require("./rules/require-named-capture-group"), "require-unicode-regexp": require("./rules/require-unicode-regexp"), "require-yield": require("./rules/require-yield"), "rest-spread-spacing": require("./rules/rest-spread-spacing"), diff --git a/lib/rules/require-named-capture-group.js b/lib/rules/prefer-named-capture-group.js similarity index 97% rename from lib/rules/require-named-capture-group.js rename to lib/rules/prefer-named-capture-group.js index 23b9783de09..36a117876d8 100644 --- a/lib/rules/require-named-capture-group.js +++ b/lib/rules/prefer-named-capture-group.js @@ -29,7 +29,7 @@ module.exports = { description: "enforce using named capture group in regular expression", category: "Best Practices", recommended: false, - url: "https://eslint.org/docs/rules/require-named-capture-group" + url: "https://eslint.org/docs/rules/prefer-named-capture-group" }, schema: [], diff --git a/tests/lib/rules/require-named-capture-group.js b/tests/lib/rules/prefer-named-capture-group.js similarity index 89% rename from tests/lib/rules/require-named-capture-group.js rename to tests/lib/rules/prefer-named-capture-group.js index 51986b45456..0c293acc03c 100644 --- a/tests/lib/rules/require-named-capture-group.js +++ b/tests/lib/rules/prefer-named-capture-group.js @@ -1,5 +1,5 @@ /** - * @fileoverview Tests for require-named-capture-group rule. + * @fileoverview Tests for prefer-named-capture-group rule. * @author Pig Fang */ @@ -9,7 +9,7 @@ // Requirements //------------------------------------------------------------------------------ -const rule = require("../../../lib/rules/require-named-capture-group"), +const rule = require("../../../lib/rules/prefer-named-capture-group"), RuleTester = require("../../../lib/testers/rule-tester"); //------------------------------------------------------------------------------ @@ -18,7 +18,7 @@ const rule = require("../../../lib/rules/require-named-capture-group"), const ruleTester = new RuleTester({ parserOptions: { ecmaVersion: 2018 } }); -ruleTester.run("require-named-capture-group", rule, { +ruleTester.run("prefer-named-capture-group", rule, { valid: [ "/normal_regex/", "/(?:[0-9]{4})/", diff --git a/tools/rule-types.json b/tools/rule-types.json index c4f75c8eaa9..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", @@ -235,7 +236,6 @@ "require-atomic-updates": "problem", "require-await": "suggestion", "require-jsdoc": "suggestion", - "require-named-capture-group": "suggestion", "require-unicode-regexp": "suggestion", "require-yield": "suggestion", "rest-spread-spacing": "layout", From 1481a54a9197c43c08798009823e7d7c68097f1d Mon Sep 17 00:00:00 2001 From: Pig Fang Date: Sat, 16 Feb 2019 10:29:35 +0800 Subject: [PATCH 04/11] Update: use `ReferenceTracker` --- lib/rules/prefer-named-capture-group.js | 33 ++++++++++++++++--------- 1 file changed, 22 insertions(+), 11 deletions(-) diff --git a/lib/rules/prefer-named-capture-group.js b/lib/rules/prefer-named-capture-group.js index 36a117876d8..8cf9f7f639d 100644 --- a/lib/rules/prefer-named-capture-group.js +++ b/lib/rules/prefer-named-capture-group.js @@ -9,6 +9,12 @@ // Requirements //------------------------------------------------------------------------------ +const { + CALL, + CONSTRUCT, + ReferenceTracker, + getStringIfConstant +} = require("eslint-utils"); const regexpp = require("regexpp"); //------------------------------------------------------------------------------ @@ -72,17 +78,22 @@ module.exports = { checkRegex(node.regex.pattern, node); } }, - "NewExpression, CallExpression"(node) { - const { callee, arguments: [firstArg] } = node; - - if ( - callee.type === "Identifier" && - callee.name === "RegExp" && - firstArg && - firstArg.type === "Literal" && - typeof firstArg.value === "string" - ) { - checkRegex(firstArg.value, node); + 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]); + + if (regex) { + checkRegex(regex, node); + } } } }; From 55be93825004e819c1bcdfb87d011a3134d13bbf Mon Sep 17 00:00:00 2001 From: Pig Fang Date: Sat, 16 Feb 2019 10:35:17 +0800 Subject: [PATCH 05/11] Update: ignore regexp syntax errors --- lib/rules/prefer-named-capture-group.js | 33 +++++++++++-------- tests/lib/rules/prefer-named-capture-group.js | 3 +- 2 files changed, 21 insertions(+), 15 deletions(-) diff --git a/lib/rules/prefer-named-capture-group.js b/lib/rules/prefer-named-capture-group.js index 8cf9f7f639d..d5fc16af283 100644 --- a/lib/rules/prefer-named-capture-group.js +++ b/lib/rules/prefer-named-capture-group.js @@ -55,21 +55,26 @@ module.exports = { * @returns {void} */ function checkRegex(regex, node) { - const ast = parser.parsePattern(regex); - - regexpp.visitRegExpAST(ast, { - onCapturingGroupEnter(group) { - if (!group.name) { - context.report({ - node, - messageId: "required", - data: { - group: group.raw - } - }); + try { + const ast = parser.parsePattern(regex); + + regexpp.visitRegExpAST(ast, { + onCapturingGroupEnter(group) { + if (!group.name) { + context.report({ + node, + messageId: "required", + data: { + group: group.raw + } + }); + } } - } - }); + }); + } catch (_) { + + // ignore regex syntax errors + } } return { diff --git a/tests/lib/rules/prefer-named-capture-group.js b/tests/lib/rules/prefer-named-capture-group.js index 0c293acc03c..11d6fa17853 100644 --- a/tests/lib/rules/prefer-named-capture-group.js +++ b/tests/lib/rules/prefer-named-capture-group.js @@ -30,7 +30,8 @@ ruleTester.run("prefer-named-capture-group", rule, { "RegExp()", "RegExp(foo)", "RegExp('')", - "RegExp('(?[0-9]{4})')" + "RegExp('(?[0-9]{4})')", + "RegExp('(')" // invalid regexp should be ignored ], invalid: [ From 1976f971ecdfa99a0d7acb709322011d4e577683 Mon Sep 17 00:00:00 2001 From: Pig Fang Date: Sat, 16 Feb 2019 10:48:18 +0800 Subject: [PATCH 06/11] Update: detect "unicode" flag --- lib/rules/prefer-named-capture-group.js | 10 ++++++---- tests/lib/rules/prefer-named-capture-group.js | 4 +++- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/lib/rules/prefer-named-capture-group.js b/lib/rules/prefer-named-capture-group.js index d5fc16af283..35d6c8c7f95 100644 --- a/lib/rules/prefer-named-capture-group.js +++ b/lib/rules/prefer-named-capture-group.js @@ -52,11 +52,12 @@ module.exports = { * * @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) { + function checkRegex(regex, node, uFlag) { try { - const ast = parser.parsePattern(regex); + const ast = parser.parsePattern(regex, 0, regex.length, uFlag); regexpp.visitRegExpAST(ast, { onCapturingGroupEnter(group) { @@ -80,7 +81,7 @@ module.exports = { return { Literal(node) { if (node.regex) { - checkRegex(node.regex.pattern, node); + checkRegex(node.regex.pattern, node, node.regex.flags.includes("u")); } }, Program() { @@ -95,9 +96,10 @@ module.exports = { for (const { node } of tracker.iterateGlobalReferences(traceMap)) { const regex = getStringIfConstant(node.arguments[0]); + const flags = getStringIfConstant(node.arguments[1]); if (regex) { - checkRegex(regex, node); + 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 index 11d6fa17853..27d7ca667e3 100644 --- a/tests/lib/rules/prefer-named-capture-group.js +++ b/tests/lib/rules/prefer-named-capture-group.js @@ -23,6 +23,7 @@ ruleTester.run("prefer-named-capture-group", rule, { "/normal_regex/", "/(?:[0-9]{4})/", "/(?[0-9]{4})/", + "/\\u{1F680}/u", "new RegExp()", "new RegExp(foo)", "new RegExp('')", @@ -31,7 +32,8 @@ ruleTester.run("prefer-named-capture-group", rule, { "RegExp(foo)", "RegExp('')", "RegExp('(?[0-9]{4})')", - "RegExp('(')" // invalid regexp should be ignored + "RegExp('(')", // invalid regexp should be ignored + "RegExp('\\\\u{1F680}', 'u')" ], invalid: [ From ec4ba4e422a5faaa8096889f8a5b4dbe3f198a3d Mon Sep 17 00:00:00 2001 From: Pig Fang Date: Sat, 16 Feb 2019 11:29:16 +0800 Subject: [PATCH 07/11] Update: report group instead of AST node --- lib/rules/prefer-named-capture-group.js | 12 +++++ tests/lib/rules/prefer-named-capture-group.js | 45 ++++++++++++++++--- 2 files changed, 52 insertions(+), 5 deletions(-) diff --git a/lib/rules/prefer-named-capture-group.js b/lib/rules/prefer-named-capture-group.js index 35d6c8c7f95..26d7eba0fd3 100644 --- a/lib/rules/prefer-named-capture-group.js +++ b/lib/rules/prefer-named-capture-group.js @@ -62,9 +62,21 @@ module.exports = { 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 } diff --git a/tests/lib/rules/prefer-named-capture-group.js b/tests/lib/rules/prefer-named-capture-group.js index 27d7ca667e3..88047d7c9fa 100644 --- a/tests/lib/rules/prefer-named-capture-group.js +++ b/tests/lib/rules/prefer-named-capture-group.js @@ -39,21 +39,56 @@ ruleTester.run("prefer-named-capture-group", rule, { invalid: [ { code: "/([0-9]{4})/", - errors: [{ messageId: "required", type: "Literal", data: { group: "([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})" } }] + 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})" } }] + 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})" } }, - { messageId: "required", type: "Literal", data: { group: "(\\w{5})" } } + { + 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 + } ] } ] From 6b7d4e94c19124fbbab1d45cf9c21b825b2d0d84 Mon Sep 17 00:00:00 2001 From: Pig Fang Date: Thu, 21 Feb 2019 14:56:37 +0800 Subject: [PATCH 08/11] Docs: improve words --- docs/rules/prefer-named-capture-group.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/rules/prefer-named-capture-group.md b/docs/rules/prefer-named-capture-group.md index 58135035b08..ff8790c836d 100644 --- a/docs/rules/prefer-named-capture-group.md +++ b/docs/rules/prefer-named-capture-group.md @@ -1,6 +1,6 @@ # Suggest using named capture group in regular expression (prefer-named-capture-group) -With the landing of ECMAScript 2018, named capture group can be used in regular expression, which can improve the readability. +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})/; @@ -8,7 +8,7 @@ const regex = /(?[0-9]{4})/; ## Rule Details -This rule is aimed at using named capture group instead of numbered capture group in regular expression. +This rule is aimed at using named capture groups instead of numbered capture groups in regular expressions. Examples of **incorrect** code for this rule: From c047e4113c4bac8762c39adfd5f225bbd23aa715 Mon Sep 17 00:00:00 2001 From: Pig Fang Date: Thu, 21 Feb 2019 14:59:26 +0800 Subject: [PATCH 09/11] Chore: enable early return --- lib/rules/prefer-named-capture-group.js | 57 +++++++++++++------------ 1 file changed, 30 insertions(+), 27 deletions(-) diff --git a/lib/rules/prefer-named-capture-group.js b/lib/rules/prefer-named-capture-group.js index 26d7eba0fd3..a78b6d3e0ee 100644 --- a/lib/rules/prefer-named-capture-group.js +++ b/lib/rules/prefer-named-capture-group.js @@ -56,38 +56,41 @@ module.exports = { * @returns {void} */ function checkRegex(regex, node, uFlag) { + let ast; + try { - const ast = parser.parsePattern(regex, 0, regex.length, uFlag); - - 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 - } - }); - } - } - }); + 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 { From 880c546dfdd656e5bc7b9602f8d5cf12bc389e13 Mon Sep 17 00:00:00 2001 From: Pig Fang Date: Thu, 21 Feb 2019 15:01:18 +0800 Subject: [PATCH 10/11] Update: improve the message --- lib/rules/prefer-named-capture-group.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/rules/prefer-named-capture-group.js b/lib/rules/prefer-named-capture-group.js index a78b6d3e0ee..0316e4ab996 100644 --- a/lib/rules/prefer-named-capture-group.js +++ b/lib/rules/prefer-named-capture-group.js @@ -41,7 +41,7 @@ module.exports = { schema: [], messages: { - required: "Capture group '{{group}}' in regular expression should be named." + required: "Capture group '{{group}}' in regular expression should be named. Or you may want to use a non-capturing group." } }, From fbf51a583ce04aa16c2c3201ff5633b0fc94141c Mon Sep 17 00:00:00 2001 From: Kevin Partington Date: Sat, 2 Mar 2019 09:20:32 +0800 Subject: [PATCH 11/11] Chore: simplify message Co-Authored-By: g-plane --- lib/rules/prefer-named-capture-group.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/rules/prefer-named-capture-group.js b/lib/rules/prefer-named-capture-group.js index 0316e4ab996..07e69f022c7 100644 --- a/lib/rules/prefer-named-capture-group.js +++ b/lib/rules/prefer-named-capture-group.js @@ -41,7 +41,7 @@ module.exports = { schema: [], messages: { - required: "Capture group '{{group}}' in regular expression should be named. Or you may want to use a non-capturing group." + required: "Capture group '{{group}}' should be converted to a named or non-capturing group." } },