Skip to content

Commit

Permalink
Add new Performance/RedundantSplitRegexpArgument cop.
Browse files Browse the repository at this point in the history
Apply reviews

Remove wrong configuration

Fix auto-correction with escaping chars

Ignore string special chars

Guarantee string will behave the same when using special chars

Apply review

Add changelog
  • Loading branch information
mfbmina committed Jan 14, 2021
1 parent eb26b4a commit 1ae6d24
Show file tree
Hide file tree
Showing 7 changed files with 135 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

* [#190](https://github.com/rubocop-hq/rubocop-performance/pull/190): Add new `Performance/RedundantSplitRegexpArgument` cop. ([@mfbmina][])
* [#173](https://github.com/rubocop-hq/rubocop-performance/pull/173): Add new `Performance/BlockGivenWithExplicitBlock` cop. ([@fatkodima][])
* [#136](https://github.com/rubocop-hq/rubocop-performance/issues/136): Add new `Performance/MethodObjectAsBlock` cop. ([@fatkodima][])
* [#151](https://github.com/rubocop-hq/rubocop-performance/issues/151): Add new `Performance/ConstantRegexp` 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 @@ -215,6 +215,11 @@ Performance/RedundantSortBlock:
Enabled: 'pending'
VersionAdded: '1.7'

Performance/RedundantSplitRegexpArgument:
Description: 'This cop identifies places where `split` argument can be replaced from a deterministic regexp to a string.'
Enabled: pending
VersionAdded: '1.10'

Performance/RedundantStringChars:
Description: 'Checks for redundant `String#chars`.'
Enabled: 'pending'
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 @@ -41,6 +41,7 @@ Performance cops optimization analysis for your projects.
* 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#performanceredundantsplitregexpargument[Performance/RedundantSplitRegexpArgument]
* xref:cops_performance.adoc#performanceredundantstringchars[Performance/RedundantStringChars]
* xref:cops_performance.adoc#performanceregexpmatch[Performance/RegexpMatch]
* xref:cops_performance.adoc#performancereverseeach[Performance/ReverseEach]
Expand Down
26 changes: 26 additions & 0 deletions docs/modules/ROOT/pages/cops_performance.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -1302,6 +1302,32 @@ array.sort { |a, b| a <=> b }
array.sort
----

== Performance/RedundantSplitRegexpArgument

|===
| Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged

| Pending
| Yes
| Yes
| 1.9
| -
|===

This cop identifies places where `split` argument can be replaced from
a deterministic regexp to a string.

=== Examples

[source,ruby]
----
# bad
'a,b,c'.split(/,/)
# good
'a,b,c'.split(',')
----

== Performance/RedundantStringChars

|===
Expand Down
51 changes: 51 additions & 0 deletions lib/rubocop/cop/performance/redundant_split_regexp_argument.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
# frozen_string_literal: true

module RuboCop
module Cop
module Performance
# This cop identifies places where `split` argument can be replaced from
# a deterministic regexp to a string.
#
# @example
# # bad
# 'a,b,c'.split(/,/)
#
# # good
# 'a,b,c'.split(',')
class RedundantSplitRegexpArgument < Base
extend AutoCorrector

MSG = 'Use string as argument instead of regexp.'
RESTRICT_ON_SEND = %i[split].freeze
DETERMINISTIC_REGEX = /\A(?:#{LITERAL_REGEX})+\Z/.freeze
STR_SPECIAL_CHARS = %w[\n \" \' \\ \t \b \f \r].freeze

def_node_matcher :split_call_with_regexp?, <<~PATTERN
{(send !nil? :split {regexp})}
PATTERN

def on_send(node)
return unless split_call_with_regexp?(node)
return unless determinist_regexp?(node.first_argument)

add_offense(node.first_argument) do |corrector|
autocorrect(corrector, node)
end
end

private

def determinist_regexp?(first_argument)
DETERMINISTIC_REGEX.match?(first_argument.source)
end

def autocorrect(corrector, node)
new_argument = node.first_argument.source[1..-2]
new_argument.delete!('\\') unless STR_SPECIAL_CHARS.include?(new_argument)

corrector.replace(node.first_argument, "\"#{new_argument}\"")
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 @@ -31,6 +31,7 @@
require_relative 'performance/redundant_match'
require_relative 'performance/redundant_merge'
require_relative 'performance/redundant_sort_block'
require_relative 'performance/redundant_split_regexp_argument'
require_relative 'performance/redundant_string_chars'
require_relative 'performance/regexp_match'
require_relative 'performance/reverse_each'
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
# frozen_string_literal: true

RSpec.describe RuboCop::Cop::Performance::RedundantSplitRegexpArgument do
subject(:cop) { described_class.new }

it 'accepts methods other than split' do
expect_no_offenses("'a,b,c'.insert(2, 'a')")
end

it 'accepts when split does not receive a regexp' do
expect_no_offenses("'a,b,c'.split(',')")
end

it 'accepts when split does not receive a deterministic regexp' do
expect_no_offenses("'a,b,c'.split(/,+/)")
end

it 'registers an offense when the method is split and correctly removes escaping characters' do
expect_offense(<<~RUBY)
'a,b,c'.split(/\\./)
^^^^ Use string as argument instead of regexp.
RUBY

expect_correction(<<~RUBY)
'a,b,c'.split(".")
RUBY
end

it 'registers an offense when the method is split and corrects correctly special string chars' do
expect_offense(<<~RUBY)
"foo\\nbar\\nbaz\\n".split(/\\n/)
^^^^ Use string as argument instead of regexp.
RUBY

expect_correction(<<~RUBY)
"foo\\nbar\\nbaz\\n".split("\\n")
RUBY
end

it 'registers an offense when the method is split' do
expect_offense(<<~RUBY)
'a,b,c'.split(/,/)
^^^ Use string as argument instead of regexp.
RUBY

expect_correction(<<~RUBY)
'a,b,c'.split(",")
RUBY
end
end

0 comments on commit 1ae6d24

Please sign in to comment.