Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make Style/ZeroLengthPredicate aware of array.length.zero? #11327

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#11327](https://github.com/rubocop/rubocop/pull/11327): Make `Style/ZeroLengthPredicate` aware of `array.length.zero?`. ([@koic][])
2 changes: 1 addition & 1 deletion lib/rubocop/cop/style/block_comments.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ def on_new_investigation
eq_begin, eq_end, contents = parts(comment)

corrector.remove(eq_begin)
unless contents.length.zero?
unless contents.empty?
corrector.replace(
contents,
contents.source.gsub(/\A/, '# ').gsub(/\n\n/, "\n#\n").gsub(/\n(?=[^#])/, "\n# ")
Expand Down
45 changes: 31 additions & 14 deletions lib/rubocop/cop/style/zero_length_predicate.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,58 +34,75 @@ module Style
class ZeroLengthPredicate < Base
extend AutoCorrector

ZERO_MSG = 'Use `empty?` instead of `%<lhs>s %<opr>s %<rhs>s`.'
NONZERO_MSG = 'Use `!empty?` instead of `%<lhs>s %<opr>s %<rhs>s`.'
ZERO_MSG = 'Use `empty?` instead of `%<current>s`.'
NONZERO_MSG = 'Use `!empty?` instead of `%<current>s`.'

RESTRICT_ON_SEND = %i[size length].freeze

def on_send(node)
check_zero_length_predicate(node)
check_nonzero_length_predicate(node)
check_zero_length_comparison(node)
check_nonzero_length_comparison(node)
end

private

def check_zero_length_predicate(node)
zero_length_predicate = zero_length_predicate(node.parent)
return unless zero_length_predicate
return unless (length_method = zero_length_predicate(node.parent))

lhs, opr, rhs = zero_length_predicate
offense = node.loc.selector.join(node.parent.source_range.end)
message = format(ZERO_MSG, current: "#{length_method}.zero?")

add_offense(offense, message: message) do |corrector|
corrector.replace(offense, 'empty?')
end
end

def check_zero_length_comparison(node)
zero_length_comparison = zero_length_comparison(node.parent)
return unless zero_length_comparison

lhs, opr, rhs = zero_length_comparison

return if non_polymorphic_collection?(node.parent)

add_offense(
node.parent, message: format(ZERO_MSG, lhs: lhs, opr: opr, rhs: rhs)
node.parent, message: format(ZERO_MSG, current: "#{lhs} #{opr} #{rhs}")
) do |corrector|
corrector.replace(node.parent, replacement(node.parent))
end
end

def check_nonzero_length_predicate(node)
nonzero_length_predicate = nonzero_length_predicate(node.parent)
return unless nonzero_length_predicate
def check_nonzero_length_comparison(node)
nonzero_length_comparison = nonzero_length_comparison(node.parent)
return unless nonzero_length_comparison

lhs, opr, rhs = nonzero_length_predicate
lhs, opr, rhs = nonzero_length_comparison

return if non_polymorphic_collection?(node.parent)

add_offense(
node.parent, message: format(NONZERO_MSG, lhs: lhs, opr: opr, rhs: rhs)
node.parent, message: format(NONZERO_MSG, current: "#{lhs} #{opr} #{rhs}")
) do |corrector|
corrector.replace(node.parent, replacement(node.parent))
end
end

# @!method zero_length_predicate(node)
def_node_matcher :zero_length_predicate, <<~PATTERN
(send (send (...) ${:length :size}) :zero?)
PATTERN

# @!method zero_length_comparison(node)
def_node_matcher :zero_length_comparison, <<~PATTERN
{(send (send (...) ${:length :size}) $:== (int $0))
(send (int $0) $:== (send (...) ${:length :size}))
(send (send (...) ${:length :size}) $:< (int $1))
(send (int $1) $:> (send (...) ${:length :size}))}
PATTERN

# @!method nonzero_length_predicate(node)
def_node_matcher :nonzero_length_predicate, <<~PATTERN
# @!method nonzero_length_comparison(node)
def_node_matcher :nonzero_length_comparison, <<~PATTERN
{(send (send (...) ${:length :size}) ${:> :!=} (int $0))
(send (int $0) ${:< :!=} (send (...) ${:length :size}))}
PATTERN
Expand Down
44 changes: 44 additions & 0 deletions spec/rubocop/cop/style/zero_length_predicate_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,28 @@
RUBY
end

it 'registers an offense for `array.length.zero?`' do
expect_offense(<<~RUBY)
[1, 2, 3].length.zero?
^^^^^^^^^^^^ Use `empty?` instead of `length.zero?`.
RUBY

expect_correction(<<~RUBY)
[1, 2, 3].empty?
RUBY
end

it 'registers an offense for `array.size.zero?`' do
expect_offense(<<~'RUBY')
[1, 2, 3].size.zero?
^^^^^^^^^^ Use `empty?` instead of `size.zero?`.
RUBY

expect_correction(<<~'RUBY')
[1, 2, 3].empty?
RUBY
end

it 'registers an offense for `0 == array.length`' do
expect_offense(<<~'RUBY')
0 == [1, 2, 3].length
Expand Down Expand Up @@ -134,6 +156,28 @@
RUBY
end

it 'registers an offense for `!array.length.zero?`' do
expect_offense(<<~RUBY)
![1, 2, 3].length.zero?
^^^^^^^^^^^^ Use `empty?` instead of `length.zero?`.
RUBY

expect_correction(<<~RUBY)
![1, 2, 3].empty?
RUBY
end

it 'registers an offense for `!array.size.zero?`' do
expect_offense(<<~'RUBY')
![1, 2, 3].size.zero?
^^^^^^^^^^ Use `empty?` instead of `size.zero?`.
RUBY

expect_correction(<<~'RUBY')
![1, 2, 3].empty?
RUBY
end

it 'registers an offense for `0 < array.length' do
expect_offense(<<~'RUBY')
0 < [1, 2, 3].length
Expand Down