Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add regexp/no-invalid-regexp rule #288

Merged
merged 3 commits into from
Aug 10, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 | |
Expand Down
1 change: 1 addition & 0 deletions docs/rules/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 | |
Expand Down
64 changes: 64 additions & 0 deletions docs/rules/no-invalid-regexp.md
Original file line number Diff line number Diff line change
@@ -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)
64 changes: 64 additions & 0 deletions lib/rules/no-invalid-regexp.ts
Original file line number Diff line number Diff line change
@@ -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 })
},
})
2 changes: 2 additions & 0 deletions lib/utils/rules.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -80,6 +81,7 @@ export const rules = [
noEmptyGroup,
noEmptyLookaroundsAssertion,
noEscapeBackspace,
noInvalidRegexp,
noInvisibleCharacter,
noLazyEnds,
noLegacyFeatures,
Expand Down
48 changes: 48 additions & 0 deletions tests/lib/rules/no-invalid-regexp.ts
Original file line number Diff line number Diff line change
@@ -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,
},
],
},
],
})