diff --git a/.changeset/unlucky-humans-fry.md b/.changeset/unlucky-humans-fry.md new file mode 100644 index 000000000..7ac5cb1f2 --- /dev/null +++ b/.changeset/unlucky-humans-fry.md @@ -0,0 +1,5 @@ +--- +"eslint-plugin-regexp": minor +--- + +Add suggestions for `regexp/no-useless-assertion` diff --git a/README.md b/README.md index 14c627a03..7d7b4b382 100644 --- a/README.md +++ b/README.md @@ -128,7 +128,7 @@ The `plugin:regexp/all` config enables all rules. It's meant for testing, not fo | [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 | | ✅ | | | | [no-super-linear-backtracking](https://ota-meshi.github.io/eslint-plugin-regexp/rules/no-super-linear-backtracking.html) | disallow exponential and polynomial backtracking | ✅ | | 🔧 | | | [no-super-linear-move](https://ota-meshi.github.io/eslint-plugin-regexp/rules/no-super-linear-move.html) | disallow quantifiers that cause quadratic moves | | | | | -| [no-useless-assertions](https://ota-meshi.github.io/eslint-plugin-regexp/rules/no-useless-assertions.html) | disallow assertions that are known to always accept (or reject) | ✅ | | | | +| [no-useless-assertions](https://ota-meshi.github.io/eslint-plugin-regexp/rules/no-useless-assertions.html) | disallow assertions that are known to always accept (or reject) | ✅ | | | 💡 | | [no-useless-backreference](https://ota-meshi.github.io/eslint-plugin-regexp/rules/no-useless-backreference.html) | disallow useless backreferences in regular expressions | ✅ | | | | | [no-useless-dollar-replacements](https://ota-meshi.github.io/eslint-plugin-regexp/rules/no-useless-dollar-replacements.html) | disallow useless `$` replacements in replacement string | ✅ | | | | | [strict](https://ota-meshi.github.io/eslint-plugin-regexp/rules/strict.html) | disallow not strictly valid regular expressions | ✅ | | 🔧 | 💡 | diff --git a/docs/rules/index.md b/docs/rules/index.md index 535d969b5..62167017f 100644 --- a/docs/rules/index.md +++ b/docs/rules/index.md @@ -34,7 +34,7 @@ sidebarDepth: 0 | [no-potentially-useless-backreference](no-potentially-useless-backreference.md) | disallow backreferences that reference a group that might not be matched | | ✅ | | | | [no-super-linear-backtracking](no-super-linear-backtracking.md) | disallow exponential and polynomial backtracking | ✅ | | 🔧 | | | [no-super-linear-move](no-super-linear-move.md) | disallow quantifiers that cause quadratic moves | | | | | -| [no-useless-assertions](no-useless-assertions.md) | disallow assertions that are known to always accept (or reject) | ✅ | | | | +| [no-useless-assertions](no-useless-assertions.md) | disallow assertions that are known to always accept (or reject) | ✅ | | | 💡 | | [no-useless-backreference](no-useless-backreference.md) | disallow useless backreferences in regular expressions | ✅ | | | | | [no-useless-dollar-replacements](no-useless-dollar-replacements.md) | disallow useless `$` replacements in replacement string | ✅ | | | | | [strict](strict.md) | disallow not strictly valid regular expressions | ✅ | | 🔧 | 💡 | diff --git a/docs/rules/no-useless-assertions.md b/docs/rules/no-useless-assertions.md index 5eecad910..4185f8f33 100644 --- a/docs/rules/no-useless-assertions.md +++ b/docs/rules/no-useless-assertions.md @@ -9,13 +9,15 @@ since: "v0.9.0" 💼 This rule is enabled 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 assertions that are known to always accept (or reject) ## :book: Rule Details -Some assertion are unnecessary because the rest of the pattern forces them to +Some assertions are unnecessary because the rest of the pattern forces them to always be accept (or reject). diff --git a/lib/rules/no-useless-assertions.ts b/lib/rules/no-useless-assertions.ts index 808b14769..6d8268d95 100644 --- a/lib/rules/no-useless-assertions.ts +++ b/lib/rules/no-useless-assertions.ts @@ -1,10 +1,12 @@ import type { RegExpVisitor } from "@eslint-community/regexpp/visitor" import type { + Alternative, Assertion, EdgeAssertion, Element, LookaroundAssertion, Node, + Pattern, WordBoundaryAssertion, } from "@eslint-community/regexpp/ast" import type { RegExpContext } from "../utils" @@ -203,6 +205,73 @@ function createReorderingGetFirstCharAfter( } } +function removeAlternative( + alternative: Alternative, +): [Element | Pattern, string] { + const parent = alternative.parent + if (parent.alternatives.length > 1) { + // we can just remove the alternative + let { start, end } = alternative + if (parent.alternatives[0] === alternative) { + end++ + } else { + start-- + } + const before = parent.raw.slice(0, start - parent.start) + const after = parent.raw.slice(end - parent.start) + return [parent, before + after] + } + + // we have to remove the parent as well + + switch (parent.type) { + case "Pattern": + return [parent, "[]"] + + case "Assertion": { + // the inner part of the assertion always rejects + const assertionParent = parent.parent + if (parent.negate) { + // the assertion always accepts + return [ + assertionParent.type === "Quantifier" + ? assertionParent + : parent, + "", + ] + } + if (assertionParent.type === "Quantifier") { + if (assertionParent.min === 0) { + return [assertionParent, ""] + } + return removeAlternative(assertionParent.parent) + } + return removeAlternative(assertionParent) + } + + case "CapturingGroup": { + // we don't remove capturing groups + const before = parent.raw.slice(0, alternative.start - parent.start) + const after = parent.raw.slice(alternative.end - parent.start) + return [parent, `${before}[]${after}`] + } + + case "Group": { + const groupParent = parent.parent + if (groupParent.type === "Quantifier") { + if (groupParent.min === 0) { + return [groupParent, ""] + } + return removeAlternative(groupParent.parent) + } + return removeAlternative(groupParent) + } + + default: + return assertNever(parent) + } +} + const messages = { alwaysRejectByChar: "{{assertion}} will always reject because it is {{followedOrPreceded}} by a character.", @@ -226,6 +295,10 @@ const messages = { "The {{kind}} {{assertion}} will always {{acceptOrReject}}.", alwaysForNegativeLookaround: "The negative {{kind}} {{assertion}} will always {{acceptOrReject}}.", + + acceptSuggestion: "Remove the assertion. (Replace with empty string.)", + rejectSuggestion: + "Remove branch of the assertion. (Replace with empty set.)", } export default createRule("no-useless-assertions", { @@ -236,6 +309,7 @@ export default createRule("no-useless-assertions", { category: "Possible Errors", recommended: true, }, + hasSuggestions: true, schema: [], messages, type: "problem", @@ -245,15 +319,44 @@ export default createRule("no-useless-assertions", { node, flags, getRegexpLocation, + fixReplaceNode, }: RegExpContext): RegExpVisitor.Handlers { const reported = new Set() + function replaceWithEmptyString(assertion: Assertion) { + if (assertion.parent.type === "Quantifier") { + // the assertion always accepts does not consume characters, we can remove the quantifier as well. + return fixReplaceNode(assertion.parent, "") + } + return fixReplaceNode(assertion, "") + } + + function replaceWithEmptySet(assertion: Assertion) { + if (assertion.parent.type === "Quantifier") { + if (assertion.parent.min === 0) { + // the assertion always rejects does not consume characters, we can remove the quantifier as well. + return fixReplaceNode(assertion.parent, "") + } + const [element, replacement] = removeAlternative( + assertion.parent.parent, + ) + return fixReplaceNode(element, replacement) + } + const [element, replacement] = removeAlternative( + assertion.parent, + ) + return fixReplaceNode(element, replacement) + } + function report( assertion: Assertion, messageId: keyof typeof messages, - data: Record, + data: Record & { + acceptOrReject: "accept" | "reject" + }, ) { reported.add(assertion) + const { acceptOrReject } = data context.report({ node, @@ -263,6 +366,15 @@ export default createRule("no-useless-assertions", { assertion: mention(assertion), ...data, }, + suggest: [ + { + messageId: `${acceptOrReject}Suggestion`, + fix: + acceptOrReject === "accept" + ? replaceWithEmptyString(assertion) + : replaceWithEmptySet(assertion), + }, + ], }) } @@ -295,6 +407,7 @@ export default createRule("no-useless-assertions", { if (next.char.isEmpty) { report(assertion, "alwaysAcceptByChar", { followedOrPreceded, + acceptOrReject: "accept", }) } } else { @@ -306,6 +419,7 @@ export default createRule("no-useless-assertions", { { followedOrPreceded, startOrEnd: assertion.kind, + acceptOrReject: "accept", }, ) } @@ -317,6 +431,7 @@ export default createRule("no-useless-assertions", { // since the m flag isn't present any character will result in trivial rejection report(assertion, "alwaysRejectByChar", { followedOrPreceded, + acceptOrReject: "reject", }) } else { // only if the character is a sub set of /./, will the assertion trivially reject @@ -325,11 +440,15 @@ export default createRule("no-useless-assertions", { report( assertion, "alwaysRejectByNonLineTerminator", - { followedOrPreceded }, + { + followedOrPreceded, + acceptOrReject: "reject", + }, ) } else if (next.char.isSubsetOf(lineTerminator)) { report(assertion, "alwaysAcceptByLineTerminator", { followedOrPreceded, + acceptOrReject: "accept", }) } } diff --git a/tests/lib/rules/no-useless-assertions.ts b/tests/lib/rules/no-useless-assertions.ts index b7c755f7e..7d50ebec9 100644 --- a/tests/lib/rules/no-useless-assertions.ts +++ b/tests/lib/rules/no-useless-assertions.ts @@ -64,6 +64,12 @@ tester.run("no-useless-assertions", rule as any, { { message: "'\\b' will always reject because it is preceded by a word character and followed by a word character.", + suggestions: [ + { + messageId: "rejectSuggestion", + output: String.raw`/[]/`, + }, + ], }, ], }, @@ -73,6 +79,12 @@ tester.run("no-useless-assertions", rule as any, { { message: "'\\b' will always reject because it is preceded by a word character and followed by a word character.", + suggestions: [ + { + messageId: "rejectSuggestion", + output: String.raw`/[]/v`, + }, + ], }, ], }, @@ -82,6 +94,12 @@ tester.run("no-useless-assertions", rule as any, { { message: "'\\b' will always reject because it is preceded by a non-word character and followed by a non-word character.", + suggestions: [ + { + messageId: "rejectSuggestion", + output: String.raw`/[]/`, + }, + ], }, ], }, @@ -91,6 +109,12 @@ tester.run("no-useless-assertions", rule as any, { { message: "'\\b' will always accept because it is preceded by a non-word character and followed by a word character.", + suggestions: [ + { + messageId: "acceptSuggestion", + output: String.raw`/,b/`, + }, + ], }, ], }, @@ -100,6 +124,12 @@ tester.run("no-useless-assertions", rule as any, { { message: "'\\b' will always accept because it is preceded by a word character and followed by a non-word character.", + suggestions: [ + { + messageId: "acceptSuggestion", + output: String.raw`/a,/`, + }, + ], }, ], }, @@ -110,6 +140,12 @@ tester.run("no-useless-assertions", rule as any, { { message: "'\\B' will always accept because it is preceded by a word character and followed by a word character.", + suggestions: [ + { + messageId: "acceptSuggestion", + output: String.raw`/ab/`, + }, + ], }, ], }, @@ -119,6 +155,12 @@ tester.run("no-useless-assertions", rule as any, { { message: "'\\B' will always accept because it is preceded by a non-word character and followed by a non-word character.", + suggestions: [ + { + messageId: "acceptSuggestion", + output: String.raw`/,,/`, + }, + ], }, ], }, @@ -128,6 +170,12 @@ tester.run("no-useless-assertions", rule as any, { { message: "'\\B' will always reject because it is preceded by a non-word character and followed by a word character.", + suggestions: [ + { + messageId: "rejectSuggestion", + output: String.raw`/[]/`, + }, + ], }, ], }, @@ -137,6 +185,12 @@ tester.run("no-useless-assertions", rule as any, { { message: "'\\B' will always reject because it is preceded by a word character and followed by a non-word character.", + suggestions: [ + { + messageId: "rejectSuggestion", + output: String.raw`/[]/`, + }, + ], }, ], }, @@ -147,6 +201,12 @@ tester.run("no-useless-assertions", rule as any, { { message: "'^' will always reject because it is preceded by a non-line-terminator character.", + suggestions: [ + { + messageId: "rejectSuggestion", + output: String.raw`/[]/m`, + }, + ], }, ], }, @@ -156,6 +216,12 @@ tester.run("no-useless-assertions", rule as any, { { message: "'^' will always accept because it is preceded by a line-terminator character.", + suggestions: [ + { + messageId: "acceptSuggestion", + output: String.raw`/\nfoo/m`, + }, + ], }, ], }, @@ -165,6 +231,12 @@ tester.run("no-useless-assertions", rule as any, { { message: "'^' will always reject because it is preceded by a character.", + suggestions: [ + { + messageId: "rejectSuggestion", + output: String.raw`/[]/`, + }, + ], }, ], }, @@ -174,6 +246,12 @@ tester.run("no-useless-assertions", rule as any, { { message: "'^' will always reject because it is preceded by a character.", + suggestions: [ + { + messageId: "rejectSuggestion", + output: String.raw`/[]/`, + }, + ], }, ], }, @@ -184,6 +262,12 @@ tester.run("no-useless-assertions", rule as any, { { message: "'$' will always reject because it is followed by a non-line-terminator character.", + suggestions: [ + { + messageId: "rejectSuggestion", + output: String.raw`/[]/m`, + }, + ], }, ], }, @@ -202,6 +286,12 @@ tester.run("no-useless-assertions", rule as any, { { message: "'$' will always reject because it is followed by a character.", + suggestions: [ + { + messageId: "rejectSuggestion", + output: String.raw`/[]/`, + }, + ], }, ], }, @@ -211,6 +301,12 @@ tester.run("no-useless-assertions", rule as any, { { message: "'$' will always reject because it is followed by a character.", + suggestions: [ + { + messageId: "rejectSuggestion", + output: String.raw`/[]/`, + }, + ], }, ], }, @@ -454,5 +550,105 @@ tester.run("no-useless-assertions", rule as any, { "The lookahead '(?=(?=[a-f])(?=[aA])\\w(?<=[aA])(?<=[a-f]))' will always accept.", ], }, + + { + code: String.raw`/foo(?:\.(?!$)|$)$/`, + errors: [ + { + message: + "The negative lookahead '(?!$)' will always reject.", + suggestions: [ + { + messageId: "rejectSuggestion", + output: String.raw`/foo(?:$)$/`, + }, + ], + }, + { + message: + "'$' will always accept because it is never followed by a character.", + suggestions: [ + { + messageId: "acceptSuggestion", + output: String.raw`/foo(?:\.(?!$)|)$/`, + }, + ], + }, + ], + }, + { + code: String.raw`/(?=a)+a/`, + errors: [ + { + message: "The lookahead '(?=a)' will always accept.", + suggestions: [ + { + messageId: "acceptSuggestion", + output: String.raw`/a/`, + }, + ], + }, + ], + }, + { + code: String.raw`/(?!a)*a/`, + errors: [ + { + message: + "The negative lookahead '(?!a)' will always reject.", + suggestions: [ + { + messageId: "rejectSuggestion", + output: String.raw`/a/`, + }, + ], + }, + ], + }, + { + code: String.raw`/a(\B)b/`, + errors: [ + { + message: + "'\\B' will always accept because it is preceded by a word character and followed by a word character.", + suggestions: [ + { + messageId: "acceptSuggestion", + output: String.raw`/a()b/`, + }, + ], + }, + ], + }, + { + code: String.raw`/a(\b)b/`, + errors: [ + { + message: + "'\\b' will always reject because it is preceded by a word character and followed by a word character.", + suggestions: [ + { + messageId: "rejectSuggestion", + output: String.raw`/a([])b/`, + }, + ], + }, + ], + }, + { + code: String.raw`/foo|b(?:a(?:\br)|oo)|baz/`, + errors: [ + { + message: + "'\\b' will always reject because it is preceded by a word character and followed by a word character.", + suggestions: [ + { + messageId: "rejectSuggestion", + output: String.raw`/foo|b(?:oo)|baz/`, + }, + ], + }, + ], + }, ], })