Merge #6769

6769: Fix `remove` when block method appears in gem name r=segiddins a=dduugg

Thanks so much for the contribution!
To make reviewing this PR a bit easier, please fill out answers to the following questions.

### What was the end-user problem that led to this PR?

#6768

### What was your diagnosis of the problem?

`remove_nested_blocks` considers any appearance of text matching a block method (`group source env install_if`) to be an occurrence of a nested block.

### What is your fix for the problem, implemented in this PR?

`remove_nested_blocks` should only reduce the scope of where a nested block method can occur.

### Why did you choose this fix out of the possible options?

Another approach would use word delimiters rather than `starts_with?` to find nested block methods, but this is probably fine.


Co-authored-by: Douglas Eichelberger <dduugg@gmail.com>

lib/bundler/injector.rb

@@ -123,7 +123,7 @@ def append_to(gemfile_path, new_gem_lines)
       end
     end
 
-    # evalutes a gemfile to remove the specified gem
+    # evaluates a gemfile to remove the specified gem
     # from it.
     def remove_deps(gemfile_path)
       initial_gemfile = IO.readlines(gemfile_path)
@@ -136,8 +136,8 @@ def remove_deps(gemfile_path)
 
       removed_deps = remove_gems_from_dependencies(builder, @deps, gemfile_path)
 
-      # abort the opertion if no gems were removed
-      # no need to operate on gemfile furthur
+      # abort the operation if no gems were removed
+      # no need to operate on gemfile further
       return [] if removed_deps.empty?
 
       cleaned_gemfile = remove_gems_from_gemfile(@deps, gemfile_path)
@@ -153,8 +153,8 @@ def remove_deps(gemfile_path)
 
     # @param [Dsl]      builder Dsl object of current Gemfile.
     # @param [Array]    gems Array of names of gems to be removed.
-    # @param [Pathname] path of the Gemfile
-    # @return [Array]   removed_deps Array of removed dependencies.
+    # @param [Pathname] gemfile_path Path of the Gemfile.
+    # @return [Array]   Array of removed dependencies.
     def remove_gems_from_dependencies(builder, gems, gemfile_path)
       removed_deps = []
 
@@ -206,7 +206,7 @@ def remove_nested_blocks(gemfile, block_name)
         nested_blocks -= 1
 
         gemfile.each_with_index do |line, index|
-          next unless !line.nil? && line.include?(block_name)
+          next unless !line.nil? && line.strip.start_with?(block_name)
           if gemfile[index + 1] =~ /^\s*end\s*$/
             gemfile[index] = nil
             gemfile[index + 1] = nil
@@ -222,7 +222,7 @@ def remove_nested_blocks(gemfile, block_name)
     # @param [Array] removed_deps      Array of removed dependencies.
     # @param [Array] initial_gemfile   Contents of original Gemfile before any operation.
     def cross_check_for_errors(gemfile_path, original_deps, removed_deps, initial_gemfile)
-      # evalute the new gemfile to look for any failure cases
+      # evaluate the new gemfile to look for any failure cases
       builder = Dsl.new
       builder.eval_gemfile(gemfile_path)
 

spec/commands/remove_spec.rb

@@ -140,6 +140,30 @@
       end
     end
 
+    context "when gem to be removed is outside block" do
+      it "does not modify group" do
+        gemfile <<-G
+          source "file://#{gem_repo1}"
+
+          gem "rack"
+          group :test do
+            gem "coffee-script-source"
+          end
+        G
+
+        bundle! "remove rack"
+
+        expect(out).to include("rack was removed.")
+        gemfile_should_be <<-G
+          source "file://#{gem_repo1}"
+
+          group :test do
+            gem "coffee-script-source"
+          end
+        G
+      end
+    end
+
     context "when an empty block is also present" do
       it "removes all empty blocks" do
         gemfile <<-G