From fed8c04e6bc52d46e3a008f0216bccb47ed92fd8 Mon Sep 17 00:00:00 2001 From: Jason Date: Sat, 25 Nov 2017 22:06:31 +0800 Subject: [PATCH 1/9] feat(compiler): output source range for compiler errors --- flow/compiler.js | 30 +++- src/compiler/codegen/index.js | 13 +- src/compiler/create-compiler.js | 26 +++- src/compiler/directives/on.js | 3 +- src/compiler/error-detector.js | 58 ++++---- src/compiler/helpers.js | 67 +++++++-- src/compiler/optimizer.js | 2 +- src/compiler/parser/html-parser.js | 23 ++- src/compiler/parser/index.js | 136 ++++++++++++------ src/platforms/web/compiler/directives/html.js | 2 +- .../web/compiler/directives/model.js | 11 +- src/platforms/web/compiler/directives/text.js | 2 +- src/platforms/web/compiler/modules/class.js | 4 +- src/platforms/web/compiler/modules/style.js | 4 +- src/platforms/weex/compiler/modules/class.js | 4 +- src/platforms/weex/compiler/modules/props.js | 7 + src/platforms/weex/compiler/modules/style.js | 4 +- src/server/optimizing-compiler/modules.js | 8 +- src/sfc/parser.js | 9 +- .../modules/compiler/compiler-options.spec.js | 15 ++ 20 files changed, 302 insertions(+), 126 deletions(-) diff --git a/flow/compiler.js b/flow/compiler.js index a5ebc5f3862..4d2da6fefd3 100644 --- a/flow/compiler.js +++ b/flow/compiler.js @@ -16,6 +16,7 @@ declare type CompilerOptions = { shouldDecodeTags?: boolean; shouldDecodeNewlines?: boolean; shouldDecodeNewlinesForHref?: boolean; + outputSourceRange?: boolean; // for ssr optimization compiler scopeId?: string; @@ -27,13 +28,19 @@ declare type CompilerOptions = { comments?: boolean }; +declare type WarningMessage = { + msg: string; + start?: number; + end?: number; +}; + declare type CompiledResult = { ast: ?ASTElement; render: string; staticRenderFns: Array; stringRenderFns?: Array; - errors?: Array; - tips?: Array; + errors?: Array; + tips?: Array; }; declare type ModuleOptions = { @@ -50,10 +57,13 @@ declare type ModuleOptions = { declare type ASTModifiers = { [key: string]: boolean }; declare type ASTIfCondition = { exp: ?string; block: ASTElement }; declare type ASTIfConditions = Array; +declare type ASTAttr = { name: string; value: string; start?: number; end?: number }; declare type ASTElementHandler = { value: string; modifiers: ?ASTModifiers; + start?: number; + end?: number; }; declare type ASTElementHandlers = { @@ -66,6 +76,8 @@ declare type ASTDirective = { value: string; arg: ?string; modifiers: ?ASTModifiers; + start?: number; + end?: number; }; declare type ASTNode = ASTElement | ASTText | ASTExpression; @@ -73,11 +85,15 @@ declare type ASTNode = ASTElement | ASTText | ASTExpression; declare type ASTElement = { type: 1; tag: string; - attrsList: Array<{ name: string; value: string }>; + attrsList: Array; + rawAttrsList?: Array; attrsMap: { [key: string]: string | null }; parent: ASTElement | void; children: Array; + start?: number; + end?: number; + processed?: true; static?: boolean; @@ -87,8 +103,8 @@ declare type ASTElement = { hasBindings?: boolean; text?: string; - attrs?: Array<{ name: string; value: string }>; - props?: Array<{ name: string; value: string }>; + attrs?: Array; + props?: Array; plain?: boolean; pre?: true; ns?: string; @@ -155,6 +171,8 @@ declare type ASTExpression = { static?: boolean; // 2.4 ssr optimization ssrOptimizability?: number; + start?: number; + end?: number; }; declare type ASTText = { @@ -164,6 +182,8 @@ declare type ASTText = { isComment?: boolean; // 2.4 ssr optimization ssrOptimizability?: number; + start?: number; + end?: number; }; // SFC-parser related declarations diff --git a/src/compiler/codegen/index.js b/src/compiler/codegen/index.js index 7b8a1826d7a..0bf63b8d144 100644 --- a/src/compiler/codegen/index.js +++ b/src/compiler/codegen/index.js @@ -3,7 +3,7 @@ import { genHandlers } from './events' import baseDirectives from '../directives/index' import { camelize, no, extend } from 'shared/util' -import { baseWarn, pluckModuleFunction } from '../helpers' +import { baseWarn, getRawAttr, pluckModuleFunction } from '../helpers' type TransformFunction = (el: ASTElement, code: string) => string; type DataGenFunction = (el: ASTElement) => string; @@ -115,7 +115,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. `, + getRawAttr(el, 'v-once') ) return genElement(el, state) } @@ -187,6 +188,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.`, + getRawAttr(el, 'v-for'), true /* tip */ ) } @@ -318,7 +320,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) @@ -485,7 +490,7 @@ function genComponent ( })` } -function genProps (props: Array<{ name: string, value: string }>): string { +function genProps (props: Array): string { let res = '' for (let i = 0; i < props.length; i++) { const prop = props[i] diff --git a/src/compiler/create-compiler.js b/src/compiler/create-compiler.js index b6690d8ad20..a021aaa3f53 100644 --- a/src/compiler/create-compiler.js +++ b/src/compiler/create-compiler.js @@ -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 (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 = @@ -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 diff --git a/src/compiler/directives/on.js b/src/compiler/directives/on.js index 893a678ede8..8dcf72d65ee 100644 --- a/src/compiler/directives/on.js +++ b/src/compiler/directives/on.js @@ -1,10 +1,11 @@ /* @flow */ import { warn } from 'core/util/index' +import { getRawAttr } from '../helpers' export default function on (el: ASTElement, dir: ASTDirective) { if (process.env.NODE_ENV !== 'production' && dir.modifiers) { - warn(`v-on without argument does not support modifiers.`) + warn(`v-on without argument does not support modifiers.`, getRawAttr(el, 'v-on')) } el.wrapListeners = (code: string) => `_g(${code},${dir.value})` } diff --git a/src/compiler/error-detector.js b/src/compiler/error-detector.js index e729c499a33..19a7e0af129 100644 --- a/src/compiler/error-detector.js +++ b/src/compiler/error-detector.js @@ -1,7 +1,10 @@ /* @flow */ +import { getRawAttr } from './helpers' 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' + ( @@ -19,89 +22,92 @@ const unaryOperatorsRE = new RegExp('\\b' + ( const stripStringRE = /'(?:[^'\\]|\\.)*'|"(?:[^"\\]|\\.)*"|`(?:[^`\\]|\\.)*\$\{|\}(?:[^`\\]|\\.)*`|`(?:[^`\\]|\\.)*`/g // detect problematic expressions in a template -export function detectErrors (ast: ?ASTNode): Array { - const errors: Array = [] +export function detectErrors (ast: ?ASTNode, warn: Function) { if (ast) { - checkNode(ast, errors) + checkNode(ast, warn) } - return errors } -function checkNode (node: ASTNode, errors: Array) { +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 = getRawAttr(node, 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) { +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) { - 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 + 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) { +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 ) } } diff --git a/src/compiler/helpers.js b/src/compiler/helpers.js index 1349fa364f4..c4b27b3cee3 100644 --- a/src/compiler/helpers.js +++ b/src/compiler/helpers.js @@ -3,7 +3,9 @@ import { emptyObject } from 'shared/util' import { parseFilters } from './parser/filter-parser' -export function baseWarn (msg: string) { +type Range = { start?: number, end?: number }; + +export function baseWarn (msg: string, range?: Range) { console.error(`[Vue compiler]: ${msg}`) } @@ -16,12 +18,12 @@ export function pluckModuleFunction ( : [] } -export function addProp (el: ASTElement, name: string, value: string) { - (el.props || (el.props = [])).push({ name, value }) +export function addProp (el: ASTElement, name: string, value: string, range?: Range) { + (el.props || (el.props = [])).push(setRange({ name, value }, range)) } -export function addAttr (el: ASTElement, name: string, value: string) { - (el.attrs || (el.attrs = [])).push({ name, value }) +export function addAttr (el: ASTElement, name: string, value: string, range?: Range) { + (el.attrs || (el.attrs = [])).push(setRange({ name, value }, range)) } export function addDirective ( @@ -30,9 +32,10 @@ export function addDirective ( rawName: string, value: string, arg: ?string, - modifiers: ?ASTModifiers + modifiers: ?ASTModifiers, + range?: Range ) { - (el.directives || (el.directives = [])).push({ name, rawName, value, arg, modifiers }) + (el.directives || (el.directives = [])).push(setRange({ name, rawName, value, arg, modifiers }, range)) } export function addHandler ( @@ -41,7 +44,8 @@ export function addHandler ( value: string, modifiers: ?ASTModifiers, important?: boolean, - warn?: Function + warn?: ?Function, + range?: Range ) { modifiers = modifiers || emptyObject // warn prevent and passive modifier @@ -52,7 +56,8 @@ export function addHandler ( ) { warn( 'passive and prevent can\'t be used together. ' + - 'Passive handler can\'t prevent default event.' + 'Passive handler can\'t prevent default event.', + range ) } @@ -91,7 +96,7 @@ export function addHandler ( events = el.events || (el.events = {}) } - const newHandler: any = { value } + const newHandler: any = setRange({ value }, range) if (modifiers !== emptyObject) { newHandler.modifiers = modifiers } @@ -107,6 +112,33 @@ export function addHandler ( } } +export function getRawAttr ( + el: ASTElement, + name: string +) { + const list = el.rawAttrsList + if (!list) { + return + } + for (let i = list.length - 1; i >= 0; i--) { + if (list[i].name === name) { + return list[i] + } + } +} + +export function getRawBindingAttr ( + el: ASTElement, + name: string +) { + if (!el.rawAttrsList) { + return + } + return getRawAttr(el, ':' + name) || + getRawAttr(el, 'v-bind:' + name) || + getRawAttr(el, name) +} + export function getBindingAttr ( el: ASTElement, name: string, @@ -149,3 +181,18 @@ export function getAndRemoveAttr ( } return val } + +function setRange ( + item: any, + range?: { start?: number, end?: number } +) { + if (range) { + if (range.start != null) { + item.start = range.start + } + if (range.end != null) { + item.end = range.end + } + } + return item +} diff --git a/src/compiler/optimizer.js b/src/compiler/optimizer.js index ce43203500a..e63a0e8d8d7 100644 --- a/src/compiler/optimizer.js +++ b/src/compiler/optimizer.js @@ -30,7 +30,7 @@ export function optimize (root: ?ASTElement, options: CompilerOptions) { function genStaticKeys (keys: string): Function { return makeMap( - 'type,tag,attrsList,attrsMap,plain,parent,children,attrs' + + 'type,tag,attrsList,attrsMap,plain,parent,children,attrs,start,end,rawAttrsList' + (keys ? ',' + keys : '') ) } diff --git a/src/compiler/parser/html-parser.js b/src/compiler/parser/html-parser.js index 86955e0e7ab..15225efa2da 100644 --- a/src/compiler/parser/html-parser.js +++ b/src/compiler/parser/html-parser.js @@ -73,7 +73,7 @@ export function parseHTML (html, options) { if (commentEnd >= 0) { if (options.shouldKeepComment) { - options.comment(html.substring(4, commentEnd)) + options.comment(html.substring(4, commentEnd), index, index + commentEnd + 3) } advance(commentEnd + 3) continue @@ -133,16 +133,18 @@ export function parseHTML (html, options) { rest = html.slice(textEnd) } text = html.substring(0, textEnd) - advance(textEnd) } if (textEnd < 0) { text = html - html = '' + } + + if (text) { + advance(text.length) } if (options.chars && text) { - options.chars(text) + options.chars(text, index - text.length, index) } } else { let endTagLength = 0 @@ -171,7 +173,7 @@ export function parseHTML (html, options) { if (html === last) { options.chars && options.chars(html) if (process.env.NODE_ENV !== 'production' && !stack.length && options.warn) { - options.warn(`Mal-formatted tag at end of template: "${html}"`) + options.warn(`Mal-formatted tag at end of template: "${html}"`, { start: index }) } break } @@ -196,7 +198,9 @@ export function parseHTML (html, options) { advance(start[0].length) let end, attr while (!(end = html.match(startTagClose)) && (attr = html.match(attribute))) { + attr.start = index advance(attr[0].length) + attr.end = index match.attrs.push(attr) } if (end) { @@ -241,10 +245,14 @@ export function parseHTML (html, options) { name: args[1], value: decodeAttr(value, shouldDecodeNewlines) } + if (options.outputSourceRange) { + attrs[i].start = args.start + args[0].match(/^\s*/).length + attrs[i].end = args.end + } } if (!unary) { - stack.push({ tag: tagName, lowerCasedTag: tagName.toLowerCase(), attrs: attrs }) + stack.push({ tag: tagName, lowerCasedTag: tagName.toLowerCase(), attrs: attrs, start: match.start, end: match.end }) lastTag = tagName } @@ -282,7 +290,8 @@ export function parseHTML (html, options) { options.warn ) { options.warn( - `tag <${stack[i].tag}> has no matching end tag.` + `tag <${stack[i].tag}> has no matching end tag.`, + { start: stack[i].start } ) } if (options.end) { diff --git a/src/compiler/parser/index.js b/src/compiler/parser/index.js index e971524e21b..d8ebe37e261 100644 --- a/src/compiler/parser/index.js +++ b/src/compiler/parser/index.js @@ -14,8 +14,10 @@ import { baseWarn, addHandler, addDirective, + getRawAttr, getBindingAttr, getAndRemoveAttr, + getRawBindingAttr, pluckModuleFunction } from '../helpers' @@ -41,11 +43,9 @@ let platformIsPreTag let platformMustUseProp let platformGetTagNamespace -type Attr = { name: string; value: string }; - export function createASTElement ( tag: string, - attrs: Array, + attrs: Array, parent: ASTElement | void ): ASTElement { return { @@ -85,10 +85,10 @@ export function parse ( let inPre = false let warned = false - function warnOnce (msg) { + function warnOnce (msg, range) { if (!warned) { warned = true - warn(msg) + warn(msg, range) } } @@ -110,7 +110,8 @@ export function parse ( shouldDecodeNewlines: options.shouldDecodeNewlines, shouldDecodeNewlinesForHref: options.shouldDecodeNewlinesForHref, shouldKeepComment: options.comments, - start (tag, attrs, unary) { + outputSourceRange: options.outputSourceRange, + start (tag, attrs, unary, start, end) { // check namespace. // inherit parent ns if there is one const ns = (currentParent && currentParent.ns) || platformGetTagNamespace(tag) @@ -126,12 +127,18 @@ export function parse ( element.ns = ns } + if (options.outputSourceRange) { + element.rawAttrsList = element.attrsList.slice() + element.start = start + } + if (isForbiddenTag(element) && !isServerRendering()) { element.forbidden = true process.env.NODE_ENV !== 'production' && warn( 'Templates should only be responsible for mapping the state to the ' + 'UI. Avoid placing tags with side-effects in your templates, such as ' + - `<${tag}>` + ', as they will not be parsed.' + `<${tag}>` + ', as they will not be parsed.', + { start: element.start } ) } @@ -165,13 +172,15 @@ export function parse ( if (el.tag === 'slot' || el.tag === 'template') { warnOnce( `Cannot use <${el.tag}> as component root element because it may ` + - 'contain multiple nodes.' + 'contain multiple nodes.', + { start: el.start } ) } if (el.attrsMap.hasOwnProperty('v-for')) { warnOnce( 'Cannot use v-for on stateful component root element because ' + - 'it renders multiple elements.' + 'it renders multiple elements.', + getRawAttr(el, 'v-for') ) } } @@ -193,7 +202,8 @@ export function parse ( warnOnce( `Component template should contain exactly one root element. ` + `If you are using v-if on multiple elements, ` + - `use v-else-if to chain them instead.` + `use v-else-if to chain them instead.`, + { start: element.start } ) } } @@ -221,7 +231,7 @@ export function parse ( } }, - end () { + end (tag, start, end) { // remove trailing whitespace const element = stack[stack.length - 1] const lastNode = element.children[element.children.length - 1] @@ -231,19 +241,24 @@ export function parse ( // pop stack stack.length -= 1 currentParent = stack[stack.length - 1] + if (options.outputSourceRange) { + element.end = end + } endPre(element) }, - chars (text: string) { + chars (text: string, start: number, end: number) { if (!currentParent) { if (process.env.NODE_ENV !== 'production') { if (text === template) { warnOnce( - 'Component template requires a root element, rather than just text.' + 'Component template requires a root element, rather than just text.', + { start } ) } else if ((text = text.trim())) { warnOnce( - `text "${text}" outside root element will be ignored.` + `text "${text}" outside root element will be ignored.`, + { start } ) } } @@ -264,26 +279,39 @@ export function parse ( : preserveWhitespace && children.length ? ' ' : '' if (text) { let expression + let child: ?ASTNode if (!inVPre && text !== ' ' && (expression = parseText(text, delimiters))) { - children.push({ + child = { type: 2, expression, text - }) + } } else if (text !== ' ' || !children.length || children[children.length - 1].text !== ' ') { - children.push({ + child = { type: 3, text - }) + } + } + if (child) { + if (options.outputSourceRange) { + child.start = start + child.end = end + } + children.push(child) } } }, - comment (text: string) { - currentParent.children.push({ + comment (text: string, start, end) { + const child: ASTText = { type: 3, text, isComment: true - }) + } + if (options.outputSourceRange) { + child.start = start + child.end = end + } + currentParent.children.push(child) } }) return root @@ -296,13 +324,18 @@ function processPre (el) { } function processRawAttrs (el) { - const l = el.attrsList.length - if (l) { - const attrs = el.attrs = new Array(l) - for (let i = 0; i < l; i++) { + const list = el.attrsList + const len = list.length + if (len) { + const attrs: Array = el.attrs = new Array(len) + for (let i = 0; i < len; i++) { attrs[i] = { - name: el.attrsList[i].name, - value: JSON.stringify(el.attrsList[i].value) + name: list[i].name, + value: JSON.stringify(list[i].value) + } + if (list[i].start != null) { + attrs[i].start = list[i].start + attrs[i].end = list[i].end } } } else if (!el.pre) { @@ -331,7 +364,10 @@ function processKey (el) { const exp = getBindingAttr(el, 'key') if (exp) { if (process.env.NODE_ENV !== 'production' && el.tag === 'template') { - warn(`