Fix false positives for mixin in one-component-per-file and require-n…

…ame-property rule (#1419)
vuejs · Jan 22, 2021 · 6894340 · 6894340
1 parent 3f20a08
commit 6894340
Show file tree

Hide file tree

Showing 5 changed files with 70 additions and 27 deletions.
diff --git a/lib/rules/one-component-per-file.js b/lib/rules/one-component-per-file.js
@@ -4,6 +4,7 @@
  */
 'use strict'
 const utils = require('../utils')
+const { getVueComponentDefinitionType } = require('../utils')
 
 // ------------------------------------------------------------------------------
 // Rule Definition
@@ -30,7 +31,13 @@ module.exports = {
 
     return Object.assign(
       {},
-      utils.executeOnVueComponent(context, (node) => {
+      utils.executeOnVueComponent(context, (node, type) => {
+        if (type === 'definition') {
+          const defType = getVueComponentDefinitionType(node)
+          if (defType === 'mixin') {
+            return
+          }
+        }
         components.push(node)
       }),
       {

diff --git a/lib/rules/require-name-property.js b/lib/rules/require-name-property.js
@@ -5,6 +5,7 @@
 'use strict'
 
 const utils = require('../utils')
+const { getVueComponentDefinitionType } = require('../utils')
 
 /**
  * @param {Property | SpreadElement} node
@@ -31,7 +32,14 @@ module.exports = {
   },
   /** @param {RuleContext} context */
   create(context) {
-    return utils.executeOnVue(context, (component) => {
+    return utils.executeOnVue(context, (component, type) => {
+      if (type === 'definition') {
+        const defType = getVueComponentDefinitionType(component)
+        if (defType === 'mixin') {
+          return
+        }
+      }
+
       if (component.properties.some(isNameProperty)) return
 
       context.report({

diff --git a/lib/utils/index.js b/lib/utils/index.js
@@ -1003,6 +1003,13 @@ module.exports = {
   },
 
   getVueObjectType,
+  /**
+   * Get the Vue component definition type from given node
+   * Vue.component('xxx', {}) || component('xxx', {})
+   * @param {ObjectExpression} node Node to check
+   * @returns {'component' | 'mixin' | 'extend' | 'createApp' | 'defineComponent' | null}
+   */
+  getVueComponentDefinitionType,
   compositingVisitors,
 
   /**
@@ -1876,14 +1883,15 @@ function isVueComponentFile(node, path) {
 }
 
 /**
- * Check whether given node is Vue component
+ * Get the Vue component definition type from given node
  * Vue.component('xxx', {}) || component('xxx', {})
- * @param {ESNode} node Node to check
- * @returns {boolean}
+ * @param {ObjectExpression} node Node to check
+ * @returns {'component' | 'mixin' | 'extend' | 'createApp' | 'defineComponent' | null}
  */
-function isVueComponent(node) {
-  if (node.type === 'CallExpression') {
-    const callee = node.callee
+function getVueComponentDefinitionType(node) {
+  const parent = getParent(node)
+  if (parent.type === 'CallExpression') {
+    const callee = parent.callee
 
     if (callee.type === 'MemberExpression') {
       const calleeObject = skipTSAsExpression(callee.object)
@@ -1893,48 +1901,51 @@ function isVueComponent(node) {
         if (calleeObject.name === 'Vue') {
           // for Vue.js 2.x
           // Vue.component('xxx', {}) || Vue.mixin({}) || Vue.extend('xxx', {})
-          const isFullVueComponentForVue2 =
-            propName &&
-            ['component', 'mixin', 'extend'].includes(propName) &&
-            isObjectArgument(node)
-
-          return Boolean(isFullVueComponentForVue2)
+          const maybeFullVueComponentForVue2 =
+            propName && isObjectArgument(parent)
+
+          return maybeFullVueComponentForVue2 &&
+            (propName === 'component' ||
+              propName === 'mixin' ||
+              propName === 'extend')
+            ? propName
+            : null
         }
 
         // for Vue.js 3.x
         // app.component('xxx', {}) || app.mixin({})
-        const isFullVueComponent =
-          propName &&
-          ['component', 'mixin'].includes(propName) &&
-          isObjectArgument(node)
+        const maybeFullVueComponent = propName && isObjectArgument(parent)
 
-        return Boolean(isFullVueComponent)
+        return maybeFullVueComponent &&
+          (propName === 'component' || propName === 'mixin')
+          ? propName
+          : null
       }
     }
 
     if (callee.type === 'Identifier') {
       if (callee.name === 'component') {
         // for Vue.js 2.x
         // component('xxx', {})
-        const isDestructedVueComponent = isObjectArgument(node)
-        return isDestructedVueComponent
+        const isDestructedVueComponent = isObjectArgument(parent)
+        return isDestructedVueComponent ? 'component' : null
       }
       if (callee.name === 'createApp') {
         // for Vue.js 3.x
         // createApp({})
-        const isAppVueComponent = isObjectArgument(node)
-        return isAppVueComponent
+        const isAppVueComponent = isObjectArgument(parent)
+        return isAppVueComponent ? 'createApp' : null
       }
       if (callee.name === 'defineComponent') {
         // for Vue.js 3.x
         // defineComponent({})
-        const isDestructedVueComponent = isObjectArgument(node)
-        return isDestructedVueComponent
+        const isDestructedVueComponent = isObjectArgument(parent)
+        return isDestructedVueComponent ? 'defineComponent' : null
       }
     }
   }
 
-  return false
+  return null
 
   /** @param {CallExpression} node */
   function isObjectArgument(node) {
@@ -1986,7 +1997,7 @@ function getVueObjectType(context, node) {
   } else if (parent.type === 'CallExpression') {
     // Vue.component('xxx', {}) || component('xxx', {})
     if (
-      isVueComponent(parent) &&
+      getVueComponentDefinitionType(node) != null &&
       skipTSAsExpression(parent.arguments.slice(-1)[0]) === node
     ) {
       return 'definition'

diff --git a/tests/lib/rules/one-component-per-file.js b/tests/lib/rules/one-component-per-file.js
@@ -51,6 +51,13 @@ ruleTester.run('one-component-per-file', rule, {
           }
         }
       }`
+    },
+    {
+      filename: 'test.js',
+      code: `
+        Vue.mixin({})
+        Vue.component('name', {})
+      `
     }
   ],
   invalid: [

diff --git a/tests/lib/rules/require-name-property.js b/tests/lib/rules/require-name-property.js
@@ -41,6 +41,16 @@ ruleTester.run('require-name-property', rule, {
         }
       `,
       parserOptions
+    },
+    {
+      code: `
+        Vue.mixin({
+          methods: {
+            $foo () {}
+          }
+        })
+      `,
+      parserOptions
     }
   ],