Skip to content

Commit bdf16f8

Browse files
RunDevelopmentota-meshi
RunDevelopment
and
ota-meshi
authored
Add suggestions for regexp/no-dupe-disjunctions (#406)
* Removed usage of deprected isDisjointWith function * Added partial parser * Implemented nested subsets * Added tests for nested subset * Added nested prefix subsets * Added tests for nested prefix subsets * Added missing docs * Improved deduping * Update lib/rules/no-dupe-disjunctions.ts Co-authored-by: Yosuke Ota <otameshiyo23@gmail.com> * Fixed linting error * Fixed and documented optimization * Added failing tests for bug * Fixed incorrect partial NFA construction * Fixed capturing group warning for nested alternatives * Proper message escaping * Improved messages * Don't repeat the loc code * Add suggestions for `regexp/no-dupe-disjunctions` * Implemented review comment Co-authored-by: Yosuke Ota <otameshiyo23@gmail.com>
1 parent 249e29e commit bdf16f8

File tree

4 files changed

+264
-62
lines changed

4 files changed

+264
-62
lines changed

‎lib/rules/no-dupe-characters-character-class.ts‎

Lines changed: 17 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -14,13 +14,12 @@ import {
1414
createRule,
1515
defineRegexpVisitor,
1616
toCharSetSource,
17-
mightCreateNewElement,
17+
fixRemoveCharacterClassElement,
1818
} from "../utils"
1919
import type { CharRange, CharSet } from "refa"
2020
import { JS } from "refa"
2121
import type { ReadonlyFlags } from "regexp-ast-analysis"
2222
import { toCharSet } from "regexp-ast-analysis"
23-
import type { Rule } from "eslint"
2423
import { mentionChar } from "../utils/mention"
2524

2625
interface Grouping {
@@ -106,61 +105,6 @@ function inRange({ min, max }: CharRange, char: number): boolean {
106105
return min <= char && char <= max
107106
}
108107

109-
/**
110-
* Removes the given character class element from its character class.
111-
*/
112-
function fixRemove(
113-
context: RegExpContext,
114-
element: CharacterClassElement,
115-
): Rule.ReportDescriptor["fix"] {
116-
const parent = element.parent
117-
if (parent.type !== "CharacterClass") {
118-
throw new Error("Only call this function for character class elements.")
119-
}
120-
121-
return context.fixReplaceNode(element, () => {
122-
const textBefore = parent.raw.slice(0, element.start - parent.start)
123-
const textAfter = parent.raw.slice(element.end - parent.start)
124-
125-
if (mightCreateNewElement(textBefore, textAfter)) {
126-
return null
127-
}
128-
129-
const elementBefore: CharacterClassElement | undefined =
130-
parent.elements[parent.elements.indexOf(element) - 1]
131-
const elementAfter: CharacterClassElement | undefined =
132-
parent.elements[parent.elements.indexOf(element) + 1]
133-
134-
if (
135-
elementBefore &&
136-
elementAfter &&
137-
elementBefore.type === "Character" &&
138-
elementBefore.raw === "-" &&
139-
elementAfter.type === "Character"
140-
) {
141-
// e.g. [\s0-\s9] -> [\s0-9] is incorrect
142-
return null
143-
}
144-
145-
// add a backslash if ...
146-
if (
147-
// ... the text character is a dash
148-
// e.g. [a\w-b] -> [a\-b], [\w-b] -> [-b], [\s\w-b] -> [\s-b]
149-
(textAfter.startsWith("-") &&
150-
elementBefore &&
151-
elementBefore.type === "Character") ||
152-
// ... the next character is a caret and the caret will then be the
153-
// first character in the character class
154-
// e.g. [a^b] -> [\^b], [ba^] -> [b^]
155-
(textAfter.startsWith("^") && !parent.negate && !elementBefore)
156-
) {
157-
return "\\"
158-
}
159-
160-
return ""
161-
})
162-
}
163-
164108
export default createRule("no-dupe-characters-character-class", {
165109
meta: {
166110
type: "suggestion",
@@ -202,7 +146,10 @@ export default createRule("no-dupe-characters-character-class", {
202146
data: {
203147
duplicate: mentionChar(duplicate),
204148
},
205-
fix: fixRemove(regexpContext, duplicate),
149+
fix: fixRemoveCharacterClassElement(
150+
regexpContext,
151+
duplicate,
152+
),
206153
})
207154
} else {
208155
context.report({
@@ -213,7 +160,10 @@ export default createRule("no-dupe-characters-character-class", {
213160
duplicate: mentionChar(duplicate),
214161
element: mentionChar(element),
215162
},
216-
fix: fixRemove(regexpContext, duplicate),
163+
fix: fixRemoveCharacterClassElement(
164+
regexpContext,
165+
duplicate,
166+
),
217167
})
218168
}
219169
}
@@ -259,7 +209,10 @@ export default createRule("no-dupe-characters-character-class", {
259209
subsetElement: mentionChar(subsetElement),
260210
element: mentionChar(element),
261211
},
262-
fix: fixRemove(regexpContext, subsetElement),
212+
fix: fixRemoveCharacterClassElement(
213+
regexpContext,
214+
subsetElement,
215+
),
263216
})
264217
}
265218

@@ -283,7 +236,10 @@ export default createRule("no-dupe-characters-character-class", {
283236
.map((e) => e.raw)
284237
.join("")}' (${elements.map(mentionChar).join(", ")})`,
285238
},
286-
fix: fixRemove(regexpContext, subsetElement),
239+
fix: fixRemoveCharacterClassElement(
240+
regexpContext,
241+
subsetElement,
242+
),
287243
})
288244
}
289245

‎lib/rules/no-dupe-disjunctions.ts‎

Lines changed: 148 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,12 @@ import type {
99
Quantifier,
1010
} from "regexpp/ast"
1111
import type { RegExpContext } from "../utils"
12-
import { createRule, defineRegexpVisitor } from "../utils"
12+
import {
13+
createRule,
14+
defineRegexpVisitor,
15+
fixRemoveCharacterClassElement,
16+
fixRemoveAlternative,
17+
} from "../utils"
1318
import { isCoveredNode, isEqualNodes } from "../utils/regexp-ast"
1419
import type { Expression, FiniteAutomaton, NoParent, ReadonlyNFA } from "refa"
1520
import {
@@ -35,6 +40,8 @@ import { canReorder } from "../utils/reorder-alternatives"
3540
import { mention, mentionChar } from "../utils/mention"
3641
import type { NestedAlternative } from "../utils/partial-parser"
3742
import { PartialParser } from "../utils/partial-parser"
43+
import type { Rule } from "eslint"
44+
import { getAllowedCharRanges, inRange } from "../utils/char-ranges"
3845

3946
type ParentNode = Group | CapturingGroup | Pattern | LookaroundAssertion
4047

@@ -837,6 +844,34 @@ function mentionNested(nested: NestedAlternative): string {
837844
return mentionChar(nested)
838845
}
839846

847+
/**
848+
* Returns a fix that removes the given alternative.
849+
*/
850+
function fixRemoveNestedAlternative(
851+
context: RegExpContext,
852+
alternative: NestedAlternative,
853+
) {
854+
switch (alternative.type) {
855+
case "Alternative":
856+
return fixRemoveAlternative(context, alternative)
857+
858+
case "Character":
859+
case "CharacterClassRange":
860+
case "CharacterSet": {
861+
if (alternative.parent.type !== "CharacterClass") {
862+
// This isn't supposed to happen. We can't just remove the only
863+
// alternative of its parent
864+
return () => null
865+
}
866+
867+
return fixRemoveCharacterClassElement(context, alternative)
868+
}
869+
870+
default:
871+
throw assertNever(alternative)
872+
}
873+
}
874+
840875
const enum ReportOption {
841876
all = "all",
842877
trivial = "trivial",
@@ -873,6 +908,7 @@ export default createRule("no-dupe-disjunctions", {
873908
category: "Possible Errors",
874909
recommended: true,
875910
},
911+
hasSuggestions: true,
876912
schema: [
877913
{
878914
type: "object",
@@ -905,6 +941,9 @@ export default createRule("no-dupe-disjunctions", {
905941
"Unexpected superset. This alternative is a superset of {{others}}. It might be possible to remove the other alternative(s).{{cap}}{{exp}}",
906942
overlap:
907943
"Unexpected overlap. This alternative overlaps with {{others}}. The overlap is {{expr}}.{{cap}}{{exp}}",
944+
945+
remove: "Remove the {{alternative}} {{type}}.",
946+
replaceRange: "Replace {{range}} with {{replacement}}.",
908947
},
909948
type: "suggestion", // "problem",
910949
},
@@ -917,6 +956,8 @@ export default createRule("no-dupe-disjunctions", {
917956
const report: ReportOption =
918957
context.options[0]?.report ?? ReportOption.trivial
919958

959+
const allowedRanges = getAllowedCharRanges(undefined, context)
960+
920961
/**
921962
* Create visitor
922963
*/
@@ -1104,6 +1145,103 @@ export default createRule("no-dupe-disjunctions", {
11041145
}
11051146
}
11061147

1148+
/** Prints the given character. */
1149+
function printChar(char: number): string {
1150+
if (inRange(allowedRanges, char)) {
1151+
return String.fromCodePoint(char)
1152+
}
1153+
1154+
if (char === 0) return "\\0"
1155+
if (char <= 0xff)
1156+
return `\\x${char.toString(16).padStart(2, "0")}`
1157+
if (char <= 0xffff)
1158+
return `\\u${char.toString(16).padStart(4, "0")}`
1159+
1160+
return `\\u{${char.toString(16)}}`
1161+
}
1162+
1163+
/** Returns suggestions for fixing the given report */
1164+
function getSuggestions(
1165+
result: Result,
1166+
): Rule.SuggestionReportDescriptor[] {
1167+
if (result.type === "Overlap" || result.type === "Superset") {
1168+
// the types of results cannot be trivially fixed by
1169+
// removing an alternative.
1170+
return []
1171+
}
1172+
1173+
const alternative =
1174+
result.type === "NestedSubset" ||
1175+
result.type === "PrefixNestedSubset"
1176+
? result.nested
1177+
: result.alternative
1178+
1179+
const containsCapturingGroup = hasSomeDescendant(
1180+
alternative,
1181+
(d) => d.type === "CapturingGroup",
1182+
)
1183+
if (containsCapturingGroup) {
1184+
// we can't just remove a capturing group
1185+
return []
1186+
}
1187+
1188+
if (
1189+
alternative.type === "Character" &&
1190+
alternative.parent.type === "CharacterClassRange"
1191+
) {
1192+
const range = alternative.parent
1193+
1194+
let replacement
1195+
if (range.min.value + 1 === range.max.value) {
1196+
replacement =
1197+
range.min === alternative
1198+
? range.max.raw
1199+
: range.min.raw
1200+
} else {
1201+
if (range.min === alternative) {
1202+
// replace with {min+1}-{max}
1203+
const min = printChar(range.min.value + 1)
1204+
replacement = `${min}-${range.max.raw}`
1205+
} else {
1206+
// replace with {min}-{max-1}
1207+
const max = printChar(range.max.value - 1)
1208+
replacement = `${range.min.raw}-${max}`
1209+
}
1210+
}
1211+
1212+
return [
1213+
{
1214+
messageId: "replaceRange",
1215+
data: {
1216+
range: mentionChar(range),
1217+
replacement: mention(replacement),
1218+
},
1219+
fix: regexpContext.fixReplaceNode(
1220+
range,
1221+
replacement,
1222+
),
1223+
},
1224+
]
1225+
}
1226+
1227+
return [
1228+
{
1229+
messageId: "remove",
1230+
data: {
1231+
alternative: mentionNested(alternative),
1232+
type:
1233+
alternative.type === "Alternative"
1234+
? "alternative"
1235+
: "element",
1236+
},
1237+
fix: fixRemoveNestedAlternative(
1238+
regexpContext,
1239+
alternative,
1240+
),
1241+
},
1242+
]
1243+
}
1244+
11071245
/** Report the given result. */
11081246
function reportResult(result: Result, { stared }: FilterInfo) {
11091247
let exp
@@ -1136,13 +1274,16 @@ export default createRule("no-dupe-disjunctions", {
11361274
result.others.map((a) => a.raw).join("|"),
11371275
)
11381276

1277+
const suggest = getSuggestions(result)
1278+
11391279
switch (result.type) {
11401280
case "Duplicate":
11411281
context.report({
11421282
node,
11431283
loc,
11441284
messageId: "duplicate",
11451285
data: { exp, cap, others },
1286+
suggest,
11461287
})
11471288
break
11481289

@@ -1152,6 +1293,7 @@ export default createRule("no-dupe-disjunctions", {
11521293
loc,
11531294
messageId: "subset",
11541295
data: { exp, cap, others },
1296+
suggest,
11551297
})
11561298
break
11571299

@@ -1167,6 +1309,7 @@ export default createRule("no-dupe-disjunctions", {
11671309
root: mention(result.alternative),
11681310
nested: mentionNested(result.nested),
11691311
},
1312+
suggest,
11701313
})
11711314
break
11721315

@@ -1176,6 +1319,7 @@ export default createRule("no-dupe-disjunctions", {
11761319
loc,
11771320
messageId: "prefixSubset",
11781321
data: { exp, cap, others },
1322+
suggest,
11791323
})
11801324
break
11811325

@@ -1191,6 +1335,7 @@ export default createRule("no-dupe-disjunctions", {
11911335
root: mention(result.alternative),
11921336
nested: mentionNested(result.nested),
11931337
},
1338+
suggest,
11941339
})
11951340
break
11961341

@@ -1200,6 +1345,7 @@ export default createRule("no-dupe-disjunctions", {
12001345
loc,
12011346
messageId: "superset",
12021347
data: { exp, cap, others },
1348+
suggest,
12031349
})
12041350
break
12051351

@@ -1216,6 +1362,7 @@ export default createRule("no-dupe-disjunctions", {
12161362
faToSource(result.overlap, flags),
12171363
),
12181364
},
1365+
suggest,
12191366
})
12201367
break
12211368

0 commit comments

Comments
 (0)