From bc2108289dca686d143646f20b14a5f59e624447 Mon Sep 17 00:00:00 2001 From: RunDevelopment Date: Fri, 16 Apr 2021 18:29:32 +0200 Subject: [PATCH] Updated `no-useless-non-greedy` rule --- README.md | 2 +- docs/rules/README.md | 2 +- docs/rules/no-useless-non-greedy.md | 20 +++- lib/rules/no-useless-non-greedy.ts | 117 ++++++++++++++++++----- tests/lib/rules/no-useless-non-greedy.ts | 33 +++++-- 5 files changed, 138 insertions(+), 36 deletions(-) diff --git a/README.md b/README.md index 580b482ac..805752290 100644 --- a/README.md +++ b/README.md @@ -110,7 +110,7 @@ The rules with the following star :star: are included in the `plugin:regexp/reco | [regexp/no-useless-escape](https://ota-meshi.github.io/eslint-plugin-regexp/rules/no-useless-escape.html) | disallow unnecessary escape characters in RegExp | | | [regexp/no-useless-exactly-quantifier](https://ota-meshi.github.io/eslint-plugin-regexp/rules/no-useless-exactly-quantifier.html) | disallow unnecessary exactly quantifier | :star: | | [regexp/no-useless-non-capturing-group](https://ota-meshi.github.io/eslint-plugin-regexp/rules/no-useless-non-capturing-group.html) | disallow unnecessary Non-capturing group | :wrench: | -| [regexp/no-useless-non-greedy](https://ota-meshi.github.io/eslint-plugin-regexp/rules/no-useless-non-greedy.html) | disallow unnecessary quantifier non-greedy (`?`) | :wrench: | +| [regexp/no-useless-non-greedy](https://ota-meshi.github.io/eslint-plugin-regexp/rules/no-useless-non-greedy.html) | disallow unnecessarily non-greedy quantifiers | :wrench: | | [regexp/no-useless-range](https://ota-meshi.github.io/eslint-plugin-regexp/rules/no-useless-range.html) | disallow unnecessary range of characters by using a hyphen | :wrench: | | [regexp/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 | :star::wrench: | | [regexp/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/README.md b/docs/rules/README.md index 31381e9e5..8dfd30462 100644 --- a/docs/rules/README.md +++ b/docs/rules/README.md @@ -38,7 +38,7 @@ The rules with the following star :star: are included in the `plugin:regexp/reco | [regexp/no-useless-escape](./no-useless-escape.md) | disallow unnecessary escape characters in RegExp | | | [regexp/no-useless-exactly-quantifier](./no-useless-exactly-quantifier.md) | disallow unnecessary exactly quantifier | :star: | | [regexp/no-useless-non-capturing-group](./no-useless-non-capturing-group.md) | disallow unnecessary Non-capturing group | :wrench: | -| [regexp/no-useless-non-greedy](./no-useless-non-greedy.md) | disallow unnecessary quantifier non-greedy (`?`) | :wrench: | +| [regexp/no-useless-non-greedy](./no-useless-non-greedy.md) | disallow unnecessarily non-greedy quantifiers | :wrench: | | [regexp/no-useless-range](./no-useless-range.md) | disallow unnecessary range of characters by using a hyphen | :wrench: | | [regexp/no-useless-two-nums-quantifier](./no-useless-two-nums-quantifier.md) | disallow unnecessary `{n,m}` quantifier | :star::wrench: | | [regexp/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-non-greedy.md b/docs/rules/no-useless-non-greedy.md index e08ca29a5..edd069ce7 100644 --- a/docs/rules/no-useless-non-greedy.md +++ b/docs/rules/no-useless-non-greedy.md @@ -2,18 +2,30 @@ pageClass: "rule-details" sidebarDepth: 0 title: "regexp/no-useless-non-greedy" -description: "disallow unnecessary quantifier non-greedy (`?`)" +description: "disallow unnecessarily non-greedy quantifiers" since: "v0.3.0" --- # regexp/no-useless-non-greedy -> disallow unnecessary quantifier non-greedy (`?`) +> disallow unnecessarily non-greedy quantifiers - :wrench: The `--fix` option on the [command line](https://eslint.org/docs/user-guide/command-line-interface#fixing-problems) can automatically fix some of the problems reported by this rule. ## :book: Rule Details -This rule reports unnecessary quantifier non-greedy (`?`). +This rule reports lazy quantifiers that don't need to by lazy. + +There are two reasons why a lazy quantifier doesn't have to lazy: + +1. It's a constant quantifier (e.g. `a{3}?`). + +2. The quantifier is effectively possessive (e.g. `a+?b`). + + Whether a quantifier (let's call it _q_) is effectively possessive depends on the expression after it (let's call it _e_). _q_ is effectively possessive if _q_ cannot accept the character accepted by _e_ and _e_ cannot accept the characters accepted by _q_. + + In the example above, the character `a` and the character `b` do not overlap. Therefore the quantifier `a+` is possessive. + + Since an effectively possessive quantifier cannot give up characters to the expression after it, it doesn't matter whether the quantifier greedy or lazy. However, greedy quantifiers should be preferred because they require less characters to write and are easier to visually parse. @@ -25,11 +37,13 @@ var foo = /a*?/; var foo = /a+?/; var foo = /a{4,}?/; var foo = /a{2,4}?/; +var foo = /a[\s\S]*?bar/; /* ✗ BAD */ var foo = /a{1}?/; var foo = /a{4}?/; var foo = /a{2,2}?/; +var foo = /ab+?c/; ``` diff --git a/lib/rules/no-useless-non-greedy.ts b/lib/rules/no-useless-non-greedy.ts index 2a2b50504..aa6b486b5 100644 --- a/lib/rules/no-useless-non-greedy.ts +++ b/lib/rules/no-useless-non-greedy.ts @@ -1,16 +1,53 @@ -import type { Expression } from "estree" +import type { Rule, SourceCode } from "eslint" +import type { Expression, SourceLocation } from "estree" +import { + getMatchingDirection, + getFirstConsumedChar, + getFirstCharAfter, +} from "regexp-ast-analysis" +import type { Quantifier } from "regexpp/ast" import type { RegExpVisitor } from "regexpp/visitor" import { createRule, defineRegexpVisitor, getRegexpLocation, getRegexpRange, + parseFlags, } from "../utils" +/** + * Returns a fix that makes the given quantifier greedy. + */ +function makeGreedy( + sourceCode: SourceCode, + node: Expression, + qNode: Quantifier, +) { + return (fixer: Rule.RuleFixer): Rule.Fix | null => { + const range = getRegexpRange(sourceCode, node, qNode) + if (range == null) { + return null + } + return fixer.removeRange([range[1] - 1, range[1]]) + } +} + +/** + * Returns the source location of the lazy modifier of the given quantifier. + */ +function getLazyLoc( + sourceCode: SourceCode, + node: Expression, + qNode: Quantifier, +): SourceLocation { + const offset = qNode.raw.length - 1 + return getRegexpLocation(sourceCode, node, qNode, [offset, offset + 1]) +} + export default createRule("no-useless-non-greedy", { meta: { docs: { - description: "disallow unnecessary quantifier non-greedy (`?`)", + description: "disallow unnecessarily non-greedy quantifiers", // TODO In the major version // recommended: true, recommended: false, @@ -18,7 +55,9 @@ export default createRule("no-useless-non-greedy", { fixable: "code", schema: [], messages: { - unexpected: "Unexpected quantifier non-greedy.", + constant: "Unexpected non-greedy constant quantifier.", + possessive: + "Unexpected non-greedy constant quantifier. The quantifier is effectively possessive, so it doesn't matter whether it is greedy or not.", }, type: "suggestion", // "problem", }, @@ -29,33 +68,61 @@ export default createRule("no-useless-non-greedy", { * Create visitor * @param node */ - function createVisitor(node: Expression): RegExpVisitor.Handlers { + function createVisitor( + node: Expression, + _pattern: string, + flagsStr: string, + ): RegExpVisitor.Handlers { + const flags = parseFlags(flagsStr) + return { onQuantifierEnter(qNode) { - if (qNode.greedy === false && qNode.min === qNode.max) { - const offset = qNode.raw.length - 1 + if (qNode.greedy) { + return + } + + if (qNode.min === qNode.max) { + // a constant lazy quantifier (e.g. /a{2}?/) + context.report({ node, - loc: getRegexpLocation(sourceCode, node, qNode, [ - offset, - offset + 1, - ]), - messageId: "unexpected", - fix(fixer) { - const range = getRegexpRange( - sourceCode, - node, - qNode, - ) - if (range == null) { - return null - } - return fixer.removeRange([ - range[1] - 1, - range[1], - ]) - }, + loc: getLazyLoc(sourceCode, node, qNode), + messageId: "constant", + fix: makeGreedy(sourceCode, node, qNode), }) + return + } + + // This is more tricky. + // The basic idea here is that if the first character of the + // quantified element and the first character of whatever + // comes after the quantifier are always different, then the + // lazy modifier doesn't matter. + // E.g. /a+?b+/ == /a+b+/ + + const matchingDir = getMatchingDirection(qNode) + const firstChar = getFirstConsumedChar( + qNode, + matchingDir, + flags, + ) + if (!firstChar.empty) { + const after = getFirstCharAfter( + qNode, + matchingDir, + flags, + ) + if ( + !after.edge && + firstChar.char.isDisjointWith(after.char) + ) { + context.report({ + node, + loc: getLazyLoc(sourceCode, node, qNode), + messageId: "possessive", + fix: makeGreedy(sourceCode, node, qNode), + }) + } } }, } diff --git a/tests/lib/rules/no-useless-non-greedy.ts b/tests/lib/rules/no-useless-non-greedy.ts index 6046f747c..954451c14 100644 --- a/tests/lib/rules/no-useless-non-greedy.ts +++ b/tests/lib/rules/no-useless-non-greedy.ts @@ -9,14 +9,24 @@ const tester = new RuleTester({ }) tester.run("no-useless-non-greedy", rule as any, { - valid: [`/a*?/`, `/a+?/`, `/a{4,}?/`, `/a{2,4}?/`, `/a{2,2}/`, `/a{3}/`], + valid: [ + `/a*?/`, + `/a+?/`, + `/a{4,}?/`, + `/a{2,4}?/`, + `/a{2,2}/`, + `/a{3}/`, + `/a+?b*/`, + `/[\\s\\S]+?bar/`, + `/a??a?/`, + ], invalid: [ { code: `/a{1}?/`, output: `/a{1}/`, errors: [ { - message: "Unexpected quantifier non-greedy.", + messageId: "constant", line: 1, column: 6, endLine: 1, @@ -29,7 +39,7 @@ tester.run("no-useless-non-greedy", rule as any, { output: `/a{4}/`, errors: [ { - message: "Unexpected quantifier non-greedy.", + messageId: "constant", line: 1, column: 6, endLine: 1, @@ -40,20 +50,31 @@ tester.run("no-useless-non-greedy", rule as any, { { code: `/a{2,2}?/`, output: `/a{2,2}/`, - errors: ["Unexpected quantifier non-greedy."], + errors: [{ messageId: "constant" }], }, { code: String.raw`const s = "\\d{1}?" new RegExp(s)`, output: String.raw`const s = "\\d{1}" new RegExp(s)`, - errors: ["Unexpected quantifier non-greedy."], + errors: [{ messageId: "constant" }], }, { code: String.raw`const s = "\\d"+"{1}?" new RegExp(s)`, output: null, - errors: ["Unexpected quantifier non-greedy."], + errors: [{ messageId: "constant" }], + }, + + { + code: `/a+?b+/`, + output: `/a+b+/`, + errors: [{ messageId: "possessive" }], + }, + { + code: `/(?:a|cd)+?(?:b+|zzz)/`, + output: `/(?:a|cd)+(?:b+|zzz)/`, + errors: [{ messageId: "possessive" }], }, ], })