New cop to detect `.distinct(:col_name)` antipattern

[jamiemccarthy]

Is your feature request related to a problem? Please describe.

A common misunderstanding of the relational model is to use DISTINCT where GROUP BY was intended. The same mistake in ActiveRecord sees developers writing Relation.distinct(:column_name) with the belief that only that column's values will be distinct.

In fact, in SQL, SELECT DISTINCT is the keyword. When present, it ensures all rows returned are unique, across all returned columns.

ActiveRecord permits .distinct(:column_name) and treats it exactly like .distinct or .distinct(true), because it's only concerned with the truth or falsity of its argument.

Using it this way happens to accidentally work, in the same way that writing SELECT DISTINCT x, y, z happens to work even if the engineer wrongly believes it's x that is getting grouped-on.

Using distinct in this way is an antipattern because the code is misleading.

Describe the solution you'd like

A Rails cop should detect distinct with a String or Symbol argument and flag it with a warning. Autocorrect should remove the argument, which defaults to true.

This gives engineers a chance to make their code less-misleading, and learn a little bit about ActiveRecord too.

Describe alternatives you've considered

Alternative approaches:

• Flag any argument other than true/false/nil. But one could picture cases where the engineer uses odd truthy values like 1 for some reason, and it may be difficult to anticipate the boundary line between mistake and cleverness.

• Flag any argument that is blank? but truthy, such as {} or []. Those should not be autocorrected, though, because the author's intention seems unclear and the correction would change behavior. I doubt this is worth doing, because this mistake seems extremely unlikely.

• Check for known column names of the relation and only flag those — but this is harder and has little benefit. If an engineer calls distinct("misspelled_column_name") they are probably still laboring under a misapprehension that should be corrected.

Additional context

I am proposing this because I've seen this mistake in production code. 😺

I'm happy to write this cop if the project maintainers consider it useful.