[Fix rubocop#115] DeletePrefix and DeleteSuffix cops detects sub/sub!…

… String methods
fatkodima · May 24, 2020 · d1933cc · d1933cc
1 parent 6dc7ba7
commit d1933cc
Show file tree

Hide file tree

Showing 6 changed files with 226 additions and 14 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,10 @@
 
 ## master (unreleased)
 
+### New features
+
+* [#115](https://github.com/rubocop-hq/rubocop-performance/issues/115): Support `String#sub` and `String#sub!` methods for `Performance/DeletePrefix` and `Performance/DeleteSuffix` cops. ([@fatkodima][])
+
 ### Bug fixes
 
 * [#111](https://github.com/rubocop-hq/rubocop-performance/issues/111): Fix an error for `Performance/DeletePrefix` and `Performance/DeleteSuffix` cops when using autocorrection with RuboCop 0.81 or lower. ([@koic][])

diff --git a/lib/rubocop/cop/performance/delete_prefix.rb b/lib/rubocop/cop/performance/delete_prefix.rb
@@ -5,7 +5,7 @@ module Cop
     module Performance
       # In Ruby 2.5, `String#delete_prefix` has been added.
       #
-      # This cop identifies places where `gsub(/\Aprefix/, '')`
+      # This cop identifies places where `gsub(/\Aprefix/, '')` and `sub(/\Aprefix/, '')`
       # can be replaced by `delete_prefix('prefix')`.
       #
       # The `delete_prefix('prefix')` method is faster than
@@ -19,6 +19,11 @@ module Performance
       #   str.gsub(/^prefix/, '')
       #   str.gsub!(/^prefix/, '')
       #
+      #   str.sub(/\Aprefix/, '')
+      #   str.sub!(/\Aprefix/, '')
+      #   str.sub(/^prefix/, '')
+      #   str.sub!(/^prefix/, '')
+      #
       #   # good
       #   str.delete_prefix('prefix')
       #   str.delete_prefix!('prefix')
@@ -33,15 +38,17 @@ class DeletePrefix < Cop
 
         PREFERRED_METHODS = {
           gsub: :delete_prefix,
-          gsub!: :delete_prefix!
+          gsub!: :delete_prefix!,
+          sub: :delete_prefix,
+          sub!: :delete_prefix!
         }.freeze
 
-        def_node_matcher :gsub_method?, <<~PATTERN
-          (send $!nil? ${:gsub :gsub!} (regexp (str $#literal_at_start?) (regopt)) (str $_))
+        def_node_matcher :delete_prefix_candidate?, <<~PATTERN
+          (send $!nil? ${:gsub :gsub! :sub :sub!} (regexp (str $#literal_at_start?) (regopt)) (str $_))
         PATTERN
 
         def on_send(node)
-          gsub_method?(node) do |_, bad_method, _, replace_string|
+          delete_prefix_candidate?(node) do |_, bad_method, _, replace_string|
             return unless replace_string.blank?
 
             good_method = PREFERRED_METHODS[bad_method]
@@ -53,7 +60,7 @@ def on_send(node)
         end
 
         def autocorrect(node)
-          gsub_method?(node) do |receiver, bad_method, regexp_str, _|
+          delete_prefix_candidate?(node) do |receiver, bad_method, regexp_str, _|
             lambda do |corrector|
               good_method = PREFERRED_METHODS[bad_method]
               regexp_str = drop_start_metacharacter(regexp_str)

diff --git a/lib/rubocop/cop/performance/delete_suffix.rb b/lib/rubocop/cop/performance/delete_suffix.rb
@@ -5,7 +5,7 @@ module Cop
     module Performance
       # In Ruby 2.5, `String#delete_suffix` has been added.
       #
-      # This cop identifies places where `gsub(/suffix\z/, '')`
+      # This cop identifies places where `gsub(/suffix\z/, '')` and `sub(/suffix\z/, '')`
       # can be replaced by `delete_suffix('suffix')`.
       #
       # The `delete_suffix('suffix')` method is faster than
@@ -19,6 +19,11 @@ module Performance
       #   str.gsub(/suffix$/, '')
       #   str.gsub!(/suffix$/, '')
       #
+      #   str.sub(/suffix\z/, '')
+      #   str.sub!(/suffix\z/, '')
+      #   str.sub(/suffix$/, '')
+      #   str.sub!(/suffix$/, '')
+      #
       #   # good
       #   str.delete_suffix('suffix')
       #   str.delete_suffix!('suffix')
@@ -33,15 +38,17 @@ class DeleteSuffix < Cop
 
         PREFERRED_METHODS = {
           gsub: :delete_suffix,
-          gsub!: :delete_suffix!
+          gsub!: :delete_suffix!,
+          sub: :delete_suffix,
+          sub!: :delete_suffix!
         }.freeze
 
-        def_node_matcher :gsub_method?, <<~PATTERN
-          (send $!nil? ${:gsub :gsub!} (regexp (str $#literal_at_end?) (regopt)) (str $_))
+        def_node_matcher :delete_suffix_candidate?, <<~PATTERN
+          (send $!nil? ${:gsub :gsub! :sub :sub!} (regexp (str $#literal_at_end?) (regopt)) (str $_))
         PATTERN
 
         def on_send(node)
-          gsub_method?(node) do |_, bad_method, _, replace_string|
+          delete_suffix_candidate?(node) do |_, bad_method, _, replace_string|
             return unless replace_string.blank?
 
             good_method = PREFERRED_METHODS[bad_method]
@@ -53,7 +60,7 @@ def on_send(node)
         end
 
         def autocorrect(node)
-          gsub_method?(node) do |receiver, bad_method, regexp_str, _|
+          delete_suffix_candidate?(node) do |receiver, bad_method, regexp_str, _|
             lambda do |corrector|
               good_method = PREFERRED_METHODS[bad_method]
               regexp_str = drop_end_metacharacter(regexp_str)

diff --git a/manual/cops_performance.md b/manual/cops_performance.md
@@ -267,7 +267,7 @@ Enabled | Yes | Yes  | 1.6 | -
 
 In Ruby 2.5, `String#delete_prefix` has been added.
 
-This cop identifies places where `gsub(/\Aprefix/, '')`
+This cop identifies places where `gsub(/\Aprefix/, '')` and `sub(/\Aprefix/, '')`
 can be replaced by `delete_prefix('prefix')`.
 
 The `delete_prefix('prefix')` method is faster than
@@ -282,6 +282,11 @@ str.gsub!(/\Aprefix/, '')
 str.gsub(/^prefix/, '')
 str.gsub!(/^prefix/, '')
 
+str.sub(/\Aprefix/, '')
+str.sub!(/\Aprefix/, '')
+str.sub(/^prefix/, '')
+str.sub!(/^prefix/, '')
+
 # good
 str.delete_prefix('prefix')
 str.delete_prefix!('prefix')
@@ -299,7 +304,7 @@ Enabled | Yes | Yes  | 1.6 | -
 
 In Ruby 2.5, `String#delete_suffix` has been added.
 
-This cop identifies places where `gsub(/suffix\z/, '')`
+This cop identifies places where `gsub(/suffix\z/, '')` and `sub(/suffix\z/, '')`
 can be replaced by `delete_suffix('suffix')`.
 
 The `delete_suffix('suffix')` method is faster than
@@ -314,6 +319,11 @@ str.gsub!(/suffix\z/, '')
 str.gsub(/suffix$/, '')
 str.gsub!(/suffix$/, '')
 
+str.sub(/suffix\z/, '')
+str.sub!(/suffix\z/, '')
+str.sub(/suffix$/, '')
+str.sub!(/suffix$/, '')
+
 # good
 str.delete_suffix('suffix')
 str.delete_suffix!('suffix')

diff --git a/spec/rubocop/cop/performance/delete_prefix_spec.rb b/spec/rubocop/cop/performance/delete_prefix_spec.rb
@@ -15,6 +15,18 @@
         str.gsub!(/\\Aprefix/, '')
       RUBY
     end
+
+    it "does not register an offense when using `sub(/\Aprefix/, '')`" do
+      expect_no_offenses(<<~RUBY)
+        str.sub(/\\Aprefix/, '')
+      RUBY
+    end
+
+    it "does not register an offense when using `sub!(/\Aprefix/, '')`" do
+      expect_no_offenses(<<~RUBY)
+        str.sub!(/\\Aprefix/, '')
+      RUBY
+    end
   end
 
   context 'TargetRubyVersion >= 2.5', :ruby25 do
@@ -40,6 +52,28 @@
           str.delete_prefix!('prefix')
         RUBY
       end
+
+      it "registers an offense and corrects when `sub(/\Aprefix/, '')`" do
+        expect_offense(<<~RUBY)
+          str.sub(/\\Aprefix/, '')
+              ^^^ Use `delete_prefix` instead of `sub`.
+        RUBY
+
+        expect_correction(<<~RUBY)
+          str.delete_prefix('prefix')
+        RUBY
+      end
+
+      it "registers an offense and corrects when `sub!(/\Aprefix/, '')`" do
+        expect_offense(<<~RUBY)
+          str.sub!(/\\Aprefix/, '')
+              ^^^^ Use `delete_prefix!` instead of `sub!`.
+        RUBY
+
+        expect_correction(<<~RUBY)
+          str.delete_prefix!('prefix')
+        RUBY
+      end
     end
 
     context 'when using `^` as starting pattern' do
@@ -64,6 +98,28 @@
           str.delete_prefix!('prefix')
         RUBY
       end
+
+      it 'registers an offense and corrects when using `sub`' do
+        expect_offense(<<~RUBY)
+          str.sub(/^prefix/, '')
+              ^^^ Use `delete_prefix` instead of `sub`.
+        RUBY
+
+        expect_correction(<<~RUBY)
+          str.delete_prefix('prefix')
+        RUBY
+      end
+
+      it 'registers an offense and corrects when using `sub!`' do
+        expect_offense(<<~RUBY)
+          str.sub!(/^prefix/, '')
+              ^^^^ Use `delete_prefix!` instead of `sub!`.
+        RUBY
+
+        expect_correction(<<~RUBY)
+          str.delete_prefix!('prefix')
+        RUBY
+      end
     end
 
     context 'when using non-starting pattern' do
@@ -78,6 +134,18 @@
           str.gsub!(/pattern/, '')
         RUBY
       end
+
+      it 'does not register an offense when using `sub`' do
+        expect_no_offenses(<<~RUBY)
+          str.sub(/pattern/, '')
+        RUBY
+      end
+
+      it 'does not register an offense when using `sub!`' do
+        expect_no_offenses(<<~RUBY)
+          str.sub!(/pattern/, '')
+        RUBY
+      end
     end
 
     context 'with starting pattern `\A` and ending pattern `\z`' do
@@ -92,6 +160,18 @@
           str.gsub!(/\\Aprefix\\z/, '')
         RUBY
       end
+
+      it 'does not register an offense and corrects when using `sub`' do
+        expect_no_offenses(<<~RUBY)
+          str.sub(/\\Aprefix\\z/, '')
+        RUBY
+      end
+
+      it 'does not register an offense and corrects when using `sub!`' do
+        expect_no_offenses(<<~RUBY)
+          str.sub!(/\\Aprefix\\z/, '')
+        RUBY
+      end
     end
 
     context 'when using a non-blank string as replacement string' do
@@ -106,6 +186,18 @@
           str.gsub!(/\\Aprefix/, 'foo')
         RUBY
       end
+
+      it 'does not register an offense and corrects when using `sub`' do
+        expect_no_offenses(<<~RUBY)
+          str.sub(/\\Aprefix/, 'foo')
+        RUBY
+      end
+
+      it 'does not register an offense and corrects when using `sub!`' do
+        expect_no_offenses(<<~RUBY)
+          str.sub!(/\\Aprefix/, 'foo')
+        RUBY
+      end
     end
 
     it 'does not register an offense when using `delete_prefix`' do