Merge pull request #4045 from jlgoedert/improve_bundle_remove_comment…

…_handling

Prevent remove command from deleting gemfile lines that are comments

(cherry picked from commit 2d0fa4a)

bundler/lib/bundler/injector.rb

@@ -179,11 +179,11 @@ def remove_gems_from_dependencies(builder, gems, gemfile_path)
     # @param [Pathname] gemfile_path The Gemfile from which to remove dependencies.
     def remove_gems_from_gemfile(gems, gemfile_path)
       patterns = /gem\s+(['"])#{Regexp.union(gems)}\1|gem\s*\((['"])#{Regexp.union(gems)}\2\)/
-
       new_gemfile = []
       multiline_removal = false
       IO.readlines(gemfile_path).each do |line|
-        if line.match(patterns)
+        match_data = line.match(patterns)
+        if match_data && is_not_within_comment?(line, match_data)
           multiline_removal = line.rstrip.end_with?(",")
           # skip lines which match the regex
           next
@@ -207,6 +207,13 @@ def remove_gems_from_gemfile(gems, gemfile_path)
       new_gemfile.join.chomp
     end
 
+    # @param [String] line          Individual line of gemfile content.
+    # @param [MatchData] match_data Data about Regex match.
+    def is_not_within_comment?(line, match_data)
+      match_start_index = match_data.offset(0).first
+      !line[0..match_start_index].include?("#")
+    end
+
     # @param [Array] gemfile       Array of gemfile contents.
     # @param [String] block_name   Name of block name to look for.
     def remove_nested_blocks(gemfile, block_name)

bundler/spec/commands/remove_spec.rb

@@ -83,7 +83,7 @@
     end
   end
 
-  describe "remove mutiple gems from gemfile" do
+  describe "remove multiple gems from gemfile" do
     context "when all gems are present in gemfile" do
       it "shows success fir all removed gems" do
         gemfile <<-G
@@ -210,7 +210,7 @@
       end
     end
 
-    context "when the gem belongs to mutiple groups" do
+    context "when the gem belongs to multiple groups" do
       it "removes the groups" do
         gemfile <<-G
           source "#{file_uri_for(gem_repo1)}"
@@ -616,4 +616,84 @@
       expect(out).to include("foo could not be removed.")
     end
   end
+
+  describe "with comments that mention gems" do
+    context "when comment is a separate line comment" do
+      it "does not remove the line comment" do
+        gemfile <<-G
+          source "#{file_uri_for(gem_repo1)}"
+
+          # gem "rack" might be used in the future
+          gem "rack"
+        G
+
+        bundle "remove rack"
+
+        expect(out).to include("rack was removed.")
+        gemfile_should_be <<-G
+          source "#{file_uri_for(gem_repo1)}"
+
+          # gem "rack" might be used in the future
+        G
+      end
+    end
+
+    context "when gem specified for removal has an inline comment" do
+      it "removes the inline comment" do
+        gemfile <<-G
+          source "#{file_uri_for(gem_repo1)}"
+
+          gem "rack" # this can be removed
+        G
+
+        bundle "remove rack"
+
+        expect(out).to include("rack was removed.")
+        gemfile_should_be <<-G
+          source "#{file_uri_for(gem_repo1)}"
+        G
+      end
+    end
+
+    context "when gem specified for removal is mentioned in other gem's comment" do
+      it "does not remove other gem" do
+        gemfile <<-G
+          source "#{file_uri_for(gem_repo1)}"
+          gem "puma" # implements interface provided by gem "rack"
+
+          gem "rack"
+        G
+
+        bundle "remove rack"
+
+        expect(out).to_not include("puma was removed.")
+        expect(out).to include("rack was removed.")
+        gemfile_should_be <<-G
+          source "#{file_uri_for(gem_repo1)}"
+          gem "puma" # implements interface provided by gem "rack"
+        G
+      end
+    end
+
+    context "when gem specified for removal has a comment that mentions other gem" do
+      it "does not remove other gem" do
+        gemfile <<-G
+          source "#{file_uri_for(gem_repo1)}"
+          gem "puma" # implements interface provided by gem "rack"
+
+          gem "rack"
+        G
+
+        bundle "remove puma"
+
+        expect(out).to include("puma was removed.")
+        expect(out).to_not include("rack was removed.")
+        gemfile_should_be <<-G
+          source "#{file_uri_for(gem_repo1)}"
+
+          gem "rack"
+        G
+      end
+    end
+  end
 end