Skip to content

Fixed wrong location calculation when including CRLF. #74

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

Merged
merged 2 commits into from
Oct 12, 2020
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
43 changes: 35 additions & 8 deletions src/common/location-calculator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,17 +23,20 @@ export class LocationCalculator {
private ltOffsets: number[]
private baseOffset: number
private baseIndexOfGap: number
private shiftOffset: number

/**
* Initialize this calculator.
* @param gapOffsets The list of the offset of removed characters in tokenization phase.
* @param ltOffsets The list of the offset of line terminators.
* @param baseOffset The base offset to calculate locations.
* @param shiftOffset The shift offset to calculate locations.
*/
public constructor(
gapOffsets: number[],
ltOffsets: number[],
baseOffset?: number,
shiftOffset: number = 0,
) {
this.gapOffsets = gapOffsets
this.ltOffsets = ltOffsets
Expand All @@ -42,6 +45,7 @@ export class LocationCalculator {
this.baseOffset === 0
? 0
: sortedLastIndex(gapOffsets, this.baseOffset)
this.shiftOffset = shiftOffset
}

/**
Expand All @@ -54,6 +58,21 @@ export class LocationCalculator {
this.gapOffsets,
this.ltOffsets,
this.baseOffset + offset,
this.shiftOffset,
)
}

/**
* Get sub calculator that shifts the given offset.
* @param offset The shift of new sub calculator.
* @returns Sub calculator.
*/
public getSubCalculatorShift(offset: number): LocationCalculator {
return new LocationCalculator(
this.gapOffsets,
this.ltOffsets,
this.baseOffset,
this.shiftOffset + offset,
)
}

Expand Down Expand Up @@ -91,7 +110,7 @@ export class LocationCalculator {
* @returns The location of the index.
*/
public getLocation(index: number): Location {
return this._getLocation(this.baseOffset + index)
return this._getLocation(this.baseOffset + index + this.shiftOffset)
}

/**
Expand All @@ -100,20 +119,27 @@ export class LocationCalculator {
* @returns The offset of the index.
*/
public getOffsetWithGap(index: number): number {
return this.baseOffset + index + this._getGap(index)
const shiftOffset = this.shiftOffset
return (
this.baseOffset +
index +
shiftOffset +
this._getGap(index + shiftOffset)
)
}

/**
* Modify the location information of the given node with using the base offset and gaps of this calculator.
* @param node The node to modify their location.
*/
public fixLocation<T extends HasLocation>(node: T): T {
const shiftOffset = this.shiftOffset
const range = node.range
const loc = node.loc
const gap0 = this._getGap(range[0])
const gap1 = this._getGap(range[1])
const d0 = this.baseOffset + Math.max(0, gap0)
const d1 = this.baseOffset + Math.max(0, gap1)
const gap0 = this._getGap(range[0] + shiftOffset)
const gap1 = this._getGap(range[1] + shiftOffset)
const d0 = this.baseOffset + Math.max(0, gap0) + shiftOffset
const d1 = this.baseOffset + Math.max(0, gap1) + shiftOffset

if (d0 !== 0) {
range[0] += d0
Expand All @@ -138,8 +164,9 @@ export class LocationCalculator {
* @param error The error to modify their location.
*/
public fixErrorLocation(error: ParseError) {
const gap = this._getGap(error.index)
const diff = this.baseOffset + Math.max(0, gap)
const shiftOffset = this.shiftOffset
const gap = this._getGap(error.index + shiftOffset)
const diff = this.baseOffset + Math.max(0, gap) + shiftOffset

error.index += diff

Expand Down
30 changes: 18 additions & 12 deletions src/script/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,7 @@ function parseExpressionBody(
try {
const ast = parseScriptFragment(
`0(${code})`,
locationCalculator.getSubCalculatorAfter(-2),
locationCalculator.getSubCalculatorShift(-2),
parserOptions,
).ast
const tokens = ast.tokens || []
Expand Down Expand Up @@ -431,9 +431,12 @@ function parseFilter(
// Parse the callee.
if (calleeCode.trim()) {
const spaces = /^\s*/u.exec(calleeCode)![0]
const subCalculator = locationCalculator.getSubCalculatorShift(
spaces.length,
)
const { ast } = parseScriptFragment(
`${spaces}"${calleeCode.trim()}"`,
locationCalculator,
`"${calleeCode.trim()}"`,
subCalculator,
parserOptions,
)
const statement = ast.body[0] as ESLintExpressionStatement
Expand All @@ -455,12 +458,13 @@ function parseFilter(
expression.callee = {
type: "Identifier",
parent: expression,
range: [callee.range[0], callee.range[1] - 2],
range: [
callee.range[0],
subCalculator.getOffsetWithGap(calleeCode.trim().length),
],
loc: {
start: callee.loc.start,
end: locationCalculator.getLocation(
callee.range[1] - callee.range[0] - 1,
),
end: subCalculator.getLocation(calleeCode.trim().length),
},
name: String(callee.value),
}
Expand All @@ -478,7 +482,9 @@ function parseFilter(
if (argsCode != null) {
const { ast } = parseScriptFragment(
`0${argsCode}`,
locationCalculator.getSubCalculatorAfter(paren - 1),
locationCalculator
.getSubCalculatorAfter(paren)
.getSubCalculatorShift(-1),
parserOptions,
)
const statement = ast.body[0] as ESLintExpressionStatement
Expand Down Expand Up @@ -684,7 +690,7 @@ export function parseExpression(
// Parse a filter
const retF = parseFilter(
filterCode,
locationCalculator.getSubCalculatorAfter(prevLoc + 1),
locationCalculator.getSubCalculatorShift(prevLoc + 1),
parserOptions,
)
if (retF) {
Expand Down Expand Up @@ -731,7 +737,7 @@ export function parseVForExpression(
const replaced = processedCode !== code
const ast = parseScriptFragment(
`for(let ${processedCode});`,
locationCalculator.getSubCalculatorAfter(-8),
locationCalculator.getSubCalculatorShift(-8),
parserOptions,
).ast
const tokens = ast.tokens || []
Expand Down Expand Up @@ -829,7 +835,7 @@ function parseVOnExpressionBody(
try {
const ast = parseScriptFragment(
`void function($event){${code}}`,
locationCalculator.getSubCalculatorAfter(-22),
locationCalculator.getSubCalculatorShift(-22),
parserOptions,
).ast
const references = analyzeExternalReferences(ast, parserOptions)
Expand Down Expand Up @@ -905,7 +911,7 @@ export function parseSlotScopeExpression(
try {
const ast = parseScriptFragment(
`void function(${code}) {}`,
locationCalculator.getSubCalculatorAfter(-14),
locationCalculator.getSubCalculatorShift(-14),
parserOptions,
).ast
const statement = ast.body[0] as ESLintExpressionStatement
Expand Down
19 changes: 19 additions & 0 deletions test/ast.js
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,25 @@ describe("Template AST", () => {
assert.strictEqual(actualText, expectedText)
})

it("should have correct range on windows(CRLF).", () => {
const sourceForWin = source.replace(/\r?\n/gu, "\r\n")
const actualForWin = parser.parseForESLint(
sourceForWin,
Object.assign({ filePath: sourcePath }, PARSER_OPTIONS)
)

const resultPath = path.join(ROOT, `${name}/token-ranges.json`)
const expectedText = fs.readFileSync(resultPath, "utf8")
const tokens = getAllTokens(actualForWin.ast).map(t =>
sourceForWin
.slice(t.range[0], t.range[1])
.replace(/\r?\n/gu, "\n")
)
const actualText = JSON.stringify(tokens, null, 4)

assert.strictEqual(actualText, expectedText)
})

it("should have correct location.", () => {
const lines = source.match(/[^\r\n]*(?:\r?\n|$)/gu) || []
lines.push(String.fromCodePoint(0))
Expand Down
Loading