diff --git a/.changeset/swift-guests-trade.md b/.changeset/swift-guests-trade.md new file mode 100644 index 000000000..91c5c8535 --- /dev/null +++ b/.changeset/swift-guests-trade.md @@ -0,0 +1,5 @@ +--- +"eslint-plugin-regexp": minor +--- + +Add suggestions for `regexp/no-empty-alternative` diff --git a/README.md b/README.md index 561ec723e..92ff1f6db 100644 --- a/README.md +++ b/README.md @@ -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 | ✅ | | | | diff --git a/docs/rules/index.md b/docs/rules/index.md index 31e251aac..6c37ceb52 100644 --- a/docs/rules/index.md +++ b/docs/rules/index.md @@ -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 | ✅ | | | | diff --git a/docs/rules/no-empty-alternative.md b/docs/rules/no-empty-alternative.md index f355e367b..eb4b10c8b 100644 --- a/docs/rules/no-empty-alternative.md +++ b/docs/rules/no-empty-alternative.md @@ -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). + > disallow alternatives without elements @@ -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?`. + ```js diff --git a/lib/rules/no-empty-alternative.ts b/lib/rules/no-empty-alternative.ts index 008b56f08..67c794f63 100644 --- a/lib/rules/no-empty-alternative.ts +++ b/lib/rules/no-empty-alternative.ts @@ -1,5 +1,6 @@ import type { RegExpVisitor } from "@eslint-community/regexpp/visitor" import type { + Alternative, CapturingGroup, Group, Pattern, @@ -7,6 +8,49 @@ import type { 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: { @@ -16,8 +60,10 @@ 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", }, @@ -25,6 +71,7 @@ export default createRule("no-empty-alternative", { function createVisitor({ node, getRegexpLocation, + fixReplaceNode, }: RegExpContext): RegExpVisitor.Handlers { function verifyAlternatives( regexpNode: CapturingGroup | Group | Pattern, @@ -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, @@ -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 diff --git a/tests/lib/rules/no-empty-alternative.ts b/tests/lib/rules/no-empty-alternative.ts index b8397022a..010d8e7c7 100644 --- a/tests/lib/rules/no-empty-alternative.ts +++ b/tests/lib/rules/no-empty-alternative.ts @@ -15,9 +15,9 @@ 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: `/(?:||||)??/` }], }, ], }, @@ -25,9 +25,9 @@ tester.run("no-empty-alternative", rule as any, { code: `/(a+|b+|)/`, errors: [ { - message: "No empty alternatives. Use quantifiers instead.", - line: 1, + messageId: "empty", column: 8, + suggestions: [{ output: `/((?:a+|b+)?)/` }], }, ], }, @@ -35,9 +35,36 @@ tester.run("no-empty-alternative", rule as any, { code: String.raw`/(?:\|\|||\|)/`, errors: [ { - message: "No empty alternatives. Use quantifiers instead.", - line: 1, + messageId: "empty", column: 10, + suggestions: [], + }, + ], + }, + { + code: String.raw`/(?a|b|)/`, + errors: [ + { + messageId: "empty", + suggestions: [{ output: String.raw`/(?(?: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/` }], }, ], },