From e4f71bd3ce34bdca2cb2f29df60f710be1d93946 Mon Sep 17 00:00:00 2001 From: yosuke ota Date: Thu, 21 Jan 2021 19:59:49 +0900 Subject: [PATCH] Fix false positives for mixin in one-component-per-file and require-name-property rule --- lib/rules/one-component-per-file.js | 9 +++- lib/rules/require-name-property.js | 10 +++- lib/utils/index.js | 61 +++++++++++++---------- tests/lib/rules/one-component-per-file.js | 7 +++ tests/lib/rules/require-name-property.js | 10 ++++ 5 files changed, 70 insertions(+), 27 deletions(-) diff --git a/lib/rules/one-component-per-file.js b/lib/rules/one-component-per-file.js index 619942c1d..59deba90e 100644 --- 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 index e8930ceb4..6bb57b244 100644 --- 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 index 94d9994d2..35521255b 100644 --- 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,22 +1901,25 @@ 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 } } @@ -1916,25 +1927,25 @@ function isVueComponent(node) { 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 index 201b391f3..5cdabfa85 100644 --- 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 index c79ee9f2a..eae44a9ca 100644 --- 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 } ],