Repository: ota-meshi/eslint-plugin-regexp (ts)

Issue:
`regexp/no-invalid-regexp` for invalid regexes
**Motivation**
While developing #282, I noticed that [ESLint's `no-invalid-regexp`](https://eslint.org/docs/rules/no-invalid-regexp) only validates expression of the form `RegExp(<string literal>)`. It doesn't support the concatenation of string literals and all the fancy stuff #282 can do.

E.g. ESLint's `no-invalid-regexp` will detect `RegExp("([")` but it can't detect `RegExp("([" + "")`. We can detect this, so why not report it?

**Description**
Add a new rule `regexp/no-invalid-regexp` that behaves like ESLint `no-invalid-regexp` but uses `PatternSource` under the hood.

The best way to implement this might be to extend the `defineRegexpVisitor` to allow a new visitor type that doesn't require a parsed RegExpp AST.

**Examples**

<!-- Add a few examples of regexes that the rule does and does not report. -->

```js
/* ✓ GOOD */
var foo = RegExp("foo")
var foo = RegExp("([" + "])")

/* ✗ BAD */
var foo = RegExp("([")
var foo = new RegExp("([" + "\\" + "])", "i")
```


Pull request description:
Add `regexp/no-invalid-regexp` rule
 This fixes #283.

This is a branch of #287.

---

The implementation of the rule is quite simple. The only interesting part is the report location. While not documented, regexpp actually tells us where the syntax error occurs. The only problem is that this position isn't exact. It's off by one at times (see code comments). That being said, the error reporting is quite good.

![image](https://user-images.githubusercontent.com/20878432/128925982-ec46875d-7f38-4a60-a155-32264939404d.png)


Patch:
diff --git a/README.md b/README.md
index 68de45a..051e65a 100644
--- a/README.md
+++ b/README.md
@@ -111,6 +111,7 @@ The rules with the following star :star: are included in the `plugin:regexp/reco
 | [regexp/no-empty-group](https://ota-meshi.github.io/eslint-plugin-regexp/rules/no-empty-group.html) | disallow empty group | :star: |
 | [regexp/no-empty-lookarounds-assertion](https://ota-meshi.github.io/eslint-plugin-regexp/rules/no-empty-lookarounds-assertion.html) | disallow empty lookahead assertion or empty lookbehind assertion | :star: |
 | [regexp/no-escape-backspace](https://ota-meshi.github.io/eslint-plugin-regexp/rules/no-escape-backspace.html) | disallow escape backspace (`[\b]`) | :star: |
+| [regexp/no-invalid-regexp](https://ota-meshi.github.io/eslint-plugin-regexp/rules/no-invalid-regexp.html) | disallow invalid regular expression strings in `RegExp` constructors |  |
 | [regexp/no-lazy-ends](https://ota-meshi.github.io/eslint-plugin-regexp/rules/no-lazy-ends.html) | disallow lazy quantifiers at the end of an expression |  |
 | [regexp/no-optional-assertion](https://ota-meshi.github.io/eslint-plugin-regexp/rules/no-optional-assertion.html) | disallow optional assertions |  |
 | [regexp/no-potentially-useless-backreference](https://ota-meshi.github.io/eslint-plugin-regexp/rules/no-potentially-useless-backreference.html) | disallow backreferences that reference a group that might not be matched |  |
diff --git a/docs/rules/README.md b/docs/rules/README.md
index e5e8402..fe2790c 100644
--- a/docs/rules/README.md
+++ b/docs/rules/README.md
@@ -20,6 +20,7 @@ The rules with the following star :star: are included in the `plugin:regexp/reco
 | [regexp/no-empty-group](./no-empty-group.md) | disallow empty group | :star: |
 | [regexp/no-empty-lookarounds-assertion](./no-empty-lookarounds-assertion.md) | disallow empty lookahead assertion or empty lookbehind assertion | :star: |
 | [regexp/no-escape-backspace](./no-escape-backspace.md) | disallow escape backspace (`[\b]`) | :star: |
+| [regexp/no-invalid-regexp](./no-invalid-regexp.md) | disallow invalid regular expression strings in `RegExp` constructors |  |
 | [regexp/no-lazy-ends](./no-lazy-ends.md) | disallow lazy quantifiers at the end of an expression |  |
 | [regexp/no-optional-assertion](./no-optional-assertion.md) | disallow optional assertions |  |
 | [regexp/no-potentially-useless-backreference](./no-potentially-useless-backreference.md) | disallow backreferences that reference a group that might not be matched |  |
diff --git a/docs/rules/no-invalid-regexp.md b/docs/rules/no-invalid-regexp.md
new file mode 100644
index 0000000..a5b588b
--- /dev/null
+++ b/docs/rules/no-invalid-regexp.md
@@ -0,0 +1,64 @@
+---
+pageClass: "rule-details"
+sidebarDepth: 0
+title: "regexp/no-invalid-regexp"
+description: "disallow invalid regular expression strings in `RegExp` constructors"
+---
+# regexp/no-invalid-regexp
+
+> disallow invalid regular expression strings in `RegExp` constructors
+
+- :exclamation: <badge text="This rule has not been released yet." vertical="middle" type="error"> ***This rule has not been released yet.*** </badge>
+
+## :book: Rule Details
+
+This rule reports invalid regular expression patterns given to `RegExp` constructors.
+
+<eslint-code-block>
+
+```js
+/* eslint regexp/no-invalid-regexp: "error" */
+
+/* ✓ GOOD */
+RegExp('foo')
+RegExp('[a' + ']')
+
+/* ✗ BAD */
+RegExp('\\')
+RegExp('[a-Z]*')
+RegExp('\\p{Foo}', 'u')
+
+const space = '\\s*'
+RegExp('=' + space + '+(\\w+)', 'u')
+```
+
+</eslint-code-block>
+
+### Differences to ESLint's `no-invalid-regexp` rule
+
+This rule is almost functionally equivalent to ESLint's [no-invalid-regexp] rule. The only difference is that this rule doesn't valid flags (see [no-non-standard-flag](./no-non-standard-flag.html)).
+
+There are two reasons we provide this rule:
+
+1. Better error reporting.
+
+    Instead of reporting the whole invalid string, this rule will try to report the exact position of the syntax error.
+
+2. Better support for complex constructor calls.
+
+    ESLint's rule only validates `RegExp` constructors called with simple string literals. This rule also supports operations (e.g. string concatenation) and variables to some degree.
+
+## :wrench: Options
+
+Nothing.
+
+## :books: Further reading
+
+- [no-invalid-regexp]
+
+[no-invalid-regexp]: https://eslint.org/docs/rules/no-invalid-regexp
+
+## :mag: Implementation
+
+- [Rule source](https://github.com/ota-meshi/eslint-plugin-regexp/blob/master/lib/rules/no-invalid-regexp.ts)
+- [Test source](https://github.com/ota-meshi/eslint-plugin-regexp/blob/master/tests/lib/rules/no-invalid-regexp.ts)
diff --git a/lib/rules/no-invalid-regexp.ts b/lib/rules/no-invalid-regexp.ts
new file mode 100644
index 0000000..2820b0c
--- /dev/null
+++ b/lib/rules/no-invalid-regexp.ts
@@ -0,0 +1,64 @@
+import type { RegExpContextForInvalid } from "../utils"
+import { createRule, defineRegexpVisitor } from "../utils"
+
+/** Returns the position of the error */
+function getErrorIndex(error: SyntaxError): number | null {
+    // eslint-disable-next-line @typescript-eslint/no-explicit-any -- x
+    const index = (error as any).index
+    if (typeof index === "number") {
+        return index
+    }
+    return null
+}
+
+export default createRule("no-invalid-regexp", {
+    meta: {
+        docs: {
+            description:
+                "disallow invalid regular expression strings in `RegExp` constructors",
+            category: "Possible Errors",
+            // TODO Switch to recommended in the major version.
+            // TODO When setting `recommended: true`, do not forget to disable ESLint's no-invalid-regexp in recommended.ts
+            // recommended: true,
+            recommended: false,
+        },
+        schema: [],
+        messages: {
+            error: "{{message}}",
+        },
+        type: "problem",
+    },
+    create(context) {
+        /** Visit invalid regexes */
+        function visitInvalid(regexpContext: RegExpContextForInvalid): void {
+            const { node, error, patternSource } = regexpContext
+
+            let loc = undefined
+
+            const index = getErrorIndex(error)
+            if (
+                index !== null &&
+                index >= 0 &&
+                index <= patternSource.value.length
+            ) {
+                // The error index regexpp reports is a little weird.
+                // It's either spot on or one character to the right.
+                // Since we can't know which index is correct, we will report
+                // both positions.
+                loc = patternSource.getAstLocation({
+                    start: Math.max(index - 1, 0),
+                    end: Math.min(index + 1, patternSource.value.length),
+                })
+            }
+
+            context.report({
+                node,
+                loc: loc ?? undefined,
+                messageId: "error",
+                data: { message: error.message },
+            })
+        }
+
+        return defineRegexpVisitor(context, { visitInvalid })
+    },
+})
diff --git a/lib/utils/rules.ts b/lib/utils/rules.ts
index 3e5ed9b..f391fcd 100644
--- a/lib/utils/rules.ts
+++ b/lib/utils/rules.ts
@@ -13,6 +13,7 @@ import noEmptyCapturingGroup from "../rules/no-empty-capturing-group"
 import noEmptyGroup from "../rules/no-empty-group"
 import noEmptyLookaroundsAssertion from "../rules/no-empty-lookarounds-assertion"
 import noEscapeBackspace from "../rules/no-escape-backspace"
+import noInvalidRegexp from "../rules/no-invalid-regexp"
 import noInvisibleCharacter from "../rules/no-invisible-character"
 import noLazyEnds from "../rules/no-lazy-ends"
 import noLegacyFeatures from "../rules/no-legacy-features"
@@ -80,6 +81,7 @@ export const rules = [
     noEmptyGroup,
     noEmptyLookaroundsAssertion,
     noEscapeBackspace,
+    noInvalidRegexp,
     noInvisibleCharacter,
     noLazyEnds,
     noLegacyFeatures,


Test patch:
diff --git a/tests/lib/rules/no-invalid-regexp.ts b/tests/lib/rules/no-invalid-regexp.ts
new file mode 100644
index 0000000..7b545b3
--- /dev/null
+++ b/tests/lib/rules/no-invalid-regexp.ts
@@ -0,0 +1,48 @@
+import { RuleTester } from "eslint"
+import rule from "../../../lib/rules/no-invalid-regexp"
+
+const tester = new RuleTester({
+    parserOptions: {
+        ecmaVersion: 2020,
+        sourceType: "module",
+    },
+})
+
+tester.run("no-invalid-regexp", rule as any, {
+    valid: [`/regexp/`, `RegExp("(" + ")")`],
+    invalid: [
+        {
+            code: `RegExp("(")`,
+            errors: [
+                {
+                    message:
+                        "Invalid regular expression: /(/: Unterminated group",
+                    column: 9,
+                    endColumn: 10,
+                },
+            ],
+        },
+        {
+            code: `RegExp("(" + "(")`,
+            errors: [
+                {
+                    message:
+                        "Invalid regular expression: /((/: Unterminated group",
+                    column: 15,
+                    endColumn: 16,
+                },
+            ],
+        },
+        {
+            code: `RegExp("[a-Z] some valid stuff")`,
+            errors: [
+                {
+                    message:
+                        "Invalid regular expression: /[a-Z] some valid stuff/: Range out of order in character class",
+                    column: 12,
+                    endColumn: 14,
+                },
+            ],
+        },
+    ],
+})