Skip to content

Add new Rails/InGroupsOf cop - #1654

Open
jcoleman wants to merge 1 commit into
rubocop:masterfrom
jcoleman:rails-in-groups-of-cop
Open

Add new Rails/InGroupsOf cop#1654
jcoleman wants to merge 1 commit into
rubocop:masterfrom
jcoleman:rails-in-groups-of-cop

Conversation

@jcoleman

Copy link
Copy Markdown

What

Prefer Ruby's built-in Enumerable#each_slice over Active Support's in_groups_of when the collection is chunked without an intentional fill value.

The cop reports calls with no second argument and calls passing an explicit false (which is equivalent to each_slice). Any other explicit second argument is treated as an intentional fill value and left alone.

An explicit EnforcedStyle instead asks for an explicit second argument on calls that omit one, making the padding behavior obvious at the call site; in that style any explicit second argument, including false, is accepted.

Autocorrection is not supported because an omitted second argument may reflect an intentional desire to pad with nil rather than a bug.

Why

We've noticed that it's in_groups_of is a common source of bugs. Calling in_groups_of without a second argument pads the final group with nil, which is easy to overlook and frequently leads to a NoMethodError on nil (or another type error) downstream.

While in_groups_of is useful in e.g. HTML table generation (where padding is likely the default requirement), the now-builtin Ruby each_slice is semantically more correct by default for most code we've seen.


Before submitting the PR make sure the following are checked:

  • The PR relates to only one subject with a clear title and description in grammatically correct, complete sentences.
  • Wrote good commit messages.
  • Commit message starts with [Fix #issue-number] (if the related issue exists).
  • Feature branch is up-to-date with master (if not - rebase it).
  • Squashed related commits together.
  • Added tests.
  • Ran bundle exec rake default. It executes all tests and runs RuboCop on its own code.
  • Added an entry (file) to the changelog folder named {change_type}_{change_description}.md if the new code introduces user-observable changes. See changelog entry format for details.
  • If this is a new cop, consider making a corresponding update to the Rails Style Guide. Prefer each_slice to in_groups_of rails-style-guide#382

Prefer Ruby's built-in `Enumerable#each_slice` over Active Support's
`in_groups_of` when the collection is chunked without an intentional fill
value. Calling `in_groups_of` without a second argument pads the final group
with `nil`, which is easy to overlook and frequently leads to a NoMethodError
on nil (or another type error) downstream.

The cop reports calls with no second argument and calls passing an explicit
`false` (which is equivalent to `each_slice`). Any other explicit second
argument is treated as an intentional fill value and left alone.

An `explicit` EnforcedStyle instead asks for an explicit second argument on
calls that omit one, making the padding behavior obvious at the call site; in
that style any explicit second argument, including `false`, is accepted.

Autocorrection is not supported because an omitted second argument may reflect
an intentional desire to pad with `nil` rather than a bug.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.

1 participant