From f75cbbae29beb956a3599b4a5a0c21ab049b3a28 Mon Sep 17 00:00:00 2001 From: Michael Schmidt Date: Mon, 2 Oct 2023 01:16:26 +0200 Subject: [PATCH] Add `regexp/no-useless-set-operand` rule (#625) * Add `regexp/no-useless-set-operand` rule * Added docs * Create nervous-yaks-destroy.md * Apply suggestions from code review Co-authored-by: Yosuke Ota * npm run update --------- Co-authored-by: Yosuke Ota --- .changeset/nervous-yaks-destroy.md | 5 + README.md | 1 + docs/rules/index.md | 1 + docs/rules/no-useless-set-operand.md | 56 +++++ lib/configs/recommended.ts | 1 + lib/rules/no-useless-set-operand.ts | 240 ++++++++++++++++++++++ lib/utils/rules.ts | 2 + tests/lib/rules/no-useless-set-operand.ts | 106 ++++++++++ 8 files changed, 412 insertions(+) create mode 100644 .changeset/nervous-yaks-destroy.md create mode 100644 docs/rules/no-useless-set-operand.md create mode 100644 lib/rules/no-useless-set-operand.ts create mode 100644 tests/lib/rules/no-useless-set-operand.ts diff --git a/.changeset/nervous-yaks-destroy.md b/.changeset/nervous-yaks-destroy.md new file mode 100644 index 000000000..e698ea1d6 --- /dev/null +++ b/.changeset/nervous-yaks-destroy.md @@ -0,0 +1,5 @@ +--- +"eslint-plugin-regexp": major +--- + +Add `regexp/no-useless-set-operand` rule diff --git a/README.md b/README.md index bf9689a03..95afc5cf5 100644 --- a/README.md +++ b/README.md @@ -155,6 +155,7 @@ The `plugin:regexp/all` config enables all rules. It's meant for testing, not fo | [no-useless-lazy](https://ota-meshi.github.io/eslint-plugin-regexp/rules/no-useless-lazy.html) | disallow unnecessarily non-greedy quantifiers | ✅ | | 🔧 | | | [no-useless-quantifier](https://ota-meshi.github.io/eslint-plugin-regexp/rules/no-useless-quantifier.html) | disallow quantifiers that can be removed | ✅ | | 🔧 | 💡 | | [no-useless-range](https://ota-meshi.github.io/eslint-plugin-regexp/rules/no-useless-range.html) | disallow unnecessary character ranges | ✅ | | 🔧 | | +| [no-useless-set-operand](https://ota-meshi.github.io/eslint-plugin-regexp/rules/no-useless-set-operand.html) | disallow unnecessary elements in expression character classes | ✅ | | 🔧 | | | [no-useless-two-nums-quantifier](https://ota-meshi.github.io/eslint-plugin-regexp/rules/no-useless-two-nums-quantifier.html) | disallow unnecessary `{n,m}` quantifier | ✅ | | 🔧 | | | [no-zero-quantifier](https://ota-meshi.github.io/eslint-plugin-regexp/rules/no-zero-quantifier.html) | disallow quantifiers with a maximum of zero | ✅ | | | 💡 | | [optimal-lookaround-quantifier](https://ota-meshi.github.io/eslint-plugin-regexp/rules/optimal-lookaround-quantifier.html) | disallow the alternatives of lookarounds that end with a non-constant quantifier | | ✅ | | 💡 | diff --git a/docs/rules/index.md b/docs/rules/index.md index 11ed9bc2a..2392df264 100644 --- a/docs/rules/index.md +++ b/docs/rules/index.md @@ -62,6 +62,7 @@ sidebarDepth: 0 | [no-useless-lazy](no-useless-lazy.md) | disallow unnecessarily non-greedy quantifiers | ✅ | | 🔧 | | | [no-useless-quantifier](no-useless-quantifier.md) | disallow quantifiers that can be removed | ✅ | | 🔧 | 💡 | | [no-useless-range](no-useless-range.md) | disallow unnecessary character ranges | ✅ | | 🔧 | | +| [no-useless-set-operand](no-useless-set-operand.md) | disallow unnecessary elements in expression character classes | ✅ | | 🔧 | | | [no-useless-two-nums-quantifier](no-useless-two-nums-quantifier.md) | disallow unnecessary `{n,m}` quantifier | ✅ | | 🔧 | | | [no-zero-quantifier](no-zero-quantifier.md) | disallow quantifiers with a maximum of zero | ✅ | | | 💡 | | [optimal-lookaround-quantifier](optimal-lookaround-quantifier.md) | disallow the alternatives of lookarounds that end with a non-constant quantifier | | ✅ | | 💡 | diff --git a/docs/rules/no-useless-set-operand.md b/docs/rules/no-useless-set-operand.md new file mode 100644 index 000000000..9ffa74836 --- /dev/null +++ b/docs/rules/no-useless-set-operand.md @@ -0,0 +1,56 @@ +--- +pageClass: "rule-details" +sidebarDepth: 0 +title: "regexp/no-useless-set-operand" +description: "disallow unnecessary elements in expression character classes" +--- +# regexp/no-useless-set-operand + +💼 This rule is enabled in the ✅ `plugin:regexp/recommended` config. + +🔧 This rule is automatically fixable by the [`--fix` CLI option](https://eslint.org/docs/latest/user-guide/command-line-interface#--fix). + + + +> disallow unnecessary elements in expression character classes + +## :book: Rule Details + +The `v` flag added set operations for character classes, e.g. `[\w&&\D]` and `[\w--\d]`, but there are no limitations on what operands can be used. This rule reports any unnecessary operands. + + + +```js +/* eslint regexp/no-useless-set-operand: "error" */ + +/* ✓ GOOD */ +foo = /[\w--\d]/v +foo = /[\w--[\d_]]/v + +/* ✗ BAD */ +foo = /[\w--[\d$]]/v +foo = /[\w&&\d]/v +foo = /[\w&&\s]/v +foo = /[\w&&[\d\s]]/v +foo = /[\w&&[^\d\s]]/v +foo = /[\w--\s]/v +foo = /[\d--\w]/v +foo = /[\w--[\d\s]]/v +foo = /[\w--[^\d\s]]/v + +``` + + + +## :wrench: Options + +Nothing. + +## :rocket: Version + +:exclamation: ***This rule has not been released yet.*** + +## :mag: Implementation + +- [Rule source](https://github.com/ota-meshi/eslint-plugin-regexp/blob/master/lib/rules/no-useless-set-operand.ts) +- [Test source](https://github.com/ota-meshi/eslint-plugin-regexp/blob/master/tests/lib/rules/no-useless-set-operand.ts) diff --git a/lib/configs/recommended.ts b/lib/configs/recommended.ts index d11220902..2d1d26146 100644 --- a/lib/configs/recommended.ts +++ b/lib/configs/recommended.ts @@ -50,6 +50,7 @@ export const rules = { "regexp/no-useless-non-capturing-group": "error", "regexp/no-useless-quantifier": "error", "regexp/no-useless-range": "error", + "regexp/no-useless-set-operand": "error", "regexp/no-useless-two-nums-quantifier": "error", "regexp/no-zero-quantifier": "error", "regexp/optimal-lookaround-quantifier": "warn", diff --git a/lib/rules/no-useless-set-operand.ts b/lib/rules/no-useless-set-operand.ts new file mode 100644 index 000000000..2f7d05df7 --- /dev/null +++ b/lib/rules/no-useless-set-operand.ts @@ -0,0 +1,240 @@ +import type { RegExpVisitor } from "@eslint-community/regexpp/visitor" +import type { + CharacterClassElement, + ClassSetOperand, + ExpressionCharacterClass, + Node, + StringAlternative, +} from "@eslint-community/regexpp/ast" +import type { RegExpContext } from "../utils" +import { createRule, defineRegexpVisitor } from "../utils" +import { toUnicodeSet } from "regexp-ast-analysis" + +type FlatElement = CharacterClassElement | StringAlternative + +function getFlatElements( + node: ClassSetOperand | ExpressionCharacterClass["expression"], +): readonly FlatElement[] { + if (node.type === "ClassStringDisjunction") { + return node.alternatives + } + if (node.type === "CharacterClass") { + const nested: FlatElement[] = [] + // eslint-disable-next-line func-style -- x + const addElement = (element: CharacterClassElement) => { + if (element.type === "ClassStringDisjunction") { + nested.push(...element.alternatives) + } else if (element.type === "CharacterClass") { + if (!element.negate) { + nested.push(...element.elements) + } + nested.push(element) + } else { + nested.push(element) + } + } + node.elements.forEach(addElement) + return nested + } + + return [] +} + +function removeDescendant(root: Node, e: FlatElement): string { + let { start, end } = e + + if (e.type === "StringAlternative") { + if (e.parent.alternatives.length === 1) { + // we have to remove the whole string disjunction + // eslint-disable-next-line no-param-reassign -- x + e = e.parent + start = e.start + end = e.end + } else { + // remove one adjacent | symbol + if (e.parent.alternatives.at(-1) === e) { + start-- + } else { + end++ + } + } + } + + const before = root.raw.slice(0, start - root.start) + const after = root.raw.slice(end - root.start) + return before + after +} + +export default createRule("no-useless-set-operand", { + meta: { + docs: { + description: + "disallow unnecessary elements in expression character classes", + category: "Best Practices", + recommended: true, + }, + schema: [], + messages: { + intersectionDisjoint: + "'{{left}}' and '{{right}}' are disjoint, so the result of the intersection is always going to be the empty set.", + intersectionSubset: + "'{{sub}}' is a subset of '{{super}}', so the result of the intersection is always going to be '{{sub}}'.", + intersectionRemove: + "'{{expr}}' can be removed without changing the result of the intersection.", + subtractionDisjoint: + "'{{left}}' and '{{right}}' are disjoint, so the subtraction doesn't do anything.", + subtractionSubset: + "'{{left}}' is a subset of '{{right}}', so the result of the subtraction is always going to be the empty set.", + subtractionRemove: + "'{{expr}}' can be removed without changing the result of the subtraction.", + }, + fixable: "code", + type: "suggestion", + }, + create(context) { + function createVisitor( + regexpContext: RegExpContext, + ): RegExpVisitor.Handlers { + const { node, flags, getRegexpLocation, fixReplaceNode } = + regexpContext + + if (!flags.unicodeSets) { + // set operations are only available with the `v` flag + return {} + } + + function fixRemoveExpression( + expr: ExpressionCharacterClass["expression"], + ) { + if (expr.parent.type === "ExpressionCharacterClass") { + const cc = expr.parent + return fixReplaceNode(cc, cc.negate ? "[^]" : "[]") + } + return fixReplaceNode(expr, "[]") + } + + return { + onClassIntersectionEnter(iNode) { + const leftSet = toUnicodeSet(iNode.left, flags) + const rightSet = toUnicodeSet(iNode.right, flags) + + if (leftSet.isDisjointWith(rightSet)) { + context.report({ + node, + loc: getRegexpLocation(iNode), + messageId: "intersectionDisjoint", + data: { + left: iNode.left.raw, + right: iNode.right.raw, + }, + fix: fixRemoveExpression(iNode), + }) + return + } + + if (leftSet.isSubsetOf(rightSet)) { + context.report({ + node, + loc: getRegexpLocation(iNode), + messageId: "intersectionSubset", + data: { + sub: iNode.left.raw, + super: iNode.right.raw, + }, + fix: fixReplaceNode(iNode, iNode.left.raw), + }) + return + } + if (rightSet.isSubsetOf(leftSet)) { + context.report({ + node, + loc: getRegexpLocation(iNode), + messageId: "intersectionSubset", + data: { + sub: iNode.right.raw, + super: iNode.left.raw, + }, + fix: fixReplaceNode(iNode, iNode.right.raw), + }) + return + } + + const toRemoveRight = getFlatElements(iNode.right).filter( + (e) => leftSet.isDisjointWith(toUnicodeSet(e, flags)), + ) + const toRemoveLeft = getFlatElements(iNode.left).filter( + (e) => rightSet.isDisjointWith(toUnicodeSet(e, flags)), + ) + for (const e of [...toRemoveRight, ...toRemoveLeft]) { + context.report({ + node, + loc: getRegexpLocation(e), + messageId: "subtractionRemove", + data: { + expr: e.raw, + }, + fix: fixReplaceNode( + iNode, + removeDescendant(iNode, e), + ), + }) + } + }, + onClassSubtractionEnter(sNode) { + const leftSet = toUnicodeSet(sNode.left, flags) + const rightSet = toUnicodeSet(sNode.right, flags) + + if (leftSet.isDisjointWith(rightSet)) { + context.report({ + node, + loc: getRegexpLocation(sNode), + messageId: "subtractionDisjoint", + data: { + left: sNode.left.raw, + right: sNode.right.raw, + }, + fix: fixReplaceNode(sNode, sNode.left.raw), + }) + return + } + + if (leftSet.isSubsetOf(rightSet)) { + context.report({ + node, + loc: getRegexpLocation(sNode), + messageId: "subtractionSubset", + data: { + left: sNode.left.raw, + right: sNode.right.raw, + }, + fix: fixRemoveExpression(sNode), + }) + return + } + + const toRemove = getFlatElements(sNode.right).filter((e) => + leftSet.isDisjointWith(toUnicodeSet(e, flags)), + ) + for (const e of toRemove) { + context.report({ + node, + loc: getRegexpLocation(e), + messageId: "subtractionRemove", + data: { + expr: e.raw, + }, + fix: fixReplaceNode( + sNode, + removeDescendant(sNode, e), + ), + }) + } + }, + } + } + + return defineRegexpVisitor(context, { + createVisitor, + }) + }, +}) diff --git a/lib/utils/rules.ts b/lib/utils/rules.ts index e69b79c44..05c9f3923 100644 --- a/lib/utils/rules.ts +++ b/lib/utils/rules.ts @@ -47,6 +47,7 @@ import noUselessNonCapturingGroup from "../rules/no-useless-non-capturing-group" import noUselessNonGreedy from "../rules/no-useless-non-greedy" import noUselessQuantifier from "../rules/no-useless-quantifier" import noUselessRange from "../rules/no-useless-range" +import noUselessSetOperand from "../rules/no-useless-set-operand" import noUselessTwoNumsQuantifier from "../rules/no-useless-two-nums-quantifier" import noZeroQuantifier from "../rules/no-zero-quantifier" import optimalLookaroundQuantifier from "../rules/optimal-lookaround-quantifier" @@ -130,6 +131,7 @@ export const rules = [ noUselessNonGreedy, noUselessQuantifier, noUselessRange, + noUselessSetOperand, noUselessTwoNumsQuantifier, noZeroQuantifier, optimalLookaroundQuantifier, diff --git a/tests/lib/rules/no-useless-set-operand.ts b/tests/lib/rules/no-useless-set-operand.ts new file mode 100644 index 000000000..f62d229bb --- /dev/null +++ b/tests/lib/rules/no-useless-set-operand.ts @@ -0,0 +1,106 @@ +import { RuleTester } from "eslint" +import rule from "../../../lib/rules/no-useless-set-operand" + +const tester = new RuleTester({ + parserOptions: { + ecmaVersion: "latest", + sourceType: "module", + }, +}) + +tester.run("no-useless-set-operand", rule as any, { + valid: [String.raw`/[\w--\d]/v`], + invalid: [ + { + code: String.raw`/[\w&&\d]/v`, + output: String.raw`/[\d]/v`, + errors: [ + "'\\d' is a subset of '\\w', so the result of the intersection is always going to be '\\d'.", + ], + }, + { + code: String.raw`/[\w&&\s]/v`, + output: String.raw`/[]/v`, + errors: [ + "'\\w' and '\\s' are disjoint, so the result of the intersection is always going to be the empty set.", + ], + }, + { + code: String.raw`/[^\w&&\s]/v`, + output: String.raw`/[^]/v`, + errors: [ + "'\\w' and '\\s' are disjoint, so the result of the intersection is always going to be the empty set.", + ], + }, + { + code: String.raw`/[\w&&[\d\s]]/v`, + output: String.raw`/[\w&&[\d]]/v`, + errors: [ + "'\\s' can be removed without changing the result of the subtraction.", + ], + }, + { + code: String.raw`/[\w&&[^\d\s]]/v`, + output: String.raw`/[\w&&[^\d]]/v`, + errors: [ + "'\\s' can be removed without changing the result of the subtraction.", + ], + }, + { + code: String.raw`/[\w--\s]/v`, + output: String.raw`/[\w]/v`, + errors: [ + "'\\w' and '\\s' are disjoint, so the subtraction doesn't do anything.", + ], + }, + { + code: String.raw`/[\d--\w]/v`, + output: String.raw`/[]/v`, + errors: [ + "'\\d' is a subset of '\\w', so the result of the subtraction is always going to be the empty set.", + ], + }, + { + code: String.raw`/[^\d--\w]/v`, + output: String.raw`/[^]/v`, + errors: [ + "'\\d' is a subset of '\\w', so the result of the subtraction is always going to be the empty set.", + ], + }, + { + code: String.raw`/[\w--[\d\s]]/v`, + output: String.raw`/[\w--[\d]]/v`, + errors: [ + "'\\s' can be removed without changing the result of the subtraction.", + ], + }, + { + code: String.raw`/[\w--[^\d\s]]/v`, + output: String.raw`/[\w--[^\d]]/v`, + errors: [ + "'\\s' can be removed without changing the result of the subtraction.", + ], + }, + { + code: String.raw`/[\w--[a\q{aa|b}]]/v`, + output: String.raw`/[\w--[a\q{b}]]/v`, + errors: [ + "'aa' can be removed without changing the result of the subtraction.", + ], + }, + { + code: String.raw`/[\w--[a\q{aa}]]/v`, + output: String.raw`/[\w--[a]]/v`, + errors: [ + "'aa' can be removed without changing the result of the subtraction.", + ], + }, + { + code: String.raw`/[\w--\q{a|aa}]/v`, + output: String.raw`/[\w--\q{a}]/v`, + errors: [ + "'aa' can be removed without changing the result of the subtraction.", + ], + }, + ], +})