Merge pull request #190 from mfbmina/regexp-in-split

Cop for deterministic regexp on String#split

CHANGELOG.md

@@ -26,6 +26,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][])

config/default.yml

@@ -221,6 +221,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'

docs/modules/ROOT/pages/cops.adoc

@@ -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]

docs/modules/ROOT/pages/cops_performance.adoc

@@ -1310,6 +1310,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
 
 |===

lib/rubocop/cop/performance/redundant_split_regexp_argument.rb

@@ -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

lib/rubocop/cop/performance_cops.rb

@@ -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'

spec/rubocop/cop/performance/redundant_split_regexp_argument_spec.rb

@@ -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