Skip to content

Add suggestions for regexp/no-empty-alternative #621

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

Merged
merged 4 commits into from
Oct 1, 2023
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
5 changes: 5 additions & 0 deletions .changeset/swift-guests-trade.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"eslint-plugin-regexp": minor
---

Add suggestions for `regexp/no-empty-alternative`
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ The `plugin:regexp/all` config enables all rules. It's meant for testing, not fo
| [no-contradiction-with-assertion](https://ota-meshi.github.io/eslint-plugin-regexp/rules/no-contradiction-with-assertion.html) | disallow elements that contradict assertions | ✅ | | | 💡 |
| [no-control-character](https://ota-meshi.github.io/eslint-plugin-regexp/rules/no-control-character.html) | disallow control characters | | | | 💡 |
| [no-dupe-disjunctions](https://ota-meshi.github.io/eslint-plugin-regexp/rules/no-dupe-disjunctions.html) | disallow duplicate disjunctions | ✅ | | | 💡 |
| [no-empty-alternative](https://ota-meshi.github.io/eslint-plugin-regexp/rules/no-empty-alternative.html) | disallow alternatives without elements | | ✅ | | |
| [no-empty-alternative](https://ota-meshi.github.io/eslint-plugin-regexp/rules/no-empty-alternative.html) | disallow alternatives without elements | | ✅ | | 💡 |
| [no-empty-capturing-group](https://ota-meshi.github.io/eslint-plugin-regexp/rules/no-empty-capturing-group.html) | disallow capturing group that captures empty. | ✅ | | | |
| [no-empty-character-class](https://ota-meshi.github.io/eslint-plugin-regexp/rules/no-empty-character-class.html) | disallow character classes that match no characters | ✅ | | | |
| [no-empty-group](https://ota-meshi.github.io/eslint-plugin-regexp/rules/no-empty-group.html) | disallow empty group | ✅ | | | |
Expand Down
2 changes: 1 addition & 1 deletion docs/rules/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ sidebarDepth: 0
| [no-contradiction-with-assertion](no-contradiction-with-assertion.md) | disallow elements that contradict assertions | ✅ | | | 💡 |
| [no-control-character](no-control-character.md) | disallow control characters | | | | 💡 |
| [no-dupe-disjunctions](no-dupe-disjunctions.md) | disallow duplicate disjunctions | ✅ | | | 💡 |
| [no-empty-alternative](no-empty-alternative.md) | disallow alternatives without elements | | ✅ | | |
| [no-empty-alternative](no-empty-alternative.md) | disallow alternatives without elements | | ✅ | | 💡 |
| [no-empty-capturing-group](no-empty-capturing-group.md) | disallow capturing group that captures empty. | ✅ | | | |
| [no-empty-character-class](no-empty-character-class.md) | disallow character classes that match no characters | ✅ | | | |
| [no-empty-group](no-empty-group.md) | disallow empty group | ✅ | | | |
Expand Down
5 changes: 5 additions & 0 deletions docs/rules/no-empty-alternative.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ since: "v0.8.0"

⚠️ This rule _warns_ in the ✅ `plugin:regexp/recommended` config.

💡 This rule is manually fixable by [editor suggestions](https://eslint.org/docs/developer-guide/working-with-rules#providing-suggestions).

<!-- end auto-generated rule header -->

> disallow alternatives without elements
Expand All @@ -19,6 +21,9 @@ While (re-)writing long regular expressions, it can happen that one forgets to
remove the `|` character of a former alternative. This rule tries to point out
these potential mistakes by reporting all empty alternatives.

If the empty alternative is supposed to match the empty string, then use a
quantifier instead. For example, `a|` should be written as `a?`.

<eslint-code-block>

```js
Expand Down
66 changes: 63 additions & 3 deletions lib/rules/no-empty-alternative.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,56 @@
import type { RegExpVisitor } from "@eslint-community/regexpp/visitor"
import type {
Alternative,
CapturingGroup,
Group,
Pattern,
} from "@eslint-community/regexpp/ast"
import type { RegExpContext } from "../utils"
import { createRule, defineRegexpVisitor } from "../utils"

/**
* Returns the source before and after the alternatives of the given capturing group.
*/
function getCapturingGroupOuterSource(node: CapturingGroup): [string, string] {
const first = node.alternatives[0]
const last = node.alternatives[node.alternatives.length - 1]

const innerStart = first.start - node.start
const innerEnd = last.end - node.start
return [node.raw.slice(0, innerStart), node.raw.slice(innerEnd)]
}

function getFixedNode(
regexpNode: CapturingGroup | Group | Pattern,
alt: Alternative,
): string | null {
let quant
if (regexpNode.alternatives.at(0) === alt) {
quant = "??"
} else if (regexpNode.alternatives.at(-1) === alt) {
quant = "?"
} else {
return null
}

const innerAlternatives = regexpNode.alternatives
.filter((a) => a !== alt)
.map((a) => a.raw)
.join("|")

let replacement = `(?:${innerAlternatives})${quant}`
if (regexpNode.type === "CapturingGroup") {
const [before, after] = getCapturingGroupOuterSource(regexpNode)
replacement = `${before}${replacement}${after}`
} else if (regexpNode.parent?.type === "Quantifier") {
// if the parent is a quantifier, then we need to wrap the replacement
// in a non-capturing group
replacement = `(?:${replacement})`
}

return replacement
}

export default createRule("no-empty-alternative", {
meta: {
docs: {
Expand All @@ -16,15 +60,18 @@ export default createRule("no-empty-alternative", {
default: "warn",
},
schema: [],
hasSuggestions: true,
messages: {
empty: "No empty alternatives. Use quantifiers instead.",
empty: "This empty alternative might be a mistake. If not, use a quantifier instead.",
suggest: "Use a quantifier instead.",
},
type: "problem",
},
create(context) {
function createVisitor({
node,
getRegexpLocation,
fixReplaceNode,
}: RegExpContext): RegExpVisitor.Handlers {
function verifyAlternatives(
regexpNode: CapturingGroup | Group | Pattern,
Expand All @@ -34,12 +81,12 @@ export default createRule("no-empty-alternative", {
// the parser and one alternative is already handled by other rules.
for (let i = 0; i < regexpNode.alternatives.length; i++) {
const alt = regexpNode.alternatives[i]
const last = i === regexpNode.alternatives.length - 1
const isLast = i === regexpNode.alternatives.length - 1
if (alt.elements.length === 0) {
// Since empty alternative have a width of 0, it's hard to underline their location.
// So we will report the location of the `|` that causes the empty alternative.
const index = alt.start
const loc = last
const loc = isLast
? getRegexpLocation({
start: index - 1,
end: index,
Expand All @@ -49,10 +96,23 @@ export default createRule("no-empty-alternative", {
end: index + 1,
})

const fixed = getFixedNode(regexpNode, alt)

context.report({
node,
loc,
messageId: "empty",
suggest: fixed
? [
{
messageId: "suggest",
fix: fixReplaceNode(
regexpNode,
fixed,
),
},
]
: undefined,
})
// don't report the same node multiple times
return
Expand Down
39 changes: 33 additions & 6 deletions tests/lib/rules/no-empty-alternative.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,29 +15,56 @@ tester.run("no-empty-alternative", rule as any, {
code: `/|||||/`,
errors: [
{
message: "No empty alternatives. Use quantifiers instead.",
line: 1,
messageId: "empty",
column: 2,
suggestions: [{ output: `/(?:||||)??/` }],
},
],
},
{
code: `/(a+|b+|)/`,
errors: [
{
message: "No empty alternatives. Use quantifiers instead.",
line: 1,
messageId: "empty",
column: 8,
suggestions: [{ output: `/((?:a+|b+)?)/` }],
},
],
},
{
code: String.raw`/(?:\|\|||\|)/`,
errors: [
{
message: "No empty alternatives. Use quantifiers instead.",
line: 1,
messageId: "empty",
column: 10,
suggestions: [],
},
],
},
{
code: String.raw`/(?<name>a|b|)/`,
errors: [
{
messageId: "empty",
suggestions: [{ output: String.raw`/(?<name>(?:a|b)?)/` }],
},
],
},
{
code: String.raw`/(?:a|b|)f/`,
errors: [
{
messageId: "empty",
suggestions: [{ output: String.raw`/(?:a|b)?f/` }],
},
],
},
{
code: String.raw`/(?:a|b|)+f/`,
errors: [
{
messageId: "empty",
suggestions: [{ output: String.raw`/(?:(?:a|b)?)+f/` }],
},
],
},
Expand Down