diff --git a/CHANGELOG.md b/CHANGELOG.md index d77a6c97e9..3219e384e7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ ### Bug fixes * [#247](https://github.com/rubocop/rubocop-performance/issues/247): Fix an incorrect auto-correct for `Performance/MapCompact` when using multi-line trailing dot method calls. ([@koic][]) +* [#249](https://github.com/rubocop/rubocop-performance/issues/249): Fix a false positive for `Performance/RedundantStringChars` when using `str.chars.last` and `str.chars.drop`. ([@koic][]) ### Changes diff --git a/docs/modules/ROOT/pages/cops_performance.adoc b/docs/modules/ROOT/pages/cops_performance.adoc index b6395e8003..4caea9b1ff 100644 --- a/docs/modules/ROOT/pages/cops_performance.adoc +++ b/docs/modules/ROOT/pages/cops_performance.adoc @@ -1447,28 +1447,28 @@ str[0..2].chars # bad str.chars.first str.chars.first(2) -str.chars.last -str.chars.last(2) # good str[0] str[0...2].chars -str[-1] -str[-2..-1].chars # bad str.chars.take(2) -str.chars.drop(2) str.chars.length str.chars.size str.chars.empty? # good str[0...2].chars -str[2..-1].chars str.length str.size str.empty? + +# For example, if the receiver is a blank string, it will be incompatible. +# If a negative value is specified for the receiver, `nil` is returned. +str.chars.last # Incompatible with `str[-1]`. +str.chars.last(2) # Incompatible with `str[-2..-1].chars`. +str.chars.drop(2) # Incompatible with `str[2..-1].chars`. ---- == Performance/RegexpMatch diff --git a/lib/rubocop/cop/performance/redundant_string_chars.rb b/lib/rubocop/cop/performance/redundant_string_chars.rb index 9d5eb625c6..20d9bc0c0c 100644 --- a/lib/rubocop/cop/performance/redundant_string_chars.rb +++ b/lib/rubocop/cop/performance/redundant_string_chars.rb @@ -16,35 +16,35 @@ module Performance # # bad # str.chars.first # str.chars.first(2) - # str.chars.last - # str.chars.last(2) # # # good # str[0] # str[0...2].chars - # str[-1] - # str[-2..-1].chars # # # bad # str.chars.take(2) - # str.chars.drop(2) # str.chars.length # str.chars.size # str.chars.empty? # # # good # str[0...2].chars - # str[2..-1].chars # str.length # str.size # str.empty? # + # # For example, if the receiver is a blank string, it will be incompatible. + # # If a negative value is specified for the receiver, `nil` is returned. + # str.chars.last # Incompatible with `str[-1]`. + # str.chars.last(2) # Incompatible with `str[-2..-1].chars`. + # str.chars.drop(2) # Incompatible with `str[2..-1].chars`. + # class RedundantStringChars < Base include RangeHelp extend AutoCorrector MSG = 'Use `%s` instead of `%s`.' - RESTRICT_ON_SEND = %i[[] slice first last take drop length size empty?].freeze + RESTRICT_ON_SEND = %i[[] slice first take length size empty?].freeze def_node_matcher :redundant_chars_call?, <<~PATTERN (send $(send _ :chars) $_ $...) @@ -80,7 +80,6 @@ def build_message(method, args) format(MSG, good_method: good_method, bad_method: bad_method) end - # rubocop:disable Metrics/CyclomaticComplexity, Metrics/MethodLength def build_good_method(method, args) case method when :[], :slice @@ -91,21 +90,12 @@ def build_good_method(method, args) else '[0]' end - when :last - if args.any? - "[-#{args.first.source}..-1].chars" - else - '[-1]' - end when :take "[0...#{args.first.source}].chars" - when :drop - "[#{args.first.source}..-1].chars" else ".#{method}" end end - # rubocop:enable Metrics/CyclomaticComplexity, Metrics/MethodLength def build_bad_method(method, args) case method diff --git a/spec/rubocop/cop/performance/redundant_string_chars_spec.rb b/spec/rubocop/cop/performance/redundant_string_chars_spec.rb index 21c79ef4f7..df256631fe 100644 --- a/spec/rubocop/cop/performance/redundant_string_chars_spec.rb +++ b/spec/rubocop/cop/performance/redundant_string_chars_spec.rb @@ -45,25 +45,15 @@ RUBY end - it 'registers an offense and corrects when using `str.chars.last`' do - expect_offense(<<~RUBY) + it 'does not register an offense when using `str.chars.last`' do + expect_no_offenses(<<~RUBY) str.chars.last - ^^^^^^^^^^ Use `[-1]` instead of `chars.last`. - RUBY - - expect_correction(<<~RUBY) - str[-1] RUBY end - it 'registers an offense and corrects when using `str.chars.last(2)`' do - expect_offense(<<~RUBY) + it 'does not register an offense when using `str.chars.last(2)`' do + expect_no_offenses(<<~RUBY) str.chars.last(2) - ^^^^^^^^^^^^^ Use `[-2..-1].chars` instead of `chars.last(2)`. - RUBY - - expect_correction(<<~RUBY) - str[-2..-1].chars RUBY end @@ -78,14 +68,9 @@ RUBY end - it 'registers an offense and corrects when using `str.chars.drop(2)`' do - expect_offense(<<~RUBY) + it 'does not register an offense when using `str.chars.drop(2)`' do + expect_no_offenses(<<~RUBY) str.chars.drop(2) - ^^^^^^^^^^^^^ Use `[2..-1].chars` instead of `chars.drop(2)`. - RUBY - - expect_correction(<<~RUBY) - str[2..-1].chars RUBY end