Skip to content
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

Add suggestions for regexp/no-useless-assertion #666

Merged
merged 3 commits into from
Oct 21, 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/unlucky-humans-fry.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"eslint-plugin-regexp": minor
---

Add suggestions for `regexp/no-useless-assertion`
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 | ✅ | | 🔧 | 💡 |
Expand Down
2 changes: 1 addition & 1 deletion docs/rules/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 | ✅ | | 🔧 | 💡 |
Expand Down
4 changes: 3 additions & 1 deletion docs/rules/no-useless-assertions.md
Original file line number Diff line number Diff line change
Expand Up @@ -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).

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

> 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).

<eslint-code-block>
Expand Down
123 changes: 121 additions & 2 deletions lib/rules/no-useless-assertions.ts
Original file line number Diff line number Diff line change
@@ -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"
Expand Down Expand Up @@ -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.",
Expand All @@ -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", {
Expand All @@ -236,6 +309,7 @@ export default createRule("no-useless-assertions", {
category: "Possible Errors",
recommended: true,
},
hasSuggestions: true,
schema: [],
messages,
type: "problem",
Expand All @@ -245,15 +319,44 @@ export default createRule("no-useless-assertions", {
node,
flags,
getRegexpLocation,
fixReplaceNode,
}: RegExpContext): RegExpVisitor.Handlers {
const reported = new Set<Assertion>()

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<string, string>,
data: Record<string, string> & {
acceptOrReject: "accept" | "reject"
},
) {
reported.add(assertion)
const { acceptOrReject } = data

context.report({
node,
Expand All @@ -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),
},
],
})
}

Expand Down Expand Up @@ -295,6 +407,7 @@ export default createRule("no-useless-assertions", {
if (next.char.isEmpty) {
report(assertion, "alwaysAcceptByChar", {
followedOrPreceded,
acceptOrReject: "accept",
})
}
} else {
Expand All @@ -306,6 +419,7 @@ export default createRule("no-useless-assertions", {
{
followedOrPreceded,
startOrEnd: assertion.kind,
acceptOrReject: "accept",
},
)
}
Expand All @@ -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
Expand All @@ -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",
})
}
}
Expand Down
Loading
Loading