Skip to content

Commit

Permalink
Add new Performance/RedundantStringChars cop
Browse files Browse the repository at this point in the history
  • Loading branch information
fatkodima committed Jun 24, 2020
1 parent c0402b7 commit f615047
Show file tree
Hide file tree
Showing 7 changed files with 329 additions and 0 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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][])
Expand Down
5 changes: 5 additions & 0 deletions config/default.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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`,
Expand Down
1 change: 1 addition & 0 deletions docs/modules/ROOT/pages/cops.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
52 changes: 52 additions & 0 deletions docs/modules/ROOT/pages/cops_performance.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -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

|===
Expand Down
137 changes: 137 additions & 0 deletions lib/rubocop/cop/performance/redundant_string_chars.rb
Original file line number Diff line number Diff line change
@@ -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
1 change: 1 addition & 0 deletions lib/rubocop/cop/performance_cops.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
132 changes: 132 additions & 0 deletions spec/rubocop/cop/performance/redundant_string_chars_spec.rb
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit f615047

Please sign in to comment.