Add note to redundant-all section about all receiver being an association

[masato-bkn]

As pointed out #347, when the receiver of all is an association, there are methods whose behavior changes when all is omitted.

So, I've added a note to caution against omitting these methods.

[masato-bkn]

I'm torn about whether to add delete and destroy to this list for the following reasons:

• These methods are not the targets for verification of cop.

• The expected arguments differ depending on whether all is present or not. In the first place, it's not possible to omit all. Even if omitted, unlike delete_all, it seems unlikely that one wouldn't notice the change in behavior:thinking:

  user.articles.all.delete(id_or_array) # takes an id or an array as an argument
  # https://api.rubyonrails.org/classes/ActiveRecord/Persistence/ClassMethods.html#method-i-delete
  
  user.articles.delete(Article.first) # takes a records as an argument
  # https://api.rubyonrails.org/classes/ActiveRecord/Associations/CollectionProxy.html#method-i-delete
  
  user.articles.all.destroy(id) # takes an id as an argument
  # https://api.rubyonrails.org/classes/ActiveRecord/Persistence/ClassMethods.html#method-i-destroy
  
  user.articles.destroy(Article.first) # takes a records as an argument
  # https://api.rubyonrails.org/classes/ActiveRecord/Associations/CollectionProxy.html#method-i-destroy

[masato-bkn]

I think it might be better to leave the specifics of the differences in behavior to the official Rails documentation, rather than detailing them in this style guide. So, I only included the URL to the documentation and did not write about how each method behaves differently in the style guide.

[pirj]

Can we consider ‘delete*’ “query methods”?
If not, we should emphasize on that.

[pirj]

Can we consider ‘delete*’ “query methods”?
The cop goes beyond just query methods.
What if we briefly mention that ‘all’ is redundant except for those not getting i to details?

[masato-bkn]

Thank you for your comment!

It's a tough call, but I consider delete* as query methods. Upon checking the ActiveRecord::Querying::QUERYING_METHODS constant in ActiveRecord, it includes methods like delete* and update*. Given this, in the context of ActiveRecord, it could be argued that delete* is also treated as a query method.

However, for those unfamiliar with these specifics, we might consider revising the phrasing.

What if we briefly mention that ‘all’ is redundant except for those not getting into details?

What specific wording do you think would be appropriate? I'd appreciate any suggestions you might have.

[masato-bkn]

I made revisions based on the feedback. How does this look? 5efdfa8

[pirj]

Thanks!

[koic]

Please remove the redundant blank lines in the list items and surround the method names with backticks.

Suggested change

	* delete - https://api.rubyonrails.org/classes/ActiveRecord/Persistence/ClassMethods.html#method-i-delete[with all] / https://api.rubyonrails.org/classes/ActiveRecord/Associations/CollectionProxy.html#method-i-delete[without all]
	* delete_all - https://api.rubyonrails.org/classes/ActiveRecord/Relation.html#method-i-delete_all[with all] / https://api.rubyonrails.org/classes/ActiveRecord/Associations/CollectionProxy.html#method-i-delete_all[without all]
	* destroy - https://api.rubyonrails.org/classes/ActiveRecord/Persistence/ClassMethods.html#method-i-destroy[with all] / https://api.rubyonrails.org/classes/ActiveRecord/Associations/CollectionProxy.html#method-i-destroy[without all]
	* destroy_all - https://api.rubyonrails.org/classes/ActiveRecord/Relation.html#method-i-destroy_all[with all] / https://api.rubyonrails.org/classes/ActiveRecord/Associations/CollectionProxy.html#method-i-destroy_all[without all]
	* `delete` - https://api.rubyonrails.org/classes/ActiveRecord/Persistence/ClassMethods.html#method-i-delete[with all] / https://api.rubyonrails.org/classes/ActiveRecord/Associations/CollectionProxy.html#method-i-delete[without all]
	* `delete_all` - https://api.rubyonrails.org/classes/ActiveRecord/Relation.html#method-i-delete_all[with all] / https://api.rubyonrails.org/classes/ActiveRecord/Associations/CollectionProxy.html#method-i-delete_all[without all]
	* `destroy` - https://api.rubyonrails.org/classes/ActiveRecord/Persistence/ClassMethods.html#method-i-destroy[with all] / https://api.rubyonrails.org/classes/ActiveRecord/Associations/CollectionProxy.html#method-i-destroy[without all]
	* `destroy_all` - https://api.rubyonrails.org/classes/ActiveRecord/Relation.html#method-i-destroy_all[with all] / https://api.rubyonrails.org/classes/ActiveRecord/Associations/CollectionProxy.html#method-i-destroy_all[without all]

[masato-bkn]

I've addressed it with d41903b.

[koic]

This looks good. Can you squash your commits into one?

FYI, in reality, as a reviewer wanting to see the differences, it's sufficient to check using "Compare" link, so there's no need to commits as a work log :-)

[masato-bkn]

Thank you all for your reviews! I have squashed the commits, so please check it.

[koic]

Thanks!