Skip to content

Commit

Permalink
fix: more precise line/col for parse Error message, #613
Browse files Browse the repository at this point in the history
  • Loading branch information
harttle committed May 24, 2023
1 parent 92b9577 commit 1039a31
Show file tree
Hide file tree
Showing 13 changed files with 94 additions and 64 deletions.
4 changes: 2 additions & 2 deletions src/parser/tokenizer.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ describe('Tokenizer', function () {
it('should throw when {% raw %} not closed', function () {
const html = '{%raw%} {%endraw {%raw%}'
const tokenizer = new Tokenizer(html)
expect(() => tokenizer.readTopLevelTokens()).toThrow('raw "{%raw%} {%end..." not closed, line:1, col:8')
expect(() => tokenizer.readTopLevelTokens()).toThrow('raw "{%raw%} {%endraw {%raw%}" not closed, line:1, col:8')
})
it('should read output token', function () {
const html = '<p>{{foo | date: "%Y-%m-%d"}}</p>'
Expand Down Expand Up @@ -191,7 +191,7 @@ describe('Tokenizer', function () {
it('should throw if tag not closed', function () {
const html = '{% assign foo = bar {{foo}}'
const tokenizer = new Tokenizer(html)
expect(() => tokenizer.readTopLevelTokens()).toThrow(/tag "{% assign foo..." not closed/)
expect(() => tokenizer.readTopLevelTokens()).toThrow('tag "{% assign foo = bar {{foo}}" not closed, line:1, col:1')
})
it('should throw if output not closed', function () {
const tokenizer = new Tokenizer('{{name}')
Expand Down
59 changes: 35 additions & 24 deletions src/parser/tokenizer.ts
Original file line number Diff line number Diff line change
@@ -1,23 +1,25 @@
import { TagToken, HTMLToken, HashToken, QuotedToken, LiquidTagToken, OutputToken, ValueToken, Token, RangeToken, FilterToken, TopLevelToken, PropertyAccessToken, OperatorToken, LiteralToken, IdentifierToken, NumberToken } from '../tokens'
import { Trie, createTrie, ellipsis, literalValues, assert, TokenizationError, TYPES, QUOTE, BLANK, IDENTIFIER } from '../util'
import { Trie, createTrie, ellipsis, literalValues, TokenizationError, TYPES, QUOTE, BLANK, IDENTIFIER } from '../util'
import { Operators, Expression } from '../render'
import { NormalizedFullOptions, defaultOptions } from '../liquid-options'
import { FilterArg } from './filter-arg'
import { matchOperator } from './match-operator'
import { whiteSpaceCtrl } from './whitespace-ctrl'

export class Tokenizer {
p = 0
p: number
N: number
private rawBeginAt = -1
private opTrie: Trie

constructor (
public input: string,
operators: Operators = defaultOptions.operators,
public file?: string
public file?: string,
private range?: [number, number]
) {
this.N = input.length
this.p = range ? range[0] : 0
this.N = range ? range[1] : input.length
this.opTrie = createTrie(operators)
}

Expand Down Expand Up @@ -57,11 +59,14 @@ export class Tokenizer {
readFilter (): FilterToken | null {
this.skipBlank()
if (this.end()) return null
assert(this.peek() === '|', () => `expected "|" before filter`)
this.assert(this.peek() === '|', () => `expected "|" before filter`)
this.p++
const begin = this.p
const name = this.readIdentifier()
if (!name.size()) return null
if (!name.size()) {
this.assert(this.end(), () => `expected filter name`)
return null
}
const args = []
this.skipBlank()
if (this.peek() === ':') {
Expand All @@ -70,7 +75,7 @@ export class Tokenizer {
const arg = this.readFilterArg()
arg && args.push(arg)
this.skipBlank()
assert(this.end() || this.peek() === ',' || this.peek() === '|', () => `unexpected character ${this.snapshot()}`)
this.assert(this.end() || this.peek() === ',' || this.peek() === '|', () => `unexpected character ${this.snapshot()}`)
} while (this.peek() === ',')
} else if (this.peek() === '|' || this.end()) {
// do nothing
Expand Down Expand Up @@ -121,7 +126,7 @@ export class Tokenizer {
const { file, input } = this
const begin = this.p
if (this.readToDelimiter(options.tagDelimiterRight) === -1) {
throw this.mkError(`tag ${this.snapshot(begin)} not closed`, begin)
throw this.error(`tag ${this.snapshot(begin)} not closed`, begin)
}
const token = new TagToken(input, begin, this.p, options, file)
if (token.name === 'raw') this.rawBeginAt = begin
Expand All @@ -145,7 +150,7 @@ export class Tokenizer {
const { outputDelimiterRight } = options
const begin = this.p
if (this.readToDelimiter(outputDelimiterRight) === -1) {
throw this.mkError(`output ${this.snapshot(begin)} not closed`, begin)
throw this.error(`output ${this.snapshot(begin)} not closed`, begin)
}
return new OutputToken(input, begin, this.p, options, file)
}
Expand Down Expand Up @@ -174,32 +179,38 @@ export class Tokenizer {
this.p++
}
}
throw this.mkError(`raw ${this.snapshot(this.rawBeginAt)} not closed`, begin)
throw this.error(`raw ${this.snapshot(this.rawBeginAt)} not closed`, begin)
}

readLiquidTagTokens (options: NormalizedFullOptions = defaultOptions): LiquidTagToken[] {
const tokens: LiquidTagToken[] = []
while (this.p < this.N) {
const token = this.readLiquidTagToken(options)
if (token.name) tokens.push(token)
token && tokens.push(token)
}
return tokens
}

readLiquidTagToken (options: NormalizedFullOptions): LiquidTagToken {
const { file, input } = this
readLiquidTagToken (options: NormalizedFullOptions): LiquidTagToken | undefined {
this.skipBlank()
if (this.end()) return

const begin = this.p
let end = this.N
if (this.readToDelimiter('\n') !== -1) end = this.p
return new LiquidTagToken(input, begin, end, options, file)
this.readToDelimiter('\n')
const end = this.p
return new LiquidTagToken(this.input, begin, end, options, this.file)
}

error (msg: string, pos: number) {
return new TokenizationError(msg, new IdentifierToken(this.input, pos, this.N, this.file))
}

mkError (msg: string, begin: number) {
return new TokenizationError(msg, new IdentifierToken(this.input, begin, this.N, this.file))
assert (pred: unknown, msg: string | (() => string), pos: number = this.p) {
if (!pred) throw this.error(typeof msg === 'function' ? msg() : msg, pos)
}

snapshot (begin: number = this.p) {
return JSON.stringify(ellipsis(this.input.slice(begin), 16))
return JSON.stringify(ellipsis(this.input.slice(begin), 32))
}

/**
Expand All @@ -212,7 +223,7 @@ export class Tokenizer {
readIdentifier (): IdentifierToken {
this.skipBlank()
const begin = this.p
while (this.peekType() & IDENTIFIER) ++this.p
while (!this.end() && this.peekType() & IDENTIFIER) ++this.p
return new IdentifierToken(this.input, begin, this.p, this.file)
}

Expand Down Expand Up @@ -250,7 +261,7 @@ export class Tokenizer {
}

remaining () {
return this.input.slice(this.p)
return this.input.slice(this.p, this.N)
}

advance (i = 1) {
Expand Down Expand Up @@ -323,7 +334,7 @@ export class Tokenizer {

readValueOrThrow (): ValueToken {
const value = this.readValue()
assert(value, () => `unexpected token ${this.snapshot()}, value expected`)
this.assert(value, () => `unexpected token ${this.snapshot()}, value expected`)
return value!
}

Expand Down Expand Up @@ -372,8 +383,8 @@ export class Tokenizer {
return TYPES[this.input.charCodeAt(this.p + n)]
}

peek (n = 0) {
return this.input[this.p + n]
peek (n = 0): string {
return this.p + n >= this.N ? '' : this.input[this.p + n]
}

skipBlank () {
Expand Down
3 changes: 3 additions & 0 deletions src/render/expression.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@ export class Expression {
}
return operands[0]
}
public valid () {
return !!this.postfix.length
}
}

export function * evalToken (token: Token | undefined, ctx: Context, lenient = false): IterableIterator<unknown> {
Expand Down
11 changes: 7 additions & 4 deletions src/tags/assign.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,17 @@
import { Value, assert, Tokenizer, Liquid, TopLevelToken, TagToken, Context, Tag } from '..'
import { Value, Liquid, TopLevelToken, TagToken, Context, Tag } from '..'
export default class extends Tag {
private key: string
private value: Value

constructor (token: TagToken, remainTokens: TopLevelToken[], liquid: Liquid) {
super(token, remainTokens, liquid)
const tokenizer = new Tokenizer(token.args, liquid.options.operators)
this.key = tokenizer.readIdentifier().content
const tokenizer = token.tokenizer
this.key = token.tokenizer.readIdentifier().content
tokenizer.assert(this.key, () => `expected variable name in ${token.getText()}`)

tokenizer.skipBlank()
assert(tokenizer.peek() === '=', () => `illegal token ${token.getText()}`)
tokenizer.assert(tokenizer.peek() === '=', () => `expected "=" in ${token.getText()}`)

tokenizer.advance()
this.value = new Value(tokenizer.remaining(), this.liquid)
}
Expand Down
5 changes: 2 additions & 3 deletions src/tags/liquid.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
import { Template, Tokenizer, Emitter, Liquid, TopLevelToken, TagToken, Context, Tag } from '..'
import { Template, Emitter, Liquid, TopLevelToken, TagToken, Context, Tag } from '..'

export default class extends Tag {
templates: Template[]
constructor (token: TagToken, remainTokens: TopLevelToken[], liquid: Liquid) {
super(token, remainTokens, liquid)
const tokenizer = new Tokenizer(token.args, this.liquid.options.operators)
const tokens = tokenizer.readLiquidTagTokens(this.liquid.options)
const tokens = token.tokenizer.readLiquidTagTokens(this.liquid.options)
this.templates = this.liquid.parser.parseTokens(tokens)
}
* render (ctx: Context, emitter: Emitter): Generator<unknown, void, unknown> {
Expand Down
1 change: 1 addition & 0 deletions src/template/value.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ export class Value {
public constructor (str: string, liquid: Liquid) {
const tokenizer = new Tokenizer(str, liquid.options.operators)
this.initial = tokenizer.readExpression()
assert(this.initial.valid(), `invalid value expression: "${str}"`)
this.filters = tokenizer.readFilters().map(({ name, args }) => new Filter(name, this.getFilter(liquid, name), args, liquid))
}
public * value (ctx: Context, lenient?: boolean): Generator<unknown, unknown, unknown> {
Expand Down
27 changes: 15 additions & 12 deletions src/tokens/delimited-token.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
import { Token } from './token'
import { TokenKind } from '../parser'
import { last } from '../util'
import { TYPES, BLANK } from '../util'

export abstract class DelimitedToken extends Token {
public trimLeft = false
public trimRight = false
public content: string
public contentRange: [number, number]
public constructor (
kind: TokenKind,
content: string,
[contentBegin, contentEnd]: [number, number],
input: string,
begin: number,
end: number,
Expand All @@ -17,16 +17,19 @@ export abstract class DelimitedToken extends Token {
file?: string
) {
super(kind, input, begin, end, file)
this.content = this.getText()
const tl = content[0] === '-'
const tr = last(content) === '-'
this.content = content
.slice(
tl ? 1 : 0,
tr ? -1 : content.length
)
.trim()
const tl = input[contentBegin] === '-'
const tr = input[contentEnd - 1] === '-'

let l = tl ? contentBegin + 1 : contentBegin
let r = tr ? contentEnd - 1 : contentEnd
while (l < r && (TYPES[input.charCodeAt(l)] & BLANK)) l++
while (r > l && (TYPES[input.charCodeAt(r - 1)] & BLANK)) r--

this.contentRange = [l, r]
this.trimLeft = tl || trimLeft
this.trimRight = tr || trimRight
}
get content () {
return this.input.slice(this.contentRange[0], this.contentRange[1])
}
}
5 changes: 4 additions & 1 deletion src/tokens/liquid-tag-token.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@ import { TokenizationError } from '../util'
import { NormalizedFullOptions } from '../liquid-options'
import { Tokenizer, TokenKind } from '../parser'

/**
* LiquidTagToken is different from TagToken by not having delimiters `{%` or `%}`
*/
export class LiquidTagToken extends DelimitedToken {
public name: string
public args: string
Expand All @@ -14,7 +17,7 @@ export class LiquidTagToken extends DelimitedToken {
file?: string
) {
const value = input.slice(begin, end)
super(TokenKind.Tag, value, input, begin, end, false, false, file)
super(TokenKind.Tag, [begin, end], input, begin, end, false, false, file)

if (!/\S/.test(value)) {
// A line that contains only whitespace.
Expand Down
4 changes: 2 additions & 2 deletions src/tokens/output-token.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ export class OutputToken extends DelimitedToken {
file?: string
) {
const { trimOutputLeft, trimOutputRight, outputDelimiterLeft, outputDelimiterRight } = options
const value = input.slice(begin + outputDelimiterLeft.length, end - outputDelimiterRight.length)
super(TokenKind.Output, value, input, begin, end, trimOutputLeft, trimOutputRight, file)
const valueRange: [number, number] = [begin + outputDelimiterLeft.length, end - outputDelimiterRight.length]
super(TokenKind.Output, valueRange, input, begin, end, trimOutputLeft, trimOutputRight, file)
}
}
19 changes: 10 additions & 9 deletions src/tokens/tag-token.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import type { NormalizedFullOptions } from '../liquid-options'

export class TagToken extends DelimitedToken {
public name: string
public args: string
public tokenizer: Tokenizer
public constructor (
input: string,
begin: number,
Expand All @@ -14,14 +14,15 @@ export class TagToken extends DelimitedToken {
file?: string
) {
const { trimTagLeft, trimTagRight, tagDelimiterLeft, tagDelimiterRight } = options
const value = input.slice(begin + tagDelimiterLeft.length, end - tagDelimiterRight.length)
super(TokenKind.Tag, value, input, begin, end, trimTagLeft, trimTagRight, file)
const [valueBegin, valueEnd] = [begin + tagDelimiterLeft.length, end - tagDelimiterRight.length]
super(TokenKind.Tag, [valueBegin, valueEnd], input, begin, end, trimTagLeft, trimTagRight, file)

const tokenizer = new Tokenizer(this.content, options.operators)
this.name = tokenizer.readTagName()
if (!this.name) throw new TokenizationError(`illegal tag syntax`, this)

tokenizer.skipBlank()
this.args = tokenizer.remaining()
this.tokenizer = new Tokenizer(input, options.operators, file, this.contentRange)
this.name = this.tokenizer.readTagName()
if (!this.name) throw new TokenizationError(`illegal tag syntax, tag name expected`, this)
this.tokenizer.skipBlank()
}
get args (): string {
return this.tokenizer.input.slice(this.tokenizer.p, this.contentRange[1])
}
}
2 changes: 1 addition & 1 deletion test/integration/filters/array.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ describe('filters/array', function () {
it('should throw when comma missing', async () => {
const src = '{% assign beatles = "John, Paul, George, Ringo" | split: ", " %}' +
'{{ beatles | join " and " }}'
return expect(render(src)).rejects.toThrow('unexpected token at "\\" and \\"", line:1, col:65')
return expect(render(src)).rejects.toThrow('expected ":" after filter name, line:1, col:65')

This comment has been minimized.

Copy link
@TomasHubelbauer

TomasHubelbauer May 28, 2023

Contributor

I see you've updated this test but I don't see where this message was implement. I suppose the PR is still WIP. Are you planning to address #610 here as well? If so, thank you! Good work!

})
})
describe('last', () => {
Expand Down
4 changes: 2 additions & 2 deletions test/integration/tags/assign.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ describe('tags/assign', function () {
it('should throw when variable name illegal', function () {
const src = '{% assign / %}'
const ctx = {}
return expect(liquid.parseAndRender(src, ctx)).rejects.toThrow(/illegal/)
return expect(liquid.parseAndRender(src, ctx)).rejects.toThrow(/expected variable name/)
})
it('should support assign to a string', async function () {
const src = '{% assign foo="bar" %}{{foo}}'
Expand All @@ -15,7 +15,7 @@ describe('tags/assign', function () {
})
it('should throw when variable value illegal', function () {
const src = '{% assign foo = “bar” %}'
expect(() => liquid.parse(src)).toThrow(/unexpected token at "“bar”"/)
expect(() => liquid.parse(src)).toThrow(/invalid value expression: " “bar”"/)
expect(() => liquid.parse(src)).toThrow(ParseError)
})
it('should support assign to a number', async function () {
Expand Down
Loading

0 comments on commit 1039a31

Please sign in to comment.