From 2d15606c761d8cac7192a6aaa37d6ddf0b2dcfac Mon Sep 17 00:00:00 2001 From: Armano Date: Mon, 14 Aug 2017 23:50:31 +0200 Subject: [PATCH 1/5] Add rule `no-template-shadow`. fixes #101 --- docs/rules/no-template-shadow.md | 46 ++++++++++ lib/rules/no-template-shadow.js | 78 ++++++++++++++++ tests/lib/rules/no-template-shadow.js | 127 ++++++++++++++++++++++++++ 3 files changed, 251 insertions(+) create mode 100644 docs/rules/no-template-shadow.md create mode 100644 lib/rules/no-template-shadow.js create mode 100644 tests/lib/rules/no-template-shadow.js diff --git a/docs/rules/no-template-shadow.md b/docs/rules/no-template-shadow.md new file mode 100644 index 000000000..6cc866ad4 --- /dev/null +++ b/docs/rules/no-template-shadow.md @@ -0,0 +1,46 @@ +# Disallow variable declarations from shadowing variables declared in the outer scope. (no-template-shadow) + +`no-shadow` should report variable definitions of v-for directives or scope attributes if those shadows the variables in parent scopes. + +## :book: Rule Details + +This rule aims to eliminate shadowed variable declarations of v-for directives or scope attributes. + +:-1: Examples of **incorrect** code for this rule: + +```html + + +``` + +:+1: Examples of **correct** code for this rule: + +```html + + +``` + +## :wrench: Options + +Nothing. diff --git a/lib/rules/no-template-shadow.js b/lib/rules/no-template-shadow.js new file mode 100644 index 000000000..284b46f27 --- /dev/null +++ b/lib/rules/no-template-shadow.js @@ -0,0 +1,78 @@ +/** + * @fileoverview Disallow variable declarations from shadowing variables declared in the outer scope. + * @author Armano + */ +'use strict' + +// ------------------------------------------------------------------------------ +// Requirements +// ------------------------------------------------------------------------------ + +const utils = require('../utils') + +// ------------------------------------------------------------------------------ +// Rule Definition +// ------------------------------------------------------------------------------ + +const GROUP_NAMES = ['props', 'computed', 'data', 'methods'] + +module.exports = { + meta: { + docs: { + description: 'Disallow variable declarations from shadowing variables declared in the outer scope.', + category: 'Possible Errors', + recommended: false + }, + fixable: null, + schema: [] + }, + + create (context) { + const jsVars = new Set() + let scope = { + parent: null, + nodes: [] + } + + // ---------------------------------------------------------------------- + // Public + // ---------------------------------------------------------------------- + + utils.registerTemplateBodyVisitor(context, { + VElement (node) { + scope = { + parent: scope, + nodes: scope.nodes.slice() // make copy + } + if (node.variables) { + for (const variable of node.variables) { + const varNode = variable.id + const name = varNode.name + if (scope.nodes.some(node => node.name === name) || jsVars.has(name)) { + context.report({ + node: varNode, + loc: varNode.loc, + message: "Variable '{{name}}' is already declared in the upper scope.", + data: { + name + } + }) + } else { + scope.nodes.push(varNode) + } + } + } + }, + 'VElement:exit' (node) { + scope = scope.parent + } + }) + + return utils.executeOnVue(context, (obj) => { + const properties = Array.from(utils.iterateProperties(obj, new Set(GROUP_NAMES))) + for (const node of properties) { + jsVars.add(node.name) + } + }) + } +} diff --git a/tests/lib/rules/no-template-shadow.js b/tests/lib/rules/no-template-shadow.js new file mode 100644 index 000000000..d3dca2e81 --- /dev/null +++ b/tests/lib/rules/no-template-shadow.js @@ -0,0 +1,127 @@ +/** + * @fileoverview Disallow variable declarations from shadowing variables declared in the outer scope. + * @author Armano + */ +'use strict' + +// ------------------------------------------------------------------------------ +// Requirements +// ------------------------------------------------------------------------------ + +const rule = require('../../../lib/rules/no-template-shadow') +const RuleTester = require('eslint').RuleTester + +// ------------------------------------------------------------------------------ +// Tests +// ------------------------------------------------------------------------------ + +const ruleTester = new RuleTester({ + parser: 'vue-eslint-parser', + parserOptions: { + ecmaVersion: 2015, + sourceType: 'module' + } +}) + +ruleTester.run('no-template-shadow', rule, { + + valid: [ + '', + '', + '', + '', + '', + { + filename: 'test.vue', + code: ` + ` + } + ], + + invalid: [ + { + filename: 'test.vue', + code: '', + errors: [{ + message: "Variable 'i' is already declared in the upper scope.", + type: 'Identifier' + }] + }, + { + filename: 'test.vue', + code: ` + `, + errors: [{ + message: "Variable 'i' is already declared in the upper scope.", + type: 'Identifier' + }, { + message: "Variable 'i' is already declared in the upper scope.", + type: 'Identifier' + }] + }, + { + filename: 'test.vue', + code: ` + `, + errors: [{ + message: "Variable 'i' is already declared in the upper scope.", + type: 'Identifier' + }, { + message: "Variable 'i' is already declared in the upper scope.", + type: 'Identifier' + }] + }, + { + filename: 'test.vue', + code: ` + `, + errors: [{ + message: "Variable 'i' is already declared in the upper scope.", + type: 'Identifier' + }, { + message: "Variable 'f' is already declared in the upper scope.", + type: 'Identifier' + }] + } + ] +}) From b1d17ed4e9357eff9eeb0081d333feb2ed946b01 Mon Sep 17 00:00:00 2001 From: Armano Date: Sun, 17 Sep 2017 17:57:30 +0200 Subject: [PATCH 2/5] fix/merge changes with master --- lib/rules/no-template-shadow.js | 8 +++----- lib/utils/index.js | 2 +- tests/lib/rules/no-template-shadow.js | 1 + 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/lib/rules/no-template-shadow.js b/lib/rules/no-template-shadow.js index 284b46f27..86b3ebcae 100644 --- a/lib/rules/no-template-shadow.js +++ b/lib/rules/no-template-shadow.js @@ -38,7 +38,7 @@ module.exports = { // Public // ---------------------------------------------------------------------- - utils.registerTemplateBodyVisitor(context, { + return utils.defineTemplateBodyVisitor(context, { VElement (node) { scope = { parent: scope, @@ -66,13 +66,11 @@ module.exports = { 'VElement:exit' (node) { scope = scope.parent } - }) - - return utils.executeOnVue(context, (obj) => { + }, utils.executeOnVue(context, (obj) => { const properties = Array.from(utils.iterateProperties(obj, new Set(GROUP_NAMES))) for (const node of properties) { jsVars.add(node.name) } - }) + })) } } diff --git a/lib/utils/index.js b/lib/utils/index.js index 957a9d6c8..9e8950b3e 100644 --- a/lib/utils/index.js +++ b/lib/utils/index.js @@ -26,7 +26,7 @@ module.exports = { * * @param {RuleContext} context The rule context to use parser services. * @param {Object} templateBodyVisitor The visitor to traverse the template body. - * @param {Object} scriptVisitor The visitor to traverse the script. + * @param {Object} [scriptVisitor] The visitor to traverse the script. * @returns {Object} The merged visitor. */ defineTemplateBodyVisitor (context, templateBodyVisitor, scriptVisitor) { diff --git a/tests/lib/rules/no-template-shadow.js b/tests/lib/rules/no-template-shadow.js index d3dca2e81..79cfaa43f 100644 --- a/tests/lib/rules/no-template-shadow.js +++ b/tests/lib/rules/no-template-shadow.js @@ -31,6 +31,7 @@ ruleTester.run('no-template-shadow', rule, { '', '', '', + '', { filename: 'test.vue', code: ` + ``` + + ```html + -``` + ``` :+1: Examples of **correct** code for this rule: diff --git a/lib/rules/no-template-shadow.js b/lib/rules/no-template-shadow.js index 86b3ebcae..0f4cacbb5 100644 --- a/lib/rules/no-template-shadow.js +++ b/lib/rules/no-template-shadow.js @@ -19,9 +19,10 @@ const GROUP_NAMES = ['props', 'computed', 'data', 'methods'] module.exports = { meta: { docs: { - description: 'Disallow variable declarations from shadowing variables declared in the outer scope.', - category: 'Possible Errors', - recommended: false + description: 'disallow variable declarations from shadowing variables declared in the outer scope', + category: 'essential', + recommended: false, + url: 'https://github.com/vuejs/eslint-plugin-vue/blob/v5.0.0-beta.1/docs/rules/no-template-shadow.md' }, fixable: null, schema: [] diff --git a/tests/lib/rules/no-template-shadow.js b/tests/lib/rules/no-template-shadow.js index 79cfaa43f..22b0fd116 100644 --- a/tests/lib/rules/no-template-shadow.js +++ b/tests/lib/rules/no-template-shadow.js @@ -36,6 +36,7 @@ ruleTester.run('no-template-shadow', rule, { filename: 'test.vue', code: ` `, errors: [{ message: "Variable 'i' is already declared in the upper scope.", - type: 'Identifier' - }, { - message: "Variable 'i' is already declared in the upper scope.", - type: 'Identifier' + type: 'Identifier', + line: 2 }] }, { @@ -94,10 +91,36 @@ ruleTester.run('no-template-shadow', rule, { `, errors: [{ message: "Variable 'i' is already declared in the upper scope.", - type: 'Identifier' + type: 'Identifier', + line: 2 }, { message: "Variable 'i' is already declared in the upper scope.", - type: 'Identifier' + type: 'Identifier', + line: 3 + }] + }, + { + filename: 'test.vue', + code: ` + `, + errors: [{ + message: "Variable 'i' is already declared in the upper scope.", + type: 'Identifier', + line: 2 + }, { + message: "Variable 'i' is already declared in the upper scope.", + type: 'Identifier', + line: 3 }] }, { @@ -118,10 +141,12 @@ ruleTester.run('no-template-shadow', rule, { `, errors: [{ message: "Variable 'i' is already declared in the upper scope.", - type: 'Identifier' + type: 'Identifier', + line: 2 }, { message: "Variable 'f' is already declared in the upper scope.", - type: 'Identifier' + type: 'Identifier', + line: 3 }] } ] From 2a3a92b6823fd6beb569f245e27189405ca6330c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Sajn=C3=B3g?= Date: Mon, 30 Jul 2018 12:42:42 +0200 Subject: [PATCH 4/5] Add more test cases --- lib/rules/no-template-shadow.js | 1 - tests/lib/rules/no-template-shadow.js | 62 ++++++++++++++++++++++++++- 2 files changed, 61 insertions(+), 2 deletions(-) diff --git a/lib/rules/no-template-shadow.js b/lib/rules/no-template-shadow.js index 0f4cacbb5..4482c80f6 100644 --- a/lib/rules/no-template-shadow.js +++ b/lib/rules/no-template-shadow.js @@ -21,7 +21,6 @@ module.exports = { docs: { description: 'disallow variable declarations from shadowing variables declared in the outer scope', category: 'essential', - recommended: false, url: 'https://github.com/vuejs/eslint-plugin-vue/blob/v5.0.0-beta.1/docs/rules/no-template-shadow.md' }, fixable: null, diff --git a/tests/lib/rules/no-template-shadow.js b/tests/lib/rules/no-template-shadow.js index 22b0fd116..c52f4ddb4 100644 --- a/tests/lib/rules/no-template-shadow.js +++ b/tests/lib/rules/no-template-shadow.js @@ -18,7 +18,7 @@ const RuleTester = require('eslint').RuleTester const ruleTester = new RuleTester({ parser: 'vue-eslint-parser', parserOptions: { - ecmaVersion: 2015, + ecmaVersion: 2018, sourceType: 'module' } }) @@ -46,6 +46,30 @@ ruleTester.run('no-template-shadow', rule, { } } ` + }, + { + filename: 'test.vue', + code: ` + ` } ], @@ -148,6 +172,42 @@ ruleTester.run('no-template-shadow', rule, { type: 'Identifier', line: 3 }] + }, + { + filename: 'test.vue', + code: ` + `, + errors: [{ + message: "Variable 'e' is already declared in the upper scope.", + type: 'Identifier', + line: 6 + }, { + message: "Variable 'f' is already declared in the upper scope.", + type: 'Identifier', + line: 7 + }] } ] }) From 10a8c8e9afbbc5ef4bb2055e10e82f6524e97482 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Sajn=C3=B3g?= Date: Mon, 30 Jul 2018 15:00:09 +0200 Subject: [PATCH 5/5] Change no-template-shadow category to strongly-recommended --- lib/rules/no-template-shadow.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/rules/no-template-shadow.js b/lib/rules/no-template-shadow.js index 4482c80f6..6a476d67d 100644 --- a/lib/rules/no-template-shadow.js +++ b/lib/rules/no-template-shadow.js @@ -20,7 +20,7 @@ module.exports = { meta: { docs: { description: 'disallow variable declarations from shadowing variables declared in the outer scope', - category: 'essential', + category: 'strongly-recommended', url: 'https://github.com/vuejs/eslint-plugin-vue/blob/v5.0.0-beta.1/docs/rules/no-template-shadow.md' }, fixable: null,