-
-
Notifications
You must be signed in to change notification settings - Fork 678
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
⭐️New: Add rule no-template-shadow
.
#158
Changes from all commits
2d15606
b1d17ed
526e58f
aba2016
2a3a92b
10a8c8e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,56 @@ | ||
# Disallow variable declarations from shadowing variables declared in the outer scope. (no-template-shadow) | ||
|
||
`no-template-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 | ||
<template> | ||
<div> | ||
<div v-for="i in 5"> | ||
<div v-for="i in 10"></div> | ||
</div> | ||
</div> | ||
</template> | ||
``` | ||
|
||
```html | ||
<template> | ||
<div> | ||
<div v-for="i in 5"></div> | ||
</div> | ||
</template> | ||
<script> | ||
export default { | ||
data () { | ||
return { | ||
i: 10 | ||
} | ||
} | ||
} | ||
</script> | ||
``` | ||
|
||
:+1: Examples of **correct** code for this rule: | ||
|
||
```html | ||
<template> | ||
<div v-for="i in 5"></div> | ||
<div v-for="i in 5"></div> | ||
</template> | ||
<script> | ||
export default { | ||
computed: { | ||
f () { } | ||
} | ||
} | ||
</script> | ||
``` | ||
|
||
## :wrench: Options | ||
|
||
Nothing. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,76 @@ | ||
/** | ||
* @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: 'strongly-recommended', | ||
url: 'https://github.com/vuejs/eslint-plugin-vue/blob/v5.0.0-beta.1/docs/rules/no-template-shadow.md' | ||
}, | ||
fixable: null, | ||
schema: [] | ||
}, | ||
|
||
create (context) { | ||
const jsVars = new Set() | ||
let scope = { | ||
parent: null, | ||
nodes: [] | ||
} | ||
|
||
// ---------------------------------------------------------------------- | ||
// Public | ||
// ---------------------------------------------------------------------- | ||
|
||
return utils.defineTemplateBodyVisitor(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 | ||
} | ||
}, utils.executeOnVue(context, (obj) => { | ||
const properties = Array.from(utils.iterateProperties(obj, new Set(GROUP_NAMES))) | ||
for (const node of properties) { | ||
jsVars.add(node.name) | ||
} | ||
})) | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why square brackets here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It indicates an |
||
* @returns {Object} The merged visitor. | ||
*/ | ||
defineTemplateBodyVisitor (context, templateBodyVisitor, scriptVisitor) { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,213 @@ | ||
/** | ||
* @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: 2018, | ||
sourceType: 'module' | ||
} | ||
}) | ||
|
||
ruleTester.run('no-template-shadow', rule, { | ||
|
||
valid: [ | ||
'', | ||
'<template><div></div></template>', | ||
'<template><div v-for="i in 5"></div></template>', | ||
'<template><div v-for="i in 5"><div v-for="b in 5"></div></div></template>', | ||
'<template><div v-for="i in 5"></div><div v-for="i in 5"></div></template>', | ||
'<template> <ul v-for="i in 5"> <li> <span v-for="j in 10">{{i}},{{j}}</span> </li> </ul> </template>', | ||
{ | ||
filename: 'test.vue', | ||
code: `<template> | ||
<div v-for="i in 5"></div> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's add case with |
||
<div v-for="i in f"></div> | ||
<div v-for="i in 5"></div> | ||
</template> | ||
<script> | ||
export default { | ||
computed: { | ||
f () { } | ||
} | ||
} | ||
</script>` | ||
}, | ||
{ | ||
filename: 'test.vue', | ||
code: `<template> | ||
<div v-for="i in b" /> | ||
<div v-for="b in c" /> | ||
<div v-for="d in f" /> | ||
</template> | ||
<script> | ||
export default { | ||
...a, | ||
data() { | ||
return { | ||
...b, | ||
c: [1, 2, 3] | ||
} | ||
}, | ||
computed: { | ||
...d, | ||
e, | ||
['f']: [1, 2], | ||
} | ||
} | ||
</script>` | ||
} | ||
], | ||
|
||
invalid: [ | ||
{ | ||
filename: 'test.vue', | ||
code: '<template><div v-for="i in 5"><div v-for="i in 5"></div></div></template>', | ||
errors: [{ | ||
message: "Variable 'i' is already declared in the upper scope.", | ||
type: 'Identifier' | ||
}] | ||
}, | ||
{ | ||
filename: 'test.vue', | ||
code: `<template> | ||
<div v-for="i in 5"></div> | ||
</template> | ||
<script> | ||
export default { | ||
data: { | ||
i: 7 | ||
} | ||
} | ||
</script>`, | ||
errors: [{ | ||
message: "Variable 'i' is already declared in the upper scope.", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please add line on which the following error is expected |
||
type: 'Identifier', | ||
line: 2 | ||
}] | ||
}, | ||
{ | ||
filename: 'test.vue', | ||
code: `<template> | ||
<div v-for="i in 5"></div> | ||
<div v-for="i in 5"></div> | ||
</template> | ||
<script> | ||
export default { | ||
data: { | ||
i: 7 | ||
} | ||
} | ||
</script>`, | ||
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 | ||
}] | ||
}, | ||
{ | ||
filename: 'test.vue', | ||
code: `<template> | ||
<div v-for="i in 5"> | ||
<div v-for="i in 5"></div> | ||
</div> | ||
</template> | ||
<script> | ||
export default { | ||
data: { | ||
i: 7 | ||
} | ||
} | ||
</script>`, | ||
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 | ||
}] | ||
}, | ||
{ | ||
filename: 'test.vue', | ||
code: `<template> | ||
<div v-for="i in 5"></div> | ||
<div v-for="f in 5"></div> | ||
</template> | ||
<script> | ||
export default { | ||
computed: { | ||
i () { } | ||
}, | ||
methods: { | ||
f () { } | ||
} | ||
} | ||
</script>`, | ||
errors: [{ | ||
message: "Variable 'i' is already declared in the upper scope.", | ||
type: 'Identifier', | ||
line: 2 | ||
}, { | ||
message: "Variable 'f' is already declared in the upper scope.", | ||
type: 'Identifier', | ||
line: 3 | ||
}] | ||
}, | ||
{ | ||
filename: 'test.vue', | ||
code: `<template> | ||
<div v-for="i in c" /> | ||
<div v-for="a in c" /> | ||
<div v-for="b in c" /> | ||
<div v-for="d in c" /> | ||
<div v-for="e in f" /> | ||
<div v-for="f in c" /> | ||
</template> | ||
<script> | ||
export default { | ||
...a, | ||
data() { | ||
return { | ||
...b, | ||
c: [1, 2, 3] | ||
} | ||
}, | ||
computed: { | ||
...d, | ||
e, | ||
['f']: [1, 2], | ||
} | ||
} | ||
</script>`, | ||
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 | ||
}] | ||
} | ||
] | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should present two or three cases here. First - a simple template example, and second with shadowed
data
ormethod
property declarations. e.g.:1.