[Fix rubocop#249] Fix a false positive for `Performance/RedundantStri…

…ngChars`

Fixes rubocop#249.

This PR fixes a false positive for `Performance/RedundantStringChars` when
using `str.chars.last` and `str.chars.drop`.

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
 

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

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 `%<good_method>s` instead of `%<bad_method>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

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