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

Add explicit braced property access parsing #260

Merged
merged 6 commits into from Oct 9, 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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 9 additions & 0 deletions src/parser/tokenizer.ts
Expand Up @@ -207,6 +207,15 @@ export class Tokenizer {
const value = this.readQuoted() || this.readRange()
if (value) return value

if (this.peek() === '[') {
this.p++
const prop = this.readQuoted()
if (!prop) return
if (this.peek() !== ']') return
this.p++
return new PropertyAccessToken(prop, [], this.p)
}

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about ["foo"]["bar"] or ["foo"].bar?

We need adapt the following logic (the PropertyAccessToken part) to support this case (remove the variable constructor argument and make it the first props maybe?).

Copy link
Contributor Author

@wyozi wyozi Oct 9, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense, although the number of potential valid syntaxes are getting a bit scary. Do you think that could be postponed into the future and this feature shipped as is (once I fix the tests passing)?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the number of potential valid syntaxes are getting a bit scary

Seems it can be done by consume the [, call readValue recursively(as in props brackets), and consume the ]. This way the following cases will be supported also:

{{["foo"].bar}}
{{[0].bar}}
{{[key].bar}}

const variable = this.readWord()
if (!variable.size()) return

Expand Down
2 changes: 1 addition & 1 deletion src/render/expression.ts
Expand Up @@ -42,7 +42,7 @@ export class Expression {
export function evalToken (token: Token | undefined, ctx: Context): any {
assert(ctx, () => 'unable to evaluate: context not defined')
if (TypeGuards.isPropertyAccessToken(token)) {
const variable = token.variable.getText()
const variable = token.getVariableAsText()
const props: string[] = token.props.map(prop => evalToken(prop, ctx))
return ctx.get([variable, ...props])
}
Expand Down
11 changes: 10 additions & 1 deletion src/tokens/property-access-token.ts
Expand Up @@ -2,13 +2,22 @@ import { Token } from './token'
import { WordToken } from './word-token'
import { QuotedToken } from './quoted-token'
import { TokenKind } from '../parser/token-kind'
import { parseStringLiteral } from '../parser/parse-string-literal'

export class PropertyAccessToken extends Token {
constructor (
public variable: WordToken,
public variable: WordToken | QuotedToken,
public props: (WordToken | QuotedToken | PropertyAccessToken)[],
end: number
) {
super(TokenKind.PropertyAccess, variable.input, variable.begin, end, variable.file)
}

getVariableAsText() {
if (this.variable instanceof WordToken) {
return this.variable.getText()
} else {
return parseStringLiteral(this.variable.getText())
}
}
}
5 changes: 5 additions & 0 deletions test/e2e/issues.ts
Expand Up @@ -18,4 +18,9 @@ describe('Issues', function () {
const html = await engine.parseAndRender(template)
expect(html).to.equal('foo')
})
it('#259 complex property access with braces is not supported', async () => {
const engine = new Liquid()
const html = engine.parseAndRenderSync('{{ ["complex key"] }}', { 'complex key': 'foo' })
expect(html).to.equal('foo')
})
})
13 changes: 13 additions & 0 deletions test/unit/parser/tokenizer.ts
Expand Up @@ -34,6 +34,19 @@ describe('Tokenize', function () {
it('should read property access value', () => {
expect(new Tokenizer('a[b]["c d"]').readValueOrThrow().getText()).to.equal('a[b]["c d"]')
})
it('should read quoted property access value', () => {
const value = new Tokenizer('["a prop"]').readValue()
expect(value).to.be.instanceOf(PropertyAccessToken)
expect((value as PropertyAccessToken).variable.getText()).to.equal('"a prop"')
})
it('should throw for broken quoted property access', () => {
const tokenizer = new Tokenizer('[5]')
expect(() => tokenizer.readValueOrThrow()).to.throw()
})
it('should throw for incomplete quoted property access', () => {
const tokenizer = new Tokenizer('["a prop"')
expect(() => tokenizer.readValueOrThrow()).to.throw()
})
it('should read hash', () => {
const hash1 = new Tokenizer('foo: 3').readHash()
expect(hash1!.name.content).to.equal('foo')
Expand Down
21 changes: 21 additions & 0 deletions test/unit/tokens/property-access-token.ts
@@ -0,0 +1,21 @@
import * as chai from 'chai'
import * as sinonChai from 'sinon-chai'
import { PropertyAccessToken } from '../../../src/tokens/property-access-token'
import { QuotedToken } from '../../../src/tokens/quoted-token'
import { WordToken } from '../../../src/tokens/word-token'

chai.use(sinonChai)
const expect = chai.expect

describe('PropertyAccessToken', function () {
describe('getVariableAsText', function () {
it('should return correct value for WordToken', function () {
const token = new PropertyAccessToken(new WordToken('foo', 0, 3), [], 3)
expect(token.getVariableAsText()).to.equal('foo')
})
it('should return correct value for QuotedToken', function () {
const token = new PropertyAccessToken(new QuotedToken('"foo bar"', 0, 9), [], 9)
expect(token.getVariableAsText()).to.equal('foo bar')
})
})
})