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

Add new Style/RedundantAssignment cop #8235

Merged

Conversation

fatkodima
Copy link
Contributor

Closes #7373

Can confirm this is a popular pattern - I have seen those many times. Also I have found 3 offenses in rails codebase and 1 in rubocops own.

This cop finds the pattern recursively, not just as last 2 expressions in method's body.

@marcandre
Copy link
Contributor

Cool 😄

I didn't test it but I fear your code will erroneously flag the following, right?

def foo
  bar = something

  bar
ensure
  something_else(bar)
end

@fatkodima fatkodima force-pushed the redundant-assignment-before-return branch from 6205193 to 99eda9f Compare July 4, 2020 06:26
@fatkodima
Copy link
Contributor Author

@marcandre Oh, wow, great catch! 💪

Changed handling of ensure nodes and added test case for this.

@bbatsov
Copy link
Collaborator

bbatsov commented Jul 5, 2020

I like the proposed cop. I'm only wondering if we should shorten its name and drop BeforeReturn, as I can't imagine another case of redundant assignment - I guess anything else goes to the unused assignment category.

@fatkodima fatkodima force-pushed the redundant-assignment-before-return branch from 99eda9f to 19b41e6 Compare July 5, 2020 21:59
@fatkodima fatkodima changed the title Add new Style/RedundantAssignmentBeforeReturn cop Add new Style/RedundantAssignment cop Jul 5, 2020
@fatkodima
Copy link
Contributor Author

@bbatsov Agreed, I can not imagine too. Stripped that BeforeReturn from the cop name.

@bbatsov bbatsov merged commit 903915d into rubocop:master Jul 6, 2020
@bbatsov
Copy link
Collaborator

bbatsov commented Jul 6, 2020

Thanks!

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.

Detect useless assignment and return
3 participants