Skip to content

Commit f719c81

Browse files
RunDevelopmentRunDevelopment
authored
Add suggestions for regexp/no-empty-alternative (#621)
* Add suggestions for `regexp/no-empty-alternative` * Updated generated files * Create swift-guests-trade.md
1 parent fe34c05 commit f719c81

File tree

6 files changed

+108
-11
lines changed

6 files changed

+108
-11
lines changed

‎.changeset/swift-guests-trade.md

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"eslint-plugin-regexp": minor
3+
---
4+
5+
Add suggestions for `regexp/no-empty-alternative`

‎README.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ The `plugin:regexp/all` config enables all rules. It's meant for testing, not fo
112112
| [no-contradiction-with-assertion](https://ota-meshi.github.io/eslint-plugin-regexp/rules/no-contradiction-with-assertion.html) | disallow elements that contradict assertions | ✅ | | | 💡 |
113113
| [no-control-character](https://ota-meshi.github.io/eslint-plugin-regexp/rules/no-control-character.html) | disallow control characters | | | | 💡 |
114114
| [no-dupe-disjunctions](https://ota-meshi.github.io/eslint-plugin-regexp/rules/no-dupe-disjunctions.html) | disallow duplicate disjunctions | ✅ | | | 💡 |
115-
| [no-empty-alternative](https://ota-meshi.github.io/eslint-plugin-regexp/rules/no-empty-alternative.html) | disallow alternatives without elements | | ✅ | | |
115+
| [no-empty-alternative](https://ota-meshi.github.io/eslint-plugin-regexp/rules/no-empty-alternative.html) | disallow alternatives without elements | | ✅ | | 💡 |
116116
| [no-empty-capturing-group](https://ota-meshi.github.io/eslint-plugin-regexp/rules/no-empty-capturing-group.html) | disallow capturing group that captures empty. | ✅ | | | |
117117
| [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 | ✅ | | | |
118118
| [no-empty-group](https://ota-meshi.github.io/eslint-plugin-regexp/rules/no-empty-group.html) | disallow empty group | ✅ | | | |

‎docs/rules/index.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ sidebarDepth: 0
1919
| [no-contradiction-with-assertion](no-contradiction-with-assertion.md) | disallow elements that contradict assertions | ✅ | | | 💡 |
2020
| [no-control-character](no-control-character.md) | disallow control characters | | | | 💡 |
2121
| [no-dupe-disjunctions](no-dupe-disjunctions.md) | disallow duplicate disjunctions | ✅ | | | 💡 |
22-
| [no-empty-alternative](no-empty-alternative.md) | disallow alternatives without elements | | ✅ | | |
22+
| [no-empty-alternative](no-empty-alternative.md) | disallow alternatives without elements | | ✅ | | 💡 |
2323
| [no-empty-capturing-group](no-empty-capturing-group.md) | disallow capturing group that captures empty. | ✅ | | | |
2424
| [no-empty-character-class](no-empty-character-class.md) | disallow character classes that match no characters | ✅ | | | |
2525
| [no-empty-group](no-empty-group.md) | disallow empty group | ✅ | | | |

‎docs/rules/no-empty-alternative.md

+5
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ since: "v0.8.0"
99

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

12+
💡 This rule is manually fixable by [editor suggestions](https://eslint.org/docs/developer-guide/working-with-rules#providing-suggestions).
13+
1214
<!-- end auto-generated rule header -->
1315

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

24+
If the empty alternative is supposed to match the empty string, then use a
25+
quantifier instead. For example, `a|` should be written as `a?`.
26+
2227
<eslint-code-block>
2328

2429
```js

‎lib/rules/no-empty-alternative.ts

+63-3
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,56 @@
11
import type { RegExpVisitor } from "@eslint-community/regexpp/visitor"
22
import type {
3+
Alternative,
34
CapturingGroup,
45
Group,
56
Pattern,
67
} from "@eslint-community/regexpp/ast"
78
import type { RegExpContext } from "../utils"
89
import { createRule, defineRegexpVisitor } from "../utils"
910

11+
/**
12+
* Returns the source before and after the alternatives of the given capturing group.
13+
*/
14+
function getCapturingGroupOuterSource(node: CapturingGroup): [string, string] {
15+
const first = node.alternatives[0]
16+
const last = node.alternatives[node.alternatives.length - 1]
17+
18+
const innerStart = first.start - node.start
19+
const innerEnd = last.end - node.start
20+
return [node.raw.slice(0, innerStart), node.raw.slice(innerEnd)]
21+
}
22+
23+
function getFixedNode(
24+
regexpNode: CapturingGroup | Group | Pattern,
25+
alt: Alternative,
26+
): string | null {
27+
let quant
28+
if (regexpNode.alternatives.at(0) === alt) {
29+
quant = "??"
30+
} else if (regexpNode.alternatives.at(-1) === alt) {
31+
quant = "?"
32+
} else {
33+
return null
34+
}
35+
36+
const innerAlternatives = regexpNode.alternatives
37+
.filter((a) => a !== alt)
38+
.map((a) => a.raw)
39+
.join("|")
40+
41+
let replacement = `(?:${innerAlternatives})${quant}`
42+
if (regexpNode.type === "CapturingGroup") {
43+
const [before, after] = getCapturingGroupOuterSource(regexpNode)
44+
replacement = `${before}${replacement}${after}`
45+
} else if (regexpNode.parent?.type === "Quantifier") {
46+
// if the parent is a quantifier, then we need to wrap the replacement
47+
// in a non-capturing group
48+
replacement = `(?:${replacement})`
49+
}
50+
51+
return replacement
52+
}
53+
1054
export default createRule("no-empty-alternative", {
1155
meta: {
1256
docs: {
@@ -16,15 +60,18 @@ export default createRule("no-empty-alternative", {
1660
default: "warn",
1761
},
1862
schema: [],
63+
hasSuggestions: true,
1964
messages: {
20-
empty: "No empty alternatives. Use quantifiers instead.",
65+
empty: "This empty alternative might be a mistake. If not, use a quantifier instead.",
66+
suggest: "Use a quantifier instead.",
2167
},
2268
type: "problem",
2369
},
2470
create(context) {
2571
function createVisitor({
2672
node,
2773
getRegexpLocation,
74+
fixReplaceNode,
2875
}: RegExpContext): RegExpVisitor.Handlers {
2976
function verifyAlternatives(
3077
regexpNode: CapturingGroup | Group | Pattern,
@@ -34,12 +81,12 @@ export default createRule("no-empty-alternative", {
3481
// the parser and one alternative is already handled by other rules.
3582
for (let i = 0; i < regexpNode.alternatives.length; i++) {
3683
const alt = regexpNode.alternatives[i]
37-
const last = i === regexpNode.alternatives.length - 1
84+
const isLast = i === regexpNode.alternatives.length - 1
3885
if (alt.elements.length === 0) {
3986
// Since empty alternative have a width of 0, it's hard to underline their location.
4087
// So we will report the location of the `|` that causes the empty alternative.
4188
const index = alt.start
42-
const loc = last
89+
const loc = isLast
4390
? getRegexpLocation({
4491
start: index - 1,
4592
end: index,
@@ -49,10 +96,23 @@ export default createRule("no-empty-alternative", {
4996
end: index + 1,
5097
})
5198

99+
const fixed = getFixedNode(regexpNode, alt)
100+
52101
context.report({
53102
node,
54103
loc,
55104
messageId: "empty",
105+
suggest: fixed
106+
? [
107+
{
108+
messageId: "suggest",
109+
fix: fixReplaceNode(
110+
regexpNode,
111+
fixed,
112+
),
113+
},
114+
]
115+
: undefined,
56116
})
57117
// don't report the same node multiple times
58118
return

‎tests/lib/rules/no-empty-alternative.ts

+33-6
Original file line numberDiff line numberDiff line change
@@ -15,29 +15,56 @@ tester.run("no-empty-alternative", rule as any, {
1515
code: `/|||||/`,
1616
errors: [
1717
{
18-
message: "No empty alternatives. Use quantifiers instead.",
19-
line: 1,
18+
messageId: "empty",
2019
column: 2,
20+
suggestions: [{ output: `/(?:||||)??/` }],
2121
},
2222
],
2323
},
2424
{
2525
code: `/(a+|b+|)/`,
2626
errors: [
2727
{
28-
message: "No empty alternatives. Use quantifiers instead.",
29-
line: 1,
28+
messageId: "empty",
3029
column: 8,
30+
suggestions: [{ output: `/((?:a+|b+)?)/` }],
3131
},
3232
],
3333
},
3434
{
3535
code: String.raw`/(?:\|\|||\|)/`,
3636
errors: [
3737
{
38-
message: "No empty alternatives. Use quantifiers instead.",
39-
line: 1,
38+
messageId: "empty",
4039
column: 10,
40+
suggestions: [],
41+
},
42+
],
43+
},
44+
{
45+
code: String.raw`/(?<name>a|b|)/`,
46+
errors: [
47+
{
48+
messageId: "empty",
49+
suggestions: [{ output: String.raw`/(?<name>(?:a|b)?)/` }],
50+
},
51+
],
52+
},
53+
{
54+
code: String.raw`/(?:a|b|)f/`,
55+
errors: [
56+
{
57+
messageId: "empty",
58+
suggestions: [{ output: String.raw`/(?:a|b)?f/` }],
59+
},
60+
],
61+
},
62+
{
63+
code: String.raw`/(?:a|b|)+f/`,
64+
errors: [
65+
{
66+
messageId: "empty",
67+
suggestions: [{ output: String.raw`/(?:(?:a|b)?)+f/` }],
4168
},
4269
],
4370
},

0 commit comments

Comments
 (0)