Skip to content

Enforce valid names for computed properties #1106

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 9 commits into from
Jan 18, 2018
19 changes: 18 additions & 1 deletion src/parse/index.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import { isIdentifierStart, isIdentifierChar } from 'acorn';
import { locate, Location } from 'locate-character';
import fragment from './state/fragment';
import { whitespace } from '../utils/patterns';
import { trimStart, trimEnd } from '../utils/trim';
import getCodeFrame from '../utils/getCodeFrame';
import reservedNames from '../utils/reservedNames';
import fullCharCodeAt from '../utils/fullCharCodeAt';
import hash from './utils/hash';
import { Node, Parsed } from '../interfaces';
import CompileError from '../utils/CompileError';
Expand Down Expand Up @@ -147,7 +149,22 @@ export class Parser {

readIdentifier() {
const start = this.index;
const identifier = this.read(/[a-zA-Z_$][a-zA-Z0-9_$]*/);

let i = this.index;

const code = fullCharCodeAt(this.template, i);
if (!isIdentifierStart(code, true)) return null;

i += code <= 0xffff ? 1 : 2;

while (i < this.template.length) {
const code = fullCharCodeAt(this.template, i);

if (!isIdentifierChar(code, true)) break;
i += code <= 0xffff ? 1 : 2;
}

const identifier = this.template.slice(this.index, this.index = i);

if (reservedNames.has(identifier)) {
this.error(`'${identifier}' is a reserved word in JavaScript and cannot be used here`, start);
Expand Down
10 changes: 10 additions & 0 deletions src/utils/fullCharCodeAt.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// Adapted from https://github.com/acornjs/acorn/blob/6584815dca7440e00de841d1dad152302fdd7ca5/src/tokenize.js
// Reproduced under MIT License https://github.com/acornjs/acorn/blob/master/LICENSE

export default function fullCharCodeAt(str: string, i: number): number {
let code = str.charCodeAt(i)
if (code <= 0xd7ff || code >= 0xe000) return code;

let next = str.charCodeAt(i + 1);
return (code << 10) + next - 0x35fdc00;
}
15 changes: 15 additions & 0 deletions src/utils/isValidIdentifier.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import { isIdentifierStart, isIdentifierChar } from 'acorn';
import fullCharCodeAt from './fullCharCodeAt';

export default function isValidIdentifier(str: string): boolean {
let i = 0;

while (i < str.length) {
const code = fullCharCodeAt(str, i);
if (!(i === 0 ? isIdentifierStart : isIdentifierChar)(code, true)) return false;

i += code <= 0xffff ? 1 : 2;
}

return true;
}
20 changes: 20 additions & 0 deletions src/validate/js/propValidators/computed.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
import checkForDupes from '../utils/checkForDupes';
import checkForComputedKeys from '../utils/checkForComputedKeys';
import getName from '../../../utils/getName';
import isValidIdentifier from '../../../utils/isValidIdentifier';
import reservedNames from '../../../utils/reservedNames';
import { Validator } from '../../';
import { Node } from '../../../interfaces';
import walkThroughTopFunctionScope from '../../../utils/walkThroughTopFunctionScope';
Expand All @@ -22,6 +25,23 @@ export default function computed(validator: Validator, prop: Node) {
checkForComputedKeys(validator, prop.value.properties);

prop.value.properties.forEach((computation: Node) => {
const name = getName(computation.key);

if (!isValidIdentifier(name)) {
const suggestion = name.replace(/[^_$a-z0-9]/ig, '_').replace(/^\d/, '_$&');
Copy link
Contributor

@emilos emilos Jan 14, 2018

Choose a reason for hiding this comment

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

should this be moved to utilities?

Copy link
Member Author

Choose a reason for hiding this comment

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

I tend to favour leaving things like this inline until you need them in more than one place, otherwise I find you can end up with a lot of premature abstractions

validator.error(
`Computed property name '${name}' is invalid — must be a valid identifier such as ${suggestion}`,
computation.start
);
}

if (reservedNames.has(name)) {
validator.error(
`Computed property name '${name}' is invalid — cannot be a JavaScript reserved word`,
computation.start
);
}

if (!isFunctionExpression.has(computation.value.type)) {
validator.error(
`Computed properties can be function expressions or arrow function expressions`,
Expand Down
3 changes: 3 additions & 0 deletions test/parser/samples/unusual-identifier/input.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{{#each things as 𐊧}}
<p>{{𐊧}}</p>
{{/each}}
46 changes: 46 additions & 0 deletions test/parser/samples/unusual-identifier/output.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
{
"hash": 795130236,
"html": {
"start": 0,
"end": 47,
"type": "Fragment",
"children": [
{
"start": 0,
"end": 47,
"type": "EachBlock",
"expression": {
"type": "Identifier",
"start": 8,
"end": 14,
"name": "things"
},
"children": [
{
"start": 24,
"end": 37,
"type": "Element",
"name": "p",
"attributes": [],
"children": [
{
"start": 27,
"end": 33,
"type": "MustacheTag",
"expression": {
"type": "Identifier",
"start": 29,
"end": 31,
"name": "𐊧"
}
}
]
}
],
"context": "𐊧"
}
]
},
"css": null,
"js": null
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
[{
"message":
"Computed property name 'new' is invalid — cannot be a JavaScript reserved word",
"loc": {
"line": 9,
"column": 3
},
"pos": 87
}]
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<script>
export default {
data() {
return {
a: 1
};
},
computed: {
new: a => a * 2
}
};
</script>
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
[{
"message": "Computed property name 'with-hyphen' is invalid — must be a valid identifier such as with_hyphen",
"loc": {
"line": 9,
"column": 3
},
"pos": 87
}]
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<script>
export default {
data() {
return {
a: 1
};
},
computed: {
"with-hyphen": a => a * 2
}
};
</script>