From e0f7c4a60832e0f951cd6f440fc4c2854d05a608 Mon Sep 17 00:00:00 2001 From: Flare Date: Thu, 20 Apr 2023 11:22:06 +0800 Subject: [PATCH 1/5] Add `propProps` option to vue/no-mutating-props (#1371) --- docs/rules/no-mutating-props.md | 72 +++++++++- lib/rules/no-mutating-props.js | 142 +++++++++++++------- lib/utils/index.js | 73 +++++----- tests/lib/rules/no-mutating-props.js | 194 ++++++++++++++++++++++++++- 4 files changed, 398 insertions(+), 83 deletions(-) diff --git a/docs/rules/no-mutating-props.md b/docs/rules/no-mutating-props.md index 960ebe900..0623ed5a4 100644 --- a/docs/rules/no-mutating-props.md +++ b/docs/rules/no-mutating-props.md @@ -22,6 +22,8 @@ This rule reports mutation of component props. +``` + + ## :books: Further Reading diff --git a/lib/rules/no-mutating-props.js b/lib/rules/no-mutating-props.js index f79613d9c..21f6ae6ef 100644 --- a/lib/rules/no-mutating-props.js +++ b/lib/rules/no-mutating-props.js @@ -4,6 +4,10 @@ */ 'use strict' +/** + * @typedef {{name?: string, set: Set}} PropsInfo + */ + const utils = require('../utils') const { findVariable } = require('@eslint-community/eslint-utils') @@ -84,6 +88,19 @@ function isVmReference(node) { return false } +/** + * @param { object } options + * @param { boolean } options.propProps avoid mutating the value of a prop but leaving the reference the same + */ +function parseOptions(options) { + return Object.assign( + { + propProps: true + }, + options + ) +} + module.exports = { meta: { type: 'suggestion', @@ -94,12 +111,21 @@ module.exports = { }, fixable: null, // or "code" or "whitespace" schema: [ - // fill in your schema + { + type: 'object', + properties: { + propProps: { + type: 'boolean' + } + }, + additionalProperties: false + } ] }, /** @param {RuleContext} context */ create(context) { - /** @type {Map>} */ + const { propProps } = parseOptions(context.options[0]) + /** @type {Map} */ const propsMap = new Map() /** @type { { type: 'export' | 'mark' | 'definition', object: ObjectExpression } | { type: 'setup', object: CallExpression } | null } */ let vueObjectData = null @@ -138,9 +164,10 @@ module.exports = { /** * @param {MemberExpression|Identifier} props * @param {string} name + * @param {boolean} isRootProps */ - function verifyMutating(props, name) { - const invalid = utils.findMutating(props) + function verifyMutating(props, name, isRootProps = false) { + const invalid = utils.findMutating(props, propProps, isRootProps) if (invalid) { report(invalid.node, name) } @@ -192,8 +219,9 @@ module.exports = { /** * @param {Identifier} prop * @param {string[]} path + * @param {boolean} isRootProps */ - function verifyPropVariable(prop, path) { + function verifyPropVariable(prop, path, isRootProps = false) { const variable = findVariable(context.getScope(), prop) if (!variable) { return @@ -205,7 +233,7 @@ module.exports = { } const id = reference.identifier - const invalid = utils.findMutating(id) + const invalid = utils.findMutating(id, propProps, isRootProps) if (!invalid) { continue } @@ -252,20 +280,23 @@ module.exports = { onDefinePropsEnter(node, props) { const defineVariableNames = new Set(extractDefineVariableNames()) - const propsSet = new Set( - props - .map((p) => p.propName) - .filter( - /** - * @returns {propName is string} - */ - (propName) => - utils.isDef(propName) && - !GLOBALS_WHITE_LISTED.has(propName) && - !defineVariableNames.has(propName) - ) - ) - propsMap.set(node, propsSet) + const propsInfo = { + name: '', + set: new Set( + props + .map((p) => p.propName) + .filter( + /** + * @returns {propName is string} + */ + (propName) => + utils.isDef(propName) && + !GLOBALS_WHITE_LISTED.has(propName) && + !defineVariableNames.has(propName) + ) + ) + } + propsMap.set(node, propsInfo) vueObjectData = { type: 'setup', object: node @@ -294,22 +325,25 @@ module.exports = { target.parent.id, [] )) { - verifyPropVariable(prop, path) - propsSet.add(prop.name) + if (path.length === 0) { + propsInfo.name = prop.name + } else { + propsInfo.set.add(prop.name) + } + verifyPropVariable(prop, path, propsInfo.name === prop.name) } } }), utils.defineVueVisitor(context, { onVueObjectEnter(node) { - propsMap.set( - node, - new Set( + propsMap.set(node, { + set: new Set( utils .getComponentPropsFromOptions(node) .map((p) => p.propName) .filter(utils.isDef) ) - ) + }) }, onVueObjectExit(node, { type }) { if ( @@ -341,7 +375,11 @@ module.exports = { propsParam, [] )) { - verifyPropVariable(prop, path) + verifyPropVariable( + prop, + path, + prop.parent.type === 'FunctionExpression' + ) } }, /** @param {(Identifier | ThisExpression) & { parent: MemberExpression } } node */ @@ -359,7 +397,7 @@ module.exports = { const name = utils.getStaticPropertyName(mem) if ( name && - /** @type {Set} */ (propsMap.get(vueNode)).has(name) + /** @type {PropsInfo} */ (propsMap.get(vueNode)).set.has(name) ) { verifyMutating(mem, name) } @@ -378,9 +416,9 @@ module.exports = { const name = utils.getStaticPropertyName(mem) if ( name && - /** @type {Set} */ (propsMap.get(vueObjectData.object)).has( - name - ) + /** @type {PropsInfo} */ ( + propsMap.get(vueObjectData.object) + ).set.has(name) ) { verifyMutating(mem, name) } @@ -393,14 +431,19 @@ module.exports = { if (!isVmReference(node)) { return } - const name = node.name - if ( - name && - /** @type {Set} */ (propsMap.get(vueObjectData.object)).has( - name - ) - ) { - verifyMutating(node, name) + const propsInfo = /** @type {PropsInfo} */ ( + propsMap.get(vueObjectData.object) + ) + const isRootProps = !!node.name && propsInfo.name === node.name + const parent = node.parent + const parentProperty = + parent.type === 'MemberExpression' ? parent.property : null + const name = + isRootProps && parentProperty?.type === 'Identifier' + ? parentProperty.name + : node.name + if (name && (propsInfo.set.has(name) || isRootProps)) { + verifyMutating(node, name, isRootProps) } }, /** @param {ESNode} node */ @@ -423,12 +466,22 @@ module.exports = { return } + const propsInfo = /** @type {PropsInfo} */ ( + propsMap.get(vueObjectData.object) + ) + const nodes = utils.getMemberChaining(node) const first = nodes[0] let name - if (isVmReference(first)) { + if (isVmReference(first) && first.name !== propsInfo.name) { + if (!propProps && nodes.length > 1) { + return + } name = first.name - } else if (first.type === 'ThisExpression') { + } else if (first.type === 'ThisExpression' || isVmReference(first)) { + if (!propProps && nodes.length > 2) { + return + } const mem = nodes[1] if (!mem) { return @@ -437,12 +490,7 @@ module.exports = { } else { return } - if ( - name && - /** @type {Set} */ (propsMap.get(vueObjectData.object)).has( - name - ) - ) { + if (name && propsInfo.set.has(name)) { report(node, name) } } diff --git a/lib/utils/index.js b/lib/utils/index.js index 249097418..81f43ba12 100644 --- a/lib/utils/index.js +++ b/lib/utils/index.js @@ -1778,9 +1778,11 @@ module.exports = { /** * @param {MemberExpression|Identifier} props + * @param {boolean} propProps + * @param {boolean} isRootProps * @returns { { kind: 'assignment' | 'update' | 'call' , node: ESNode, pathNodes: MemberExpression[] } | null } */ - findMutating(props) { + findMutating(props, propProps = true, isRootProps = false) { /** @type {MemberExpression[]} */ const pathNodes = [] /** @type {MemberExpression | Identifier | ChainExpression} */ @@ -1807,50 +1809,55 @@ module.exports = { pathNodes } } - case 'UnaryExpression': { - if (target.operator === 'delete') { - return { - kind: 'update', - node: target, - pathNodes - } + case 'MemberExpression': { + if ((propProps || isRootProps) && target.object === node) { + isRootProps = false + pathNodes.push(target) + node = target + target = target.parent + continue // loop } break } - case 'CallExpression': { - if (pathNodes.length > 0 && target.callee === node) { - const mem = pathNodes[pathNodes.length - 1] - const callName = getStaticPropertyName(mem) - if ( - callName && - /^(?:push|pop|shift|unshift|reverse|splice|sort|copyWithin|fill)$/u.test( - callName - ) - ) { - // this.xxx.push() - pathNodes.pop() + } + if (propProps) { + switch (target.type) { + case 'UnaryExpression': { + if (target.operator === 'delete') { return { - kind: 'call', + kind: 'update', node: target, pathNodes } } + break } - break - } - case 'MemberExpression': { - if (target.object === node) { - pathNodes.push(target) + case 'CallExpression': { + if (pathNodes.length > 0 && target.callee === node) { + const mem = pathNodes[pathNodes.length - 1] + const callName = getStaticPropertyName(mem) + if ( + callName && + /^(?:push|pop|shift|unshift|reverse|splice|sort|copyWithin|fill)$/u.test( + callName + ) + ) { + // this.xxx.push() + pathNodes.pop() + return { + kind: 'call', + node: target, + pathNodes + } + } + } + break + } + case 'ChainExpression': { node = target target = target.parent continue // loop } - break - } - case 'ChainExpression': { - node = target - target = target.parent - continue // loop } } @@ -1875,7 +1882,7 @@ module.exports = { }), /** - * Checks whether or not the tokens of two given nodes are same. + Checks whether the tokens of two given nodes are same. * @param {ASTNode} left A node 1 to compare. * @param {ASTNode} right A node 2 to compare. * @param {ParserServices.TokenStore | SourceCode} sourceCode The ESLint source code object. diff --git a/tests/lib/rules/no-mutating-props.js b/tests/lib/rules/no-mutating-props.js index 7338d9374..4df56029b 100644 --- a/tests/lib/rules/no-mutating-props.js +++ b/tests/lib/rules/no-mutating-props.js @@ -181,6 +181,37 @@ ruleTester.run('no-mutating-props', rule, { ` }, + { + filename: 'test.vue', + code: ` + + + `, + options: [{ propProps: false }] + }, // setup { @@ -325,6 +356,43 @@ ruleTester.run('no-mutating-props', rule, { ` }, + { + filename: 'test.vue', + code: ` + + `, + options: [{ propProps: false }] + }, + { + filename: 'test.vue', + code: ` + + `, + options: [{ propProps: false }] + }, + { // script setup with shadow filename: 'test.vue', @@ -642,6 +710,48 @@ ruleTester.run('no-mutating-props', rule, { } ] }, + { + filename: 'test.vue', + code: ` + + + `, + options: [{ propProps: false }], + errors: [ + { + message: 'Unexpected mutation of "prop1" prop.', + line: 4 + }, + { + message: 'Unexpected mutation of "prop2" prop.', + line: 5 + }, + { + message: 'Unexpected mutation of "prop5" prop.', + line: 8 + }, + { + message: 'Unexpected mutation of "prop6" prop.', + line: 9 + } + ] + }, // setup { @@ -839,7 +949,7 @@ ruleTester.run('no-mutating-props', rule, { line: 3 }, { - message: 'Unexpected mutation of "props" prop.', + message: 'Unexpected mutation of "value" prop.', line: 4 } ] @@ -898,6 +1008,41 @@ ruleTester.run('no-mutating-props', rule, { } ] }, + { + filename: 'test.vue', + code: ` + + + `, + options: [{ propProps: false }], + errors: [ + { + message: 'Unexpected mutation of "a" prop.', + line: 5 + }, + { + message: 'Unexpected mutation of "b" prop.', + line: 6 + }, + { + message: 'Unexpected mutation of "a" prop.', + line: 11 + } + ] + }, { // script setup with shadow @@ -911,13 +1056,15 @@ ruleTester.run('no-mutating-props', rule, { `, errors: [ @@ -932,6 +1079,49 @@ ruleTester.run('no-mutating-props', rule, { { message: 'Unexpected mutation of "Infinity" prop.', line: 6 + }, + { + message: 'Unexpected mutation of "obj" prop.', + line: 18 + } + ] + }, + + { + filename: 'test.vue', + code: ` + + + `, + options: [{ propProps: false }], + errors: [ + { + message: 'Unexpected mutation of "a" prop.', + line: 3 + }, + { + message: 'Unexpected mutation of "a" prop.', + line: 4 + }, + { + message: 'Unexpected mutation of "a" prop.', + line: 7 + }, + { + message: 'Unexpected mutation of "a" prop.', + line: 14 } ] } From a42d9c23f52c2a77a6428007567fe360c43d32ca Mon Sep 17 00:00:00 2001 From: Flare Date: Mon, 8 May 2023 18:44:03 +0800 Subject: [PATCH 2/5] Change option name to `shallowOnly`, avoid modify function `findMutating` --- docs/rules/no-mutating-props.md | 8 +-- lib/rules/no-mutating-props.js | 46 ++++++++++------- lib/utils/index.js | 75 +++++++++++++--------------- tests/lib/rules/no-mutating-props.js | 12 ++--- 4 files changed, 73 insertions(+), 68 deletions(-) diff --git a/docs/rules/no-mutating-props.md b/docs/rules/no-mutating-props.md index 0623ed5a4..f2ffbb2f5 100644 --- a/docs/rules/no-mutating-props.md +++ b/docs/rules/no-mutating-props.md @@ -123,16 +123,16 @@ This rule reports mutation of component props. ```json { "vue/no-mutating-props": ["error", { - "propProps": true + "shallowOnly": false }] } ``` -- "propProps" (`boolean`) Avoid mutating the value of a prop but leaving the reference the same. Default is `true`. +- "shallowOnly" (`boolean`) Enables mutating the value of a prop but leaving the reference the same. Default is `true`. -### "propProps": false +### "shallowOnly": true - + ```vue diff --git a/lib/rules/no-mutating-props.js b/lib/rules/no-mutating-props.js index 21f6ae6ef..1f2dc29af 100644 --- a/lib/rules/no-mutating-props.js +++ b/lib/rules/no-mutating-props.js @@ -90,12 +90,12 @@ function isVmReference(node) { /** * @param { object } options - * @param { boolean } options.propProps avoid mutating the value of a prop but leaving the reference the same + * @param { boolean } options.shallowOnly Enables mutating the value of a prop but leaving the reference the same */ function parseOptions(options) { return Object.assign( { - propProps: true + shallowOnly: false }, options ) @@ -114,7 +114,7 @@ module.exports = { { type: 'object', properties: { - propProps: { + shallowOnly: { type: 'boolean' } }, @@ -124,7 +124,7 @@ module.exports = { }, /** @param {RuleContext} context */ create(context) { - const { propProps } = parseOptions(context.options[0]) + const { shallowOnly } = parseOptions(context.options[0]) /** @type {Map} */ const propsMap = new Map() /** @type { { type: 'export' | 'mark' | 'definition', object: ObjectExpression } | { type: 'setup', object: CallExpression } | null } */ @@ -167,8 +167,8 @@ module.exports = { * @param {boolean} isRootProps */ function verifyMutating(props, name, isRootProps = false) { - const invalid = utils.findMutating(props, propProps, isRootProps) - if (invalid) { + const invalid = utils.findMutating(props) + if (invalid && isShallowOnlyInvalid(invalid, isRootProps)) { report(invalid.node, name) } } @@ -219,9 +219,8 @@ module.exports = { /** * @param {Identifier} prop * @param {string[]} path - * @param {boolean} isRootProps */ - function verifyPropVariable(prop, path, isRootProps = false) { + function verifyPropVariable(prop, path) { const variable = findVariable(context.getScope(), prop) if (!variable) { return @@ -233,11 +232,14 @@ module.exports = { } const id = reference.identifier - const invalid = utils.findMutating(id, propProps, isRootProps) + const invalid = utils.findMutating(id) if (!invalid) { continue } let name + if (!isShallowOnlyInvalid(invalid, path.length === 0)) { + continue + } if (path.length === 0) { if (invalid.pathNodes.length === 0) { continue @@ -274,6 +276,20 @@ module.exports = { } } + /** + * Is shallowOnly false or the prop reassigned + * @param {Exclude, null>} invalid + * @param {boolean} isRootProps + * @return {boolean} + */ + function isShallowOnlyInvalid(invalid, isRootProps) { + return ( + !shallowOnly || + (invalid.pathNodes.length === (isRootProps ? 1 : 0) && + ['assignment', 'update'].includes(invalid.kind)) + ) + } + return utils.compositingVisitors( {}, utils.defineScriptSetupVisitor(context, { @@ -330,7 +346,7 @@ module.exports = { } else { propsInfo.set.add(prop.name) } - verifyPropVariable(prop, path, propsInfo.name === prop.name) + verifyPropVariable(prop, path) } } }), @@ -375,11 +391,7 @@ module.exports = { propsParam, [] )) { - verifyPropVariable( - prop, - path, - prop.parent.type === 'FunctionExpression' - ) + verifyPropVariable(prop, path) } }, /** @param {(Identifier | ThisExpression) & { parent: MemberExpression } } node */ @@ -474,12 +486,12 @@ module.exports = { const first = nodes[0] let name if (isVmReference(first) && first.name !== propsInfo.name) { - if (!propProps && nodes.length > 1) { + if (shallowOnly && nodes.length > 1) { return } name = first.name } else if (first.type === 'ThisExpression' || isVmReference(first)) { - if (!propProps && nodes.length > 2) { + if (shallowOnly && nodes.length > 2) { return } const mem = nodes[1] diff --git a/lib/utils/index.js b/lib/utils/index.js index 81f43ba12..f064ba5af 100644 --- a/lib/utils/index.js +++ b/lib/utils/index.js @@ -1778,11 +1778,9 @@ module.exports = { /** * @param {MemberExpression|Identifier} props - * @param {boolean} propProps - * @param {boolean} isRootProps - * @returns { { kind: 'assignment' | 'update' | 'call' , node: ESNode, pathNodes: MemberExpression[] } | null } + * @returns { { kind: 'assignment' | 'update' | 'call' | 'unary', node: ESNode, pathNodes: MemberExpression[] } | null } */ - findMutating(props, propProps = true, isRootProps = false) { + findMutating(props) { /** @type {MemberExpression[]} */ const pathNodes = [] /** @type {MemberExpression | Identifier | ChainExpression} */ @@ -1809,55 +1807,50 @@ module.exports = { pathNodes } } - case 'MemberExpression': { - if ((propProps || isRootProps) && target.object === node) { - isRootProps = false - pathNodes.push(target) - node = target - target = target.parent - continue // loop + case 'UnaryExpression': { + if (target.operator === 'delete') { + return { + kind: 'unary', + node: target, + pathNodes + } } break } - } - if (propProps) { - switch (target.type) { - case 'UnaryExpression': { - if (target.operator === 'delete') { + case 'CallExpression': { + if (pathNodes.length > 0 && target.callee === node) { + const mem = pathNodes[pathNodes.length - 1] + const callName = getStaticPropertyName(mem) + if ( + callName && + /^(?:push|pop|shift|unshift|reverse|splice|sort|copyWithin|fill)$/u.test( + callName + ) + ) { + // this.xxx.push() + pathNodes.pop() return { - kind: 'update', + kind: 'call', node: target, pathNodes } } - break - } - case 'CallExpression': { - if (pathNodes.length > 0 && target.callee === node) { - const mem = pathNodes[pathNodes.length - 1] - const callName = getStaticPropertyName(mem) - if ( - callName && - /^(?:push|pop|shift|unshift|reverse|splice|sort|copyWithin|fill)$/u.test( - callName - ) - ) { - // this.xxx.push() - pathNodes.pop() - return { - kind: 'call', - node: target, - pathNodes - } - } - } - break } - case 'ChainExpression': { + break + } + case 'MemberExpression': { + if (target.object === node) { + pathNodes.push(target) node = target target = target.parent continue // loop } + break + } + case 'ChainExpression': { + node = target + target = target.parent + continue // loop } } @@ -1882,7 +1875,7 @@ module.exports = { }), /** - Checks whether the tokens of two given nodes are same. + * Checks whether or not the tokens of two given nodes are same. * @param {ASTNode} left A node 1 to compare. * @param {ASTNode} right A node 2 to compare. * @param {ParserServices.TokenStore | SourceCode} sourceCode The ESLint source code object. diff --git a/tests/lib/rules/no-mutating-props.js b/tests/lib/rules/no-mutating-props.js index 4df56029b..4ee78c493 100644 --- a/tests/lib/rules/no-mutating-props.js +++ b/tests/lib/rules/no-mutating-props.js @@ -210,7 +210,7 @@ ruleTester.run('no-mutating-props', rule, { } `, - options: [{ propProps: false }] + options: [{ shallowOnly: true }] }, // setup @@ -370,7 +370,7 @@ ruleTester.run('no-mutating-props', rule, { } `, - options: [{ propProps: false }] + options: [{ shallowOnly: true }] }, { filename: 'test.vue', @@ -390,7 +390,7 @@ ruleTester.run('no-mutating-props', rule, { } `, - options: [{ propProps: false }] + options: [{ shallowOnly: true }] }, { @@ -732,7 +732,7 @@ ruleTester.run('no-mutating-props', rule, { } `, - options: [{ propProps: false }], + options: [{ shallowOnly: true }], errors: [ { message: 'Unexpected mutation of "prop1" prop.', @@ -1027,7 +1027,7 @@ ruleTester.run('no-mutating-props', rule, { `, - options: [{ propProps: false }], + options: [{ shallowOnly: true }], errors: [ { message: 'Unexpected mutation of "a" prop.', @@ -1105,7 +1105,7 @@ ruleTester.run('no-mutating-props', rule, { props.a ++ `, - options: [{ propProps: false }], + options: [{ shallowOnly: true }], errors: [ { message: 'Unexpected mutation of "a" prop.', From 9ee1482c4e602c4ebafb195249e7193ef29eff80 Mon Sep 17 00:00:00 2001 From: Flare Date: Tue, 9 May 2023 10:33:43 +0800 Subject: [PATCH 3/5] Revert the kind `unary` to `update` of `utils.findMutating` return, add some tests; --- docs/rules/no-mutating-props.md | 2 +- lib/utils/index.js | 4 ++-- tests/lib/rules/no-mutating-props.js | 21 ++++++++++++++++++++- 3 files changed, 23 insertions(+), 4 deletions(-) diff --git a/docs/rules/no-mutating-props.md b/docs/rules/no-mutating-props.md index f2ffbb2f5..62c76680d 100644 --- a/docs/rules/no-mutating-props.md +++ b/docs/rules/no-mutating-props.md @@ -128,7 +128,7 @@ This rule reports mutation of component props. } ``` -- "shallowOnly" (`boolean`) Enables mutating the value of a prop but leaving the reference the same. Default is `true`. +- "shallowOnly" (`boolean`) Enables mutating the value of a prop but leaving the reference the same. Default is `false`. ### "shallowOnly": true diff --git a/lib/utils/index.js b/lib/utils/index.js index f064ba5af..249097418 100644 --- a/lib/utils/index.js +++ b/lib/utils/index.js @@ -1778,7 +1778,7 @@ module.exports = { /** * @param {MemberExpression|Identifier} props - * @returns { { kind: 'assignment' | 'update' | 'call' | 'unary', node: ESNode, pathNodes: MemberExpression[] } | null } + * @returns { { kind: 'assignment' | 'update' | 'call' , node: ESNode, pathNodes: MemberExpression[] } | null } */ findMutating(props) { /** @type {MemberExpression[]} */ @@ -1810,7 +1810,7 @@ module.exports = { case 'UnaryExpression': { if (target.operator === 'delete') { return { - kind: 'unary', + kind: 'update', node: target, pathNodes } diff --git a/tests/lib/rules/no-mutating-props.js b/tests/lib/rules/no-mutating-props.js index 4ee78c493..5bb2d8d81 100644 --- a/tests/lib/rules/no-mutating-props.js +++ b/tests/lib/rules/no-mutating-props.js @@ -724,11 +724,18 @@ ruleTester.run('no-mutating-props', rule, {
+
`, @@ -749,6 +756,14 @@ ruleTester.run('no-mutating-props', rule, { { message: 'Unexpected mutation of "prop6" prop.', line: 9 + }, + { + message: 'Unexpected mutation of "prop10" prop.', + line: 13 + }, + { + message: 'Unexpected mutation of "prop10" prop.', + line: 22 } ] }, @@ -1037,6 +1052,10 @@ ruleTester.run('no-mutating-props', rule, { message: 'Unexpected mutation of "b" prop.', line: 6 }, + { + message: 'Unexpected mutation of "d" prop.', + line: 8 + }, { message: 'Unexpected mutation of "a" prop.', line: 11 From eebafd1310459ead1793c8da50df878a48ef9e96 Mon Sep 17 00:00:00 2001 From: Flare Date: Wed, 10 May 2023 10:15:42 +0800 Subject: [PATCH 4/5] Apply suggestions from code review Co-authored-by: Yosuke Ota --- lib/rules/no-mutating-props.js | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/lib/rules/no-mutating-props.js b/lib/rules/no-mutating-props.js index 1f2dc29af..a694aa1b0 100644 --- a/lib/rules/no-mutating-props.js +++ b/lib/rules/no-mutating-props.js @@ -448,12 +448,11 @@ module.exports = { ) const isRootProps = !!node.name && propsInfo.name === node.name const parent = node.parent - const parentProperty = - parent.type === 'MemberExpression' ? parent.property : null const name = - isRootProps && parentProperty?.type === 'Identifier' - ? parentProperty.name - : node.name + (isRootProps && + parent.type === 'MemberExpression' && + utils.getStaticPropertyName(parent)) || + node.name if (name && (propsInfo.set.has(name) || isRootProps)) { verifyMutating(node, name, isRootProps) } From 9c85f2f28a123483dcf72372b241a2acd36dabac Mon Sep 17 00:00:00 2001 From: Flare Date: Wed, 10 May 2023 11:56:05 +0800 Subject: [PATCH 5/5] Report error when mutating props with computed name in template, add tests. --- lib/rules/no-mutating-props.js | 28 ++++++++++++++++++++-------- tests/lib/rules/no-mutating-props.js | 21 ++++++++++++++++++++- 2 files changed, 40 insertions(+), 9 deletions(-) diff --git a/lib/rules/no-mutating-props.js b/lib/rules/no-mutating-props.js index a694aa1b0..61461010b 100644 --- a/lib/rules/no-mutating-props.js +++ b/lib/rules/no-mutating-props.js @@ -484,12 +484,23 @@ module.exports = { const nodes = utils.getMemberChaining(node) const first = nodes[0] let name - if (isVmReference(first) && first.name !== propsInfo.name) { - if (shallowOnly && nodes.length > 1) { - return + if (isVmReference(first)) { + if (first.name === propsInfo.name) { + // props variable + if (shallowOnly && nodes.length > 2) { + return + } + name = (nodes[1] && getPropertyNameText(nodes[1])) || first.name + } else { + if (shallowOnly && nodes.length > 1) { + return + } + name = first.name + if (!name || !propsInfo.set.has(name)) { + return + } } - name = first.name - } else if (first.type === 'ThisExpression' || isVmReference(first)) { + } else if (first.type === 'ThisExpression') { if (shallowOnly && nodes.length > 2) { return } @@ -498,12 +509,13 @@ module.exports = { return } name = utils.getStaticPropertyName(mem) + if (!name || !propsInfo.set.has(name)) { + return + } } else { return } - if (name && propsInfo.set.has(name)) { - report(node, name) - } + report(node, name) } }) ) diff --git a/tests/lib/rules/no-mutating-props.js b/tests/lib/rules/no-mutating-props.js index 5bb2d8d81..7c4884847 100644 --- a/tests/lib/rules/no-mutating-props.js +++ b/tests/lib/rules/no-mutating-props.js @@ -945,6 +945,24 @@ ruleTester.run('no-mutating-props', rule, { errors: ['Unexpected mutation of "[a]" prop.'] }, + { + filename: 'test.vue', + code: ` + + + + `, + errors: [ + { + message: 'Unexpected mutation of "[foo]" prop.', + line: 7 + } + ] + }, { filename: 'test.vue', code: ` @@ -1115,6 +1133,7 @@ ruleTester.run('no-mutating-props', rule, {