From bb35350fc01c9e7f8bd27b63dd5415ff98b084da Mon Sep 17 00:00:00 2001 From: RunDevelopment Date: Sun, 8 Aug 2021 17:08:06 +0200 Subject: [PATCH 1/7] Improve pattern source information --- lib/rules/control-character-escape.ts | 2 +- lib/rules/match-any.ts | 72 ++-- lib/rules/no-empty-alternative.ts | 16 +- lib/rules/no-invisible-character.ts | 8 +- lib/rules/no-useless-lazy.ts | 15 +- lib/rules/optimal-quantifier-concatenation.ts | 28 +- lib/rules/prefer-quantifier.ts | 40 +- lib/rules/prefer-range.ts | 73 ++-- lib/rules/prefer-w.ts | 31 +- lib/rules/sort-character-class-elements.ts | 20 +- lib/utils/ast-utils/pattern-source.ts | 390 ++++++++++++++++++ lib/utils/ast-utils/utils.ts | 112 ++++- lib/utils/index.ts | 329 +++++---------- tests/lib/rules/match-any.ts | 5 +- tests/lib/rules/negation.ts | 4 +- tests/lib/rules/no-empty-alternative.ts | 2 +- tests/lib/rules/no-useless-character-class.ts | 4 +- tests/lib/rules/no-useless-lazy.ts | 3 +- .../rules/sort-character-class-elements.ts | 3 +- .../extract-expression-references.ts | 4 +- .../ast-utils/extract-property-references.ts | 2 +- tests/lib/utils/get-usage-of-pattern.ts | 12 +- 22 files changed, 776 insertions(+), 399 deletions(-) create mode 100644 lib/utils/ast-utils/pattern-source.ts diff --git a/lib/rules/control-character-escape.ts b/lib/rules/control-character-escape.ts index 491a1923a..257c4c874 100644 --- a/lib/rules/control-character-escape.ts +++ b/lib/rules/control-character-escape.ts @@ -8,8 +8,8 @@ import { CP_TAB, createRule, defineRegexpVisitor, - isRegexpLiteral, } from "../utils" +import { isRegexpLiteral } from "../utils/ast-utils/utils" const CONTROL_CHARS = new Map([ [0, "\\0"], diff --git a/lib/rules/match-any.ts b/lib/rules/match-any.ts index 42f04d6b5..208e0aece 100644 --- a/lib/rules/match-any.ts +++ b/lib/rules/match-any.ts @@ -2,7 +2,8 @@ import type { RegExpVisitor } from "regexpp/visitor" import type { Rule } from "eslint" import type { CharacterClass, Node as RegExpNode } from "regexpp/ast" import type { RegExpContext } from "../utils" -import { createRule, defineRegexpVisitor, isRegexpLiteral } from "../utils" +import { createRule, defineRegexpVisitor } from "../utils" +import { isRegexpLiteral } from "../utils/ast-utils/utils" import { matchesAllCharacters } from "regexp-ast-analysis" const OPTION_SS1 = "[\\s\\S]" as const @@ -64,27 +65,45 @@ export default createRule("match-any", { * Fix source code * @param fixer */ - function* fix( + function fix( fixer: Rule.RuleFixer, - { node, flags, getRegexpRange, fixerApplyEscape }: RegExpContext, + { node, flags, patternSource }: RegExpContext, regexpNode: RegExpNode, - ) { - if (!preference) { - return + ): null | Rule.Fix | Rule.Fix[] { + if (!preference || !patternSource) { + return null } + if (preference === OPTION_DOTALL) { if (!flags.dotAll) { // since we can't just add flags, we cannot fix this - return + return null } if (!isRegexpLiteral(node)) { // Flag conflicts may be unavoidable and will not be autofix. - return + return null } - } - const range = getRegexpRange(regexpNode) - if (range == null) { - return + + const range = patternSource.getReplaceRange(regexpNode) + if (range == null) { + return null + } + + // Autofix to dotAll depends on the flag. + // Modify the entire regular expression literal to avoid conflicts due to flag changes. + const afterRange: [number, number] = [ + range.range[1], + node.range![1], + ] + + return [ + range.replace(fixer, "."), + // Mark regular expression flag changes to avoid conflicts due to flag changes. + fixer.replaceTextRange( + afterRange, + sourceCode.text.slice(...afterRange), + ), + ] } if ( @@ -92,27 +111,18 @@ export default createRule("match-any", { preference.startsWith("[") && preference.endsWith("]") ) { - yield fixer.replaceTextRange( - [range[0] + 1, range[1] - 1], - fixerApplyEscape(preference.slice(1, -1)), - ) - return + // We know that the first and last character are the same, + // so we only change the contents of the character class. + // This will avoid unnecessary conflicts between fixes. + const range = patternSource.getReplaceRange({ + start: regexpNode.start + 1, + end: regexpNode.end - 1, + }) + return range?.replace(fixer, preference.slice(1, -1)) ?? null } - const replacement = preference === OPTION_DOTALL ? "." : preference - yield fixer.replaceTextRange(range, fixerApplyEscape(replacement)) - - if (preference === OPTION_DOTALL) { - // Autofix to dotAll depends on the flag. - // Modify the entire regular expression literal to avoid conflicts due to flag changes. - - // Mark regular expression flag changes to avoid conflicts due to flag changes. - const afterRange: [number, number] = [range[1], node.range![1]] - yield fixer.replaceTextRange( - afterRange, - sourceCode.text.slice(...afterRange), - ) - } + const range = patternSource.getReplaceRange(regexpNode) + return range?.replace(fixer, preference) ?? null } /** diff --git a/lib/rules/no-empty-alternative.ts b/lib/rules/no-empty-alternative.ts index 27070da01..2aff2bc82 100644 --- a/lib/rules/no-empty-alternative.ts +++ b/lib/rules/no-empty-alternative.ts @@ -38,10 +38,24 @@ export default createRule("no-empty-alternative", { // the parser and one alternative is already handled by other rules. for (let i = 0; i < regexpNode.alternatives.length; i++) { const alt = regexpNode.alternatives[i] + const last = i === regexpNode.alternatives.length - 1 if (alt.elements.length === 0) { + // Since empty alternative have a width of 0, it's hard to underline their location. + // So we will report the location of the `|` that causes the empty alternative. + const index = alt.start + const loc = last + ? getRegexpLocation({ + start: index - 1, + end: index, + }) + : getRegexpLocation({ + start: index, + end: index + 1, + }) + context.report({ node, - loc: getRegexpLocation(alt), + loc, messageId: "empty", }) // don't report the same node multiple times diff --git a/lib/rules/no-invisible-character.ts b/lib/rules/no-invisible-character.ts index 430c37ba1..9f41092f6 100644 --- a/lib/rules/no-invisible-character.ts +++ b/lib/rules/no-invisible-character.ts @@ -33,7 +33,7 @@ export default createRule("no-invisible-character", { node, flags, getRegexpLocation, - getRegexpRange, + fixReplaceNode, }: RegExpContextForLiteral): RegExpVisitor.Handlers { return { onCharacterEnter(cNode) { @@ -49,11 +49,7 @@ export default createRule("no-invisible-character", { data: { instead, }, - fix(fixer) { - const range = getRegexpRange(cNode) - - return fixer.replaceTextRange(range, instead) - }, + fix: fixReplaceNode(cNode, instead), }) } }, diff --git a/lib/rules/no-useless-lazy.ts b/lib/rules/no-useless-lazy.ts index 959e50aaf..55f895791 100644 --- a/lib/rules/no-useless-lazy.ts +++ b/lib/rules/no-useless-lazy.ts @@ -13,13 +13,20 @@ import { createRule, defineRegexpVisitor } from "../utils" /** * Returns a fix that makes the given quantifier greedy. */ -function makeGreedy({ getRegexpRange }: RegExpContext, qNode: Quantifier) { +function makeGreedy({ patternSource }: RegExpContext, qNode: Quantifier) { return (fixer: Rule.RuleFixer): Rule.Fix | null => { - const range = getRegexpRange(qNode) - if (range == null) { + if (qNode.greedy) { return null } - return fixer.removeRange([range[1] - 1, range[1]]) + + const range = patternSource?.getReplaceRange({ + start: qNode.end - 1, + end: qNode.end, + }) + if (!range) { + return null + } + return range.remove(fixer) } } diff --git a/lib/rules/optimal-quantifier-concatenation.ts b/lib/rules/optimal-quantifier-concatenation.ts index 4ac202747..f216dbc70 100644 --- a/lib/rules/optimal-quantifier-concatenation.ts +++ b/lib/rules/optimal-quantifier-concatenation.ts @@ -13,7 +13,7 @@ import type { QuantifiableElement, Quantifier, } from "regexpp/ast" -import type { AST, SourceCode } from "eslint" +import type { AST } from "eslint" import type { RegExpContext, Quant } from "../utils" import { createRule, defineRegexpVisitor, quantToString } from "../utils" import { Chars, hasSomeDescendant } from "regexp-ast-analysis" @@ -489,20 +489,12 @@ function getReplacement( function getLoc( left: Element, right: Element, - sourceCode: SourceCode, - { getRegexpRange }: RegExpContext, + { patternSource }: RegExpContext, ): AST.SourceLocation | undefined { - const firstRange = getRegexpRange(left) - const lastRange = getRegexpRange(right) - - if (firstRange && lastRange) { - return { - start: sourceCode.getLocFromIndex(firstRange[0]), - end: sourceCode.getLocFromIndex(lastRange[1]), - } - } - - return undefined + return patternSource?.getAstLocation({ + start: Math.min(left.start, right.start), + end: Math.max(left.end, right.end), + }) } export default createRule("optimal-quantifier-concatenation", { @@ -568,12 +560,8 @@ export default createRule("optimal-quantifier-concatenation", { context.report({ node, loc: - getLoc( - left, - right, - context.getSourceCode(), - regexpContext, - ) ?? getRegexpLocation(aNode), + getLoc(left, right, regexpContext) ?? + getRegexpLocation(aNode), messageId: replacement.messageId, data: { left: left.raw, diff --git a/lib/rules/prefer-quantifier.ts b/lib/rules/prefer-quantifier.ts index 4e08279a3..68e380e36 100644 --- a/lib/rules/prefer-quantifier.ts +++ b/lib/rules/prefer-quantifier.ts @@ -9,6 +9,7 @@ import { isSymbol, quantToString, } from "../utils" +import type { PatternRange } from "../utils/ast-utils/pattern-source" type CharTarget = CharacterSet | Character @@ -112,15 +113,12 @@ export default createRule("prefer-quantifier", { type: "suggestion", // "problem", }, create(context) { - const sourceCode = context.getSourceCode() - /** * Create visitor */ function createVisitor({ node, - fixerApplyEscape, - getRegexpRange, + patternSource, }: RegExpContext): RegExpVisitor.Handlers { return { onAlternativeEnter(aNode) { @@ -151,24 +149,16 @@ export default createRule("prefer-quantifier", { if (!buffer || buffer.isValid()) { return } - const firstRange = getRegexpRange(buffer.elements[0]) - const lastRange = getRegexpRange( - buffer.elements[buffer.elements.length - 1], - ) - let range: [number, number] | null = null - if (firstRange && lastRange) { - range = [firstRange[0], lastRange[1]] + + const bufferRange: PatternRange = { + start: buffer.elements[0].start, + end: + buffer.elements[buffer.elements.length - 1].end, } + context.report({ node, - loc: range - ? { - start: sourceCode.getLocFromIndex( - range[0], - ), - end: sourceCode.getLocFromIndex(range[1]), - } - : undefined, + loc: patternSource?.getAstLocation(bufferRange), messageId: "unexpected", data: { type: @@ -180,13 +170,15 @@ export default createRule("prefer-quantifier", { quantifier: buffer.getQuantifier(), }, fix(fixer) { - if (range == null) { + const range = patternSource?.getReplaceRange( + bufferRange, + ) + if (!range) { return null } - return fixer.replaceTextRange( - range, - fixerApplyEscape(buffer.target.raw) + - buffer.getQuantifier(), + return range.replace( + fixer, + buffer.target.raw + buffer.getQuantifier(), ) }, }) diff --git a/lib/rules/prefer-range.ts b/lib/rules/prefer-range.ts index 13596f787..609f6bffa 100644 --- a/lib/rules/prefer-range.ts +++ b/lib/rules/prefer-range.ts @@ -7,6 +7,7 @@ import { getAllowedCharValueSchema, inRange, } from "../utils/char-ranges" +import type { PatternReplaceRange } from "../utils/ast-utils/pattern-source" export default createRule("prefer-range", { meta: { @@ -52,26 +53,36 @@ export default createRule("prefer-range", { function createVisitor( regexpContext: RegExpContext, ): RegExpVisitor.Handlers { - const { node, getRegexpRange } = regexpContext + const { node, patternSource } = regexpContext /** Get report location ranges */ function getReportRanges( nodes: (Character | CharacterClassRange)[], - ): [number, number][] | null { - const ranges: [number, number][] = [] + ): PatternReplaceRange[] | null { + const ranges: PatternReplaceRange[] = [] for (const reportNode of nodes) { - const reportRange = getRegexpRange(reportNode) + const reportRange = patternSource?.getReplaceRange( + reportNode, + ) if (!reportRange) { return null } const range = ranges.find( - (r) => r[0] <= reportRange[1] && reportRange[0] <= r[1], + (r) => + r.range[0] <= reportRange.range[1] && + reportRange.range[0] <= r.range[1], ) if (range) { - range[0] = Math.min(range[0], reportRange[0]) - range[1] = Math.max(range[1], reportRange[1]) + range.range[0] = Math.min( + range.range[0], + reportRange.range[0], + ) + range.range[1] = Math.max( + range.range[1], + reportRange.range[1], + ) } else { - ranges.push([...reportRange]) + ranges.push(reportRange) } } return ranges @@ -143,36 +154,32 @@ export default createRule("prefer-range", { group.max.value - group.min.value > 1 && group.nodes.length > 1 ) { - const ranges = getReportRanges(group.nodes) const newText = `${group.min.raw}-${group.max.raw}` - for (const range of ranges || [node.range!]) { + const ranges = getReportRanges(group.nodes) + if (!ranges) { context.report({ node, - loc: { - start: sourceCode.getLocFromIndex( - range[0], - ), - end: sourceCode.getLocFromIndex( - range[1], - ), - }, + loc: node.loc!, + messageId: "unexpected", + data: { range: newText }, + }) + continue + } + + for (const range of ranges) { + context.report({ + node, + loc: range.getAstLocation(sourceCode), messageId: "unexpected", - data: { - range: newText, + data: { range: newText }, + fix: (fixer) => { + return ranges.map((r, index) => { + if (index === 0) { + return r.replace(fixer, newText) + } + return r.remove(fixer) + }) }, - fix: ranges - ? (fixer) => { - return ranges.map((r, index) => { - if (index === 0) { - return fixer.replaceTextRange( - r, - newText, - ) - } - return fixer.removeRange(r) - }) - } - : undefined, }) } } diff --git a/lib/rules/prefer-w.ts b/lib/rules/prefer-w.ts index ba345bbaa..594342d85 100644 --- a/lib/rules/prefer-w.ts +++ b/lib/rules/prefer-w.ts @@ -84,8 +84,7 @@ export default createRule("prefer-w", { flags, getRegexpLocation, fixReplaceNode, - getRegexpRange, - fixerApplyEscape, + patternSource, toCharSet, }: RegExpContext): RegExpVisitor.Handlers { return { @@ -162,22 +161,24 @@ export default createRule("prefer-w", { .join("")}]`, instead: "\\w", }, - *fix(fixer: Rule.RuleFixer) { - const range = getRegexpRange(ccNode) - if (range == null) { - return - } - yield fixer.replaceTextRange( - getRegexpRange( - unexpectedElements.shift()!, - )!, - fixerApplyEscape("\\w"), - ) + fix(fixer: Rule.RuleFixer) { + const fixes: Rule.Fix[] = [] for (const element of unexpectedElements) { - yield fixer.removeRange( - getRegexpRange(element)!, + const range = patternSource?.getReplaceRange( + element, ) + if (!range) { + return null + } + + if (fixes.length === 0) { + // first + fixes.push(range.replace(fixer, "\\w")) + } else { + fixes.push(range.remove(fixer)) + } } + return fixes }, }) } diff --git a/lib/rules/sort-character-class-elements.ts b/lib/rules/sort-character-class-elements.ts index b8a46d0f6..937a10b71 100644 --- a/lib/rules/sort-character-class-elements.ts +++ b/lib/rules/sort-character-class-elements.ts @@ -84,9 +84,8 @@ export default createRule("sort-character-class-elements", { */ function createVisitor({ node, - fixerApplyEscape, getRegexpLocation, - getRegexpRange, + patternSource, }: RegExpContext): RegExpVisitor.Handlers { return { onCharacterClassEnter(ccNode) { @@ -112,22 +111,23 @@ export default createRule("sort-character-class-elements", { prev: moveTarget.raw, }, *fix(fixer) { - const nextRange = getRegexpRange(next) - const targetRange = getRegexpRange( + const nextRange = patternSource?.getReplaceRange( + next, + ) + const targetRange = patternSource?.getReplaceRange( moveTarget, ) + if (!targetRange || !nextRange) { return } - yield fixer.insertTextBeforeRange( - targetRange, - fixerApplyEscape( - escapeRaw(next, moveTarget), - ), + yield targetRange.insertBefore( + fixer, + escapeRaw(next, moveTarget), ) - yield fixer.removeRange(nextRange) + yield nextRange.remove(fixer) }, }) } diff --git a/lib/utils/ast-utils/pattern-source.ts b/lib/utils/ast-utils/pattern-source.ts new file mode 100644 index 000000000..629243d7c --- /dev/null +++ b/lib/utils/ast-utils/pattern-source.ts @@ -0,0 +1,390 @@ +import type { Expression, Literal, RegExpLiteral } from "estree" +import type { Rule, AST, SourceCode } from "eslint" +import { getStaticValue } from "." +import { + findVariable, + getParent, + getPropertyName, + getStringValueRange, + isRegexpLiteral, + isStringLiteral, +} from "./utils" + +/** + * The range of a node/construct within a regexp pattern. + */ +export interface PatternRange { + readonly start: number + readonly end: number +} + +export class PatternReplaceRange { + public range: AST.Range + + public type: "RegExp" | "String" + + public constructor(range: AST.Range, type: PatternReplaceRange["type"]) { + if (!range || range[0] < 0 || range[0] > range[1]) { + throw new Error(`Invalid range: ${JSON.stringify(range)}`) + } + + this.range = range + this.type = type + } + + public static fromLiteral( + node: Literal, + sourceCode: SourceCode, + nodeRange: PatternRange, + range: PatternRange, + ): PatternReplaceRange | null { + if (!node.range) { + return null + } + + const start = range.start - nodeRange.start + const end = range.end - nodeRange.start + + if (isRegexpLiteral(node)) { + const nodeStart = node.range[0] + "/".length + return new PatternReplaceRange( + [nodeStart + start, nodeStart + end], + "RegExp", + ) + } + + if (isStringLiteral(node)) { + const astRange = getStringValueRange(sourceCode, node, start, end) + + if (astRange) { + return new PatternReplaceRange(astRange, "String") + } + } + return null + } + + public getAstLocation(sourceCode: SourceCode): AST.SourceLocation { + return astRangeToLocation(sourceCode, this.range) + } + + public escape(text: string): string { + if (this.type === "String") { + return text + .replace(/\\/g, "\\\\") + .replace(/\n/g, "\\n") + .replace(/\r/g, "\\r") + .replace(/\t/g, "\\t") + } + + return text + } + + public replace(fixer: Rule.RuleFixer, text: string): Rule.Fix { + return fixer.replaceTextRange(this.range, this.escape(text)) + } + + public remove(fixer: Rule.RuleFixer): Rule.Fix { + return fixer.removeRange(this.range) + } + + public insertAfter(fixer: Rule.RuleFixer, text: string): Rule.Fix { + return fixer.insertTextAfterRange(this.range, this.escape(text)) + } + + public insertBefore(fixer: Rule.RuleFixer, text: string): Rule.Fix { + return fixer.insertTextBeforeRange(this.range, this.escape(text)) + } +} + +class PatternSegment implements PatternRange { + private readonly sourceCode: SourceCode + + public readonly node: Expression + + public readonly value: string + + public readonly start: number + + public readonly end: number + + public constructor( + sourceCode: SourceCode, + node: Expression, + value: string, + start: number, + ) { + this.sourceCode = sourceCode + this.node = node + this.value = value + this.start = start + this.end = start + value.length + } + + public contains(range: PatternRange): boolean { + return this.start <= range.start && range.end <= this.end + } + + public getReplaceRange(range: PatternRange): PatternReplaceRange | null { + if (!this.contains(range)) { + return null + } + + if (this.node.type === "Literal") { + // This will cover string literals and RegExp literals + return PatternReplaceRange.fromLiteral( + this.node, + this.sourceCode, + this, + range, + ) + } + + // e.g. /foo/.source + if ( + this.node.type === "MemberExpression" && + this.node.object.type !== "Super" && + isRegexpLiteral(this.node.object) && + getPropertyName(this.node) === "source" + ) { + return PatternReplaceRange.fromLiteral( + this.node.object, + this.sourceCode, + this, + range, + ) + } + + return null + } + + public getAstRange(range: PatternRange): AST.Range { + const replaceRange = this.getReplaceRange(range) + if (replaceRange) { + return replaceRange.range + } + + return this.node.range! + } +} + +export class PatternSource { + private readonly sourceCode: SourceCode + + public readonly node: Expression + + public readonly value: string + + private readonly segments: readonly PatternSegment[] + + private constructor( + sourceCode: SourceCode, + node: Expression, + value: string, + items: readonly PatternSegment[], + ) { + this.sourceCode = sourceCode + this.node = node + this.value = value + this.segments = items + } + + public static fromExpression( + context: Rule.RuleContext, + expression: Expression, + ): PatternSource | null { + // eslint-disable-next-line no-param-reassign -- x + expression = dereferenceOwnedConstant(context, expression) + + if (isRegexpLiteral(expression)) { + return PatternSource.fromRegExpLiteral(context, expression) + } + + const sourceCode = context.getSourceCode() + + const items: PatternSegment[] = [] + let value = "" + + for (const e of flattenPlus(context, expression)) { + const staticValue = getStaticValue(context, e) + if (!staticValue || typeof staticValue.value !== "string") { + return null + } + + items.push( + new PatternSegment( + sourceCode, + e, + staticValue.value, + value.length, + ), + ) + value += staticValue.value + } + + return new PatternSource(sourceCode, expression, value, items) + } + + public static fromRegExpLiteral( + context: Rule.RuleContext, + expression: RegExpLiteral, + ): PatternSource { + const sourceCode = context.getSourceCode() + + return new PatternSource( + sourceCode, + expression, + expression.regex.pattern, + [ + new PatternSegment( + sourceCode, + expression, + expression.regex.pattern, + 0, + ), + ], + ) + } + + private getSegment(range: PatternRange): PatternSegment | null { + const segments = this.getSegments(range) + if (segments.length === 1) { + return segments[0] + } + return null + } + + private getSegments(range: PatternRange): PatternSegment[] { + return this.segments.filter( + (item) => item.start < range.end && range.start < item.end, + ) + } + + public getReplaceRange(range: PatternRange): PatternReplaceRange | null { + const segment = this.getSegment(range) + if (segment) { + return segment.getReplaceRange(range) + } + return null + } + + public getAstRange(range: PatternRange): AST.Range { + const overlapping = this.getSegments(range) + + if (overlapping.length === 1) { + return overlapping[0].getAstRange(range) + } + + // the input range comes from multiple sources + // union all their ranges + let min = Infinity + let max = -Infinity + for (const item of overlapping) { + min = Math.min(min, item.node.range![0]) + max = Math.max(max, item.node.range![1]) + } + + if (min > max) { + return this.node.range! + } + + return [min, max] + } + + public getAstLocation(range: PatternRange): AST.SourceLocation { + return astRangeToLocation(this.sourceCode, this.getAstRange(range)) + } +} + +/** + * Flattens binary + expressions into an array. + * + * This will automatically dereference owned constants. + */ +function flattenPlus(context: Rule.RuleContext, e: Expression): Expression[] { + if (e.type === "BinaryExpression" && e.operator === "+") { + return [ + ...flattenPlus(context, e.left), + ...flattenPlus(context, e.right), + ] + } + + const deRef = dereferenceOwnedConstant(context, e) + if (deRef !== e) { + return flattenPlus(context, deRef) + } + + return [e] +} + +/** + * Converts an range into a source location. + */ +function astRangeToLocation( + sourceCode: SourceCode, + range: AST.Range, +): AST.SourceLocation { + return { + start: sourceCode.getLocFromIndex(range[0]), + end: sourceCode.getLocFromIndex(range[1]), + } +} + +/** + * If the given expression is a variables, this will dereference the variables + * if the variable is constant and only referenced by this expression. + * + * This means that only variables that are owned by this expression are + * dereferenced. + * + * In all other cases, the given expression will be returned as is. + * + * @param expression + */ +function dereferenceOwnedConstant( + context: Rule.RuleContext, + expression: Expression, +): Expression { + if (expression.type === "Identifier") { + const variable = findVariable(context, expression) + if (!variable || variable.defs.length !== 1) { + // we want a variable with 1 definition + return expression + } + + const def = variable.defs[0] + if (def.type !== "Variable") { + // we want a variable + return expression + } + + const grandParent = getParent(def.parent) + if (grandParent && grandParent.type === "ExportNamedDeclaration") { + // exported variables are not owned because they can be referenced + // by modules that import this module + return expression + } + + // we expect there two be exactly 2 references: + // 1. for initializing the variable + // 2. the reference given to this function + if (variable.references.length !== 2) { + return expression + } + + const [initRef, thisRef] = variable.references + if ( + !( + initRef.init && + initRef.writeExpr && + initRef.writeExpr === def.node.init + ) || + thisRef.identifier !== expression + ) { + return expression + } + + return def.node.init + } + + return expression +} diff --git a/lib/utils/ast-utils/utils.ts b/lib/utils/ast-utils/utils.ts index ecb1efa86..d4fb5e8f8 100644 --- a/lib/utils/ast-utils/utils.ts +++ b/lib/utils/ast-utils/utils.ts @@ -1,4 +1,4 @@ -import type { Rule } from "eslint" +import type { Rule, SourceCode, AST } from "eslint" import * as eslintUtils from "eslint-utils" import type { ArrowFunctionExpression, @@ -10,8 +10,9 @@ import type { Literal, MemberExpression, Node, + RegExpLiteral, } from "estree" -import { parseStringLiteral } from "../string-literal-parser" +import { parseStringLiteral, parseStringTokens } from "../string-literal-parser" import { baseParseReplacements } from "../replacements-utils" import type { Scope, Variable } from "eslint-scope" @@ -88,9 +89,7 @@ export function getStaticValue( } } } else if (node.type === "MemberExpression") { - const propName: string | null = !node.computed - ? (node.property as Identifier).name - : getStringIfConstant(context, node.property) + const propName = getPropertyName(node, context) if (propName === "source") { const object = getStaticValue(context, node.object) if (object && object.value instanceof RegExp) { @@ -277,3 +276,106 @@ export function parseReplacements( } }) } + +/** + * Creates source range from the given offset range of the value of the given + * string literal. + * + * @param sourceCode The ESLint source code instance. + * @param node The string literal to report. + * @returns + */ +export function getStringValueRange( + sourceCode: SourceCode, + node: Literal & { value: string }, + startOffset: number, + endOffset: number, +): AST.Range | null { + if (!node.range) { + // no range information + return null + } + if (node.value.length < endOffset) { + return null + } + + try { + const raw = sourceCode.text.slice(node.range[0] + 1, node.range[1] - 1) + let valueIndex = 0 + let start: number | null = null + for (const t of parseStringTokens(raw)) { + const endIndex = valueIndex + t.value.length + + // find start + if ( + start == null && + valueIndex <= startOffset && + startOffset < endIndex + ) { + start = t.range[0] + } + + // find end + if ( + start != null && + valueIndex < endOffset && + endOffset <= endIndex + ) { + const end = t.range[1] + const nodeStart = node.range[0] + 1 + return [nodeStart + start, nodeStart + end] + } + + valueIndex = endIndex + } + } catch { + // ignore + } + + return null +} + +/** + * Check if the given expression node is regexp literal. + */ +export function isRegexpLiteral(node: Expression): node is RegExpLiteral { + return node.type === "Literal" && "regex" in node +} + +/** + * Check if the given expression node is string literal. + */ +export function isStringLiteral( + node: Expression, +): node is Literal & { value: string } { + return node.type === "Literal" && typeof node.value === "string" +} + +/** + * Returns the string value of the property name accessed. + * + * This is guaranteed to return `null` for private properties. + * + * @param node + * @returns + */ +export function getPropertyName( + node: MemberExpression, + context?: Rule.RuleContext, +): string | null { + const prop = node.property + if (prop.type === "PrivateIdentifier") { + return null + } + + if (!node.computed) { + return (prop as Identifier).name + } + if (context) { + return getStringIfConstant(context, prop) + } + if (isStringLiteral(prop)) { + return prop.value + } + return null +} diff --git a/lib/utils/index.ts b/lib/utils/index.ts index c48614d8f..b77e9ed37 100644 --- a/lib/utils/index.ts +++ b/lib/utils/index.ts @@ -5,7 +5,6 @@ import type { Element, Node, Pattern, Quantifier } from "regexpp/ast" import { RegExpParser, visitRegExpAST } from "regexpp" import { CALL, CONSTRUCT, ReferenceTracker } from "eslint-utils" import type { Rule, AST, SourceCode } from "eslint" -import { parseStringTokens } from "./string-literal-parser" import { findVariable, getStaticValue, getStringIfConstant } from "./ast-utils" import type { ReadonlyFlags, ToCharSetElement } from "regexp-ast-analysis" // eslint-disable-next-line no-restricted-imports -- Implement RegExpContext#toCharSet @@ -14,6 +13,9 @@ import type { CharSet } from "refa" import { JS } from "refa" import type { UsageOfPattern } from "./get-usage-of-pattern" import { getUsageOfPattern } from "./get-usage-of-pattern" +import { isRegexpLiteral, isStringLiteral } from "./ast-utils/utils" +import type { PatternRange } from "./ast-utils/pattern-source" +import { PatternSource } from "./ast-utils/pattern-source" export * from "./unicode" export type ToCharSet = ( @@ -32,7 +34,7 @@ type RegExpHelpersBase = { * @returns The SourceLocation */ getRegexpLocation: ( - regexpNode: Node, + regexpNode: PatternRange, offsets?: [number, number], ) => AST.SourceLocation @@ -43,12 +45,6 @@ type RegExpHelpersBase = { */ getFlagsLocation: () => AST.SourceLocation - /** - * Escape depending on which node the string applied to fixer is applied. - * @see fixerApplyEscape - */ - fixerApplyEscape: (text: string) => string - /** * Creates a new fix that replaces the given node with a given string. * @@ -81,38 +77,28 @@ type RegExpHelpersBase = { */ getUsageOfPattern: () => UsageOfPattern + pattern: string + patternAst: Pattern + + flags: ReadonlyFlags } export type RegExpHelpersForLiteral = { - /** - * Creates source range from the given regexp node - * @param regexpNode The regexp node to report. - * @returns The SourceLocation - */ - getRegexpRange: (regexpNode: Node) => AST.Range + patternSource: PatternSource } & RegExpHelpersBase export type RegExpHelpersForSource = { - /** - * Creates source range from the given regexp node - * @param regexpNode The regexp node to report. - * @returns The SourceLocation - */ - getRegexpRange: (regexpNode: Node) => AST.Range | null + patternSource: PatternSource | null } & RegExpHelpersBase export type RegExpHelpers = RegExpHelpersForLiteral & RegExpHelpersForSource export type RegExpContextForLiteral = { node: ESTree.RegExpLiteral - pattern: string - flags: ReadonlyFlags flagsString: string ownsFlags: true regexpNode: ESTree.RegExpLiteral } & RegExpHelpersForLiteral export type RegExpContextForSource = { node: ESTree.Expression - pattern: string - flags: ReadonlyFlags flagsString: string | null ownsFlags: boolean regexpNode: ESTree.NewExpression | ESTree.CallExpression @@ -231,7 +217,6 @@ function buildRegexpVisitor( rules: RegexpRule[], programExit: (node: ESTree.Program) => void, ): RuleListener { - const sourceCode = context.getSourceCode() const parser = new RegExpParser() /** @@ -242,6 +227,7 @@ function buildRegexpVisitor( function verify( exprNode: ESTree.Expression, regexpNode: ESTree.RegExpLiteral | ESTree.CallExpression, + patternSource: PatternSource | null, pattern: string, flags: ReadonlyFlags, createVisitors: ( @@ -263,6 +249,7 @@ function buildRegexpVisitor( } const helpers = buildRegExpHelperBase({ + patternSource, exprNode, regexpNode, context, @@ -306,24 +293,30 @@ function buildRegexpVisitor( } const flagsString = node.regex.flags const flags = parseFlags(flagsString) - verify(node, node, node.regex.pattern, flags, function* (helpers) { - const regexpContext: RegExpContextForLiteral = { - node, - pattern: node.regex.pattern, - flags, - flagsString, - ownsFlags: true, - regexpNode: node, - ...helpers, - getRegexpRange: (regexpNode) => - getRegexpRange(sourceCode, node, regexpNode), - } - for (const rule of rules) { - if (rule.createLiteralVisitor) { - yield rule.createLiteralVisitor(regexpContext) + const pattern = node.regex.pattern + const patternSource = PatternSource.fromRegExpLiteral(context, node) + verify( + node, + node, + patternSource, + pattern, + flags, + function* (helpers) { + const regexpContext: RegExpContextForLiteral = { + node, + flagsString, + ownsFlags: true, + patternSource, + regexpNode: node, + ...helpers, } - } - }) + for (const rule of rules) { + if (rule.createLiteralVisitor) { + yield rule.createLiteralVisitor(regexpContext) + } + } + }, + ) }, // eslint-disable-next-line complexity -- X( Program() { @@ -336,6 +329,7 @@ function buildRegexpVisitor( newOrCall: ESTree.NewExpression | ESTree.CallExpression patternNode: ESTree.Expression pattern: string | null + patternSource: PatternSource | null flagsNode: ESTree.Expression | ESTree.SpreadElement | undefined flagsString: string | null ownsFlags: boolean @@ -376,10 +370,16 @@ function buildRegexpVisitor( pattern = String(patternResult.value) } + const patternSource = PatternSource.fromExpression( + context, + patternNode, + ) + regexpDataList.push({ newOrCall, patternNode, pattern, + patternSource, flagsNode, flagsString, ownsFlags, @@ -389,6 +389,7 @@ function buildRegexpVisitor( newOrCall, patternNode, pattern, + patternSource, flagsString, ownsFlags, } of regexpDataList) { @@ -436,23 +437,17 @@ function buildRegexpVisitor( verify( verifyPatternNode, newOrCall, + patternSource, pattern, flags, function* (helpers) { const regexpContext: RegExpContextForSource = { node: verifyPatternNode, - pattern, - flags, flagsString, ownsFlags, regexpNode: newOrCall, + patternSource, ...helpers, - getRegexpRange: (regexpNode) => - getRegexpRange( - sourceCode, - verifyPatternNode, - regexpNode, - ), } for (const rule of rules) { if (rule.createSourceVisitor) { @@ -498,12 +493,14 @@ export function compositingVisitors( * Build RegExpHelperBase */ function buildRegExpHelperBase({ + patternSource, exprNode, regexpNode, context, flags, parsedPattern, }: { + patternSource: PatternSource | null exprNode: ESTree.Expression regexpNode: ESTree.CallExpression | ESTree.RegExpLiteral context: Rule.RuleContext @@ -529,126 +526,43 @@ function buildRegExpHelperBase({ cacheCharSet.set(node, charSet) return charSet }, - getRegexpLocation: (node, offsets) => - getRegexpLocation(sourceCode, exprNode, node, offsets), + getRegexpLocation: (range, offsets) => { + if (!patternSource) { + return exprNode.loc! + } + if (offsets) { + return patternSource.getAstLocation({ + start: range.start + offsets[0], + end: range.start + offsets[1], + }) + } + return patternSource.getAstLocation(range) + }, getFlagsLocation: () => getFlagsLocation(sourceCode, regexpNode), - fixerApplyEscape: (text) => fixerApplyEscape(text, exprNode), - fixReplaceNode: (node, replacement) => - fixReplaceNode(sourceCode, exprNode, node, replacement), - fixReplaceQuant: (qNode, replacement) => - fixReplaceQuant(sourceCode, exprNode, qNode, replacement), - fixReplaceFlags: (newFlags) => - fixReplaceFlags( - sourceCode, - exprNode, - parsedPattern, - regexpNode, - newFlags, - ), + fixReplaceNode: (node, replacement) => { + if (!patternSource) { + return () => null + } + return fixReplaceNode(patternSource, node, replacement) + }, + fixReplaceQuant: (qNode, replacement) => { + if (!patternSource) { + return () => null + } + return fixReplaceQuant(patternSource, qNode, replacement) + }, + fixReplaceFlags: (newFlags) => { + if (!patternSource) { + return () => null + } + return fixReplaceFlags(patternSource, regexpNode, newFlags) + }, getUsageOfPattern: () => (cacheUsageOfPattern ??= getUsageOfPattern(regexpNode, context)), + pattern: parsedPattern.raw, patternAst: parsedPattern, - } -} - -function getRegexpRange( - sourceCode: SourceCode, - node: ESTree.RegExpLiteral, - regexpNode: Node, -): AST.Range -function getRegexpRange( - sourceCode: SourceCode, - node: ESTree.Expression, - regexpNode: Node, -): AST.Range | null -/** - * Creates source range from the given regexp node - * @param sourceCode The ESLint source code instance. - * @param node The node to report. - * @param regexpNode The regexp node to report. - * @returns The SourceLocation - */ -function getRegexpRange( - sourceCode: SourceCode, - node: ESTree.Expression, - regexpNode: Node, - offsets?: [number, number], -): AST.Range | null { - const startOffset = regexpNode.start + (offsets?.[0] ?? 0) - const endOffset = regexpNode.end + (offsets?.[1] ?? 0) - if (isRegexpLiteral(node)) { - const nodeStart = node.range![0] + 1 - return [nodeStart + startOffset, nodeStart + endOffset] - } - if (isStringLiteral(node)) { - let start: number | null = null - let end: number | null = null - try { - const sourceText = sourceCode.text.slice( - node.range![0] + 1, - node.range![1] - 1, - ) - let startIndex = 0 - for (const t of parseStringTokens(sourceText)) { - const endIndex = startIndex + t.value.length - - if ( - start == null && - startIndex <= startOffset && - startOffset < endIndex - ) { - start = t.range[0] - } - if ( - start != null && - end == null && - startIndex < endOffset && - endOffset <= endIndex - ) { - end = t.range[1] - break - } - startIndex = endIndex - } - if (start != null && end != null) { - const nodeStart = node.range![0] + 1 - return [nodeStart + start, nodeStart + end] - } - } catch { - // ignore - } - } - return null -} - -/** - * Creates SourceLocation from the given regexp node - * @param sourceCode The ESLint source code instance. - * @param node The node to report. - * @param regexpNode The regexp node to report. - * @param offsets The report location offsets. - * @returns The SourceLocation - */ -function getRegexpLocation( - sourceCode: SourceCode, - node: ESTree.Expression, - regexpNode: Node, - offsets?: [number, number], -): AST.SourceLocation { - const range = getRegexpRange(sourceCode, node, regexpNode) - if (range == null) { - return node.loc! - } - if (offsets) { - return { - start: sourceCode.getLocFromIndex(range[0] + offsets[0]), - end: sourceCode.getLocFromIndex(range[0] + offsets[1]), - } - } - return { - start: sourceCode.getLocFromIndex(range[0]), - end: sourceCode.getLocFromIndex(range[1]), + flags, } } @@ -702,62 +616,18 @@ function getFlagsLocation( } } -/** - * Check if the given expression node is regexp literal. - */ -export function isRegexpLiteral( - node: ESTree.Expression, -): node is ESTree.RegExpLiteral { - if (node.type !== "Literal") { - return false - } - if (!(node as ESTree.RegExpLiteral).regex) { - return false - } - return true -} - -/** - * Check if the given expression node is string literal. - */ -function isStringLiteral( - node: ESTree.Expression, -): node is ESTree.Literal & { value: string } { - if (node.type !== "Literal") { - return false - } - if (typeof node.value !== "string") { - return false - } - return true -} - -/** - * Escape depending on which node the string applied to fixer is applied. - */ -function fixerApplyEscape(text: string, node: ESTree.Expression): string { - if (node.type !== "Literal") { - throw new Error(`illegal node type:${node.type}`) - } - if (!(node as ESTree.RegExpLiteral).regex) { - return text.replace(/\\/gu, "\\\\") - } - return text -} - /** * Creates a new fix that replaces the given node with a given string. * * The string will automatically be escaped if necessary. */ function fixReplaceNode( - sourceCode: SourceCode, - node: ESTree.Expression, + patternSource: PatternSource, regexpNode: Node, replacement: string | (() => string | null), ) { return (fixer: Rule.RuleFixer): Rule.Fix | null => { - const range = getRegexpRange(sourceCode, node, regexpNode) + const range = patternSource.getReplaceRange(regexpNode) if (range == null) { return null } @@ -772,7 +642,7 @@ function fixReplaceNode( } } - return fixer.replaceTextRange(range, fixerApplyEscape(text, node)) + return range.replace(fixer, text) } } @@ -783,17 +653,11 @@ function fixReplaceNode( * This will not change the greediness of the quantifier. */ function fixReplaceQuant( - sourceCode: SourceCode, - node: ESTree.Expression, + patternSource: PatternSource, quantifier: Quantifier, replacement: string | Quant | (() => string | Quant | null), ) { return (fixer: Rule.RuleFixer): Rule.Fix | null => { - const range = getRegexpRange(sourceCode, node, quantifier) - if (range == null) { - return null - } - let text if (typeof replacement !== "function") { text = replacement @@ -817,10 +681,15 @@ function fixReplaceQuant( text = quantToString(text) } - return fixer.replaceTextRange( - [range[0] + offset[0], range[0] + offset[1]], - text, - ) + const range = patternSource.getReplaceRange({ + start: quantifier.start + offset[0], + end: quantifier.start + offset[1], + }) + if (range == null) { + return null + } + + return range.replace(fixer, text) } } @@ -828,9 +697,7 @@ function fixReplaceQuant( * Returns a new fixer that replaces the current flags with the given flags. */ function fixReplaceFlags( - sourceCode: SourceCode, - patternNode: ESTree.Expression, - pattern: Pattern, + patternSource: PatternSource, regexpNode: ESTree.CallExpression | ESTree.RegExpLiteral, replacement: string | (() => string | null), ) { @@ -868,16 +735,16 @@ function fixReplaceFlags( // fixes that change the pattern generally assume that flags don't // change, so we have to create conflicts. - const patternRange = getRegexpRange(sourceCode, patternNode, pattern) + const patternRange = patternSource.getReplaceRange({ + start: 0, + end: patternSource.value.length, + }) if (patternRange == null) { return null } return [ - fixer.replaceTextRange( - patternRange, - fixerApplyEscape(pattern.raw, patternNode), - ), + patternRange.replace(fixer, patternSource.value), fixer.replaceTextRange(range, newFlags), ] } diff --git a/tests/lib/rules/match-any.ts b/tests/lib/rules/match-any.ts index 6a247d363..c2f103a3f 100644 --- a/tests/lib/rules/match-any.ts +++ b/tests/lib/rules/match-any.ts @@ -162,7 +162,10 @@ tester.run("match-any", rule as any, { const s = "[\\s\\S]"+"[\\S\\s][^]." new RegExp(s, 's') `, - output: null, + output: String.raw` + const s = "[^]"+"[^][^][^]" + new RegExp(s, 's') + `, options: [{ allows: ["[^]"] }], errors: [ "Unexpected using '[\\s\\S]' to match any character.", diff --git a/tests/lib/rules/negation.ts b/tests/lib/rules/negation.ts index f84b386da..dafe63f3b 100644 --- a/tests/lib/rules/negation.ts +++ b/tests/lib/rules/negation.ts @@ -117,9 +117,7 @@ tester.run("negation", rule as any, { code: String.raw`const s ="[^\\w]" new RegExp(s) new RegExp(s)`, - output: String.raw`const s ="\\W" - new RegExp(s) - new RegExp(s)`, + output: null, errors: [ "Unexpected negated character class. Use '\\W' instead.", "Unexpected negated character class. Use '\\W' instead.", diff --git a/tests/lib/rules/no-empty-alternative.ts b/tests/lib/rules/no-empty-alternative.ts index 5d4160ece..b8397022a 100644 --- a/tests/lib/rules/no-empty-alternative.ts +++ b/tests/lib/rules/no-empty-alternative.ts @@ -27,7 +27,7 @@ tester.run("no-empty-alternative", rule as any, { { message: "No empty alternatives. Use quantifiers instead.", line: 1, - column: 9, + column: 8, }, ], }, diff --git a/tests/lib/rules/no-useless-character-class.ts b/tests/lib/rules/no-useless-character-class.ts index 92b022e0d..0f17d4794 100644 --- a/tests/lib/rules/no-useless-character-class.ts +++ b/tests/lib/rules/no-useless-character-class.ts @@ -140,8 +140,8 @@ tester.run("no-useless-character-class", rule as any, { errors: 18, }, { - code: String.raw`new RegExp("[.] [*] [+] [?] [\\^] [=] [!] [:]"+" [$] [{] [}] [(] [)] [|] [[] [\\]] [/] [\\\\]")`, - output: null, + code: String.raw`new RegExp("[.] [*] [+] [?] [\\^] [=] [!] [:]" + " [$] [{] [}] [(] [)] [|] [[] [\\]] [/] [\\\\]")`, + output: String.raw`new RegExp("\\. \\* \\+ \\? \\^ = ! :" + " \\$ \\{ } \\( \\) \\| \\[ \\] \\/ \\\\")`, options: [{ ignores: [] }], errors: 18, }, diff --git a/tests/lib/rules/no-useless-lazy.ts b/tests/lib/rules/no-useless-lazy.ts index 50bc2f541..9d6029a04 100644 --- a/tests/lib/rules/no-useless-lazy.ts +++ b/tests/lib/rules/no-useless-lazy.ts @@ -62,7 +62,8 @@ tester.run("no-useless-lazy", rule as any, { { code: String.raw`const s = "\\d"+"{1}?" new RegExp(s)`, - output: null, + output: String.raw`const s = "\\d"+"{1}" + new RegExp(s)`, errors: [{ messageId: "constant" }], }, diff --git a/tests/lib/rules/sort-character-class-elements.ts b/tests/lib/rules/sort-character-class-elements.ts index 2506f0dab..e97b1ac5e 100644 --- a/tests/lib/rules/sort-character-class-elements.ts +++ b/tests/lib/rules/sort-character-class-elements.ts @@ -219,7 +219,8 @@ tester.run("sort-character-class-elements", rule as any, { { code: String.raw`const s = "[\\d"+"\\w]" new RegExp(s, 'u')`, - output: null, + output: String.raw`const s = "[\\w\\d"+"]" + new RegExp(s, 'u')`, options: [{ order: [] }], errors: [ "Expected character class elements to be in ascending order. '\\w' should be before '\\d'.", diff --git a/tests/lib/utils/ast-utils/extract-expression-references.ts b/tests/lib/utils/ast-utils/extract-expression-references.ts index baaac8cc3..e6629bd5b 100644 --- a/tests/lib/utils/ast-utils/extract-expression-references.ts +++ b/tests/lib/utils/ast-utils/extract-expression-references.ts @@ -5,7 +5,7 @@ import type * as ESTree from "estree" import { CALL, CONSTRUCT, ReferenceTracker } from "eslint-utils" import type { ExpressionReference } from "../../../../lib/utils/ast-utils" import { extractExpressionReferences } from "../../../../lib/utils/ast-utils" -import { isRegexpLiteral } from "../../../../lib/utils" +import { isRegexpLiteral } from "../../../../lib/utils/ast-utils/utils" type ExpressionReferenceResult = { type: string; [key: string]: any } @@ -81,7 +81,7 @@ const TESTCASES: TestCase[] = [ { code: `const a = /a/ fn(a) - + function fn(b) { b.source }`, results: [ [{ type: "member", node: "b", memberExpression: "b.source" }], diff --git a/tests/lib/utils/ast-utils/extract-property-references.ts b/tests/lib/utils/ast-utils/extract-property-references.ts index 50e328601..15be5b832 100644 --- a/tests/lib/utils/ast-utils/extract-property-references.ts +++ b/tests/lib/utils/ast-utils/extract-property-references.ts @@ -4,7 +4,7 @@ import type * as ESTree from "estree" import { CALL, CONSTRUCT, ReferenceTracker } from "eslint-utils" import type { PropertyReference } from "../../../../lib/utils/ast-utils" import { extractPropertyReferences } from "../../../../lib/utils/ast-utils" -import { isRegexpLiteral } from "../../../../lib/utils" +import { isRegexpLiteral } from "../../../../lib/utils/ast-utils/utils" type PropertyReferenceResult = { [key: string]: { type: string; refs?: PropertyReferenceResult } diff --git a/tests/lib/utils/get-usage-of-pattern.ts b/tests/lib/utils/get-usage-of-pattern.ts index 62d1c1bdb..b62858b3c 100644 --- a/tests/lib/utils/get-usage-of-pattern.ts +++ b/tests/lib/utils/get-usage-of-pattern.ts @@ -1,7 +1,7 @@ import { Linter } from "eslint" import assert from "assert" import type * as ESTree from "estree" -import { isRegexpLiteral } from "../../../lib/utils" +import { isRegexpLiteral } from "../../../lib/utils/ast-utils/utils" import { getUsageOfPattern, UsageOfPattern, @@ -106,7 +106,7 @@ const TESTCASES: TestCase[] = [ code: ` getSource(/a/) toString(/b/) - + function getSource(p) { return p.source } @@ -142,7 +142,7 @@ const TESTCASES: TestCase[] = [ code: ` getSource(42, /a/) getSource(/b/, 42) - + function getSource(p, p2) { return p2.source } @@ -152,7 +152,7 @@ const TESTCASES: TestCase[] = [ { code: ` fn(/a/) - + function fn(p) { return fn(p) } @@ -164,7 +164,7 @@ const TESTCASES: TestCase[] = [ const fn = getSource fn(42, /a/) fn(/b/, 42) - + function getSource(p, p2) { return p2.source } @@ -174,7 +174,7 @@ const TESTCASES: TestCase[] = [ { code: ` getSource(42, /a/) - + function getSource(p) { return p.source } From 36dac6aa0faed2bfab52f17b5aa6f92bd86da4af Mon Sep 17 00:00:00 2001 From: RunDevelopment Date: Sun, 8 Aug 2021 17:25:28 +0200 Subject: [PATCH 2/7] Removed now useless code --- lib/utils/index.ts | 46 +++------------------------------------------- 1 file changed, 3 insertions(+), 43 deletions(-) diff --git a/lib/utils/index.ts b/lib/utils/index.ts index b77e9ed37..0d83bcf52 100644 --- a/lib/utils/index.ts +++ b/lib/utils/index.ts @@ -5,7 +5,7 @@ import type { Element, Node, Pattern, Quantifier } from "regexpp/ast" import { RegExpParser, visitRegExpAST } from "regexpp" import { CALL, CONSTRUCT, ReferenceTracker } from "eslint-utils" import type { Rule, AST, SourceCode } from "eslint" -import { findVariable, getStaticValue, getStringIfConstant } from "./ast-utils" +import { getStaticValue, getStringIfConstant } from "./ast-utils" import type { ReadonlyFlags, ToCharSetElement } from "regexp-ast-analysis" // eslint-disable-next-line no-restricted-imports -- Implement RegExpContext#toCharSet import { toCharSet } from "regexp-ast-analysis" @@ -318,7 +318,6 @@ function buildRegexpVisitor( }, ) }, - // eslint-disable-next-line complexity -- X( Program() { const tracker = new ReferenceTracker(context.getScope()) @@ -394,55 +393,16 @@ function buildRegexpVisitor( ownsFlags, } of regexpDataList) { if (typeof pattern === "string") { - let verifyPatternNode = patternNode - if (patternNode.type === "Identifier") { - const variable = findVariable(context, patternNode) - if (variable && variable.defs.length === 1) { - const def = variable.defs[0] - if ( - def.type === "Variable" && - def.parent.kind === "const" && - def.node.init && - def.node.init.type === "Literal" - ) { - let useInit = false - if (variable.references.length > 2) { - if ( - variable.references.every((ref) => { - if (ref.isWriteOnly()) { - return true - } - return regexpDataList.some( - (r) => - r.patternNode === - ref.identifier && - r.flagsString === - flagsString, - ) - }) - ) { - useInit = true - } - } else { - useInit = true - } - - if (useInit) { - verifyPatternNode = def.node.init - } - } - } - } const flags = parseFlags(flagsString || "") verify( - verifyPatternNode, + patternNode, newOrCall, patternSource, pattern, flags, function* (helpers) { const regexpContext: RegExpContextForSource = { - node: verifyPatternNode, + node: patternNode, flagsString, ownsFlags, regexpNode: newOrCall, From 6723a15f893d02aa9f5ca12da776584902d1c121 Mon Sep 17 00:00:00 2001 From: RunDevelopment Date: Sun, 8 Aug 2021 17:25:51 +0200 Subject: [PATCH 3/7] Recursively resolve constants --- lib/utils/ast-utils/pattern-source.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/utils/ast-utils/pattern-source.ts b/lib/utils/ast-utils/pattern-source.ts index 629243d7c..1a1cea3e4 100644 --- a/lib/utils/ast-utils/pattern-source.ts +++ b/lib/utils/ast-utils/pattern-source.ts @@ -383,7 +383,7 @@ function dereferenceOwnedConstant( return expression } - return def.node.init + return dereferenceOwnedConstant(context, def.node.init) } return expression From cb2e63cc7e94c0edfe8658c5be149dd074df40d2 Mon Sep 17 00:00:00 2001 From: RunDevelopment Date: Sun, 8 Aug 2021 17:35:17 +0200 Subject: [PATCH 4/7] Added more doc --- lib/utils/ast-utils/pattern-source.ts | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/lib/utils/ast-utils/pattern-source.ts b/lib/utils/ast-utils/pattern-source.ts index 1a1cea3e4..a87e55990 100644 --- a/lib/utils/ast-utils/pattern-source.ts +++ b/lib/utils/ast-utils/pattern-source.ts @@ -18,6 +18,9 @@ export interface PatternRange { readonly end: number } +/** + * A range in source code that can be edited. + */ export class PatternReplaceRange { public range: AST.Range @@ -267,6 +270,12 @@ export class PatternSource { return null } + /** + * Returns an approximate AST range for the given pattern range. + * + * DO NOT use this in fixes to edit source code. Use + * {@link PatternSource.getReplaceRange} instead. + */ public getAstRange(range: PatternRange): AST.Range { const overlapping = this.getSegments(range) @@ -290,6 +299,12 @@ export class PatternSource { return [min, max] } + /** + * Returns an approximate AST source location for the given pattern range. + * + * DO NOT use this in fixes to edit source code. Use + * {@link PatternSource.getReplaceRange} instead. + */ public getAstLocation(range: PatternRange): AST.SourceLocation { return astRangeToLocation(this.sourceCode, this.getAstRange(range)) } From 845d7403c19215c03b093a347a9cf88e330e4de3 Mon Sep 17 00:00:00 2001 From: RunDevelopment Date: Sun, 8 Aug 2021 19:01:52 +0200 Subject: [PATCH 5/7] Use pattern source to determine pattern string --- lib/rules/match-any.ts | 2 +- lib/rules/no-useless-lazy.ts | 2 +- lib/rules/optimal-quantifier-concatenation.ts | 8 +- lib/rules/prefer-quantifier.ts | 4 +- lib/rules/prefer-range.ts | 2 +- lib/rules/prefer-w.ts | 2 +- lib/rules/sort-character-class-elements.ts | 4 +- lib/utils/ast-utils/pattern-source.ts | 165 +++++++++--------- lib/utils/ast-utils/utils.ts | 73 ++++++++ lib/utils/index.ts | 151 ++++++---------- 10 files changed, 223 insertions(+), 190 deletions(-) diff --git a/lib/rules/match-any.ts b/lib/rules/match-any.ts index 208e0aece..f362dfe72 100644 --- a/lib/rules/match-any.ts +++ b/lib/rules/match-any.ts @@ -70,7 +70,7 @@ export default createRule("match-any", { { node, flags, patternSource }: RegExpContext, regexpNode: RegExpNode, ): null | Rule.Fix | Rule.Fix[] { - if (!preference || !patternSource) { + if (!preference) { return null } diff --git a/lib/rules/no-useless-lazy.ts b/lib/rules/no-useless-lazy.ts index 55f895791..0459baf90 100644 --- a/lib/rules/no-useless-lazy.ts +++ b/lib/rules/no-useless-lazy.ts @@ -19,7 +19,7 @@ function makeGreedy({ patternSource }: RegExpContext, qNode: Quantifier) { return null } - const range = patternSource?.getReplaceRange({ + const range = patternSource.getReplaceRange({ start: qNode.end - 1, end: qNode.end, }) diff --git a/lib/rules/optimal-quantifier-concatenation.ts b/lib/rules/optimal-quantifier-concatenation.ts index f216dbc70..e4c30fcf3 100644 --- a/lib/rules/optimal-quantifier-concatenation.ts +++ b/lib/rules/optimal-quantifier-concatenation.ts @@ -490,8 +490,8 @@ function getLoc( left: Element, right: Element, { patternSource }: RegExpContext, -): AST.SourceLocation | undefined { - return patternSource?.getAstLocation({ +): AST.SourceLocation { + return patternSource.getAstLocation({ start: Math.min(left.start, right.start), end: Math.max(left.end, right.end), }) @@ -559,9 +559,7 @@ export default createRule("optimal-quantifier-concatenation", { if (replacement.type === "Both") { context.report({ node, - loc: - getLoc(left, right, regexpContext) ?? - getRegexpLocation(aNode), + loc: getLoc(left, right, regexpContext), messageId: replacement.messageId, data: { left: left.raw, diff --git a/lib/rules/prefer-quantifier.ts b/lib/rules/prefer-quantifier.ts index 68e380e36..050a51f28 100644 --- a/lib/rules/prefer-quantifier.ts +++ b/lib/rules/prefer-quantifier.ts @@ -158,7 +158,7 @@ export default createRule("prefer-quantifier", { context.report({ node, - loc: patternSource?.getAstLocation(bufferRange), + loc: patternSource.getAstLocation(bufferRange), messageId: "unexpected", data: { type: @@ -170,7 +170,7 @@ export default createRule("prefer-quantifier", { quantifier: buffer.getQuantifier(), }, fix(fixer) { - const range = patternSource?.getReplaceRange( + const range = patternSource.getReplaceRange( bufferRange, ) if (!range) { diff --git a/lib/rules/prefer-range.ts b/lib/rules/prefer-range.ts index 609f6bffa..c9cae541f 100644 --- a/lib/rules/prefer-range.ts +++ b/lib/rules/prefer-range.ts @@ -61,7 +61,7 @@ export default createRule("prefer-range", { ): PatternReplaceRange[] | null { const ranges: PatternReplaceRange[] = [] for (const reportNode of nodes) { - const reportRange = patternSource?.getReplaceRange( + const reportRange = patternSource.getReplaceRange( reportNode, ) if (!reportRange) { diff --git a/lib/rules/prefer-w.ts b/lib/rules/prefer-w.ts index 594342d85..1c08c0b88 100644 --- a/lib/rules/prefer-w.ts +++ b/lib/rules/prefer-w.ts @@ -164,7 +164,7 @@ export default createRule("prefer-w", { fix(fixer: Rule.RuleFixer) { const fixes: Rule.Fix[] = [] for (const element of unexpectedElements) { - const range = patternSource?.getReplaceRange( + const range = patternSource.getReplaceRange( element, ) if (!range) { diff --git a/lib/rules/sort-character-class-elements.ts b/lib/rules/sort-character-class-elements.ts index 937a10b71..f03a64227 100644 --- a/lib/rules/sort-character-class-elements.ts +++ b/lib/rules/sort-character-class-elements.ts @@ -111,10 +111,10 @@ export default createRule("sort-character-class-elements", { prev: moveTarget.raw, }, *fix(fixer) { - const nextRange = patternSource?.getReplaceRange( + const nextRange = patternSource.getReplaceRange( next, ) - const targetRange = patternSource?.getReplaceRange( + const targetRange = patternSource.getReplaceRange( moveTarget, ) diff --git a/lib/utils/ast-utils/pattern-source.ts b/lib/utils/ast-utils/pattern-source.ts index a87e55990..a0d7820ec 100644 --- a/lib/utils/ast-utils/pattern-source.ts +++ b/lib/utils/ast-utils/pattern-source.ts @@ -2,8 +2,8 @@ import type { Expression, Literal, RegExpLiteral } from "estree" import type { Rule, AST, SourceCode } from "eslint" import { getStaticValue } from "." import { - findVariable, - getParent, + dereferenceOwnedVariable, + astRangeToLocation, getPropertyName, getStringValueRange, isRegexpLiteral, @@ -170,6 +170,19 @@ class PatternSegment implements PatternRange { } } +export interface RegExpValue { + readonly source: string + readonly flags: string + /** + * If the RegExp object is an owned RegExp literal, then this value will be + * non-null. + * + * If the RegExp object is shared or not created a literal, this will be + * `null`. + */ + readonly ownedNode: RegExpLiteral | null +} + export class PatternSource { private readonly sourceCode: SourceCode @@ -179,16 +192,34 @@ export class PatternSource { private readonly segments: readonly PatternSegment[] + /** + * If the pattern of a regexp is defined by a RegExp object, this value + * will be non-null. This is the case for simple RegExp literals + * (e.g. `/foo/`) and RegExp constructors (e.g. `RegExp(/foo/, "i")`). + * + * If the pattern source is defined by a string value + * (e.g. `RegExp("foo")`), then this will be `null`. + */ + public readonly regexpValue: RegExpValue | null + + public isStringValue(): this is PatternSource & { + readonly regexpValue: null + } { + return this.regexpValue === null + } + private constructor( sourceCode: SourceCode, node: Expression, value: string, - items: readonly PatternSegment[], + segments: readonly PatternSegment[], + regexpValue: RegExpValue | null, ) { this.sourceCode = sourceCode this.node = node this.value = value - this.segments = items + this.segments = segments + this.regexpValue = regexpValue } public static fromExpression( @@ -196,7 +227,7 @@ export class PatternSource { expression: Expression, ): PatternSource | null { // eslint-disable-next-line no-param-reassign -- x - expression = dereferenceOwnedConstant(context, expression) + expression = dereferenceOwnedVariable(context, expression) if (isRegexpLiteral(expression)) { return PatternSource.fromRegExpLiteral(context, expression) @@ -204,12 +235,29 @@ export class PatternSource { const sourceCode = context.getSourceCode() + const flat = flattenPlus(context, expression) + const items: PatternSegment[] = [] let value = "" - for (const e of flattenPlus(context, expression)) { + for (const e of flat) { const staticValue = getStaticValue(context, e) - if (!staticValue || typeof staticValue.value !== "string") { + if (!staticValue) { + return null + } + + if (flat.length === 1 && staticValue.value instanceof RegExp) { + // This means we have a non-owned reference to something that + // evaluates to an RegExp object + return PatternSource.fromRegExpObject( + context, + e, + staticValue.value.source, + staticValue.value.flags, + ) + } + + if (typeof staticValue.value !== "string") { return null } @@ -224,7 +272,28 @@ export class PatternSource { value += staticValue.value } - return new PatternSource(sourceCode, expression, value, items) + return new PatternSource(sourceCode, expression, value, items, null) + } + + private static fromRegExpObject( + context: Rule.RuleContext, + expression: Expression, + source: string, + flags: string, + ): PatternSource { + const sourceCode = context.getSourceCode() + + return new PatternSource( + sourceCode, + expression, + source, + [new PatternSegment(sourceCode, expression, source, 0)], + { + source, + flags, + ownedNode: null, + }, + ) } public static fromRegExpLiteral( @@ -245,6 +314,11 @@ export class PatternSource { 0, ), ], + { + source: expression.regex.pattern, + flags: expression.regex.flags, + ownedNode: expression, + }, ) } @@ -323,83 +397,10 @@ function flattenPlus(context: Rule.RuleContext, e: Expression): Expression[] { ] } - const deRef = dereferenceOwnedConstant(context, e) + const deRef = dereferenceOwnedVariable(context, e) if (deRef !== e) { return flattenPlus(context, deRef) } return [e] } - -/** - * Converts an range into a source location. - */ -function astRangeToLocation( - sourceCode: SourceCode, - range: AST.Range, -): AST.SourceLocation { - return { - start: sourceCode.getLocFromIndex(range[0]), - end: sourceCode.getLocFromIndex(range[1]), - } -} - -/** - * If the given expression is a variables, this will dereference the variables - * if the variable is constant and only referenced by this expression. - * - * This means that only variables that are owned by this expression are - * dereferenced. - * - * In all other cases, the given expression will be returned as is. - * - * @param expression - */ -function dereferenceOwnedConstant( - context: Rule.RuleContext, - expression: Expression, -): Expression { - if (expression.type === "Identifier") { - const variable = findVariable(context, expression) - if (!variable || variable.defs.length !== 1) { - // we want a variable with 1 definition - return expression - } - - const def = variable.defs[0] - if (def.type !== "Variable") { - // we want a variable - return expression - } - - const grandParent = getParent(def.parent) - if (grandParent && grandParent.type === "ExportNamedDeclaration") { - // exported variables are not owned because they can be referenced - // by modules that import this module - return expression - } - - // we expect there two be exactly 2 references: - // 1. for initializing the variable - // 2. the reference given to this function - if (variable.references.length !== 2) { - return expression - } - - const [initRef, thisRef] = variable.references - if ( - !( - initRef.init && - initRef.writeExpr && - initRef.writeExpr === def.node.init - ) || - thisRef.identifier !== expression - ) { - return expression - } - - return dereferenceOwnedConstant(context, def.node.init) - } - - return expression -} diff --git a/lib/utils/ast-utils/utils.ts b/lib/utils/ast-utils/utils.ts index d4fb5e8f8..269c0b551 100644 --- a/lib/utils/ast-utils/utils.ts +++ b/lib/utils/ast-utils/utils.ts @@ -379,3 +379,76 @@ export function getPropertyName( } return null } + +/** + * Converts an range into a source location. + */ +export function astRangeToLocation( + sourceCode: SourceCode, + range: AST.Range, +): AST.SourceLocation { + return { + start: sourceCode.getLocFromIndex(range[0]), + end: sourceCode.getLocFromIndex(range[1]), + } +} + +/** + * If the given expression is a variables, this will dereference the variables + * if the variable is constant and only referenced by this expression. + * + * This means that only variables that are owned by this expression are + * dereferenced. + * + * In all other cases, the given expression will be returned as is. + * + * @param expression + */ +export function dereferenceOwnedVariable( + context: Rule.RuleContext, + expression: Expression, +): Expression { + if (expression.type === "Identifier") { + const variable = findVariable(context, expression) + if (!variable || variable.defs.length !== 1) { + // we want a variable with 1 definition + return expression + } + + const def = variable.defs[0] + if (def.type !== "Variable") { + // we want a variable + return expression + } + + const grandParent = getParent(def.parent) + if (grandParent && grandParent.type === "ExportNamedDeclaration") { + // exported variables are not owned because they can be referenced + // by modules that import this module + return expression + } + + // we expect there two be exactly 2 references: + // 1. for initializing the variable + // 2. the reference given to this function + if (variable.references.length !== 2) { + return expression + } + + const [initRef, thisRef] = variable.references + if ( + !( + initRef.init && + initRef.writeExpr && + initRef.writeExpr === def.node.init + ) || + thisRef.identifier !== expression + ) { + return expression + } + + return dereferenceOwnedVariable(context, def.node.init) + } + + return expression +} diff --git a/lib/utils/index.ts b/lib/utils/index.ts index 0d83bcf52..7f7369a8a 100644 --- a/lib/utils/index.ts +++ b/lib/utils/index.ts @@ -5,7 +5,7 @@ import type { Element, Node, Pattern, Quantifier } from "regexpp/ast" import { RegExpParser, visitRegExpAST } from "regexpp" import { CALL, CONSTRUCT, ReferenceTracker } from "eslint-utils" import type { Rule, AST, SourceCode } from "eslint" -import { getStaticValue, getStringIfConstant } from "./ast-utils" +import { getStringIfConstant } from "./ast-utils" import type { ReadonlyFlags, ToCharSetElement } from "regexp-ast-analysis" // eslint-disable-next-line no-restricted-imports -- Implement RegExpContext#toCharSet import { toCharSet } from "regexp-ast-analysis" @@ -78,31 +78,24 @@ type RegExpHelpersBase = { getUsageOfPattern: () => UsageOfPattern pattern: string - patternAst: Pattern + patternSource: PatternSource flags: ReadonlyFlags } -export type RegExpHelpersForLiteral = { - patternSource: PatternSource -} & RegExpHelpersBase -export type RegExpHelpersForSource = { - patternSource: PatternSource | null -} & RegExpHelpersBase -export type RegExpHelpers = RegExpHelpersForLiteral & RegExpHelpersForSource export type RegExpContextForLiteral = { node: ESTree.RegExpLiteral flagsString: string ownsFlags: true regexpNode: ESTree.RegExpLiteral -} & RegExpHelpersForLiteral +} & RegExpHelpersBase export type RegExpContextForSource = { node: ESTree.Expression flagsString: string | null ownsFlags: boolean regexpNode: ESTree.NewExpression | ESTree.CallExpression -} & RegExpHelpersForSource +} & RegExpHelpersBase export type RegExpContext = RegExpContextForLiteral | RegExpContextForSource type RegexpRule = { @@ -227,8 +220,7 @@ function buildRegexpVisitor( function verify( exprNode: ESTree.Expression, regexpNode: ESTree.RegExpLiteral | ESTree.CallExpression, - patternSource: PatternSource | null, - pattern: string, + patternSource: PatternSource, flags: ReadonlyFlags, createVisitors: ( helpers: RegExpHelpersBase, @@ -238,9 +230,9 @@ function buildRegexpVisitor( try { parsedPattern = parser.parsePattern( - pattern, + patternSource.value, 0, - pattern.length, + patternSource.value.length, flags.unicode, ) } catch { @@ -293,30 +285,21 @@ function buildRegexpVisitor( } const flagsString = node.regex.flags const flags = parseFlags(flagsString) - const pattern = node.regex.pattern const patternSource = PatternSource.fromRegExpLiteral(context, node) - verify( - node, - node, - patternSource, - pattern, - flags, - function* (helpers) { - const regexpContext: RegExpContextForLiteral = { - node, - flagsString, - ownsFlags: true, - patternSource, - regexpNode: node, - ...helpers, - } - for (const rule of rules) { - if (rule.createLiteralVisitor) { - yield rule.createLiteralVisitor(regexpContext) - } + verify(node, node, patternSource, flags, function* (helpers) { + const regexpContext: RegExpContextForLiteral = { + node, + flagsString, + ownsFlags: true, + regexpNode: node, + ...helpers, + } + for (const rule of rules) { + if (rule.createLiteralVisitor) { + yield rule.createLiteralVisitor(regexpContext) } - }, - ) + } + }) }, Program() { const tracker = new ReferenceTracker(context.getScope()) @@ -327,9 +310,7 @@ function buildRegexpVisitor( const regexpDataList: { newOrCall: ESTree.NewExpression | ESTree.CallExpression patternNode: ESTree.Expression - pattern: string | null - patternSource: PatternSource | null - flagsNode: ESTree.Expression | ESTree.SpreadElement | undefined + patternSource: PatternSource flagsString: string | null ownsFlags: boolean }[] = [] @@ -343,43 +324,29 @@ function buildRegexpVisitor( if (!patternNode || patternNode.type === "SpreadElement") { continue } - const patternResult = getStaticValue(context, patternNode) - if (!patternResult || patternResult.value == null) { - continue - } - let pattern: string | null = null - let { flagsString, ownsFlags } = - flagsNode && flagsNode.type !== "SpreadElement" - ? { - flagsString: getStringIfConstant( - context, - flagsNode, - ), - ownsFlags: isStringLiteral(flagsNode), - } - : { flagsString: null, ownsFlags: false } - if (patternResult.value instanceof RegExp) { - pattern = patternResult.value.source - if (!flagsNode) { - // If no flag is given, the flag is also cloned. - flagsString = patternResult.value.flags - ownsFlags = true - } - } else { - pattern = String(patternResult.value) - } const patternSource = PatternSource.fromExpression( context, patternNode, ) + if (!patternSource) { + continue + } + + let flagsString = null + let ownsFlags = false + if (flagsNode && flagsNode.type !== "SpreadElement") { + flagsString = getStringIfConstant(context, flagsNode) + ownsFlags = isStringLiteral(flagsNode) + } else if (patternSource.regexpValue) { + flagsString = patternSource.regexpValue.flags + ownsFlags = Boolean(patternSource.regexpValue.ownedNode) + } regexpDataList.push({ newOrCall, patternNode, - pattern, patternSource, - flagsNode, flagsString, ownsFlags, }) @@ -387,38 +354,31 @@ function buildRegexpVisitor( for (const { newOrCall, patternNode, - pattern, patternSource, flagsString, ownsFlags, } of regexpDataList) { - if (typeof pattern === "string") { - const flags = parseFlags(flagsString || "") - verify( - patternNode, - newOrCall, - patternSource, - pattern, - flags, - function* (helpers) { - const regexpContext: RegExpContextForSource = { - node: patternNode, - flagsString, - ownsFlags, - regexpNode: newOrCall, - patternSource, - ...helpers, - } - for (const rule of rules) { - if (rule.createSourceVisitor) { - yield rule.createSourceVisitor( - regexpContext, - ) - } + const flags = parseFlags(flagsString || "") + verify( + patternNode, + newOrCall, + patternSource, + flags, + function* (helpers) { + const regexpContext: RegExpContextForSource = { + node: patternNode, + flagsString, + ownsFlags, + regexpNode: newOrCall, + ...helpers, + } + for (const rule of rules) { + if (rule.createSourceVisitor) { + yield rule.createSourceVisitor(regexpContext) } - }, - ) - } + } + }, + ) } }, } @@ -460,7 +420,7 @@ function buildRegExpHelperBase({ flags, parsedPattern, }: { - patternSource: PatternSource | null + patternSource: PatternSource exprNode: ESTree.Expression regexpNode: ESTree.CallExpression | ESTree.RegExpLiteral context: Rule.RuleContext @@ -522,6 +482,7 @@ function buildRegExpHelperBase({ pattern: parsedPattern.raw, patternAst: parsedPattern, + patternSource, flags, } } From 105e8f052debd32e8e4ebbbadf2fb8bc661d1f83 Mon Sep 17 00:00:00 2001 From: RunDevelopment Date: Mon, 9 Aug 2021 12:35:47 +0200 Subject: [PATCH 6/7] Improved dereferenceOwnedVariable --- lib/utils/ast-utils/utils.ts | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/lib/utils/ast-utils/utils.ts b/lib/utils/ast-utils/utils.ts index 269c0b551..e10110deb 100644 --- a/lib/utils/ast-utils/utils.ts +++ b/lib/utils/ast-utils/utils.ts @@ -394,15 +394,27 @@ export function astRangeToLocation( } /** - * If the given expression is a variables, this will dereference the variables - * if the variable is constant and only referenced by this expression. + * If the given expression is the identifier of an owned variable, then the + * value of the variable will be returned. * - * This means that only variables that are owned by this expression are - * dereferenced. + * Owned means that the variable is readonly and only referenced by this + * expression. * * In all other cases, the given expression will be returned as is. * - * @param expression + * Note: This will recursively dereference owned variables. I.e. of the given + * identifier resolves to a variable `a` that is assigned an owned variable `b`, + * then this will return the value of `b`. Example: + * + * ```js + * const c = 5; + * const b = c; + * const a = b; + * + * foo(a); + * ``` + * + * Dereferencing `a` in `foo(a)` will return `5`. */ export function dereferenceOwnedVariable( context: Rule.RuleContext, @@ -416,8 +428,8 @@ export function dereferenceOwnedVariable( } const def = variable.defs[0] - if (def.type !== "Variable") { - // we want a variable + if (def.type !== "Variable" || def.node.id.type !== "Identifier") { + // we want a simple variable return expression } From 35b65031fa6b30dc88ed7012bdaa7a8e8babf0bb Mon Sep 17 00:00:00 2001 From: RunDevelopment Date: Mon, 9 Aug 2021 13:52:33 +0200 Subject: [PATCH 7/7] Smarter variable dereferencing --- lib/utils/ast-utils/utils.ts | 120 +++++++++++++++++++++++++++-------- 1 file changed, 93 insertions(+), 27 deletions(-) diff --git a/lib/utils/ast-utils/utils.ts b/lib/utils/ast-utils/utils.ts index e10110deb..e73026594 100644 --- a/lib/utils/ast-utils/utils.ts +++ b/lib/utils/ast-utils/utils.ts @@ -1,4 +1,4 @@ -import type { Rule, SourceCode, AST } from "eslint" +import type { Rule, SourceCode, AST, Scope } from "eslint" import * as eslintUtils from "eslint-utils" import type { ArrowFunctionExpression, @@ -14,7 +14,6 @@ import type { } from "estree" import { parseStringLiteral, parseStringTokens } from "../string-literal-parser" import { baseParseReplacements } from "../replacements-utils" -import type { Scope, Variable } from "eslint-scope" /** * Get a parent node @@ -34,10 +33,44 @@ export function getParent(node: Node | null): E | null { export function findVariable( context: Rule.RuleContext, node: Identifier, -): Variable | null { +): Scope.Variable | null { return eslintUtils.findVariable(getScope(context, node), node) } +type SimpleVariable = Scope.Variable & { + defs: [ + Scope.Definition & { type: "Variable" } & { node: { id: Identifier } }, + ] +} + +/** + * Finds a variable of the form `{var,let,const} identifier ( = )?`. + * + * The returned variable is also guaranteed to have exactly one definition. + * + * @param context + * @param expression + */ +function findSimpleVariable( + context: Rule.RuleContext, + identifier: Identifier, +): SimpleVariable | null { + const variable = findVariable(context, identifier) + + if (!variable || variable.defs.length !== 1) { + // we want a variable with 1 definition + return null + } + + const def = variable.defs[0] + if (def.type !== "Variable" || def.node.id.type !== "Identifier") { + // we want a simple variable + return null + } + + return variable as SimpleVariable +} + /** * Get the value of a given node if it's a constant of string. */ @@ -62,12 +95,10 @@ type GetStaticValueResult = | { value: unknown } | { value: undefined; optional?: true } -/* eslint-disable complexity -- ignore */ /** * Get the value of a given node if it's a static value. */ export function getStaticValue( - /* eslint-enable complexity -- ignore */ context: Rule.RuleContext, node: Node, ): GetStaticValueResult | null { @@ -113,20 +144,9 @@ export function getStaticValue( } return { value } } else if (node.type === "Identifier") { - const variable = findVariable(context, node) - - if (variable != null && variable.defs.length === 1) { - const def = variable.defs[0] - if ( - def.type === "Variable" && - def.parent && - def.parent.type === "VariableDeclaration" && - def.parent.kind === "const" && - def.node.id.type === "Identifier" && - def.node.init - ) { - return getStaticValue(context, def.node.init) - } + const deRef = dereferenceVariable(context, node) + if (deRef !== node) { + return getStaticValue(context, deRef) } } return eslintUtils.getStaticValue(node, getScope(context, node)) @@ -135,9 +155,13 @@ export function getStaticValue( /** * Gets the scope for the current node */ -export function getScope(context: Rule.RuleContext, currentNode: Node): Scope { +export function getScope( + context: Rule.RuleContext, + currentNode: Node, +): Scope.Scope { // eslint-disable-next-line @typescript-eslint/no-explicit-any -- ignore - const scopeManager = (context.getSourceCode() as any).scopeManager + const scopeManager: Scope.ScopeManager = (context.getSourceCode() as any) + .scopeManager // eslint-disable-next-line @typescript-eslint/no-explicit-any -- ignore let node: any = currentNode @@ -421,17 +445,14 @@ export function dereferenceOwnedVariable( expression: Expression, ): Expression { if (expression.type === "Identifier") { - const variable = findVariable(context, expression) - if (!variable || variable.defs.length !== 1) { + const variable = findSimpleVariable(context, expression) + + if (!variable) { // we want a variable with 1 definition return expression } const def = variable.defs[0] - if (def.type !== "Variable" || def.node.id.type !== "Identifier") { - // we want a simple variable - return expression - } const grandParent = getParent(def.parent) if (grandParent && grandParent.type === "ExportNamedDeclaration") { @@ -464,3 +485,48 @@ export function dereferenceOwnedVariable( return expression } + +/** + * If the given expression is the identifier of a variable, then the value of + * the variable will be returned if that value can be statically known. + * + * This method assumes that the value of the variable is immutable. This is + * important because it means that expression that resolve to primitives + * (numbers, string, ...) behave as expected. However, if the value is mutable + * (e.g. arrays and objects), then the object might be mutated. This is because + * objects are passed by reference. So the reference can be statically known + * (the value of the variable) but the value of the object cannot be statically + * known. If the object is immutable (e.g. RegExp and symbols), then they behave + * like primitives. + */ +export function dereferenceVariable( + context: Rule.RuleContext, + expression: Expression, +): Expression { + if (expression.type === "Identifier") { + const variable = findSimpleVariable(context, expression) + + if (variable) { + const def = variable.defs[0] + if (def.node.init) { + if (def.parent.kind === "const") { + // const variables are always what they are initialized to + return dereferenceVariable(context, def.node.init) + } + + // we might still be able to dereference var and let variables, + // they just have to never re-assigned + const refs = variable.references + + const inits = refs.filter((r) => r.init).length + const reads = refs.filter((r) => r.isReadOnly()).length + if (inits === 1 && reads + inits === refs.length) { + // there is only one init and all other references only read + return dereferenceVariable(context, def.node.init) + } + } + } + } + + return expression +}