Skip to content

Commit

Permalink
New: add rule "prefer-named-capture-group" (fixes #11381) (#11392)
Browse files Browse the repository at this point in the history 
* 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>
  • Loading branch information
@g-plane @platinumazure
g-plane authored and platinumazure committed Mar 2, 2019
1 parent a44f750 commit ec59ec0
Show file tree
Hide file tree
Showing 5 changed files with 264 additions and 1 deletion.
43 changes: 43 additions & 0 deletions docs/rules/prefer-named-capture-group.md
View file
@@ -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)
1 change: 1 addition & 0 deletions lib/built-in-rules-index.js
View file
Expand Up @@ -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"),
Expand Down
123 changes: 123 additions & 0 deletions lib/rules/prefer-named-capture-group.js
View file
@@ -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"));
}
}
}
};
}
};
95 changes: 95 additions & 0 deletions tests/lib/rules/prefer-named-capture-group.js
View file
@@ -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
}
]
}
]
});
3 changes: 2 additions & 1 deletion tools/rule-types.json
View file
Expand Up @@ -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",
Expand Down Expand Up @@ -264,4 +265,4 @@
"wrap-regex": "layout",
"yield-star-spacing": "layout",
"yoda": "suggestion"
}
}

0 comments on commit ec59ec0

Please sign in to comment.