From b31a1aa8870474b2ca782c45d55edac2932d4cc2 Mon Sep 17 00:00:00 2001 From: Jason Date: Sat, 22 Dec 2018 10:58:32 +0800 Subject: [PATCH] feat(compiler): output source range for compiler errors (#7127) ref #6338 --- flow/compiler.js | 33 +++- src/compiler/codegen/index.js | 11 +- src/compiler/create-compiler.js | 26 +++- src/compiler/error-detector.js | 57 +++---- src/compiler/helpers.js | 57 +++++-- src/compiler/optimizer.js | 2 +- src/compiler/parser/html-parser.js | 23 ++- src/compiler/parser/index.js | 144 ++++++++++++------ src/platforms/web/compiler/directives/html.js | 2 +- .../web/compiler/directives/model.js | 9 +- src/platforms/web/compiler/directives/text.js | 2 +- src/platforms/web/compiler/modules/class.js | 3 +- src/platforms/web/compiler/modules/style.js | 3 +- src/platforms/weex/compiler/modules/class.js | 3 +- src/platforms/weex/compiler/modules/props.js | 5 + src/platforms/weex/compiler/modules/style.js | 3 +- src/server/optimizing-compiler/modules.js | 8 +- src/server/optimizing-compiler/optimizer.js | 1 + src/sfc/parser.js | 33 ++-- .../modules/compiler/compiler-options.spec.js | 21 +++ test/unit/modules/sfc/sfc-parser.spec.js | 6 + 21 files changed, 325 insertions(+), 127 deletions(-) diff --git a/flow/compiler.js b/flow/compiler.js index a5f3304de52..a7acacee595 100644 --- a/flow/compiler.js +++ b/flow/compiler.js @@ -18,6 +18,7 @@ declare type CompilerOptions = { shouldDecodeTags?: boolean; shouldDecodeNewlines?: boolean; shouldDecodeNewlinesForHref?: boolean; + outputSourceRange?: boolean; // runtime user-configurable delimiters?: [string, string]; // template delimiters @@ -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; stringRenderFns?: Array; - errors?: Array; - tips?: Array; + errors?: Array; + tips?: Array; }; declare type ModuleOptions = { @@ -53,11 +60,14 @@ 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: any; start?: number; end?: number }; declare type ASTElementHandler = { value: string; params?: Array; modifiers: ?ASTModifiers; + start?: number; + end?: number; }; declare type ASTElementHandlers = { @@ -70,6 +80,8 @@ declare type ASTDirective = { value: string; arg: ?string; modifiers: ?ASTModifiers; + start?: number; + end?: number; }; declare type ASTNode = ASTElement | ASTText | ASTExpression; @@ -77,11 +89,15 @@ declare type ASTNode = ASTElement | ASTText | ASTExpression; declare type ASTElement = { type: 1; tag: string; - attrsList: Array<{ name: string; value: any }>; + attrsList: Array; attrsMap: { [key: string]: any }; + rawAttrsMap: { [key: string]: ASTAttr }; parent: ASTElement | void; children: Array; + start?: number; + end?: number; + processed?: true; static?: boolean; @@ -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; + props?: Array; plain?: boolean; pre?: true; ns?: string; @@ -160,6 +176,8 @@ declare type ASTExpression = { static?: boolean; // 2.4 ssr optimization ssrOptimizability?: number; + start?: number; + end?: number; }; declare type ASTText = { @@ -169,6 +187,8 @@ declare type ASTText = { isComment?: boolean; // 2.4 ssr optimization ssrOptimizability?: number; + start?: number; + end?: number; }; // SFC-parser related declarations @@ -179,7 +199,8 @@ declare type SFCDescriptor = { script: ?SFCBlock; styles: Array; customBlocks: Array; -}; + errors: Array; +} declare type SFCBlock = { type: string; diff --git a/src/compiler/codegen/index.js b/src/compiler/codegen/index.js index d353b6457b1..5acc548a629 100644 --- a/src/compiler/codegen/index.js +++ b/src/compiler/codegen/index.js @@ -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) } @@ -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 */ ) } @@ -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) @@ -503,7 +508,7 @@ function genComponent ( })` } -function genProps (props: Array<{ name: string, value: any }>): 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 6671dcfe385..67d2057adf9 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 (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 = @@ -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/error-detector.js b/src/compiler/error-detector.js index e729c499a33..22c3b75d3aa 100644 --- a/src/compiler/error-detector.js +++ b/src/compiler/error-detector.js @@ -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' + ( @@ -19,89 +21,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 = 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) { +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 265b05c0390..0411f1a2299 100644 --- a/src/compiler/helpers.js +++ b/src/compiler/helpers.js @@ -3,9 +3,13 @@ import { emptyObject } from 'shared/util' import { parseFilters } from './parser/filter-parser' -export function baseWarn (msg: string) { +type Range = { start?: number, end?: number }; + +/* eslint-disable no-unused-vars */ +export function baseWarn (msg: string, range?: Range) { console.error(`[Vue compiler]: ${msg}`) } +/* eslint-enable no-unused-vars */ export function pluckModuleFunction ( modules: ?Array, @@ -16,20 +20,20 @@ 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(rangeSetItem({ name, value }, range)) el.plain = false } -export function addAttr (el: ASTElement, name: string, value: any) { - (el.attrs || (el.attrs = [])).push({ name, value }) +export function addAttr (el: ASTElement, name: string, value: any, range?: Range) { + (el.attrs || (el.attrs = [])).push(rangeSetItem({ name, value }, range)) el.plain = false } // add a raw attr (use this in preTransforms) -export function addRawAttr (el: ASTElement, name: string, value: any) { +export function addRawAttr (el: ASTElement, name: string, value: any, range?: Range) { el.attrsMap[name] = value - el.attrsList.push({ name, value }) + el.attrsList.push(rangeSetItem({ name, value }, range)) } export function addDirective ( @@ -38,9 +42,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(rangeSetItem({ name, rawName, value, arg, modifiers }, range)) el.plain = false } @@ -50,7 +55,8 @@ export function addHandler ( value: string, modifiers: ?ASTModifiers, important?: boolean, - warn?: Function + warn?: ?Function, + range?: Range ) { modifiers = modifiers || emptyObject // warn prevent and passive modifier @@ -61,7 +67,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 ) } @@ -100,9 +107,7 @@ export function addHandler ( events = el.events || (el.events = {}) } - const newHandler: any = { - value: value.trim() - } + const newHandler: any = rangeSetItem({ value: value.trim() }, range) if (modifiers !== emptyObject) { newHandler.modifiers = modifiers } @@ -120,6 +125,15 @@ export function addHandler ( el.plain = false } +export function getRawBindingAttr ( + el: ASTElement, + name: string +) { + return el.rawAttrsMap[':' + name] || + el.rawAttrsMap['v-bind:' + name] || + el.rawAttrsMap[name] +} + export function getBindingAttr ( el: ASTElement, name: string, @@ -162,3 +176,18 @@ export function getAndRemoveAttr ( } return val } + +function rangeSetItem ( + 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..5f56e74cdee 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,rawAttrsMap' + (keys ? ',' + keys : '') ) } diff --git a/src/compiler/parser/html-parser.js b/src/compiler/parser/html-parser.js index b0ef2d649e8..2e6fa6d324e 100644 --- a/src/compiler/parser/html-parser.js +++ b/src/compiler/parser/html-parser.js @@ -69,7 +69,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 @@ -129,16 +129,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 @@ -167,7 +169,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 + html.length }) } break } @@ -192,7 +194,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) { @@ -231,10 +235,14 @@ export function parseHTML (html, options) { name: args[1], value: decodeAttr(value, shouldDecodeNewlines) } + if (process.env.NODE_ENV !== 'production' && 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 } @@ -269,7 +277,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 9bcd42f9ed8..8900a1d90f9 100644 --- a/src/compiler/parser/index.js +++ b/src/compiler/parser/index.js @@ -16,6 +16,7 @@ import { addDirective, getBindingAttr, getAndRemoveAttr, + getRawBindingAttr, pluckModuleFunction } from '../helpers' @@ -41,11 +42,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 { @@ -53,6 +52,7 @@ export function createASTElement ( tag, attrsList: attrs, attrsMap: makeAttrsMap(attrs), + rawAttrsMap: {}, parent, children: [] } @@ -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) } } @@ -114,7 +114,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) { // check namespace. // inherit parent ns if there is one const ns = (currentParent && currentParent.ns) || platformGetTagNamespace(tag) @@ -130,12 +131,21 @@ export function parse ( element.ns = ns } + if (process.env.NODE_ENV !== 'production' && options.outputSourceRange) { + element.start = start + element.rawAttrsMap = element.attrsList.reduce((cumulated, attr) => { + cumulated[attr.name] = attr + return cumulated + }, {}) + } + 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 } ) } @@ -169,13 +179,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.', + el.rawAttrsMap['v-for'] ) } } @@ -197,7 +209,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 +234,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 +244,24 @@ export function parse ( // pop stack stack.length -= 1 currentParent = stack[stack.length - 1] + if (process.env.NODE_ENV !== 'production' && options.outputSourceRange) { + element.end = end + } closeElement(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,27 +282,40 @@ export function parse ( : preserveWhitespace && children.length ? ' ' : '' if (text) { let res + let child: ?ASTNode if (!inVPre && text !== ' ' && (res = parseText(text, delimiters))) { - children.push({ + child = { type: 2, expression: res.expression, tokens: res.tokens, text - }) + } } else if (text !== ' ' || !children.length || children[children.length - 1].text !== ' ') { - children.push({ + child = { type: 3, text - }) + } + } + if (child) { + if (process.env.NODE_ENV !== 'production' && 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 (process.env.NODE_ENV !== 'production' && options.outputSourceRange) { + child.start = start + child.end = end + } + currentParent.children.push(child) } }) return root @@ -297,13 +328,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) { @@ -333,7 +369,10 @@ function processKey (el) { if (exp) { if (process.env.NODE_ENV !== 'production') { if (el.tag === 'template') { - warn(`