prefer-string-replace-all: Don't crash on invalid pattern (#2011)

sindresorhus · Dec 12, 2022 · 3bbe027 · 3bbe027
1 parent 8cd1ded
commit 3bbe027
Show file tree

Hide file tree

Showing 8 changed files with 60 additions and 6 deletions.
diff --git a/rules/better-regex.js b/rules/better-regex.js
@@ -6,8 +6,10 @@ const {newExpressionSelector} = require('./selectors/index.js');
 const {isStringLiteral} = require('./ast/index.js');
 
 const MESSAGE_ID = 'better-regex';
+const MESSAGE_ID_PARSE_ERROR = 'better-regex/parse-error';
 const messages = {
 	[MESSAGE_ID]: '{{original}} can be optimized to {{optimized}}.',
+	[MESSAGE_ID_PARSE_ERROR]: 'Problem parsing {{original}}: {{error}}',
 };
 
 const newRegExp = newExpressionSelector({name: 'RegExp', minimumArguments: 1});
@@ -39,11 +41,11 @@ const create = context => {
 			} catch (error) {
 				return {
 					node,
+					messageId: MESSAGE_ID_PARSE_ERROR,
 					data: {
 						original,
 						error: error.message,
 					},
-					message: 'Problem parsing {{original}}: {{error}}',
 				};
 			}
 

diff --git a/rules/prefer-string-replace-all.js b/rules/prefer-string-replace-all.js
@@ -27,11 +27,17 @@ function getPatternReplacement(node) {
 		return;
 	}
 
-	const tree = parseRegExp(pattern, flags, {
-		unicodePropertyEscape: true,
-		namedGroups: true,
-		lookbehind: true,
-	});
+	let tree;
+
+	try {
+		tree = parseRegExp(pattern, flags, {
+			unicodePropertyEscape: true,
+			namedGroups: true,
+			lookbehind: true,
+		});
+	} catch {
+		return;
+	}
 
 	const parts = tree.type === 'alternative' ? tree.body : [tree];
 	if (parts.some(part => part.type !== 'value')) {

diff --git a/test/better-regex.mjs b/test/better-regex.mjs
@@ -323,3 +323,11 @@ test({
 		},
 	],
 });
+
+test.snapshot({
+	valid: [],
+	invalid: [
+		// Invalid RegExp
+		'/(?!a)+/g',
+	],
+});
diff --git a/test/prefer-string-replace-all.mjs b/test/prefer-string-replace-all.mjs
@@ -103,5 +103,8 @@ test.snapshot({
 		'foo.replaceAll(/a]/g, _)',
 		'foo.replaceAll(/\\r\\n\\u{1f600}/gu, _)',
 		`foo.replaceAll(/a${' very'.repeat(30)} long string/g, _)`,
+
+		// Invalid RegExp #2010
+		'foo.replace(/(?!a)+/g, "")',
 	],
 });
diff --git a/test/snapshots/better-regex.mjs.md b/test/snapshots/better-regex.mjs.md
@@ -0,0 +1,19 @@
+# Snapshot report for `test/better-regex.mjs`
+
+The actual snapshot is saved in `better-regex.mjs.snap`.
+
+Generated by [AVA](https://avajs.dev).
+
+## Invalid #1
+      1 | /(?!a)+/g
+
+> Error 1/1
+
+    `␊
+    > 1 | /(?!a)+/g␊
+        | ^^^^^^^^^ Problem parsing /(?!a)+/g: ␊
+    ␊
+    /(?!a)+/g␊
+          ^␊
+    Unexpected token: "+" at 1:6.␊
+    `
diff --git a/test/snapshots/better-regex.mjs.snap b/test/snapshots/better-regex.mjs.snap
diff --git a/test/snapshots/prefer-string-replace-all.mjs.md b/test/snapshots/prefer-string-replace-all.mjs.md
@@ -613,3 +613,19 @@ Generated by [AVA](https://avajs.dev).
     > 1 | foo.replaceAll(/a very very very very very very very very very very very very very very very very very very very very very very very very very very very very very very long string/g, _)␊
         |                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ This pattern can be replaced with a string literal.␊
     `
+
+## Invalid #38
+      1 | foo.replace(/(?!a)+/g, "")
+
+> Output
+
+    `␊
+      1 | foo.replaceAll(/(?!a)+/g, "")␊
+    `
+
+> Error 1/1
+
+    `␊
+    > 1 | foo.replace(/(?!a)+/g, "")␊
+        |     ^^^^^^^ Prefer \`String#replaceAll()\` over \`String#replace()\`.␊
+    `
diff --git a/test/snapshots/prefer-string-replace-all.mjs.snap b/test/snapshots/prefer-string-replace-all.mjs.snap