Skip to content

Commit

Permalink
Add option to show warnings when props could be accessed via this (#2101
Browse files Browse the repository at this point in the history
)

Co-authored-by: David Lomas <david.lomas@knowinnovation.com>
Co-authored-by: Yosuke Ota <otameshiyo23@gmail.com>
  • Loading branch information
3 people authored Apr 13, 2023
1 parent 6c32bf5 commit 31a38a9
Show file tree
Hide file tree
Showing 4 changed files with 270 additions and 10 deletions.
71 changes: 69 additions & 2 deletions docs/rules/no-unused-properties.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ since: v7.0.0
This rule is aimed at eliminating unused properties.

::: warning Note
This rule cannot be checked for use in other components (e.g. `mixins`, Property access via `$refs`) and use in places where the scope cannot be determined.
This rule cannot check for use of properties by other components (e.g. `mixins`, property access via `$refs`) and use in places where the scope cannot be determined. Some access to properties might be implied, for example accessing data or computed via a variable such as `this[varName]`. In this case, the default is to assume all properties, methods, etc. are 'used'. See the `unreferencedOptions` for a more strict interpretation of 'use' in these cases.
:::

<eslint-code-block :rules="{'vue/no-unused-properties': ['error']}">
Expand Down Expand Up @@ -56,7 +56,8 @@ This rule cannot be checked for use in other components (e.g. `mixins`, Property
"vue/no-unused-properties": ["error", {
"groups": ["props"],
"deepData": false,
"ignorePublicMembers": false
"ignorePublicMembers": false,
"unreferencedOptions": []
}]
}
```
Expand All @@ -69,6 +70,7 @@ This rule cannot be checked for use in other components (e.g. `mixins`, Property
- `"setup"`
- `deepData` (`boolean`) If `true`, the object of the property defined in `data` will be searched deeply. Default is `false`. Include `"data"` in `groups` to use this option.
- `ignorePublicMembers` (`boolean`) If `true`, members marked with a [JSDoc `/** @public */` tag](https://jsdoc.app/tags-public.html) will be ignored. Default is `false`.
- `unreferencedOptions` (`string[]`) Array of access methods that should be interpreted as leaving properties unreferenced. Currently, two such methods are available: `unknownMemberAsUnreferenced`, and `returnAsUnreferenced`. See examples below.

### `"groups": ["props", "data"]`

Expand Down Expand Up @@ -218,6 +220,71 @@ This rule cannot be checked for use in other components (e.g. `mixins`, Property

</eslint-code-block>

### `{ "groups": ["computed"], "unreferencedOptions": ["unknownMemberAsUnreferenced"] }`

<eslint-code-block :rules="{'vue/no-unused-properties': ['error', {groups: ['computed'], unreferencedOptions: ['unknownMemberAsUnreferenced']}]}">

```vue
<template>
</template>
<script>
export default {
computed: {
one () {
return 1
},
two () {
return 2
}
},
methods: {
handler () {
/* ✓ GOOD - explicit access to computed */
const a = this.one
const i = 'two'
/* ✗ BAD - unknown access via a variable, two will be reported as unreferenced */
return this[i]
},
}
}
</script>
```

</eslint-code-block>

### `{ "groups": ["computed"], "unreferencedOptions": ["returnAsUnreferenced"] }`

<eslint-code-block :rules="{'vue/no-unused-properties': ['error', {groups: ['computed'], unreferencedOptions: ['returnAsUnreferenced']}]}">

```vue
<template>
</template>
<script>
export default {
computed: {
one () {
return 1
},
two () {
return 2
}
},
methods: {
handler () {
/* ✓ GOOD - explicit access to computed */
const a = this.one
/* ✗ BAD - any property could be accessed by returning `this`, but two will still be reported as unreferenced */
return this
},
}
}
</script>
```

</eslint-code-block>

## :rocket: Version

This rule was introduced in eslint-plugin-vue v7.0.0
Expand Down
24 changes: 22 additions & 2 deletions lib/rules/no-unused-properties.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,9 @@ const GROUP_SETUP = 'setup'
const GROUP_WATCHER = 'watch'
const GROUP_EXPOSE = 'expose'

const UNREFERENCED_UNKNOWN_MEMBER = 'unknownMemberAsUnreferenced'
const UNREFERENCED_RETURN = 'returnAsUnreferenced'

const PROPERTY_LABEL = {
props: 'property',
data: 'data',
Expand Down Expand Up @@ -206,7 +209,15 @@ module.exports = {
uniqueItems: true
},
deepData: { type: 'boolean' },
ignorePublicMembers: { type: 'boolean' }
ignorePublicMembers: { type: 'boolean' },
unreferencedOptions: {
type: 'array',
items: {
enum: [UNREFERENCED_UNKNOWN_MEMBER, UNREFERENCED_RETURN]
},
additionalItems: false,
uniqueItems: true
}
},
additionalProperties: false
}
Expand All @@ -221,8 +232,17 @@ module.exports = {
const groups = new Set(options.groups || [GROUP_PROPERTY])
const deepData = Boolean(options.deepData)
const ignorePublicMembers = Boolean(options.ignorePublicMembers)
const unreferencedOptions = new Set(options.unreferencedOptions || [])

const propertyReferenceExtractor = definePropertyReferenceExtractor(context)
const propertyReferenceExtractor = definePropertyReferenceExtractor(
context,
{
unknownMemberAsUnreferenced: unreferencedOptions.has(
UNREFERENCED_UNKNOWN_MEMBER
),
returnAsUnreferenced: unreferencedOptions.has(UNREFERENCED_RETURN)
}
)

/** @type {TemplatePropertiesContainer} */
const templatePropertiesContainer = {
Expand Down
25 changes: 20 additions & 5 deletions lib/utils/property-references.js
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,10 @@ module.exports = {
/**
* @param {RuleContext} context The rule context.
*/
function definePropertyReferenceExtractor(context) {
function definePropertyReferenceExtractor(
context,
{ unknownMemberAsUnreferenced = false, returnAsUnreferenced = false } = {}
) {
/** @type {Map<Expression, IPropertyReferences>} */
const cacheForExpression = new Map()
/** @type {Map<Pattern, IPropertyReferences>} */
Expand Down Expand Up @@ -314,9 +317,15 @@ function definePropertyReferenceExtractor(context) {
if (parent.object === node) {
// `arg.foo`
const name = utils.getStaticPropertyName(parent)
return name
? new PropertyReferencesForMember(parent, name, withInTemplate)
: ANY
if (name) {
return new PropertyReferencesForMember(
parent,
name,
withInTemplate
)
} else {
return unknownMemberAsUnreferenced ? NEVER : ANY
}
}
return NEVER
}
Expand All @@ -331,12 +340,18 @@ function definePropertyReferenceExtractor(context) {
return extractFromExpression(parent, withInTemplate)
}
case 'ArrowFunctionExpression':
case 'ReturnStatement':
case 'VExpressionContainer':
case 'Property':
case 'ArrayExpression': {
return maybeExternalUsed(parent) ? ANY : NEVER
}
case 'ReturnStatement': {
if (returnAsUnreferenced) {
return NEVER
} else {
return maybeExternalUsed(parent) ? ANY : NEVER
}
}
}
return NEVER
}
Expand Down
160 changes: 159 additions & 1 deletion tests/lib/rules/no-unused-properties.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,33 @@ const allOptions = [
]
const deepDataOptions = [{ groups: ['data'], deepData: true }]

const unreferencedOptions = {
// Report errors when accessing via unknown property, e.g. this[varName]
unknownMemberAsUnreferenced: [
{
groups: ['computed'],
unreferencedOptions: ['unknownMemberAsUnreferenced']
}
],
// Report errors when returning this
returnAsUnreferenced: [
{
groups: ['computed'],
unreferencedOptions: ['returnAsUnreferenced']
}
],
// Report all
all: [
{
groups: ['computed'],
unreferencedOptions: [
'unknownMemberAsUnreferenced',
'returnAsUnreferenced'
]
}
]
}

tester.run('no-unused-properties', rule, {
valid: [
// a property used in a script expression
Expand Down Expand Up @@ -1699,7 +1726,6 @@ tester.run('no-unused-properties', rule, {
</script>`
}
],

invalid: [
// unused property
{
Expand Down Expand Up @@ -2803,6 +2829,138 @@ tester.run('no-unused-properties', rule, {
line: 10
}
]
},

// unreferencedOptions: unknownMemberAsUnreferenced
{
filename: 'test.vue',
code: `
<script>
export default {
computed: {
one () {
return 1
},
two () {
return 2
}
},
methods: {
handler () {
const a = this.one
const i = 'two'
return this[i]
},
}
}
</script>`,
options: unreferencedOptions.unknownMemberAsUnreferenced,
errors: [
{
message: "'two' of computed property found, but never used.",
line: 8
}
]
},
// unreferencedOptions: returnAsUnreferenced
{
filename: 'test.vue',
code: `
<script>
export default {
computed: {
one () {
return 1
},
two () {
return 2
}
},
methods: {
handler () {
const a = this.one
return this
},
}
}
</script>`,
options: unreferencedOptions.returnAsUnreferenced,
errors: [
{
message: "'two' of computed property found, but never used.",
line: 8
}
]
},
// unreferencedOptions: returnAsUnreferenced via variable with deepData
{
filename: 'test.vue',
code: `
<script>
export default {
data () {
return {
foo: {
bar: 1
},
baz: 2
}
},
methods: {
handler () {
const vm = this
console.log(vm.baz)
return vm.foo
},
}
}
</script>
`,
options: [
{
groups: ['data'],
unreferencedOptions: ['returnAsUnreferenced'],
deepData: true
}
],
errors: [
{
message: "'foo.bar' of data found, but never used.",
line: 7
}
]
},
// unreferencedOptions: all
{
filename: 'test.vue',
code: `
<script>
export default {
computed: {
one () {
return 1
},
two () {
return 2
}
},
methods: {
handler () {
const a = this.one
const i = 'two'
const b = this[i]
return this
},
}
}
</script>`,
options: unreferencedOptions.all,
errors: [
{
message: "'two' of computed property found, but never used.",
line: 8
}
]
}
]
})
Expand Down

0 comments on commit 31a38a9

Please sign in to comment.