From 9c6a762c150a264989fcee3c767f753fb1e8f108 Mon Sep 17 00:00:00 2001 From: Armano Date: Thu, 8 Nov 2018 01:27:44 +0100 Subject: [PATCH] Add `no-mutating-props` rule. --- lib/rules/no-mutating-props.js | 139 +++++++++++ .../no-side-effects-in-computed-properties.js | 11 +- lib/utils/index.js | 41 +++- tests/lib/rules/no-mutating-props.js | 231 ++++++++++++++++++ tests/lib/utils/index.js | 105 +++++++- 5 files changed, 519 insertions(+), 8 deletions(-) create mode 100644 lib/rules/no-mutating-props.js create mode 100644 tests/lib/rules/no-mutating-props.js diff --git a/lib/rules/no-mutating-props.js b/lib/rules/no-mutating-props.js new file mode 100644 index 000000000..a4f409f4d --- /dev/null +++ b/lib/rules/no-mutating-props.js @@ -0,0 +1,139 @@ +/** + * @fileoverview Check if component props are not mutated + * @author 2018 Armano + */ +'use strict' + +const utils = require('../utils') + +// ------------------------------------------------------------------------------ +// Rule Definition +// ------------------------------------------------------------------------------ + +module.exports = { + meta: { + docs: { + description: 'disallow mutation of props', + category: undefined, + url: 'https://github.com/vuejs/eslint-plugin-vue/blob/v5.0.0-beta.3/docs/rules/no-mutating-props.md' + }, + fixable: null, // or "code" or "whitespace" + schema: [ + // fill in your schema + ] + }, + + create (context) { + let mutatedNodes = [] + let props = [] + + function checkForMutations () { + for (const prop of props) { + const propName = utils.getStaticPropertyName(prop.key) + + for (const node of mutatedNodes) { + if (propName === node.name) { + context.report({ + node: node.node, + message: 'Unexpected mutation of "{{key}}" prop.', + data: { key: node.name } + }) + } + } + } + mutatedNodes = [] + } + + function checkTemplateProperty (node) { + if (node.type === 'MemberExpression') { + const expression = utils.parseMemberExpression(node) + mutatedNodes.push({ + name: expression[0] === 'this' ? expression[1] : expression[0], + node + }) + } else if (node.type === 'Identifier') { + mutatedNodes.push({ + name: node.name, + node + }) + } + } + + return Object.assign({}, + { + // this.xxx <=|+=|-=> + 'AssignmentExpression' (node) { + if (node.left.type !== 'MemberExpression') return + const expression = utils.parseMemberExpression(node.left) + if (expression[0] === 'this') { + mutatedNodes.push({ + name: expression[1], + node + }) + } + }, + // this.xxx <++|--> + 'UpdateExpression > MemberExpression' (node) { + const expression = utils.parseMemberExpression(node) + if (expression[0] === 'this') { + mutatedNodes.push({ + name: expression[1], + node + }) + } + }, + // this.xxx.func() + 'CallExpression' (node) { + const expression = utils.parseMemberOrCallExpression(node) + const code = expression.join('.').replace(/\.\[/g, '[') + const MUTATION_REGEX = /(this.)((?!(concat|slice|map|filter)\().)[^\)]*((push|pop|shift|unshift|reverse|splice|sort|copyWithin|fill)\()/g + + if (MUTATION_REGEX.test(code)) { + if (expression[0] === 'this') { + mutatedNodes.push({ + name: expression[1], + node + }) + } + } + } + }, + utils.executeOnVue(context, (obj) => { + props = utils.getComponentProps(obj) + .filter(cp => cp.key) + checkForMutations() + }), + + utils.defineTemplateBodyVisitor(context, { + 'VExpressionContainer AssignmentExpression' (node) { + checkTemplateProperty(node.left) + }, + // this.xxx <++|--> + 'VExpressionContainer UpdateExpression' (node) { + checkTemplateProperty(node.argument) + }, + // this.xxx.func() + 'VExpressionContainer CallExpression' (node) { + const expression = utils.parseMemberOrCallExpression(node) + const code = expression.join('.').replace(/\.\[/g, '[') + const MUTATION_REGEX = /(this.)?((?!(concat|slice|map|filter)\().)[^\)]*((push|pop|shift|unshift|reverse|splice|sort|copyWithin|fill)\()/g + + if (MUTATION_REGEX.test(code)) { + mutatedNodes.push({ + name: expression[0] === 'this' ? expression[1] : expression[0], + node + }) + } + }, + + "VAttribute[directive=true][key.name='model'] VExpressionContainer" (node) { + checkTemplateProperty(node.expression) + }, + + "VElement[name='template']:exit" () { + checkForMutations() + } + }) + ) + } +} diff --git a/lib/rules/no-side-effects-in-computed-properties.js b/lib/rules/no-side-effects-in-computed-properties.js index 9996e7ca2..3ff167c5f 100644 --- a/lib/rules/no-side-effects-in-computed-properties.js +++ b/lib/rules/no-side-effects-in-computed-properties.js @@ -42,6 +42,9 @@ module.exports = { // this.xxx.func() 'CallExpression' (node) { const code = utils.parseMemberOrCallExpression(node) + .join('.') + .replace(/\.\[/g, '[') + const MUTATION_REGEX = /(this.)((?!(concat|slice|map|filter)\().)[^\)]*((push|pop|shift|unshift|reverse|splice|sort|copyWithin|fill)\()/g if (MUTATION_REGEX.test(code)) { @@ -52,8 +55,8 @@ module.exports = { utils.executeOnVue(context, (obj) => { const computedProperties = utils.getComputedProperties(obj) - computedProperties.forEach(cp => { - forbiddenNodes.forEach(node => { + for (const cp of computedProperties) { + for (const node of forbiddenNodes) { if ( cp.value && node.loc.start.line >= cp.value.loc.start.line && @@ -65,8 +68,8 @@ module.exports = { data: { key: cp.key } }) } - }) - }) + } + } }) ) } diff --git a/lib/utils/index.js b/lib/utils/index.js index 4a8a8cb11..69f333b74 100644 --- a/lib/utils/index.js +++ b/lib/utils/index.js @@ -365,9 +365,46 @@ module.exports = { return null }, + /** + * Get all props by looking at all component's properties + * @param {ObjectExpression} componentObject Object with component definition + * @return {Array} Array of component props in format: [{key?: String, value?: ASTNode, node: ASTNod}] + */ + getComponentProps (componentObject) { + const propsNode = componentObject.properties + .find(p => + p.type === 'Property' && + p.key.type === 'Identifier' && + p.key.name === 'props' && + (p.value.type === 'ObjectExpression' || p.value.type === 'ArrayExpression') + ) + + if (!propsNode) { + return [] + } + + let props + + if (propsNode.value.type === 'ObjectExpression') { + props = propsNode.value.properties + .filter(cp => cp.type === 'Property') + .map(cp => { + return { key: cp.key, value: this.unwrapTypes(cp.value), node: cp } + }) + } else { + props = propsNode.value.elements + .map(cp => { + const key = cp.type === 'Literal' && typeof cp.value === 'string' ? cp : null + return { key, value: null, node: cp } + }) + } + + return props + }, + /** * Get all computed properties by looking at all component's properties - * @param {ObjectExpression} Object with component definition + * @param {ObjectExpression} componentObject Object with component definition * @return {Array} Array of computed properties in format: [{key: String, value: ASTNode}] */ getComputedProperties (componentObject) { @@ -710,7 +747,7 @@ module.exports = { parsedCallee.push('this') } - return parsedCallee.reverse().join('.').replace(/\.\[/g, '[') + return parsedCallee.reverse() }, /** diff --git a/tests/lib/rules/no-mutating-props.js b/tests/lib/rules/no-mutating-props.js new file mode 100644 index 000000000..0c81f1010 --- /dev/null +++ b/tests/lib/rules/no-mutating-props.js @@ -0,0 +1,231 @@ +/** + * @fileoverview Check if component props are not mutated + * @author 2018 Armano + */ +'use strict' + +// ------------------------------------------------------------------------------ +// Requirements +// ------------------------------------------------------------------------------ + +const rule = require('../../../lib/rules/no-mutating-props') +const RuleTester = require('eslint').RuleTester + +// ------------------------------------------------------------------------------ +// Tests +// ------------------------------------------------------------------------------ + +const ruleTester = new RuleTester({ + parser: 'vue-eslint-parser', + parserOptions: { + ecmaVersion: 2018, + sourceType: 'module' + } +}) + +ruleTester.run('no-mutating-props', rule, { + + valid: [ + { + filename: 'test.vue', + code: ` + + + ` + }, + { + filename: 'test.vue', + code: ` + + + ` + }, + { + filename: 'test.vue', + code: ` + + ` + } + ], + + invalid: [ + { + filename: 'test.vue', + code: ` + + + `, + errors: [ + { + message: 'Unexpected mutation of "prop1" prop.', + line: 4 + }, + { + message: 'Unexpected mutation of "prop2" prop.', + line: 5 + }, + { + message: 'Unexpected mutation of "prop3" prop.', + line: 6 + }, + { + message: 'Unexpected mutation of "prop4" prop.', + line: 7 + }, + { + message: 'Unexpected mutation of "prop5" prop.', + line: 8 + }, + { + message: 'Unexpected mutation of "prop6" prop.', + line: 9 + }, + { + message: 'Unexpected mutation of "prop7" prop.', + line: 10 + }, + { + message: 'Unexpected mutation of "prop8" prop.', + line: 11 + } + ] + }, + { + filename: 'test.vue', + code: ` + + + `, + errors: [ + { + message: 'Unexpected mutation of "prop1" prop.', + line: 4 + }, + { + message: 'Unexpected mutation of "prop2" prop.', + line: 5 + }, + { + message: 'Unexpected mutation of "prop3" prop.', + line: 6 + }, + { + message: 'Unexpected mutation of "prop4" prop.', + line: 7 + } + ] + }, + { + filename: 'test.vue', + code: ` + + `, + errors: [ + { + message: 'Unexpected mutation of "items" prop.', + line: 16 + }, + { + message: 'Unexpected mutation of "todo" prop.', + line: 17 + }, + { + message: 'Unexpected mutation of "items" prop.', + line: 18 + } + ] + } + ] +}) diff --git a/tests/lib/utils/index.js b/tests/lib/utils/index.js index 067ddc593..e17536dcc 100644 --- a/tests/lib/utils/index.js +++ b/tests/lib/utils/index.js @@ -199,13 +199,13 @@ describe('parseMemberOrCallExpression', () => { it('should parse CallExpression', () => { node = parse(`const test = this.lorem['ipsum'].map(d => d.id).filter((a, b) => a > b).reduce((acc, d) => acc + d, 0)`) const parsed = utils.parseMemberOrCallExpression(node) - assert.equal(parsed, 'this.lorem[].map().filter().reduce()') + assert.deepEqual(parsed, ['this', 'lorem', '[]', 'map()', 'filter()', 'reduce()']) }) it('should parse MemberExpression', () => { node = parse(`const test = this.lorem['ipsum'][0].map(d => d.id).dolor.reduce((acc, d) => acc + d, 0).sit`) const parsed = utils.parseMemberOrCallExpression(node) - assert.equal(parsed, 'this.lorem[][].map().dolor.reduce().sit') + assert.deepEqual(parsed, ['this', 'lorem', '[]', '[]', 'map()', 'dolor', 'reduce()', 'sit']) }) }) @@ -247,3 +247,104 @@ describe('getRegisteredComponents', () => { ) }) }) + +describe('getComponentProps', () => { + let props + + const parse = function (code) { + const data = babelEslint.parse(code).body[0].declarations[0].init + return utils.getComponentProps(data) + } + + it('should return empty array when there is no component props', () => { + props = parse(`const test = { + name: 'test', + data() { + return {} + } + }`) + + assert.equal(props.length, 0) + }) + + it('should return empty array when component props is empty array', () => { + props = parse(`const test = { + name: 'test', + props: [] + }`) + + assert.equal(props.length, 0) + }) + + it('should return empty array when component props is empty object', () => { + props = parse(`const test = { + name: 'test', + props: {} + }`) + + assert.equal(props.length, 0) + }) + + it('should return computed props', () => { + props = parse(`const test = { + name: 'test', + ...test, + data() { + return {} + }, + props: { + ...foo, + a: String, + b: {}, + c: [String], + d + } + }`) + + assert.equal(props.length, 4, 'it detects all props') + + assert.ok(props[0].key.type === 'Identifier') + assert.ok(props[0].node.type === 'Property') + assert.ok(props[0].value.type === 'Identifier') + + assert.ok(props[1].key.type === 'Identifier') + assert.ok(props[1].node.type === 'Property') + assert.ok(props[1].value.type === 'ObjectExpression') + + assert.ok(props[2].key.type === 'Identifier') + assert.ok(props[2].node.type === 'Property') + assert.ok(props[2].value.type === 'ArrayExpression') + + assert.deepEqual(props[3].key, props[3].value) + assert.ok(props[3].node.type === 'Property') + assert.ok(props[3].value.type === 'Identifier') + }) + + it('should return computed from array props', () => { + props = parse(`const test = { + name: 'test', + data() { + return {} + }, + props: ['a', b, \`c\`, null] + }`) + + assert.equal(props.length, 4, 'it detects all props') + + assert.ok(props[0].node.type === 'Literal') + assert.deepEqual(props[0].key, props[0].node) + assert.notOk(props[0].value) + + assert.ok(props[1].node.type === 'Identifier') + assert.notOk(props[1].key) + assert.notOk(props[1].value) + + assert.ok(props[2].node.type === 'TemplateLiteral') + assert.notOk(props[2].key) + assert.notOk(props[2].value) + + assert.ok(props[3].node.type === 'Literal') + assert.notOk(props[3].key) + assert.notOk(props[3].value) + }) +})