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

Track usage of every assignment in UnusedLocalVariable #469

Merged
merged 4 commits into from
Sep 9, 2013
Merged

Track usage of every assignment in UnusedLocalVariable #469

merged 4 commits into from
Sep 9, 2013

Conversation

yujinakayama
Copy link
Collaborator

This closes #458.

It was much more complex than I expected, but I think I've done. :) UnusedLocalVariable now tracks usage of every assignment, that is neither entire variable lifetime nor only last assignment.

Before merging, I think we should consider reorganizing the new UnusedLocalVariable and UselessAssignment.

  • About local variable assignments, UnusedLocalVariable is now superior to UselessAssignment.
    • UnusedLocalVariable tracks every useless assignment in every scope.
    • UselessAssignment tracks useless assignment at the end of method.
  • UselessAssignment also tracks useless setter call at the end of method (technically it's not assignment though).

I think there are two choices:

  1. Merge the two cops into UselessAssignment. Two different logics, the new UnusedLocalVariable's one and the setter call tracking part of the current UselessAssignment, would live in a cop.
  2. Extract the setter call tracking logic to UselessSetterCall (or something), and rename UnusedLocalVariable to UselessAssignment.

I'd prefer option 2.

@bbatsov
Copy link
Collaborator

bbatsov commented Sep 8, 2013

I vote for option 2 as well.

Also rename VariableEntry to Variable.
The cop now tracks usage of each assignment
rather than usage in entire variable lifetime.

This closes #458.
* Extracted useless setter call tracking part of UselessAssignment to
  UselessSetterCall.
* Renamed UnusedLocalVariable to UselessAssignment.
@yujinakayama
Copy link
Collaborator Author

Pushed additional commit for option 2.

bbatsov added a commit that referenced this pull request Sep 9, 2013
Track usage of every assignment in UnusedLocalVariable
@bbatsov bbatsov merged commit 5d69a10 into rubocop:master Sep 9, 2013
@yujinakayama yujinakayama deleted the reassigned-but-unused-variable branch September 9, 2013 06:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Warn about re-assigned but not re-used variables
2 participants