Skip to content

Commit

Permalink
feat(compiler): output source range for compiler errors (vuejs#7127)
Browse files Browse the repository at this point in the history
  • Loading branch information
gzzhanghao authored and hefeng committed Jan 25, 2019
1 parent 743bfd9 commit 1a6f165
Show file tree
Hide file tree
Showing 21 changed files with 325 additions and 127 deletions.
33 changes: 27 additions & 6 deletions flow/compiler.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ declare type CompilerOptions = {
shouldDecodeTags?: boolean;
shouldDecodeNewlines?: boolean;
shouldDecodeNewlinesForHref?: boolean;
outputSourceRange?: boolean;

// runtime user-configurable
delimiters?: [string, string]; // template delimiters
Expand All @@ -27,13 +28,19 @@ declare type CompilerOptions = {
scopeId?: string;
};

declare type WarningMessage = {
msg: string;
start?: number;
end?: number;
};

declare type CompiledResult = {
ast: ?ASTElement;
render: string;
staticRenderFns: Array<string>;
stringRenderFns?: Array<string>;
errors?: Array<string>;
tips?: Array<string>;
errors?: Array<string | WarningMessage>;
tips?: Array<string | WarningMessage>;
};

declare type ModuleOptions = {
Expand All @@ -53,11 +60,14 @@ declare type ModuleOptions = {
declare type ASTModifiers = { [key: string]: boolean };
declare type ASTIfCondition = { exp: ?string; block: ASTElement };
declare type ASTIfConditions = Array<ASTIfCondition>;
declare type ASTAttr = { name: string; value: any; start?: number; end?: number };

declare type ASTElementHandler = {
value: string;
params?: Array<any>;
modifiers: ?ASTModifiers;
start?: number;
end?: number;
};

declare type ASTElementHandlers = {
Expand All @@ -70,18 +80,24 @@ declare type ASTDirective = {
value: string;
arg: ?string;
modifiers: ?ASTModifiers;
start?: number;
end?: number;
};

declare type ASTNode = ASTElement | ASTText | ASTExpression;

declare type ASTElement = {
type: 1;
tag: string;
attrsList: Array<{ name: string; value: any }>;
attrsList: Array<ASTAttr>;
attrsMap: { [key: string]: any };
rawAttrsMap: { [key: string]: ASTAttr };
parent: ASTElement | void;
children: Array<ASTNode>;

start?: number;
end?: number;

processed?: true;

static?: boolean;
Expand All @@ -91,8 +107,8 @@ declare type ASTElement = {
hasBindings?: boolean;

text?: string;
attrs?: Array<{ name: string; value: any }>;
props?: Array<{ name: string; value: string }>;
attrs?: Array<ASTAttr>;
props?: Array<ASTAttr>;
plain?: boolean;
pre?: true;
ns?: string;
Expand Down Expand Up @@ -160,6 +176,8 @@ declare type ASTExpression = {
static?: boolean;
// 2.4 ssr optimization
ssrOptimizability?: number;
start?: number;
end?: number;
};

declare type ASTText = {
Expand All @@ -169,6 +187,8 @@ declare type ASTText = {
isComment?: boolean;
// 2.4 ssr optimization
ssrOptimizability?: number;
start?: number;
end?: number;
};

// SFC-parser related declarations
Expand All @@ -179,7 +199,8 @@ declare type SFCDescriptor = {
script: ?SFCBlock;
styles: Array<SFCBlock>;
customBlocks: Array<SFCBlock>;
};
errors: Array<string | WarningMessage>;
}

declare type SFCBlock = {
type: string;
Expand Down
11 changes: 8 additions & 3 deletions src/compiler/codegen/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,8 @@ function genOnce (el: ASTElement, state: CodegenState): string {
}
if (!key) {
process.env.NODE_ENV !== 'production' && state.warn(
`v-once can only be used inside v-for that is keyed. `
`v-once can only be used inside v-for that is keyed. `,
el.rawAttrsMap['v-once']
)
return genElement(el, state)
}
Expand Down Expand Up @@ -202,6 +203,7 @@ export function genFor (
`<${el.tag} v-for="${alias} in ${exp}">: component lists rendered with ` +
`v-for should have explicit keys. ` +
`See https://vuejs.org/guide/list.html#key for more info.`,
el.rawAttrsMap['v-for'],
true /* tip */
)
}
Expand Down Expand Up @@ -333,7 +335,10 @@ function genInlineTemplate (el: ASTElement, state: CodegenState): ?string {
if (process.env.NODE_ENV !== 'production' && (
el.children.length !== 1 || ast.type !== 1
)) {
state.warn('Inline-template components must have exactly one child element.')
state.warn(
'Inline-template components must have exactly one child element.',
{ start: el.start }
)
}
if (ast.type === 1) {
const inlineRenderFns = generate(ast, state.options)
Expand Down Expand Up @@ -503,7 +508,7 @@ function genComponent (
})`
}

function genProps (props: Array<{ name: string, value: any }>): string {
function genProps (props: Array<ASTAttr>): string {
let res = ''
for (let i = 0; i < props.length; i++) {
const prop = props[i]
Expand Down
26 changes: 23 additions & 3 deletions src/compiler/create-compiler.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,29 @@ export function createCompilerCreator (baseCompile: Function): Function {
const finalOptions = Object.create(baseOptions)
const errors = []
const tips = []
finalOptions.warn = (msg, tip) => {

let warn = (msg, range, tip) => {
(tip ? tips : errors).push(msg)
}

if (options) {
if (process.env.NODE_ENV !== 'production' && options.outputSourceRange) {
// $flow-disable-line
const leadingSpaceLength = template.match(/^\s*/)[0].length

warn = (msg, range, tip) => {
const data: WarningMessage = { msg }
if (range) {
if (range.start != null) {
data.start = range.start + leadingSpaceLength
}
if (range.end != null) {
data.end = range.end + leadingSpaceLength
}
}
(tip ? tips : errors).push(data)
}
}
// merge custom modules
if (options.modules) {
finalOptions.modules =
Expand All @@ -38,9 +56,11 @@ export function createCompilerCreator (baseCompile: Function): Function {
}
}

const compiled = baseCompile(template, finalOptions)
finalOptions.warn = warn

const compiled = baseCompile(template.trim(), finalOptions)
if (process.env.NODE_ENV !== 'production') {
errors.push.apply(errors, detectErrors(compiled.ast))
detectErrors(compiled.ast, warn)
}
compiled.errors = errors
compiled.tips = tips
Expand Down
57 changes: 31 additions & 26 deletions src/compiler/error-detector.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

import { dirRE, onRE } from './parser/index'

type Range = { start?: number, end?: number };

// these keywords should not appear inside expressions, but operators like
// typeof, instanceof and in are allowed
const prohibitedKeywordRE = new RegExp('\\b' + (
Expand All @@ -19,89 +21,92 @@ const unaryOperatorsRE = new RegExp('\\b' + (
const stripStringRE = /'(?:[^'\\]|\\.)*'|"(?:[^"\\]|\\.)*"|`(?:[^`\\]|\\.)*\$\{|\}(?:[^`\\]|\\.)*`|`(?:[^`\\]|\\.)*`/g

// detect problematic expressions in a template
export function detectErrors (ast: ?ASTNode): Array<string> {
const errors: Array<string> = []
export function detectErrors (ast: ?ASTNode, warn: Function) {
if (ast) {
checkNode(ast, errors)
checkNode(ast, warn)
}
return errors
}

function checkNode (node: ASTNode, errors: Array<string>) {
function checkNode (node: ASTNode, warn: Function) {
if (node.type === 1) {
for (const name in node.attrsMap) {
if (dirRE.test(name)) {
const value = node.attrsMap[name]
if (value) {
const range = node.rawAttrsMap[name]
if (name === 'v-for') {
checkFor(node, `v-for="${value}"`, errors)
checkFor(node, `v-for="${value}"`, warn, range)
} else if (onRE.test(name)) {
checkEvent(value, `${name}="${value}"`, errors)
checkEvent(value, `${name}="${value}"`, warn, range)
} else {
checkExpression(value, `${name}="${value}"`, errors)
checkExpression(value, `${name}="${value}"`, warn, range)
}
}
}
}
if (node.children) {
for (let i = 0; i < node.children.length; i++) {
checkNode(node.children[i], errors)
checkNode(node.children[i], warn)
}
}
} else if (node.type === 2) {
checkExpression(node.expression, node.text, errors)
checkExpression(node.expression, node.text, warn, node)
}
}

function checkEvent (exp: string, text: string, errors: Array<string>) {
function checkEvent (exp: string, text: string, warn: Function, range?: Range) {
const stipped = exp.replace(stripStringRE, '')
const keywordMatch: any = stipped.match(unaryOperatorsRE)
if (keywordMatch && stipped.charAt(keywordMatch.index - 1) !== '$') {
errors.push(
warn(
`avoid using JavaScript unary operator as property name: ` +
`"${keywordMatch[0]}" in expression ${text.trim()}`
`"${keywordMatch[0]}" in expression ${text.trim()}`,
range
)
}
checkExpression(exp, text, errors)
checkExpression(exp, text, warn, range)
}

function checkFor (node: ASTElement, text: string, errors: Array<string>) {
checkExpression(node.for || '', text, errors)
checkIdentifier(node.alias, 'v-for alias', text, errors)
checkIdentifier(node.iterator1, 'v-for iterator', text, errors)
checkIdentifier(node.iterator2, 'v-for iterator', text, errors)
function checkFor (node: ASTElement, text: string, warn: Function, range?: Range) {
checkExpression(node.for || '', text, warn, range)
checkIdentifier(node.alias, 'v-for alias', text, warn, range)
checkIdentifier(node.iterator1, 'v-for iterator', text, warn, range)
checkIdentifier(node.iterator2, 'v-for iterator', text, warn, range)
}

function checkIdentifier (
ident: ?string,
type: string,
text: string,
errors: Array<string>
warn: Function,
range?: Range
) {
if (typeof ident === 'string') {
try {
new Function(`var ${ident}=_`)
} catch (e) {
errors.push(`invalid ${type} "${ident}" in expression: ${text.trim()}`)
warn(`invalid ${type} "${ident}" in expression: ${text.trim()}`, range)
}
}
}

function checkExpression (exp: string, text: string, errors: Array<string>) {
function checkExpression (exp: string, text: string, warn: Function, range?: Range) {
try {
new Function(`return ${exp}`)
} catch (e) {
const keywordMatch = exp.replace(stripStringRE, '').match(prohibitedKeywordRE)
if (keywordMatch) {
errors.push(
warn(
`avoid using JavaScript keyword as property name: ` +
`"${keywordMatch[0]}"\n Raw expression: ${text.trim()}`
`"${keywordMatch[0]}"\n Raw expression: ${text.trim()}`,
range
)
} else {
errors.push(
warn(
`invalid expression: ${e.message} in\n\n` +
` ${exp}\n\n` +
` Raw expression: ${text.trim()}\n`
` Raw expression: ${text.trim()}\n`,
range
)
}
}
Expand Down
Loading

0 comments on commit 1a6f165

Please sign in to comment.