From 11ab0be908d21843b4dfe5874b1e8776d23fd3f2 Mon Sep 17 00:00:00 2001 From: Milos Djermanovic Date: Mon, 9 Sep 2019 01:55:56 +0200 Subject: [PATCH 1/2] Fix: prefer-named-capture-group incorrect locations (fixes #12233) --- lib/rules/prefer-named-capture-group.js | 36 +++-- tests/lib/rules/prefer-named-capture-group.js | 125 ++++++++++++++++++ 2 files changed, 152 insertions(+), 9 deletions(-) diff --git a/lib/rules/prefer-named-capture-group.js b/lib/rules/prefer-named-capture-group.js index 07e69f022c7..dcab0c66867 100644 --- a/lib/rules/prefer-named-capture-group.js +++ b/lib/rules/prefer-named-capture-group.js @@ -50,16 +50,16 @@ module.exports = { /** * Function to check regular expression. * - * @param {string} regex The regular expression to be check. + * @param {string} pattern The regular expression pattern 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) { + function checkRegex(pattern, node, uFlag) { let ast; try { - ast = parser.parsePattern(regex, 0, regex.length, uFlag); + ast = parser.parsePattern(pattern, 0, pattern.length, uFlag); } catch (_) { // ignore regex syntax errors @@ -69,12 +69,22 @@ module.exports = { regexpp.visitRegExpAST(ast, { onCapturingGroupEnter(group) { if (!group.name) { - const locNode = node.type === "Literal" ? node : node.arguments[0]; + let locNode, rawPattern, loc; + + if (node.type === "Literal") { + locNode = node; + rawPattern = locNode.raw.slice(1, locNode.raw.lastIndexOf("/")); + } else { + locNode = node.arguments[0]; + if (locNode.type === "Literal" && typeof locNode.value === "string") { + rawPattern = locNode.raw.slice(1, -1); + } else if (locNode.type === "TemplateLiteral" && locNode.expressions.length === 0 && locNode.quasis.length === 1) { + rawPattern = locNode.quasis[0].value.raw; + } + } - context.report({ - node, - messageId: "required", - loc: { + if (pattern === rawPattern && locNode.loc.start.line === locNode.loc.end.line) { + loc = { start: { line: locNode.loc.start.line, column: locNode.loc.start.column + group.start + 1 @@ -83,7 +93,15 @@ module.exports = { line: locNode.loc.start.line, column: locNode.loc.start.column + group.end + 1 } - }, + }; + } else { + loc = locNode.loc; + } + + context.report({ + node, + messageId: "required", + loc, 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 e3a86e9ee30..7947ceef2b2 100644 --- a/tests/lib/rules/prefer-named-capture-group.js +++ b/tests/lib/rules/prefer-named-capture-group.js @@ -70,6 +70,17 @@ ruleTester.run("prefer-named-capture-group", rule, { endColumn: 19 }] }, + { + code: "new RegExp(`a(bc)d`)", + errors: [{ + messageId: "required", + type: "NewExpression", + data: { group: "(bc)" }, + line: 1, + column: 14, + endColumn: 18 + }] + }, { code: "/([0-9]{4})-(\\w{5})/", errors: [ @@ -90,6 +101,120 @@ ruleTester.run("prefer-named-capture-group", rule, { endColumn: 20 } ] + }, + + // For computed, multiline and strings with escape sequences, report the whole arguments[0] location. + { + code: "new RegExp('(' + 'a)')", + errors: [{ + messageId: "required", + type: "NewExpression", + data: { group: "(a)" }, + line: 1, + column: 12, + endColumn: 22 + }] + }, + { + code: "new RegExp('a(bc)d' + 'e')", + errors: [{ + messageId: "required", + type: "NewExpression", + data: { group: "(bc)" }, + line: 1, + column: 12, + endColumn: 26 + }] + }, + { + code: "RegExp('(a)'+'')", + errors: [{ + messageId: "required", + type: "CallExpression", + data: { group: "(a)" }, + line: 1, + column: 8, + endColumn: 16 + }] + }, + { + code: "RegExp( '' + '(ab)')", + errors: [{ + messageId: "required", + type: "CallExpression", + data: { group: "(ab)" }, + line: 1, + column: 9, + endColumn: 20 + }] + }, + { + code: "new RegExp(`(ab)${''}`)", + errors: [{ + messageId: "required", + type: "NewExpression", + data: { group: "(ab)" }, + line: 1, + column: 12, + endColumn: 23 + }] + }, + { + code: "new RegExp(`(a)\n`)", + errors: [{ + messageId: "required", + type: "NewExpression", + data: { group: "(a)" }, + line: 1, + column: 12, + endLine: 2, + endColumn: 2 + }] + }, + { + code: "RegExp(`a(b\nc)d`)", + errors: [{ + messageId: "required", + type: "CallExpression", + data: { group: "(b\nc)" }, + line: 1, + column: 8, + endLine: 2, + endColumn: 5 + }] + }, + { + code: "new RegExp('a(b)\\'')", + errors: [{ + messageId: "required", + type: "NewExpression", + data: { group: "(b)" }, + line: 1, + column: 12, + endColumn: 20 + }] + }, + { + code: "RegExp('(a)\\\\d')", + errors: [{ + messageId: "required", + type: "CallExpression", + data: { group: "(a)" }, + line: 1, + column: 8, + endColumn: 16 + }] + }, + { + code: "RegExp(`\\a(b)`)", + errors: [{ + messageId: "required", + type: "CallExpression", + data: { group: "(b)" }, + line: 1, + column: 8, + endColumn: 15 + }] } ] }); From 93c7c1a72571d0e537567c10ca75c94982ad9044 Mon Sep 17 00:00:00 2001 From: Milos Djermanovic Date: Mon, 16 Sep 2019 13:06:23 +0200 Subject: [PATCH 2/2] Don't report loc --- lib/rules/prefer-named-capture-group.js | 30 -------- tests/lib/rules/prefer-named-capture-group.js | 77 ++++++------------- 2 files changed, 22 insertions(+), 85 deletions(-) diff --git a/lib/rules/prefer-named-capture-group.js b/lib/rules/prefer-named-capture-group.js index dcab0c66867..7f4a9c3c8e3 100644 --- a/lib/rules/prefer-named-capture-group.js +++ b/lib/rules/prefer-named-capture-group.js @@ -69,39 +69,9 @@ module.exports = { regexpp.visitRegExpAST(ast, { onCapturingGroupEnter(group) { if (!group.name) { - let locNode, rawPattern, loc; - - if (node.type === "Literal") { - locNode = node; - rawPattern = locNode.raw.slice(1, locNode.raw.lastIndexOf("/")); - } else { - locNode = node.arguments[0]; - if (locNode.type === "Literal" && typeof locNode.value === "string") { - rawPattern = locNode.raw.slice(1, -1); - } else if (locNode.type === "TemplateLiteral" && locNode.expressions.length === 0 && locNode.quasis.length === 1) { - rawPattern = locNode.quasis[0].value.raw; - } - } - - if (pattern === rawPattern && locNode.loc.start.line === locNode.loc.end.line) { - 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 - } - }; - } else { - loc = locNode.loc; - } - context.report({ node, messageId: "required", - loc, 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 7947ceef2b2..4444ef479c0 100644 --- a/tests/lib/rules/prefer-named-capture-group.js +++ b/tests/lib/rules/prefer-named-capture-group.js @@ -44,8 +44,8 @@ ruleTester.run("prefer-named-capture-group", rule, { type: "Literal", data: { group: "([0-9]{4})" }, line: 1, - column: 2, - endColumn: 12 + column: 1, + endColumn: 13 }] }, { @@ -55,8 +55,8 @@ ruleTester.run("prefer-named-capture-group", rule, { type: "NewExpression", data: { group: "([0-9]{4})" }, line: 1, - column: 13, - endColumn: 23 + column: 1, + endColumn: 25 }] }, { @@ -66,8 +66,8 @@ ruleTester.run("prefer-named-capture-group", rule, { type: "CallExpression", data: { group: "([0-9]{4})" }, line: 1, - column: 9, - endColumn: 19 + column: 1, + endColumn: 21 }] }, { @@ -75,10 +75,7 @@ ruleTester.run("prefer-named-capture-group", rule, { errors: [{ messageId: "required", type: "NewExpression", - data: { group: "(bc)" }, - line: 1, - column: 14, - endColumn: 18 + data: { group: "(bc)" } }] }, { @@ -89,30 +86,25 @@ ruleTester.run("prefer-named-capture-group", rule, { type: "Literal", data: { group: "([0-9]{4})" }, line: 1, - column: 2, - endColumn: 12 + column: 1, + endColumn: 21 }, { messageId: "required", type: "Literal", data: { group: "(\\w{5})" }, line: 1, - column: 13, - endColumn: 20 + column: 1, + endColumn: 21 } ] }, - - // For computed, multiline and strings with escape sequences, report the whole arguments[0] location. { code: "new RegExp('(' + 'a)')", errors: [{ messageId: "required", type: "NewExpression", - data: { group: "(a)" }, - line: 1, - column: 12, - endColumn: 22 + data: { group: "(a)" } }] }, { @@ -120,10 +112,7 @@ ruleTester.run("prefer-named-capture-group", rule, { errors: [{ messageId: "required", type: "NewExpression", - data: { group: "(bc)" }, - line: 1, - column: 12, - endColumn: 26 + data: { group: "(bc)" } }] }, { @@ -131,10 +120,7 @@ ruleTester.run("prefer-named-capture-group", rule, { errors: [{ messageId: "required", type: "CallExpression", - data: { group: "(a)" }, - line: 1, - column: 8, - endColumn: 16 + data: { group: "(a)" } }] }, { @@ -142,10 +128,7 @@ ruleTester.run("prefer-named-capture-group", rule, { errors: [{ messageId: "required", type: "CallExpression", - data: { group: "(ab)" }, - line: 1, - column: 9, - endColumn: 20 + data: { group: "(ab)" } }] }, { @@ -153,10 +136,7 @@ ruleTester.run("prefer-named-capture-group", rule, { errors: [{ messageId: "required", type: "NewExpression", - data: { group: "(ab)" }, - line: 1, - column: 12, - endColumn: 23 + data: { group: "(ab)" } }] }, { @@ -166,9 +146,9 @@ ruleTester.run("prefer-named-capture-group", rule, { type: "NewExpression", data: { group: "(a)" }, line: 1, - column: 12, + column: 1, endLine: 2, - endColumn: 2 + endColumn: 3 }] }, { @@ -176,11 +156,7 @@ ruleTester.run("prefer-named-capture-group", rule, { errors: [{ messageId: "required", type: "CallExpression", - data: { group: "(b\nc)" }, - line: 1, - column: 8, - endLine: 2, - endColumn: 5 + data: { group: "(b\nc)" } }] }, { @@ -188,10 +164,7 @@ ruleTester.run("prefer-named-capture-group", rule, { errors: [{ messageId: "required", type: "NewExpression", - data: { group: "(b)" }, - line: 1, - column: 12, - endColumn: 20 + data: { group: "(b)" } }] }, { @@ -199,10 +172,7 @@ ruleTester.run("prefer-named-capture-group", rule, { errors: [{ messageId: "required", type: "CallExpression", - data: { group: "(a)" }, - line: 1, - column: 8, - endColumn: 16 + data: { group: "(a)" } }] }, { @@ -210,10 +180,7 @@ ruleTester.run("prefer-named-capture-group", rule, { errors: [{ messageId: "required", type: "CallExpression", - data: { group: "(b)" }, - line: 1, - column: 8, - endColumn: 15 + data: { group: "(b)" } }] } ]