improve template error detector

Author: yyx990803

flow/compiler.js

@@ -88,11 +88,13 @@ declare type ASTElement = {
   renderMethod?: ?string,
   renderArgs?: ?string,
 
-  if?: string | null,
+  if?: string,
+  ifProcessed?: boolean,
   else?: true,
   elseBlock?: ASTElement,
 
-  for?: string | null,
+  for?: string,
+  forProcessed?: boolean,
   key?: string,
   alias?: string,
   iterator1?: string,

src/compiler/codegen.js

@@ -39,9 +39,9 @@ export function generate (
 }
 
 function genElement (el: ASTElement): string {
-  if (el.for) {
+  if (el.for && !el.forProcessed) {
     return genFor(el)
-  } else if (el.if) {
+  } else if (el.if && !el.ifProcessed) {
     return genIf(el)
   } else if (el.tag === 'template' && !el.slotTarget) {
     return genChildren(el) || 'void 0'
@@ -88,7 +88,7 @@ function genElement (el: ASTElement): string {
 
 function genIf (el: ASTElement): string {
   const exp = el.if
-  el.if = null // avoid recursion
+  el.ifProcessed = true // avoid recursion
   return `(${exp})?${genElement(el)}:${genElse(el)}`
 }
 
@@ -103,7 +103,7 @@ function genFor (el: ASTElement): string {
   const alias = el.alias
   const iterator1 = el.iterator1 ? `,${el.iterator1}` : ''
   const iterator2 = el.iterator2 ? `,${el.iterator2}` : ''
-  el.for = null // avoid recursion
+  el.forProcessed = true // avoid recursion
   return `(${exp})&&_l((${exp}),` +
     `function(${alias}${iterator1}${iterator2}){` +
       `return ${genElement(el)}` +

src/compiler/error-detector.js

@@ -2,11 +2,14 @@
 
 import { dirRE } from './parser/index'
 
-const keywordRE = new RegExp('\\b' + (
-  'do,if,in,for,let,new,try,var,case,else,with,await,break,catch,class,const,' +
-  'super,throw,while,yield,delete,export,import,return,switch,typeof,default,' +
-  'extends,finally,continue,debugger,function,arguments,instanceof'
+// operators like typeof, instanceof and in are allowed
+const prohibitedKeywordRE = new RegExp('\\b' + (
+  'do,if,for,let,new,try,var,case,else,with,await,break,catch,class,const,' +
+  'super,throw,while,yield,delete,export,import,return,switch,default,' +
+  'extends,finally,continue,debugger,function,arguments'
 ).split(',').join('\\b|\\b') + '\\b')
+// check valid identifier for v-for
+const identRE = /[^\w$\.](?:[A-Za-z_$][\w$]*)/
 
 // detect problematic expressions in a template
 export function detectErrors (ast: ?ASTNode): Array<string> {
@@ -23,7 +26,11 @@ function checkNode (node: ASTNode, errors: Array<string>) {
       if (dirRE.test(name)) {
         const value = node.attrsMap[name]
         if (value) {
-          checkExpression(value, `${name}="${value}"`, errors)
+          if (name === 'v-for') {
+            checkFor(node, `v-for="${value}"`, errors)
+          } else {
+            checkExpression(value, `${name}="${value}"`, errors)
+          }
         }
       }
     }
@@ -37,17 +44,30 @@ function checkNode (node: ASTNode, errors: Array<string>) {
   }
 }
 
+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 checkIdentifier (ident: ?string, type: string, text: string, errors: Array<string>) {
+  if (typeof ident === 'string' && !identRE.test(ident)) {
+    errors.push(`- invalid ${type} "${ident}" in expression: ${text}`)
+  }
+}
+
 function checkExpression (exp: string, text: string, errors: Array<string>) {
   exp = stripToString(exp)
-  const keywordMatch = exp.match(keywordRE)
+  const keywordMatch = exp.match(prohibitedKeywordRE)
   if (keywordMatch) {
     errors.push(
       `- avoid using JavaScript keyword as property name: ` +
       `"${keywordMatch[0]}" in expression ${text}`
     )
   } else {
     try {
-      new Function(exp)
+      new Function(`return ${exp}`)
     } catch (e) {
       errors.push(`- invalid expression: ${text}`)
     }

src/compiler/parser/index.js

@@ -17,12 +17,12 @@ import {
 } from '../helpers'
 
 export const dirRE = /^v-|^@|^:/
+export const forAliasRE = /(.*)\s+(?:in|of)\s+(.*)/
+export const forIteratorRE = /\(([^,]*),([^,]*)(?:,([^,]*))?\)/
 const bindRE = /^:|^v-bind:/
 const onRE = /^@|^v-on:/
 const argRE = /:(.*)$/
 const modifierRE = /\.[^\.]+/g
-const forAliasRE = /(.*)\s+(?:in|of)\s+(.*)/
-const forIteratorRE = /\(([^,]*),([^,]*)(?:,([^,]*))?\)/
 const camelRE = /[a-z\d][A-Z]/
 
 const decodeHTMLCached = cached(decodeHTML)

test/unit/features/options/template.spec.js

@@ -60,4 +60,14 @@ describe('Options template', () => {
     expect('invalid expression: {{ a"" }}').toHaveBeenWarned()
     expect('avoid using JavaScript keyword as property name: "do" in expression {{ do + 1 }}').toHaveBeenWarned()
   })
+
+  it('warn error in generated function (v-for)', () => {
+    new Vue({
+      template: '<div><div v-for="(1, 2) in a----"></div></div>'
+    }).$mount()
+    expect('failed to compile template').toHaveBeenWarned()
+    expect('invalid v-for alias "1"').toHaveBeenWarned()
+    expect('invalid v-for iterator "2"').toHaveBeenWarned()
+    expect('invalid expression: v-for="(1, 2) in a----"').toHaveBeenWarned()
+  })
 })