Merge pull request #141 from fatkodima/string-chars-cop

Add new `Performance/RedundantStringChars` cop

CHANGELOG.md

@@ -4,6 +4,7 @@
 
 ### New features
 
+* [#141](https://github.com/rubocop-hq/rubocop-performance/pull/141): Add new `Performance/RedundantStringChars` cop. ([@fatkodima][])
 * [#127](https://github.com/rubocop-hq/rubocop-performance/pull/127): Add new `Performance/IoReadlines` cop. ([@fatkodima][])
 * [#128](https://github.com/rubocop-hq/rubocop-performance/pull/128): Add new `Performance/ReverseFirst` cop. ([@fatkodima][])
 * [#132](https://github.com/rubocop-hq/rubocop-performance/issues/132): Add new `Performance/RedundantSortBlock` cop. ([@fatkodima][])

config/default.yml

@@ -188,6 +188,11 @@ Performance/RedundantSortBlock:
   Enabled: true
   VersionAdded: '1.7'
 
+Performance/RedundantStringChars:
+  Description: 'Checks for redundant `String#chars`.'
+  Enabled: true
+  VersionAdded: '1.7'
+
 Performance/RegexpMatch:
   Description: >-
                   Use `match?` instead of `Regexp#match`, `String#match`, `Symbol#match`,

docs/modules/ROOT/pages/cops.adoc

@@ -26,6 +26,7 @@
 * xref:cops_performance.adoc#performanceredundantmatch[Performance/RedundantMatch]
 * xref:cops_performance.adoc#performanceredundantmerge[Performance/RedundantMerge]
 * xref:cops_performance.adoc#performanceredundantsortblock[Performance/RedundantSortBlock]
+* xref:cops_performance.adoc#performanceredundantstringchars[Performance/RedundantStringChars]
 * xref:cops_performance.adoc#performanceregexpmatch[Performance/RegexpMatch]
 * xref:cops_performance.adoc#performancereverseeach[Performance/ReverseEach]
 * xref:cops_performance.adoc#performancereversefirst[Performance/ReverseFirst]

docs/modules/ROOT/pages/cops_performance.adoc

@@ -1101,6 +1101,58 @@ array.sort { |a, b| a <=> b }
 array.sort
 ----
 
+== Performance/RedundantStringChars
+
+|===
+| Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged
+
+| Enabled
+| Yes
+| Yes
+| 1.7
+| -
+|===
+
+This cop checks for redundant `String#chars`.
+
+=== Examples
+
+[source,ruby]
+----
+# bad
+str.chars[0..2]
+str.chars.slice(0..2)
+
+# good
+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?
+----
+
 == Performance/RegexpMatch
 
 |===

lib/rubocop/cop/performance/redundant_string_chars.rb

@@ -0,0 +1,137 @@
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    module Performance
+      # This cop checks for redundant `String#chars`.
+      #
+      # @example
+      #   # bad
+      #   str.chars[0..2]
+      #   str.chars.slice(0..2)
+      #
+      #   # good
+      #   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?
+      #
+      class RedundantStringChars < Cop
+        include RangeHelp
+
+        MSG = 'Use `%<good_method>s` instead of `%<bad_method>s`.'
+        REPLACEABLE_METHODS = %i[[] slice first last take drop length size empty?].freeze
+
+        def_node_matcher :redundant_chars_call?, <<~PATTERN
+          (send $(send _ :chars) $#replaceable_method? $...)
+        PATTERN
+
+        def on_send(node)
+          redundant_chars_call?(node) do |receiver, method, args|
+            range = offense_range(receiver, node)
+            message = build_message(method, args)
+            add_offense(node, location: range, message: message)
+          end
+        end
+
+        def autocorrect(node)
+          redundant_chars_call?(node) do |receiver, method, args|
+            range = correction_range(receiver, node)
+            replacement = build_good_method(method, args)
+
+            lambda do |corrector|
+              corrector.replace(range, replacement)
+            end
+          end
+        end
+
+        private
+
+        def replaceable_method?(method_name)
+          REPLACEABLE_METHODS.include?(method_name)
+        end
+
+        def offense_range(receiver, node)
+          range_between(receiver.loc.selector.begin_pos, node.loc.expression.end_pos)
+        end
+
+        def correction_range(receiver, node)
+          range_between(receiver.loc.dot.begin_pos, node.loc.expression.end_pos)
+        end
+
+        def build_message(method, args)
+          good_method = build_good_method(method, args)
+          bad_method = build_bad_method(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
+            "[#{build_call_args(args)}].chars"
+          when :first
+            if args.any?
+              "[0...#{args.first.source}].chars"
+            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
+          when :[]
+            "chars[#{build_call_args(args)}]"
+          else
+            if args.any?
+              "chars.#{method}(#{build_call_args(args)})"
+            else
+              "chars.#{method}"
+            end
+          end
+        end
+
+        def build_call_args(call_args_node)
+          call_args_node.map(&:source).join(', ')
+        end
+      end
+    end
+  end
+end

lib/rubocop/cop/performance_cops.rb

@@ -26,6 +26,7 @@
 require_relative 'performance/redundant_match'
 require_relative 'performance/redundant_merge'
 require_relative 'performance/redundant_sort_block'
+require_relative 'performance/redundant_string_chars'
 require_relative 'performance/regexp_match'
 require_relative 'performance/reverse_each'
 require_relative 'performance/reverse_first'

spec/rubocop/cop/performance/redundant_string_chars_spec.rb

@@ -0,0 +1,132 @@
+# frozen_string_literal: true
+
+RSpec.describe RuboCop::Cop::Performance::RedundantStringChars do
+  subject(:cop) { described_class.new }
+
+  it 'registers an offense and corrects when using `str.chars[0..2]`' do
+    expect_offense(<<~RUBY)
+      str.chars[0..2]
+          ^^^^^^^^^^^ Use `[0..2].chars` instead of `chars[0..2]`.
+    RUBY
+
+    expect_correction(<<~RUBY)
+      str[0..2].chars
+    RUBY
+  end
+
+  it 'registers an offense and corrects when using `str.chars.slice(0..2)`' do
+    expect_offense(<<~RUBY)
+      str.chars.slice(0..2)
+          ^^^^^^^^^^^^^^^^^ Use `[0..2].chars` instead of `chars.slice(0..2)`.
+    RUBY
+
+    expect_correction(<<~RUBY)
+      str[0..2].chars
+    RUBY
+  end
+
+  it 'registers an offense and corrects when using `str.chars.first`' do
+    expect_offense(<<~RUBY)
+      str.chars.first
+          ^^^^^^^^^^^ Use `[0]` instead of `chars.first`.
+    RUBY
+
+    expect_correction(<<~RUBY)
+      str[0]
+    RUBY
+  end
+
+  it 'registers an offense and corrects when using `str.chars.first(2)`' do
+    expect_offense(<<~RUBY)
+      str.chars.first(2)
+          ^^^^^^^^^^^^^^ Use `[0...2].chars` instead of `chars.first(2)`.
+    RUBY
+
+    expect_correction(<<~RUBY)
+      str[0...2].chars
+    RUBY
+  end
+
+  it 'registers an offense and corrects when using `str.chars.last`' do
+    expect_offense(<<~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)
+      str.chars.last(2)
+          ^^^^^^^^^^^^^ Use `[-2..-1].chars` instead of `chars.last(2)`.
+    RUBY
+
+    expect_correction(<<~RUBY)
+      str[-2..-1].chars
+    RUBY
+  end
+
+  it 'registers an offense and corrects when using `str.chars.take(2)`' do
+    expect_offense(<<~RUBY)
+      str.chars.take(2)
+          ^^^^^^^^^^^^^ Use `[0...2].chars` instead of `chars.take(2)`.
+    RUBY
+
+    expect_correction(<<~RUBY)
+      str[0...2].chars
+    RUBY
+  end
+
+  it 'registers an offense and corrects when using `str.chars.drop(2)`' do
+    expect_offense(<<~RUBY)
+      str.chars.drop(2)
+          ^^^^^^^^^^^^^ Use `[2..-1].chars` instead of `chars.drop(2)`.
+    RUBY
+
+    expect_correction(<<~RUBY)
+      str[2..-1].chars
+    RUBY
+  end
+
+  it 'registers an offense and corrects when using `str.chars.length`' do
+    expect_offense(<<~RUBY)
+      str.chars.length
+          ^^^^^^^^^^^^ Use `.length` instead of `chars.length`.
+    RUBY
+
+    expect_correction(<<~RUBY)
+      str.length
+    RUBY
+  end
+
+  it 'registers an offense and corrects when using `str.chars.size`' do
+    expect_offense(<<~RUBY)
+      str.chars.size
+          ^^^^^^^^^^ Use `.size` instead of `chars.size`.
+    RUBY
+
+    expect_correction(<<~RUBY)
+      str.size
+    RUBY
+  end
+
+  it 'registers an offense and corrects when using `str.chars.empty?`' do
+    expect_offense(<<~RUBY)
+      str.chars.empty?
+          ^^^^^^^^^^^^ Use `.empty?` instead of `chars.empty?`.
+    RUBY
+
+    expect_correction(<<~RUBY)
+      str.empty?
+    RUBY
+  end
+
+  it 'does not register an offense when using `str.chars.max`' do
+    expect_no_offenses(<<~RUBY)
+      str.chars.max
+    RUBY
+  end
+end