From 76402ba37aa629d04584ee1796d215f6f2e1d101 Mon Sep 17 00:00:00 2001 From: yosuke ota Date: Fri, 6 Mar 2020 12:58:33 +0900 Subject: [PATCH 1/6] Add no-ref-as-operand rule --- docs/rules/no-ref-as-operand.md | 0 lib/rules/no-ref-as-operand.js | 126 +++++++++++++++++++++++++++ package.json | 5 +- tests/lib/rules/no-ref-as-operand.js | 119 +++++++++++++++++++++++++ 4 files changed, 248 insertions(+), 2 deletions(-) create mode 100644 docs/rules/no-ref-as-operand.md create mode 100644 lib/rules/no-ref-as-operand.js create mode 100644 tests/lib/rules/no-ref-as-operand.js diff --git a/docs/rules/no-ref-as-operand.md b/docs/rules/no-ref-as-operand.md new file mode 100644 index 000000000..e69de29bb diff --git a/lib/rules/no-ref-as-operand.js b/lib/rules/no-ref-as-operand.js new file mode 100644 index 000000000..741a6344a --- /dev/null +++ b/lib/rules/no-ref-as-operand.js @@ -0,0 +1,126 @@ +/** + * @author Yosuke Ota + * See LICENSE file in root directory for full license. + */ +'use strict' +const { ReferenceTracker, findVariable } = require('eslint-utils') + +module.exports = { + meta: { + type: 'suggestion', + docs: { + description: 'disallow use of value wrapped by `ref()` (Composition API) as an operand', + category: undefined, + url: 'https://eslint.vuejs.org/rules/no-ref-as-operand.html' + }, + fixable: 'code', + schema: [], + messages: { + requireDotValue: 'Must use `.value` to read or write the value wrapped with `ref()`.' + } + }, + create (context) { + const refCallNodes = new Set() + + const refReferences = new Set() + + function reportIfRefWrapped (node) { + if (!refReferences.has(node)) { + return + } + context.report({ + node, + messageId: 'requireDotValue' + }) + } + return { + 'Program' () { + const tracker = new ReferenceTracker(context.getScope()) + const traceMap = { + vue: { + [ReferenceTracker.ESM]: true, + ref: { + [ReferenceTracker.CALL]: true + } + } + } + + for (const { node } of tracker.iterateEsmReferences(traceMap)) { + refCallNodes.add(node) + } + }, + 'VariableDeclarator>CallExpression' (node) { + const varDecl = node.parent + if (varDecl.id.type !== 'Identifier') { + return + } + if (!refCallNodes.has(node)) { + return + } + const variable = findVariable(context.getScope(), varDecl.id) + if (!variable) { + return + } + for (const reference of variable.references) { + if (!reference.isRead()) { + continue + } + refReferences.add(reference.identifier) + } + }, + // if (refValue) + 'IfStatement>Identifier' (node) { + if (node.parent.test !== node) { + return + } + reportIfRefWrapped(node) + }, + // switch (refValue) + 'SwitchStatement>Identifier' (node) { + if (node.parent.discriminant !== node) { + return + } + reportIfRefWrapped(node) + }, + // -refValue, +refValue, !refValue, ~refValue, typeof refValue + 'UnaryExpression>Identifier' (node) { + if (node.parent.argument !== node) { + return + } + reportIfRefWrapped(node) + }, + // refValue++, refValue-- + 'UpdateExpression>Identifier' (node) { + if (node.parent.argument !== node) { + return + } + reportIfRefWrapped(node) + }, + // refValue+1, refValue-1 + 'BinaryExpression>Identifier' (node) { + reportIfRefWrapped(node) + }, + // refValue+=1, refValue-=1 + 'AssignmentExpression>Identifier' (node) { + if (node.parent.left !== node) { + return + } + reportIfRefWrapped(node) + }, + // refValue || other, refValue && other. ignore: other || refValue + 'LogicalExpression>Identifier' (node) { + if (node.parent.left !== node) { + return + } + reportIfRefWrapped(node) + }, + // refValue ? x : y + 'ConditionalExpression>Identifier' (node) { + if (node.parent.test !== node) { + return + } + reportIfRefWrapped(node) + } + } + } +} diff --git a/package.json b/package.json index 1744009a5..099b182a4 100644 --- a/package.json +++ b/package.json @@ -47,9 +47,10 @@ "eslint": "^5.0.0 || ^6.0.0" }, "dependencies": { + "eslint-utils": "^2.0.0", "natural-compare": "^1.4.0", - "vue-eslint-parser": "^7.0.0", - "semver": "^5.6.0" + "semver": "^5.6.0", + "vue-eslint-parser": "^7.0.0" }, "devDependencies": { "@types/node": "^4.2.16", diff --git a/tests/lib/rules/no-ref-as-operand.js b/tests/lib/rules/no-ref-as-operand.js new file mode 100644 index 000000000..ea55ddb23 --- /dev/null +++ b/tests/lib/rules/no-ref-as-operand.js @@ -0,0 +1,119 @@ +/** + * @author Yosuke Ota + */ +'use strict' + +const RuleTester = require('eslint').RuleTester +const rule = require('../../../lib/rules/no-ref-as-operand') + +const tester = new RuleTester({ + parser: require.resolve('vue-eslint-parser'), + parserOptions: { ecmaVersion: 2019, sourceType: 'module' } +}) + +tester.run('no-ref-as-operand', rule, { + valid: [ + ` + import { ref } from 'vue' + const count = ref(0) + console.log(count.value) // 0 + + count.value++ + console.log(count.value) // 1 + `, + ` + + ` + ], + invalid: [ + { + code: ` + import { ref } from 'vue' + let count = ref(0) + + count++ // error + console.log(count + 1) // error + console.log(1 + count) // error + `, + errors: [ + { + messageId: 'requireDotValue', + line: 5, + column: 7, + endLine: 5, + endColumn: 12 + }, + { + messageId: 'requireDotValue', + line: 6, + column: 19, + endLine: 6, + endColumn: 24 + }, + { + messageId: 'requireDotValue', + line: 7, + column: 23, + endLine: 7, + endColumn: 28 + } + ] + }, + { + code: ` + + `, + errors: [ + { + messageId: 'requireDotValue', + line: 8, + column: 13, + endLine: 8, + endColumn: 18 + }, + { + messageId: 'requireDotValue', + line: 9, + column: 25, + endLine: 9, + endColumn: 30 + }, + { + messageId: 'requireDotValue', + line: 10, + column: 29, + endLine: 10, + endColumn: 34 + } + ] + } + ] +}) From ac62c3f82bf4f16269137e0d3cc4ba537a37d860 Mon Sep 17 00:00:00 2001 From: yosuke ota Date: Fri, 6 Mar 2020 13:43:20 +0900 Subject: [PATCH 2/6] update --- docs/rules/no-ref-as-operand.md | 54 ++++++++ lib/rules/no-ref-as-operand.js | 9 +- tests/lib/rules/no-ref-as-operand.js | 197 ++++++++++++++++++++++++++- 3 files changed, 254 insertions(+), 6 deletions(-) diff --git a/docs/rules/no-ref-as-operand.md b/docs/rules/no-ref-as-operand.md index e69de29bb..a551fff4b 100644 --- a/docs/rules/no-ref-as-operand.md +++ b/docs/rules/no-ref-as-operand.md @@ -0,0 +1,54 @@ +--- +pageClass: rule-details +sidebarDepth: 0 +title: vue/no-ref-as-operand +description: disallow use of value wrapped by `ref()` (Composition API) as an operand +--- +# vue/no-ref-as-operand +> disallow use of value wrapped by `ref()` (Composition API) as an operand + +## :book: Rule Details + +This rule reports cases where a ref is used incorrectly as an operand. + + + +```vue + +``` + + + +## :books: Further reading + +- [Vue RFCs - 0013-composition-api](https://github.com/vuejs/rfcs/blob/master/active-rfcs/0013-composition-api.md) + +## :mag: Implementation + +- [Rule source](https://github.com/vuejs/eslint-plugin-vue/blob/master/lib/rules/no-ref-as-operand.js) +- [Test source](https://github.com/vuejs/eslint-plugin-vue/blob/master/tests/lib/rules/no-ref-as-operand.js) diff --git a/lib/rules/no-ref-as-operand.js b/lib/rules/no-ref-as-operand.js index 741a6344a..ec63e6b2a 100644 --- a/lib/rules/no-ref-as-operand.js +++ b/lib/rules/no-ref-as-operand.js @@ -13,7 +13,7 @@ module.exports = { category: undefined, url: 'https://eslint.vuejs.org/rules/no-ref-as-operand.html' }, - fixable: 'code', + fixable: null, schema: [], messages: { requireDotValue: 'Must use `.value` to read or write the value wrapped with `ref()`.' @@ -98,11 +98,14 @@ module.exports = { }, // refValue+1, refValue-1 'BinaryExpression>Identifier' (node) { + if (node.parent.left !== node && node.parent.right !== node) { + return + } reportIfRefWrapped(node) }, - // refValue+=1, refValue-=1 + // refValue+=1, refValue-=1, foo+=refValue, foo-=refValue 'AssignmentExpression>Identifier' (node) { - if (node.parent.left !== node) { + if (node.parent.left !== node && node.parent.right !== node) { return } reportIfRefWrapped(node) diff --git a/tests/lib/rules/no-ref-as-operand.js b/tests/lib/rules/no-ref-as-operand.js index ea55ddb23..c4dd5aa59 100644 --- a/tests/lib/rules/no-ref-as-operand.js +++ b/tests/lib/rules/no-ref-as-operand.js @@ -37,6 +37,46 @@ tester.run('no-ref-as-operand', rule, { } } + `, + ` + import { ref } from 'vue' + const count = ref(0) + if (count.value) {} + switch (count.value) {} + var foo = -count.value + var foo = +count.value + count.value++ + count.value-- + count.value + 1 + 1 - count.value + count.value || other + count.value && other + var foo = count.value ? x : y + `, + ` + import { ref } from 'vue' + let count = not_ref(0) + + count++ + `, + ` + import { ref } from 'vue' + // Probably wrong, but not checked by this rule. + const {value} = ref(0) + value++ + `, + ` + import { ref } from 'vue' + const count = ref(0) + function foo() { + let count = 0 + count++ + } + `, + ` + import { ref } from 'unknown' + const count = ref(0) + count++ ` ], invalid: [ @@ -51,21 +91,21 @@ tester.run('no-ref-as-operand', rule, { `, errors: [ { - messageId: 'requireDotValue', + message: 'Must use `.value` to read or write the value wrapped with `ref()`.', line: 5, column: 7, endLine: 5, endColumn: 12 }, { - messageId: 'requireDotValue', + message: 'Must use `.value` to read or write the value wrapped with `ref()`.', line: 6, column: 19, endLine: 6, endColumn: 24 }, { - messageId: 'requireDotValue', + message: 'Must use `.value` to read or write the value wrapped with `ref()`.', line: 7, column: 23, endLine: 7, @@ -114,6 +154,157 @@ tester.run('no-ref-as-operand', rule, { endColumn: 34 } ] + }, + { + code: ` + import { ref } from 'vue' + const foo = ref(true) + if (foo) { + // + } + `, + errors: [ + { + messageId: 'requireDotValue', + line: 4 + } + ] + }, + { + code: ` + import { ref } from 'vue' + const foo = ref(true) + switch (foo) { + // + } + `, + errors: [ + { + messageId: 'requireDotValue', + line: 4 + } + ] + }, + { + code: ` + import { ref } from 'vue' + const foo = ref(0) + var a = -foo + var b = +foo + var c = !foo + var d = ~foo + `, + errors: [ + { + messageId: 'requireDotValue', + line: 4 + }, + { + messageId: 'requireDotValue', + line: 5 + }, + { + messageId: 'requireDotValue', + line: 6 + }, + { + messageId: 'requireDotValue', + line: 7 + } + ] + }, + { + code: ` + import { ref } from 'vue' + let foo = ref(0) + foo += 1 + foo -= 1 + baz += foo + baz -= foo + `, + errors: [ + { + messageId: 'requireDotValue', + line: 4 + }, + { + messageId: 'requireDotValue', + line: 5 + }, + { + messageId: 'requireDotValue', + line: 6 + }, + { + messageId: 'requireDotValue', + line: 7 + } + ] + }, + { + code: ` + import { ref } from 'vue' + let foo = ref(true) + var a = foo || other + var b = foo && other + var c = other || foo // ignore + var d = other && foo // ignore + `, + errors: [ + { + messageId: 'requireDotValue', + line: 4 + }, + { + messageId: 'requireDotValue', + line: 5 + } + ] + }, + { + code: ` + import { ref } from 'vue' + let foo = ref(true) + var a = foo ? x : y + `, + errors: [ + { + messageId: 'requireDotValue', + line: 4 + } + ] + }, + { + code: ` + + `, + errors: [ + { + messageId: 'requireDotValue', + line: 7 + }, + { + messageId: 'requireDotValue', + line: 8 + }, + { + messageId: 'requireDotValue', + line: 9 + } + ] } ] }) From f378e38b9becca7cef45ac863a4dac7e21e9d475 Mon Sep 17 00:00:00 2001 From: yosuke ota Date: Fri, 6 Mar 2020 16:36:51 +0900 Subject: [PATCH 3/6] Add testcases --- lib/rules/no-ref-as-operand.js | 75 +++++++++++++--------------- tests/lib/rules/no-ref-as-operand.js | 30 +++++++++-- 2 files changed, 62 insertions(+), 43 deletions(-) diff --git a/lib/rules/no-ref-as-operand.js b/lib/rules/no-ref-as-operand.js index ec63e6b2a..ba127db52 100644 --- a/lib/rules/no-ref-as-operand.js +++ b/lib/rules/no-ref-as-operand.js @@ -20,12 +20,10 @@ module.exports = { } }, create (context) { - const refCallNodes = new Set() - - const refReferences = new Set() + const refReferenceIds = new Map() function reportIfRefWrapped (node) { - if (!refReferences.has(node)) { + if (!refReferenceIds.has(node)) { return } context.report({ @@ -46,68 +44,57 @@ module.exports = { } for (const { node } of tracker.iterateEsmReferences(traceMap)) { - refCallNodes.add(node) - } - }, - 'VariableDeclarator>CallExpression' (node) { - const varDecl = node.parent - if (varDecl.id.type !== 'Identifier') { - return - } - if (!refCallNodes.has(node)) { - return - } - const variable = findVariable(context.getScope(), varDecl.id) - if (!variable) { - return - } - for (const reference of variable.references) { - if (!reference.isRead()) { + const variableDeclarator = node.parent + if ( + !variableDeclarator || + variableDeclarator.type !== 'VariableDeclarator' || + variableDeclarator.id.type !== 'Identifier' + ) { + continue + } + const variable = findVariable(context.getScope(), variableDeclarator.id) + if (!variable) { continue } - refReferences.add(reference.identifier) + const variableDeclaration = ( + variableDeclarator.parent && + variableDeclarator.parent.type === 'VariableDeclaration' && + variableDeclarator.parent + ) || null + for (const reference of variable.references) { + if (!reference.isRead()) { + continue + } + + refReferenceIds.set(reference.identifier, { + variableDeclarator, + variableDeclaration + }) + } } }, // if (refValue) 'IfStatement>Identifier' (node) { - if (node.parent.test !== node) { - return - } reportIfRefWrapped(node) }, // switch (refValue) 'SwitchStatement>Identifier' (node) { - if (node.parent.discriminant !== node) { - return - } reportIfRefWrapped(node) }, // -refValue, +refValue, !refValue, ~refValue, typeof refValue 'UnaryExpression>Identifier' (node) { - if (node.parent.argument !== node) { - return - } reportIfRefWrapped(node) }, // refValue++, refValue-- 'UpdateExpression>Identifier' (node) { - if (node.parent.argument !== node) { - return - } reportIfRefWrapped(node) }, // refValue+1, refValue-1 'BinaryExpression>Identifier' (node) { - if (node.parent.left !== node && node.parent.right !== node) { - return - } reportIfRefWrapped(node) }, // refValue+=1, refValue-=1, foo+=refValue, foo-=refValue 'AssignmentExpression>Identifier' (node) { - if (node.parent.left !== node && node.parent.right !== node) { - return - } reportIfRefWrapped(node) }, // refValue || other, refValue && other. ignore: other || refValue @@ -115,6 +102,14 @@ module.exports = { if (node.parent.left !== node) { return } + // Report only constants. + const info = refReferenceIds.get(node) + if (!info) { + return + } + if (!info.variableDeclaration || info.variableDeclaration.kind !== 'const') { + return + } reportIfRefWrapped(node) }, // refValue ? x : y diff --git a/tests/lib/rules/no-ref-as-operand.js b/tests/lib/rules/no-ref-as-operand.js index c4dd5aa59..a96001cba 100644 --- a/tests/lib/rules/no-ref-as-operand.js +++ b/tests/lib/rules/no-ref-as-operand.js @@ -55,12 +55,33 @@ tester.run('no-ref-as-operand', rule, { `, ` import { ref } from 'vue' + const foo = ref(true) + if (bar) foo + `, + ` + import { ref } from 'vue' + const foo = ref(true) + var a = other || foo // ignore + var b = other && foo // ignore + + let bar = ref(true) + var a = bar || other + var b = bar || other + `, + ` + import { ref } from 'vue' let count = not_ref(0) count++ `, ` import { ref } from 'vue' + const foo = ref(0) + const bar = ref(0) + var baz = x ? foo : bar + `, + ` + import { ref } from 'vue' // Probably wrong, but not checked by this rule. const {value} = ref(0) value++ @@ -77,6 +98,11 @@ tester.run('no-ref-as-operand', rule, { import { ref } from 'unknown' const count = ref(0) count++ + `, + ` + import { ref } from 'vue' + const count = ref + count++ ` ], invalid: [ @@ -244,11 +270,9 @@ tester.run('no-ref-as-operand', rule, { { code: ` import { ref } from 'vue' - let foo = ref(true) + const foo = ref(true) var a = foo || other var b = foo && other - var c = other || foo // ignore - var d = other && foo // ignore `, errors: [ { From 81ab77d5be9610caa8c92822d9de88d9ee79494b Mon Sep 17 00:00:00 2001 From: yosuke ota Date: Sat, 7 Mar 2020 13:00:34 +0900 Subject: [PATCH 4/6] update --- docs/rules/README.md | 1 + lib/index.js | 1 + lib/rules/no-ref-as-operand.js | 2 +- 3 files changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/rules/README.md b/docs/rules/README.md index ab1f5b220..72faa9698 100644 --- a/docs/rules/README.md +++ b/docs/rules/README.md @@ -160,6 +160,7 @@ For example: | [vue/no-deprecated-slot-scope-attribute](./no-deprecated-slot-scope-attribute.md) | disallow deprecated `slot-scope` attribute (in Vue.js 2.6.0+) | :wrench: | | [vue/no-empty-pattern](./no-empty-pattern.md) | disallow empty destructuring patterns | | | [vue/no-irregular-whitespace](./no-irregular-whitespace.md) | disallow irregular whitespace | | +| [vue/no-ref-as-operand](./no-ref-as-operand.md) | disallow use of value wrapped by `ref()` (Composition API) as an operand | | | [vue/no-reserved-component-names](./no-reserved-component-names.md) | disallow the use of reserved names in component definitions | | | [vue/no-restricted-syntax](./no-restricted-syntax.md) | disallow specified syntax | | | [vue/no-static-inline-styles](./no-static-inline-styles.md) | disallow static inline `style` attributes | | diff --git a/lib/index.js b/lib/index.js index 02e65ba96..023e499c0 100644 --- a/lib/index.js +++ b/lib/index.js @@ -48,6 +48,7 @@ module.exports = { 'no-irregular-whitespace': require('./rules/no-irregular-whitespace'), 'no-multi-spaces': require('./rules/no-multi-spaces'), 'no-parsing-error': require('./rules/no-parsing-error'), + 'no-ref-as-operand': require('./rules/no-ref-as-operand'), 'no-reserved-component-names': require('./rules/no-reserved-component-names'), 'no-reserved-keys': require('./rules/no-reserved-keys'), 'no-restricted-syntax': require('./rules/no-restricted-syntax'), diff --git a/lib/rules/no-ref-as-operand.js b/lib/rules/no-ref-as-operand.js index ba127db52..ea57d9da4 100644 --- a/lib/rules/no-ref-as-operand.js +++ b/lib/rules/no-ref-as-operand.js @@ -16,7 +16,7 @@ module.exports = { fixable: null, schema: [], messages: { - requireDotValue: 'Must use `.value` to read or write the value wrapped with `ref()`.' + requireDotValue: 'Must use `.value` to read or write the value wrapped by `ref()`.' } }, create (context) { From 2657b4209b455a57c26dc1880a39463b42ec38b6 Mon Sep 17 00:00:00 2001 From: yosuke ota Date: Sat, 7 Mar 2020 13:11:50 +0900 Subject: [PATCH 5/6] Fixed testcases --- tests/lib/rules/no-ref-as-operand.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/lib/rules/no-ref-as-operand.js b/tests/lib/rules/no-ref-as-operand.js index a96001cba..b8632c88b 100644 --- a/tests/lib/rules/no-ref-as-operand.js +++ b/tests/lib/rules/no-ref-as-operand.js @@ -117,21 +117,21 @@ tester.run('no-ref-as-operand', rule, { `, errors: [ { - message: 'Must use `.value` to read or write the value wrapped with `ref()`.', + message: 'Must use `.value` to read or write the value wrapped by `ref()`.', line: 5, column: 7, endLine: 5, endColumn: 12 }, { - message: 'Must use `.value` to read or write the value wrapped with `ref()`.', + message: 'Must use `.value` to read or write the value wrapped by `ref()`.', line: 6, column: 19, endLine: 6, endColumn: 24 }, { - message: 'Must use `.value` to read or write the value wrapped with `ref()`.', + message: 'Must use `.value` to read or write the value wrapped by `ref()`.', line: 7, column: 23, endLine: 7, From f94e739dd0b45a73b6d11c2601f5f298f9292967 Mon Sep 17 00:00:00 2001 From: yosuke ota Date: Sat, 7 Mar 2020 17:43:33 +0900 Subject: [PATCH 6/6] update doc --- docs/rules/no-ref-as-operand.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docs/rules/no-ref-as-operand.md b/docs/rules/no-ref-as-operand.md index a551fff4b..e6d0b94b3 100644 --- a/docs/rules/no-ref-as-operand.md +++ b/docs/rules/no-ref-as-operand.md @@ -44,6 +44,10 @@ export default { +## :wrench: Options + +Nothing. + ## :books: Further reading - [Vue RFCs - 0013-composition-api](https://github.com/vuejs/rfcs/blob/master/active-rfcs/0013-composition-api.md)