Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve pattern source information #282

Merged
merged 7 commits into from
Aug 10, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/rules/control-character-escape.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ import {
CP_TAB,
createRule,
defineRegexpVisitor,
isRegexpLiteral,
} from "../utils"
import { isRegexpLiteral } from "../utils/ast-utils/utils"

const CONTROL_CHARS = new Map<number, string>([
[0, "\\0"],
Expand Down
70 changes: 40 additions & 30 deletions lib/rules/match-any.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -64,55 +65,64 @@ 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,
) {
): null | Rule.Fix | Rule.Fix[] {
if (!preference) {
return
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 (
regexpNode.type === "CharacterClass" &&
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
}

/**
Expand Down
16 changes: 15 additions & 1 deletion lib/rules/no-empty-alternative.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 2 additions & 6 deletions lib/rules/no-invisible-character.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ export default createRule("no-invisible-character", {
node,
flags,
getRegexpLocation,
getRegexpRange,
fixReplaceNode,
}: RegExpContextForLiteral): RegExpVisitor.Handlers {
return {
onCharacterEnter(cNode) {
Expand All @@ -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),
})
}
},
Expand Down
15 changes: 11 additions & 4 deletions lib/rules/no-useless-lazy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

Expand Down
30 changes: 8 additions & 22 deletions lib/rules/optimal-quantifier-concatenation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -489,20 +489,12 @@ function getReplacement(
function getLoc(
left: Element,
right: Element,
sourceCode: SourceCode,
{ getRegexpRange }: 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
{ patternSource }: RegExpContext,
): AST.SourceLocation {
return patternSource.getAstLocation({
start: Math.min(left.start, right.start),
end: Math.max(left.end, right.end),
})
}

export default createRule("optimal-quantifier-concatenation", {
Expand Down Expand Up @@ -567,13 +559,7 @@ export default createRule("optimal-quantifier-concatenation", {
if (replacement.type === "Both") {
context.report({
node,
loc:
getLoc(
left,
right,
context.getSourceCode(),
regexpContext,
) ?? getRegexpLocation(aNode),
loc: getLoc(left, right, regexpContext),
messageId: replacement.messageId,
data: {
left: left.raw,
Expand Down
40 changes: 16 additions & 24 deletions lib/rules/prefer-quantifier.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
isSymbol,
quantToString,
} from "../utils"
import type { PatternRange } from "../utils/ast-utils/pattern-source"

type CharTarget = CharacterSet | Character

Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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:
Expand All @@ -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(),
)
},
})
Expand Down
Loading