Improved to not report that a value is required when parsing error, f…

…or `vue/valid-v-bind-sync`, `vue/valid-v-bind`, `vue/valid-v-else-if`, `vue/valid-v-for`, `vue/valid-v-html`, `vue/valid-v-if`, `vue/valid-v-model`, `vue/valid-v-on`, `vue/valid-v-show`, `vue/valid-v-slot` and `vue/valid-v-text` rules. (#1184)
vuejs · Jun 5, 2020 · af90fed · af90fed
1 parent 50bf17a
commit af90fed
Show file tree

Hide file tree

Showing 28 changed files with 426 additions and 84 deletions.
diff --git a/lib/rules/valid-v-bind-sync.js b/lib/rules/valid-v-bind-sync.js
@@ -37,10 +37,7 @@ function isValidElement(node) {
  * @returns {boolean} `true` if the node can be LHS.
  */
 function isLhs(node) {
-  return (
-    Boolean(node) &&
-    (node.type === 'Identifier' || node.type === 'MemberExpression')
-  )
+  return node.type === 'Identifier' || node.type === 'MemberExpression'
 }
 
 // ------------------------------------------------------------------------------
@@ -85,30 +82,32 @@ module.exports = {
           })
         }
 
-        if (node.value) {
-          if (!isLhs(node.value.expression)) {
+        if (!node.value || !node.value.expression) {
+          return
+        }
+
+        if (!isLhs(node.value.expression)) {
+          context.report({
+            node,
+            loc: node.loc,
+            messageId: 'unexpectedNonLhsExpression'
+          })
+        }
+
+        for (const reference of node.value.references) {
+          const id = reference.id
+          if (id.parent.type !== 'VExpressionContainer') {
+            continue
+          }
+          const variable = reference.variable
+          if (variable) {
             context.report({
               node,
               loc: node.loc,
-              messageId: 'unexpectedNonLhsExpression'
+              messageId: 'unexpectedUpdateIterationVariable',
+              data: { varName: id.name }
             })
           }
-
-          for (const reference of node.value.references) {
-            const id = reference.id
-            if (id.parent.type !== 'VExpressionContainer') {
-              continue
-            }
-            const variable = reference.variable
-            if (variable) {
-              context.report({
-                node,
-                loc: node.loc,
-                messageId: 'unexpectedUpdateIterationVariable',
-                data: { varName: id.name }
-              })
-            }
-          }
         }
       }
     })

diff --git a/lib/rules/valid-v-bind.js b/lib/rules/valid-v-bind.js
@@ -48,7 +48,7 @@ module.exports = {
           }
         }
 
-        if (!utils.hasAttributeValue(node)) {
+        if (!node.value || utils.isEmptyValueDirective(node, context)) {
           context.report({
             node,
             loc: node.loc,

diff --git a/lib/rules/valid-v-else-if.js b/lib/rules/valid-v-else-if.js
@@ -70,7 +70,7 @@ module.exports = {
             message: "'v-else-if' directives require no modifier."
           })
         }
-        if (!utils.hasAttributeValue(node)) {
+        if (!node.value || utils.isEmptyValueDirective(node, context)) {
           context.report({
             node,
             loc: node.loc,

diff --git a/lib/rules/valid-v-else.js b/lib/rules/valid-v-else.js
@@ -70,7 +70,7 @@ module.exports = {
             message: "'v-else' directives require no modifier."
           })
         }
-        if (utils.hasAttributeValue(node)) {
+        if (node.value) {
           context.report({
             node,
             loc: node.loc,

diff --git a/lib/rules/valid-v-for.js b/lib/rules/valid-v-for.js
@@ -137,7 +137,7 @@ module.exports = {
             message: "'v-for' directives require no modifier."
           })
         }
-        if (!utils.hasAttributeValue(node)) {
+        if (!node.value || utils.isEmptyValueDirective(node, context)) {
           context.report({
             node,
             loc: node.loc,

diff --git a/lib/rules/valid-v-html.js b/lib/rules/valid-v-html.js
@@ -44,7 +44,7 @@ module.exports = {
             message: "'v-html' directives require no modifier."
           })
         }
-        if (!utils.hasAttributeValue(node)) {
+        if (!node.value || utils.isEmptyValueDirective(node, context)) {
           context.report({
             node,
             loc: node.loc,

diff --git a/lib/rules/valid-v-if.js b/lib/rules/valid-v-if.js
@@ -62,7 +62,7 @@ module.exports = {
             message: "'v-if' directives require no modifier."
           })
         }
-        if (!utils.hasAttributeValue(node)) {
+        if (!node.value || utils.isEmptyValueDirective(node, context)) {
           context.report({
             node,
             loc: node.loc,

diff --git a/lib/rules/valid-v-model.js b/lib/rules/valid-v-model.js
@@ -42,10 +42,7 @@ function isValidElement(node) {
  * @returns {boolean} `true` if the node can be LHS.
  */
 function isLhs(node) {
-  return (
-    node != null &&
-    (node.type === 'Identifier' || node.type === 'MemberExpression')
-  )
+  return node.type === 'Identifier' || node.type === 'MemberExpression'
 }
 
 /**
@@ -133,40 +130,43 @@ module.exports = {
           }
         }
 
-        if (!utils.hasAttributeValue(node)) {
+        if (!node.value || utils.isEmptyValueDirective(node, context)) {
           context.report({
             node,
             loc: node.loc,
             message: "'v-model' directives require that attribute value."
           })
+          return
+        }
+        if (!node.value.expression) {
+          // Parsing error
+          return
         }
-        if (node.value) {
-          if (!isLhs(node.value.expression)) {
+        if (!isLhs(node.value.expression)) {
+          context.report({
+            node,
+            loc: node.loc,
+            message:
+              "'v-model' directives require the attribute value which is valid as LHS."
+          })
+        }
+
+        for (const reference of node.value.references) {
+          const id = reference.id
+          if (id.parent.type !== 'VExpressionContainer') {
+            continue
+          }
+
+          const variable = getVariable(id.name, element)
+          if (variable != null) {
             context.report({
               node,
               loc: node.loc,
               message:
-                "'v-model' directives require the attribute value which is valid as LHS."
+                "'v-model' directives cannot update the iteration variable '{{varName}}' itself.",
+              data: { varName: id.name }
             })
           }
-
-          for (const reference of node.value.references) {
-            const id = reference.id
-            if (id.parent.type !== 'VExpressionContainer') {
-              continue
-            }
-
-            const variable = getVariable(id.name, element)
-            if (variable != null) {
-              context.report({
-                node,
-                loc: node.loc,
-                message:
-                  "'v-model' directives cannot update the iteration variable '{{varName}}' itself.",
-                data: { varName: id.name }
-              })
-            }
-          }
         }
       }
     })

diff --git a/lib/rules/valid-v-on.js b/lib/rules/valid-v-on.js
@@ -108,20 +108,30 @@ module.exports = {
         }
 
         if (
-          !utils.hasAttributeValue(node) &&
+          (!node.value || !node.value.expression) &&
           !node.key.modifiers.some((modifier) =>
             VERB_MODIFIERS.has(modifier.name)
           )
         ) {
-          if (node.value && sourceCode.getText(node.value.expression)) {
-            const value = sourceCode.getText(node.value)
-            context.report({
-              node,
-              loc: node.loc,
-              message:
-                'Avoid using JavaScript keyword as "v-on" value: {{value}}.',
-              data: { value }
-            })
+          if (node.value && !utils.isEmptyValueDirective(node, context)) {
+            const valueText = sourceCode.getText(node.value)
+            let innerText = valueText
+            if (
+              (valueText[0] === '"' || valueText[0] === "'") &&
+              valueText[0] === valueText[valueText.length - 1]
+            ) {
+              // quoted
+              innerText = valueText.slice(1, -1)
+            }
+            if (/^\w+$/.test(innerText)) {
+              context.report({
+                node,
+                loc: node.loc,
+                message:
+                  'Avoid using JavaScript keyword as "v-on" value: {{value}}.',
+                data: { value: valueText }
+              })
+            }
           } else {
             context.report({
               node,

diff --git a/lib/rules/valid-v-show.js b/lib/rules/valid-v-show.js
@@ -44,7 +44,7 @@ module.exports = {
             message: "'v-show' directives require no modifier."
           })
         }
-        if (!utils.hasAttributeValue(node)) {
+        if (!node.value || utils.isEmptyValueDirective(node, context)) {
           context.report({
             node,
             loc: node.loc,

diff --git a/lib/rules/valid-v-slot.js b/lib/rules/valid-v-slot.js
@@ -264,7 +264,9 @@ module.exports = {
         if (
           ownerElement === element &&
           isDefaultSlot &&
-          !utils.hasAttributeValue(node)
+          (!node.value ||
+            utils.isEmptyValueDirective(node, context) ||
+            utils.isEmptyExpressionValueDirective(node, context))
         ) {
           context.report({
             node,

diff --git a/lib/rules/valid-v-text.js b/lib/rules/valid-v-text.js
@@ -44,7 +44,7 @@ module.exports = {
             message: "'v-text' directives require no modifier."
           })
         }
-        if (!utils.hasAttributeValue(node)) {
+        if (!node.value || utils.isEmptyValueDirective(node, context)) {
           context.report({
             node,
             loc: node.loc,

diff --git a/lib/utils/index.js b/lib/utils/index.js
@@ -290,16 +290,73 @@ module.exports = {
   },
 
   /**
-   * Check whether the given attribute has their attribute value.
-   * @param {ASTNode} node The attribute node to check.
-   * @returns {boolean} `true` if the attribute has their value.
+   * Check whether the given directive attribute has their empty value (`=""`).
+   * @param {ASTNode} node The directive attribute node to check.
+   * @param {RuleContext} context The rule context to use parser services.
+   * @returns {boolean} `true` if the directive attribute has their empty value (`=""`).
    */
-  hasAttributeValue (node) {
+  isEmptyValueDirective (node, context) {
     assert(node && node.type === 'VAttribute')
-    return (
-      node.value != null &&
-      (node.value.expression != null || node.value.syntaxError != null)
-    )
+    if (node.value == null) {
+      return false
+    }
+    if (node.value.expression != null) {
+      return false
+    }
+
+    let valueText = context.getSourceCode().getText(node.value)
+    if ((valueText[0] === '"' || valueText[0] === "'") && valueText[0] === valueText[valueText.length - 1]) {
+      // quoted
+      valueText = valueText.slice(1, -1)
+    }
+    if (!valueText) {
+      // empty
+      return true
+    }
+    return false
+  },
+
+  /**
+   * Check whether the given directive attribute has their empty expression value (e.g. `=" "`, `="/* &ast;/"`).
+   * @param {ASTNode} node The directive attribute node to check.
+   * @param {RuleContext} context The rule context to use parser services.
+   * @returns {boolean} `true` if the directive attribute has their empty expression value.
+   */
+  isEmptyExpressionValueDirective (node, context) {
+    assert(node && node.type === 'VAttribute')
+    if (node.value == null) {
+      return false
+    }
+    if (node.value.expression != null) {
+      return false
+    }
+
+    const valueNode = node.value
+    const tokenStore = context.parserServices.getTemplateBodyTokenStore()
+    let quote1 = null
+    let quote2 = null
+    // `node.value` may be only comments, so cannot get the correct tokens with `tokenStore.getTokens(node.value)`.
+    for (const token of tokenStore.getTokens(node)) {
+      if (token.range[1] <= valueNode.range[0]) {
+        continue
+      }
+      if (valueNode.range[1] <= token.range[0]) {
+        // empty
+        return true
+      }
+      if (!quote1 && token.type === 'Punctuator' && (token.value === '"' || token.value === "'")) {
+        quote1 = token
+        continue
+      }
+      if (!quote2 && quote1 && token.type === 'Punctuator' && (token.value === quote1.value)) {
+        quote2 = token
+        continue
+      }
+      // not empty
+      return false
+    }
+    // empty
+    return true
   },
 
   /**

diff --git a/tests/lib/rules/valid-v-bind-sync.js b/tests/lib/rules/valid-v-bind-sync.js
@@ -130,6 +130,21 @@ tester.run('valid-v-bind-sync', rule, {
     {
       filename: 'test.vue',
       code: '<template><MyComponent :foo.sync.unknown="foo" /></template>'
+    },
+    // parsing error
+    {
+      filename: 'parsing-error.vue',
+      code: '<template><MyComponent :foo.sync="." /></template>'
+    },
+    // comment value (parsing error)
+    {
+      filename: 'comment-value.vue',
+      code: '<template><MyComponent :foo.sync="/**/" /></template>'
+    },
+    // empty value (valid-v-bind)
+    {
+      filename: 'empty-value.vue',
+      code: '<template><MyComponent :foo.sync="" /></template>'
     }
   ],
   invalid: [

diff --git a/tests/lib/rules/valid-v-bind.js b/tests/lib/rules/valid-v-bind.js
@@ -74,6 +74,16 @@ tester.run('valid-v-bind', rule, {
     {
       filename: 'test.vue',
       code: "<template><input v-bind='$attrs' /></template>"
+    },
+    // parsing error
+    {
+      filename: 'parsing-error.vue',
+      code: '<template><MyComponent :foo="." /></template>'
+    },
+    // comment value (parsing error)
+    {
+      filename: 'comment-value.vue',
+      code: '<template><MyComponent :foo="/**/" /></template>'
     }
   ],
   invalid: [
@@ -91,6 +101,12 @@ tester.run('valid-v-bind', rule, {
       filename: 'test.vue',
       code: "<template><div :aaa.unknown='bbb'></div></template>",
       errors: ["'v-bind' directives don't support the modifier 'unknown'."]
+    },
+    // empty value
+    {
+      filename: 'empty-value.vue',
+      code: '<template><MyComponent :foo="" /></template>',
+      errors: ["'v-bind' directives require an attribute value."]
     }
   ]
 })