Skip to content
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

[Fixes #294] Fix no-unused-vars rule #302

Merged
merged 3 commits into from
Dec 23, 2017
Merged

[Fixes #294] Fix no-unused-vars rule #302

merged 3 commits into from
Dec 23, 2017

Conversation

michalsnik
Copy link
Member

@michalsnik michalsnik commented Dec 21, 2017

This PR fixes no-unused-vars rule, so it doesn't throw a warning, when some of the properties are not used even when the last property was used.

As pointed in #294 currently code like this throws warnings:

<template>
  <!-- 'v'  is defined but never used, 'i' ..., 'c' ... -->
  <div v-for="(v, i, c) in foo">{{c}}</div>
</template>

In this PR I'm checking if the last property was referenced anywhere. If yes then we simply ignore other properties, if not -> we check references of all properties as before.

@mysticatea
Copy link
Member

As it happens, there is a discussion that core no-unused-vars rule should report all unused variables which are after the last used variable: eslint/eslint#9750

I like that approach.
Would you change this PR by using a for loop which goes from last to first?

for (let i = node.variables.length - 1; i >= 0 && node.varibles[i].references.length === 0; --i) {
  // report it.
}

@michalsnik
Copy link
Member Author

michalsnik commented Dec 22, 2017

I think this PR introduces exactly that @mysticatea, it works as follows:

<template>
  <div v-for="(v, i, c) in foo">{{v}}</div>
</template>

☝️ reports: i and c

<template>
  <div v-for="(v, i, c) in foo">{{i}}</div>
</template>

☝️ reports: c

<template>
  <div v-for="(v, i, c) in foo">{{c}}</div>
</template>

☝️ reports nothing

},
{
code: '<template><div v-for="(v, i, c) in foo">{{i}}</div></template>',
errors: ["'v' is defined but never used.", "'c' is defined but never used."]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are wrong in the 2nd case.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Holly molly! You're absolutely right. Thank you for being so vigilant! I'll update it tomorrow :)

@michalsnik
Copy link
Member Author

Should be good now @mysticatea

Copy link
Member

@mysticatea mysticatea left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thank you!

@mysticatea mysticatea merged commit b434ff9 into master Dec 23, 2017
@mysticatea mysticatea deleted the fix-no-unused-rule branch December 23, 2017 11:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants