Add new Performance/CollectionLiteralInLoop cop

[fatkodima]

I have seen code, like the following, a lot:

users.select do |user|
  %i[superadmin admin].include?(user.role)
end

It unnecessarily allocates array for each iteration of the loop. As soon as the iteration completes, the newly created array becomes unreachable garbage. It is better to extract it out of the loop, for example, to a local variable:

admin_roles = %i[superadmin admin]
users.select do |user|
  admin_roles.include?(user.role)
end

or to a constant:

# somewhere at the top of the class
ADMIN_ROLES = %i[superadmin admin]
...
users.select do |user|
  ADMIN_ROLES.include?(user.role)
end

This cop also works for hashes. It finds for non mutated array/hash literals inside loops.

I can provide benchmarks, if needed.

[bbatsov]

I have a feeling that the logic for immutable collections was needed somewhere else as well, so maybe it should eventually be made reusable in rubocop-ast.

[fatkodima]

1. Yes, I have found something similar https://github.com/rubocop-hq/rubocop/blob/b571ba252fba0e13331871b065b28f27c107eeda/lib/rubocop/cop/lint/void.rb#L56-L62

2. And I already used this ENUMERABLE_METHOD_NAMES constant in another PR https://github.com/rubocop-hq/rubocop-performance/pull/127/files#diff-6e1da1392214a67f5b4f8919f93f9b20R31 , so worth extracting

So how do you feel about:
3. extracting those POST_CONDITION_LOOP_TYPES and LOOP_TYPES into some properly named constants in rubocop-asts AST::Node

4. and those ENUMERABLE_METHOD_NAMES, NONMUTATING_ARRAY_METHODS and NONMUTATING_HASH_METHODS into rubocop-asts AST::CollectionNode module?

5. And probably it would be useful to extract those https://github.com/rubocop-hq/rubocop/blob/b571ba252fba0e13331871b065b28f27c107eeda/lib/rubocop/cop/lint/void.rb#L52-L54 into AST::Node.

6. And probably we should extend AST::Node with missing keywords to replace this hardcoded list? https://github.com/rubocop-hq/rubocop/blob/b571ba252fba0e13331871b065b28f27c107eeda/lib/rubocop/cop/style/redundant_self.rb#L146-L150

[bbatsov]

3-6 all make sense to me.

[marcandre]

Have you considered autocorrecting it to create a Set? Unless it calls join. This would be a stronger performance fix

[fatkodima]

No, haven't considered this. The original idea was to extract such code somewhere, not not replace it with something faster. Maybe extracted code then would be better transformed into set, but I think thats a story for another cop.

[connorshea]

Shouldn't this have been pending?