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 Rails/PluckInWhere cop #269

Merged
merged 1 commit into from
Jun 28, 2020
Merged

Conversation

fatkodima
Copy link
Contributor

@fatkodima fatkodima commented Jun 26, 2020

Closes #246

This pattern is very common and I have seen its usage a lot of times.
Maybe there are other places where pluck can be replaced with select, but in where - is the most common one I can think of right now.

@fatkodima fatkodima force-pushed the pluck_in_where-cop branch from be310e1 to 65bcce7 Compare June 26, 2020 20:27
@fatkodima fatkodima force-pushed the pluck_in_where-cop branch from 65bcce7 to 28ee6b8 Compare June 28, 2020 09:33
@fatkodima
Copy link
Contributor Author

Updated.


def in_where?(node)
send_node = node.each_ancestor(:send).first
WHERE_METHODS.include?(send_node&.method_name)
Copy link
Member

Choose a reason for hiding this comment

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

Why are safe navigation operator (&.) used?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

pluck can be not within other send node, so send_node can be nil.
I rewrote this line without & to make it a little bit clear.

@fatkodima fatkodima force-pushed the pluck_in_where-cop branch from 28ee6b8 to f0a5fbc Compare June 28, 2020 09:59
@fatkodima
Copy link
Contributor Author

Updated.

@koic koic merged commit 4e10c6e into rubocop:master Jun 28, 2020
@koic
Copy link
Member

koic commented Jun 28, 2020

Thank you! 👍

@ghost
Copy link

ghost commented Jul 6, 2020

Thank you @fatkodima! Excellent to see and learn from this PR! 🎉

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.

Performance Rails Cop proposal: Replace Pluck with Select within where clauses or subqueries
3 participants