Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cop for deterministic regexp on String#split #190

Merged
merged 1 commit into from
Jan 22, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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.10
| -
|===

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