Skip to content

Commit

Permalink
[Fix rubocop#12233] Make Style/SlicingWithRange aware of beginless …
Browse files Browse the repository at this point in the history
…range

Fixes rubocop#12233.

This PR makes `Style/SlicingWithRange` aware of redundant and beginless range.

Ruby 2.7 introduced beginless ranges. But, unlike the somewhat obscure `-1` in
`ary[1..-1]`, the `0` in `ary[0..42]` is clear as a starting point.
In fact, changing it to `ary[..42]` could potentially make it less readable.
Therefore, `ary[0..42]` should respect the original programmer's intent.
On the other hand, `ary[nil..42]` could be replaced with `ary[..42]`.
Similarly, `ary[1..nil]` could be replaced with `ary[1..]`.

Moreover, `[0..-1]` in `ary[0..-1]` is redundant and simply synonymous with `ary`.
This PR has also made adjustments to detect this case.
While this is a byproduct of supporting beginless and endless ranges,
I think including this change in `Style/SlicingWithRange` cop may be suitable
and not feel out of place.
  • Loading branch information
koic committed Jan 5, 2024
1 parent 88df571 commit 4278f84
Show file tree
Hide file tree
Showing 4 changed files with 154 additions and 15 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#12233](https://github.com/rubocop/rubocop/issues/12233): Make `Style/SlicingWithRange` aware of redundant and beginless range. ([@koic][])
2 changes: 1 addition & 1 deletion config/default.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5237,7 +5237,7 @@ Style/SingleLineMethods:
AllowIfMethodIsEmpty: true

Style/SlicingWithRange:
Description: 'Checks array slicing is done with endless ranges when suitable.'
Description: 'Checks array slicing is done with redundant, endless, and beginless ranges when suitable.'
Enabled: true
VersionAdded: '0.83'
Safe: false
Expand Down
76 changes: 65 additions & 11 deletions lib/rubocop/cop/style/slicing_with_range.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,9 @@
module RuboCop
module Cop
module Style
# Checks that arrays are sliced with endless ranges instead of
# `ary[start..-1]` on Ruby 2.6+.
# Checks that arrays are not sliced with the redundant `ary[0..-1]`, replacing it with `ary`,
# and ensures arrays are sliced with endless ranges instead of `ary[start..-1]` on Ruby 2.6+,
# and with beginless ranges instead of `ary[nil..end]` on Ruby 2.7+.
#
# @safety
# This cop is unsafe because `x..-1` and `x..` are only guaranteed to
Expand All @@ -21,42 +22,95 @@ module Style
#
# @example
# # bad
# items[1..-1]
# items[0..-1]
# items[0..nil]
# items[0...nil]
#
# # good
# items[1..]
# items
#
# # bad
# items[1..-1] # Ruby 2.6+
# items[1..nil] # Ruby 2.6+
#
# # good
# items[1..] # Ruby 2.6+
#
# # bad
# items[nil..42] # Ruby 2.7+
#
# # good
# items[..42] # Ruby 2.7+
# items[0..42] # Ruby 2.7+
#
class SlicingWithRange < Base
extend AutoCorrector
extend TargetRubyVersion

minimum_target_ruby_version 2.6

MSG = 'Prefer `%<prefer>s` over `%<current>s`.'
MSG_USELESS_RANGE = 'Remove the useless `%<prefer>s`.'
RESTRICT_ON_SEND = %i[[]].freeze

# @!method range_from_zero_till_minus_one?(node)
def_node_matcher :range_from_zero_till_minus_one?, <<~PATTERN
{
(irange (int 0) {(int -1) nil})
(erange (int 0) nil)
}
PATTERN

# @!method range_till_minus_one?(node)
def_node_matcher :range_till_minus_one?, '(irange !nil? (int -1))'
def_node_matcher :range_till_minus_one?, <<~PATTERN
{
(irange !nil? {(int -1) nil})
(erange !nil? nil)
}
PATTERN

# @!method range_from_zero?(node)
def_node_matcher :range_from_zero?, <<~PATTERN
(irange nil !nil?)
PATTERN

def on_send(node)
return unless node.arguments.one?

range_node = node.first_argument
return unless range_till_minus_one?(range_node)

prefer = preferred_method(range_node)
selector = node.loc.selector
message = format(MSG, prefer: prefer, current: selector.source)
unless (message, removal_range = offense_message_with_removal_range(range_node, selector))
return
end

add_offense(selector, message: message) do |corrector|
corrector.remove(range_node.end)
corrector.remove(removal_range)
end
end

private

def preferred_method(range_node)
def offense_message_with_removal_range(range_node, selector)
if range_from_zero_till_minus_one?(range_node)
[format(MSG_USELESS_RANGE, prefer: selector.source), selector]
elsif range_till_minus_one?(range_node)
[
format(MSG, prefer: endless(range_node), current: selector.source), range_node.end
]
elsif range_from_zero?(range_node) && target_ruby_version >= 2.7
[
format(MSG, prefer: beginless(range_node), current: selector.source), range_node.begin
]
end
end

def endless(range_node)
"[#{range_node.begin.source}#{range_node.loc.operator.source}]"
end

def beginless(range_node)
"[#{range_node.loc.operator.source}#{range_node.end.source}]"
end
end
end
end
Expand Down
90 changes: 87 additions & 3 deletions spec/rubocop/cop/style/slicing_with_range_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,54 @@

RSpec.describe RuboCop::Cop::Style::SlicingWithRange, :config do
context '<= Ruby 2.5', :ruby25 do
it 'reports no offense for array slicing with -1' do
it 'reports no offense for array slicing end with `-1`' do
expect_no_offenses(<<~RUBY)
ary[1..-1]
RUBY
end
end

context '>= Ruby 2.6', :ruby26 do
it 'reports an offense for slicing to ..-1' do
it 'reports an offense for slicing with `[0..-1]`' do
expect_offense(<<~RUBY)
ary[0..-1]
^^^^^^^ Remove the useless `[0..-1]`.
RUBY

expect_correction(<<~RUBY)
ary
RUBY
end

it 'does not register an offense for slicing with `[0...-1]`' do
expect_no_offenses(<<~RUBY)
ary[0...-1]
RUBY
end

it 'reports an offense for slicing with `[0..nil]`' do
expect_offense(<<~RUBY)
ary[0..nil]
^^^^^^^^ Remove the useless `[0..nil]`.
RUBY

expect_correction(<<~RUBY)
ary
RUBY
end

it 'reports an offense for slicing with `[0...nil]`' do
expect_offense(<<~RUBY)
ary[0...nil]
^^^^^^^^^ Remove the useless `[0...nil]`.
RUBY

expect_correction(<<~RUBY)
ary
RUBY
end

it 'reports an offense for slicing to `..-1`' do
expect_offense(<<~RUBY)
ary[1..-1]
^^^^^^^ Prefer `[1..]` over `[1..-1]`.
Expand All @@ -21,7 +60,29 @@
RUBY
end

it 'reports an offense for slicing from expression to ..-1' do
it 'reports an offense for slicing to `..nil`' do
expect_offense(<<~RUBY)
ary[1..nil]
^^^^^^^^ Prefer `[1..]` over `[1..nil]`.
RUBY

expect_correction(<<~RUBY)
ary[1..]
RUBY
end

it 'reports an offense for slicing to `...nil`' do
expect_offense(<<~RUBY)
ary[1...nil]
^^^^^^^^^ Prefer `[1...]` over `[1...nil]`.
RUBY

expect_correction(<<~RUBY)
ary[1...]
RUBY
end

it 'reports an offense for slicing from expression to `..-1`' do
expect_offense(<<~RUBY)
ary[fetch_start(true).first..-1]
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `[fetch_start(true).first..]` over `[fetch_start(true).first..-1]`.
Expand Down Expand Up @@ -49,9 +110,32 @@
ranges = [1..-1]
RUBY
end

it 'reports no offense for array slicing start with 0' do
expect_no_offenses(<<~RUBY)
ary[0..42]
RUBY
end
end

context '>= Ruby 2.7', :ruby27 do
it 'reports an offense for slicing with `[nil..42]`' do
expect_offense(<<~RUBY)
ary[nil..42]
^^^^^^^^^ Prefer `[..42]` over `[nil..42]`.
RUBY

expect_correction(<<~RUBY)
ary[..42]
RUBY
end

it 'does not register an offense for slicing with `[0..42]`' do
expect_no_offenses(<<~RUBY)
ary[0..42]
RUBY
end

it 'reports no offense for startless' do
expect_no_offenses(<<~RUBY)
ary[..-1]
Expand Down

0 comments on commit 4278f84

Please sign in to comment.