From 476d647bfc7c020527c37c82ee5fb409c5ff5fc1 Mon Sep 17 00:00:00 2001 From: Yosuke Ota Date: Sun, 7 Jun 2020 17:25:49 +0900 Subject: [PATCH] Fixed false positives for render props and template refs in `vue/no-unused-properties` rule. (#1198) --- lib/rules/no-unused-properties.js | 266 ++++++++++++++++++++---- lib/utils/index.js | 46 +++- tests/lib/rules/no-unused-properties.js | 229 ++++++++++++++++++++ 3 files changed, 497 insertions(+), 44 deletions(-) diff --git a/lib/rules/no-unused-properties.js b/lib/rules/no-unused-properties.js index 39c6a8267..8dce09b40 100644 --- a/lib/rules/no-unused-properties.js +++ b/lib/rules/no-unused-properties.js @@ -16,19 +16,34 @@ const eslintUtils = require('eslint-utils') * @typedef {import('vue-eslint-parser').AST.ESLintNode} ASTNode * @typedef {import('vue-eslint-parser').AST.ESLintObjectPattern} ObjectPattern * @typedef {import('vue-eslint-parser').AST.ESLintIdentifier} Identifier + * @typedef {import('vue-eslint-parser').AST.ESLintPattern} Pattern * @typedef {import('vue-eslint-parser').AST.ESLintThisExpression} ThisExpression + * @typedef {import('vue-eslint-parser').AST.ESLintMemberExpression} MemberExpression * @typedef {import('vue-eslint-parser').AST.ESLintFunctionExpression} FunctionExpression * @typedef {import('vue-eslint-parser').AST.ESLintArrowFunctionExpression} ArrowFunctionExpression * @typedef {import('vue-eslint-parser').AST.ESLintFunctionDeclaration} FunctionDeclaration + * @typedef {import('vue-eslint-parser').AST.VAttribute} VAttribute + * @typedef {import('vue-eslint-parser').AST.VIdentifier} VIdentifier + * @typedef {import('vue-eslint-parser').AST.VExpressionContainer} VExpressionContainer * @typedef {import('eslint').Scope.Variable} Variable * @typedef {import('eslint').Rule.RuleContext} RuleContext */ /** - * @typedef { { name: string, groupName: string, node: ASTNode } } PropertyData - * @typedef { { usedNames: Set } } TemplatePropertiesContainer - * @typedef { { properties: Array, usedNames: Set, unknown: boolean, usedPropsNames: Set, unknownProps: boolean } } VueComponentPropertiesContainer + * @typedef {import('../utils').ComponentPropertyData} ComponentPropertyData + */ +/** + * @typedef {object} TemplatePropertiesContainer + * @property {Set} usedNames + * @property {Set} refNames + * @typedef {object} VueComponentPropertiesContainer + * @property {ComponentPropertyData[]} properties + * @property {Set} usedNames + * @property {boolean} unknown + * @property {Set} usedPropsNames + * @property {boolean} unknownProps * @typedef { { node: FunctionExpression | ArrowFunctionExpression | FunctionDeclaration, index: number } } CallIdAndParamIndex - * @typedef { { usedNames: Set, unknown: boolean } } UsedProperties + * @typedef { { usedNames: UsedNames, unknown: boolean } } UsedProperties + * @typedef { (context: RuleContext) => UsedProps } UsedPropsTracker */ // ------------------------------------------------------------------------------ @@ -100,15 +115,61 @@ function getReferencesNames(references) { .map((ref) => ref.id.name) } +class UsedNames { + constructor() { + /** @type {Map} */ + this.map = new Map() + } + /** + * @returns {IterableIterator} + */ + names() { + return this.map.keys() + } + /** + * @param {string} name + * @returns {UsedPropsTracker[]} + */ + get(name) { + return this.map.get(name) || [] + } + /** + * @param {string} name + * @param {UsedPropsTracker} tracker + */ + add(name, tracker) { + const list = this.map.get(name) + if (list) { + list.push(tracker) + } else { + this.map.set(name, [tracker]) + } + } + /** + * @param {UsedNames} other + */ + addAll(other) { + other.map.forEach((trackers, name) => { + const list = this.map.get(name) + if (list) { + list.push(...trackers) + } else { + this.map.set(name, trackers) + } + }) + } +} + /** * @param {ObjectPattern} node * @returns {UsedProperties} */ function extractObjectPatternProperties(node) { - const usedNames = new Set() + const usedNames = new UsedNames() for (const prop of node.properties) { if (prop.type === 'Property') { - usedNames.add(utils.getStaticPropertyName(prop)) + const name = utils.getStaticPropertyName(prop) + usedNames.add(name, getObjectPatternPropertyPatternTracker(prop.value)) } else { // If use RestElement, everything is used! return { @@ -124,39 +185,109 @@ function extractObjectPatternProperties(node) { } /** - * @param {Identifier | ThisExpression} node + * @param {Pattern} pattern + * @returns {UsedPropsTracker} + */ +function getObjectPatternPropertyPatternTracker(pattern) { + if (pattern.type === 'ObjectPattern') { + return () => { + const result = new UsedProps() + const { usedNames, unknown } = extractObjectPatternProperties(pattern) + result.usedNames.addAll(usedNames) + result.unknown = unknown + return result + } + } + if (pattern.type === 'Identifier') { + return (context) => { + const result = new UsedProps() + const variable = findVariable(context, pattern) + if (!variable) { + return result + } + for (const reference of variable.references) { + /** @type {Identifier} */ + // @ts-ignore + const id = reference.identifier + const { usedNames, unknown, calls } = extractPatternOrThisProperties( + id, + context + ) + result.usedNames.addAll(usedNames) + result.unknown = result.unknown || unknown + result.calls.push(...calls) + } + return result + } + } else if (pattern.type === 'AssignmentPattern') { + return getObjectPatternPropertyPatternTracker(pattern.left) + } + return () => { + const result = new UsedProps() + result.unknown = true + return result + } +} + +/** + * @param {Identifier | MemberExpression | ThisExpression} node * @param {RuleContext} context * @returns {UsedProps} */ -function extractIdOrThisProperties(node, context) { - /** @type {UsedProps} */ +function extractPatternOrThisProperties(node, context) { const result = new UsedProps() const parent = node.parent if (parent.type === 'AssignmentExpression') { if (parent.right === node && parent.left.type === 'ObjectPattern') { // `({foo} = arg)` const { usedNames, unknown } = extractObjectPatternProperties(parent.left) - usedNames.forEach((name) => result.usedNames.add(name)) + result.usedNames.addAll(usedNames) result.unknown = result.unknown || unknown } + return result } else if (parent.type === 'VariableDeclarator') { - if (parent.init === node && parent.id.type === 'ObjectPattern') { - // `const {foo} = arg` - const { usedNames, unknown } = extractObjectPatternProperties(parent.id) - usedNames.forEach((name) => result.usedNames.add(name)) - result.unknown = result.unknown || unknown + if (parent.init === node) { + if (parent.id.type === 'ObjectPattern') { + // `const {foo} = arg` + const { usedNames, unknown } = extractObjectPatternProperties(parent.id) + result.usedNames.addAll(usedNames) + result.unknown = result.unknown || unknown + } else if (parent.id.type === 'Identifier') { + // `const foo = arg` + const variable = findVariable(context, parent.id) + if (!variable) { + return result + } + for (const reference of variable.references) { + /** @type {Identifier} */ + // @ts-ignore + const id = reference.identifier + const { usedNames, unknown, calls } = extractPatternOrThisProperties( + id, + context + ) + result.usedNames.addAll(usedNames) + result.unknown = result.unknown || unknown + result.calls.push(...calls) + } + } } + return result } else if (parent.type === 'MemberExpression') { if (parent.object === node) { // `arg.foo` const name = utils.getStaticPropertyName(parent) if (name) { - result.usedNames.add(name) + result.usedNames.add(name, () => + extractPatternOrThisProperties(parent, context) + ) } else { result.unknown = true } } + return result } else if (parent.type === 'CallExpression') { + // @ts-ignore const argIndex = parent.arguments.indexOf(node) if (argIndex > -1 && parent.callee.type === 'Identifier') { // `foo(arg)` @@ -195,8 +326,7 @@ function extractIdOrThisProperties(node, context) { */ class UsedProps { constructor() { - /** @type {Set} */ - this.usedNames = new Set() + this.usedNames = new UsedNames() /** @type {CallIdAndParamIndex[]} */ this.calls = [] this.unknown = false @@ -220,7 +350,7 @@ class ParamUsedProps extends UsedProps { } if (paramNode.type === 'ObjectPattern') { const { usedNames, unknown } = extractObjectPatternProperties(paramNode) - usedNames.forEach((name) => this.usedNames.add(name)) + this.usedNames.addAll(usedNames) this.unknown = this.unknown || unknown return } @@ -232,11 +362,11 @@ class ParamUsedProps extends UsedProps { /** @type {Identifier} */ // @ts-ignore const id = reference.identifier - const { usedNames, unknown, calls } = extractIdOrThisProperties( + const { usedNames, unknown, calls } = extractPatternOrThisProperties( id, context ) - usedNames.forEach((name) => this.usedNames.add(name)) + this.usedNames.addAll(usedNames) this.unknown = this.unknown || unknown this.calls.push(...calls) } @@ -325,7 +455,8 @@ module.exports = { const paramsUsedPropsMap = new Map() /** @type {TemplatePropertiesContainer} */ const templatePropertiesContainer = { - usedNames: new Set() + usedNames: new Set(), + refNames: new Set() } /** @type {Map} */ const vueComponentPropertiesContainerMap = new Map() @@ -368,6 +499,7 @@ module.exports = { function reportUnusedProperties() { for (const container of vueComponentPropertiesContainerMap.values()) { if (container.unknown) { + // unknown continue } for (const property of container.properties) { @@ -375,6 +507,7 @@ module.exports = { container.usedNames.has(property.name) || templatePropertiesContainer.usedNames.has(property.name) ) { + // used continue } if ( @@ -382,6 +515,14 @@ module.exports = { (container.unknownProps || container.usedPropsNames.has(property.name)) ) { + // used props + continue + } + if ( + property.groupName === 'setup' && + templatePropertiesContainer.refNames.has(property.name) + ) { + // used template refs continue } context.report({ @@ -399,7 +540,7 @@ module.exports = { /** * @param {UsedProps} usedProps * @param {Map>} already - * @returns {Generator} + * @returns {IterableIterator} */ function* iterateUsedProps(usedProps, already = new Map()) { yield usedProps @@ -423,6 +564,22 @@ module.exports = { } } + /** + * @param {VueComponentPropertiesContainer} container + * @param {UsedProps} baseUseProps + */ + function processParamPropsUsed(container, baseUseProps) { + for (const { usedNames, unknown } of iterateUsedProps(baseUseProps)) { + if (unknown) { + container.unknownProps = true + return + } + for (const name of usedNames.names()) { + container.usedPropsNames.add(name) + } + } + } + const scriptVisitor = Object.assign( {}, utils.defineVueVisitor(context, { @@ -448,23 +605,45 @@ module.exports = { }, onSetupFunctionEnter(node, vueData) { const container = getVueComponentPropertiesContainer(vueData.node) - const propsParam = node.params[0] - if (!propsParam) { - // no arguments - return + if (node.params[0]) { + const paramsUsedProps = getParamsUsedProps(node) + const paramUsedProps = paramsUsedProps.getParam(0) + + processParamPropsUsed(container, paramUsedProps) } - const paramsUsedProps = getParamsUsedProps(node) - const paramUsedProps = paramsUsedProps.getParam(0) + }, + onRenderFunctionEnter(node, vueData) { + const container = getVueComponentPropertiesContainer(vueData.node) + if (node.params[0]) { + // for Vue 3.x render + const paramsUsedProps = getParamsUsedProps(node) + const paramUsedProps = paramsUsedProps.getParam(0) - for (const { usedNames, unknown } of iterateUsedProps( - paramUsedProps - )) { - if (unknown) { - container.unknownProps = true + processParamPropsUsed(container, paramUsedProps) + if (container.unknownProps) { return } - for (const name of usedNames) { - container.usedPropsNames.add(name) + } + + if (vueData.functional && node.params[1]) { + // for Vue 2.x render & functional + const paramsUsedProps = getParamsUsedProps(node) + const paramUsedProps = paramsUsedProps.getParam(1) + + for (const { usedNames, unknown } of iterateUsedProps( + paramUsedProps + )) { + if (unknown) { + container.unknownProps = true + return + } + for (const usedPropsTracker of usedNames.get('props')) { + const propUsedProps = usedPropsTracker(context) + processParamPropsUsed(container, propUsedProps) + if (container.unknownProps) { + return + } + } } } }, @@ -473,14 +652,14 @@ module.exports = { return } const container = getVueComponentPropertiesContainer(vueData.node) - const usedProps = extractIdOrThisProperties(node, context) + const usedProps = extractPatternOrThisProperties(node, context) for (const { usedNames, unknown } of iterateUsedProps(usedProps)) { if (unknown) { container.unknown = true return } - for (const name of usedNames) { + for (const name of usedNames.names()) { container.usedNames.add(name) } } @@ -496,11 +675,22 @@ module.exports = { ) const templateVisitor = { + /** + * @param {VExpressionContainer} node + */ VExpressionContainer(node) { for (const name of getReferencesNames(node.references)) { templatePropertiesContainer.usedNames.add(name) } }, + /** + * @param {VAttribute} node + */ + 'VAttribute[directive=false]'(node) { + if (node.key.name === 'ref' && node.value != null) { + templatePropertiesContainer.refNames.add(node.value.value) + } + }, "VElement[parent.type!='VElement']:exit"() { reportUnusedProperties() } diff --git a/lib/utils/index.js b/lib/utils/index.js index 6b7df723f..e9f8efe50 100644 --- a/lib/utils/index.js +++ b/lib/utils/index.js @@ -698,6 +698,7 @@ module.exports = { * - `onVueObjectEnter` ... Event when Vue Object is found. * - `onVueObjectExit` ... Event when Vue Object visit ends. * - `onSetupFunctionEnter` ... Event when setup function found. + * - `onRenderFunctionEnter` ... Event when render function found. * * @param {RuleContext} context The ESLint rule context object. * @param {Object} visitor The visitor to traverse the Vue Objects. @@ -716,10 +717,35 @@ module.exports = { vueVisitor[key] = (node) => callVisitor(key, node) } + /** + * @param {ObjectExpression} node + */ vueVisitor.ObjectExpression = (node) => { const type = getVueObjectType(context, node) if (type) { - vueStack = { node, type, parent: vueStack } + vueStack = { + node, + type, + parent: vueStack, + get functional () { + /** @type {Property} */ + const functional = node.properties.find( + (p) => + p.type === 'Property' && + getStaticPropertyName(p) === 'functional' + ) + if (!functional) { + return false + } + if ( + functional.value.type === 'Literal' && + functional.value.value === false + ) { + return false + } + return true + } + } callVisitor('onVueObjectEnter', node) } callVisitor('ObjectExpression', node) @@ -731,16 +757,24 @@ module.exports = { vueStack = vueStack.parent } } - if (visitor.onSetupFunctionEnter) { - vueVisitor['Property[value.type=/^(Arrow)?FunctionExpression$/] > :function'] = (node) => { + if (visitor.onSetupFunctionEnter || visitor.onRenderFunctionEnter) { + vueVisitor[ + 'Property[value.type=/^(Arrow)?FunctionExpression$/] > :function' + ] = (node) => { /** @type {Property} */ const prop = node.parent - if (vueStack && prop.parent === vueStack.node) { - if (getStaticPropertyName(prop) === 'setup' && prop.value === node) { + if (vueStack && prop.parent === vueStack.node && prop.value === node) { + const name = getStaticPropertyName(prop) + if (name === 'setup') { callVisitor('onSetupFunctionEnter', node) + } else if (name === 'render') { + callVisitor('onRenderFunctionEnter', node) } } - callVisitor('Property[value.type=/^(Arrow)?FunctionExpression$/] > :function', node) + callVisitor( + 'Property[value.type=/^(Arrow)?FunctionExpression$/] > :function', + node + ) } } diff --git a/tests/lib/rules/no-unused-properties.js b/tests/lib/rules/no-unused-properties.js index 7c37da2f8..c5796e752 100644 --- a/tests/lib/rules/no-unused-properties.js +++ b/tests/lib/rules/no-unused-properties.js @@ -766,6 +766,221 @@ tester.run('no-unused-properties', rule, { } ` + }, + + // render & functional + { + filename: 'test.js', + code: ` + Vue.component('smart-list', { + functional: true, + props: { + items: { + type: Array, + required: true + }, + isOrdered: Boolean + }, + render: function (createElement, context) { + function appropriateListComponent () { + var items = context.props.items + + if (items.length === 0) return EmptyList + if (typeof items[0] === 'object') return TableList + if (context.props.isOrdered) return OrderedList + + return UnorderedList + } + + return createElement( + appropriateListComponent(), + context.data, + context.children + ) + } + }) + ` + }, + { + filename: 'test.js', + code: ` + Vue.component('MyButton', { + functional: true, + props: ['foo'], + render: function (createElement, {props}) { + return createElement('button', props.foo) + } + }) + ` + }, + { + filename: 'test.js', + code: ` + Vue.component('MyButton', { + functional: true, + props: ['foo'], + render: function (createElement, ctx) { + return createElement('button', fn(ctx.props)) + } + }) + + function fn(props) { + return props.foo + } + ` + }, + { + filename: 'test.js', + code: ` + Vue.component('MyButton', { + functional: true, + props: ['foo'], + render: function (createElement, ctx) { + return createElement('button', fn(ctx)) + } + }) + + function fn({props}) { + return props.foo + } + ` + }, + { + filename: 'test.js', + code: ` + Vue.component('MyButton', { + functional: true, + props: ['foo'], + render: function (createElement, {props:{foo}}) { + return createElement('button', foo) + } + }) + ` + }, + { + filename: 'test.js', + code: ` + Vue.component('MyButton', { + functional: true, + props: ['foo'], + render: function (createElement, {props:[bar]}) { + return createElement('button') + } + }) + ` + }, + { + filename: 'test.js', + code: ` + Vue.component('MyButton', { + functional: true, + props: ['foo'], + render: function (createElement, {props:bar={}}) { + return createElement('button', bar.foo) + } + }) + ` + }, + { + filename: 'test.js', + code: ` + Vue.component('MyButton', { + functional: true, + props: ['foo'], + render: function (createElement, {...foo}) { + return createElement('button') + } + }) + ` + }, + { + filename: 'test.js', + code: ` + Vue.component('MyButton', { + functional: true, + props: ['foo', 'bar'], + render: function (createElement, ctx) { + const a = ctx.props + const b = ctx.props + return createElement('button', a.foo + b.bar) + } + }) + ` + }, + { + filename: 'test.js', + code: ` + Vue.component('MyButton', { + functional: true, + props: ['foo', 'bar'], + render: function (createElement, {props: a, props: b}) { + return createElement('button', a.foo + b.bar) + } + }) + ` + }, + // render for Vue 3.x + { + filename: 'test.vue', + code: ` + export default { + props: ['foo'], + render (props) { + return h('button', props.foo) + } + }) + ` + }, + { + filename: 'test.vue', + code: ` + export default { + props: ['foo'], + render ({foo}) { + return h('button', foo) + } + }) + ` + }, + { + filename: 'test.vue', + code: ` + export default { + props: ['foo'], + render (bar) { + const {...baz} = bar + return h('button') + } + }) + ` + }, + // Vue.js 3.x Template Refs + { + filename: 'test.vue', + code: ` + + + `, + options: [{ groups: ['props', 'setup'] }] } ], @@ -1248,6 +1463,20 @@ tester.run('no-unused-properties', rule, { "'bar' of property found, but never used.", "'baz' of property found, but never used." ] + }, + + // render & not functional + { + filename: 'test.js', + code: ` + Vue.component('MyButton', { + props: ['foo'], + render: function (createElement, {props}) { + return createElement('button', props.foo) + } + }) + `, + errors: ["'foo' of property found, but never used."] } ] })