From 3c310b2e3dd7805b04f48507c65c2c0a856c2aa2 Mon Sep 17 00:00:00 2001
From: Alyssa Ross <hi@alyssa.is>
Date: Fri, 11 Nov 2016 20:08:26 +0000
Subject: [PATCH] Warn developers when uninstalling a dependency

Suggested in #1084.

Made the existing warning output entirely to STDERR, because
previously the first line went to STDERR and subsequent ones went
to STDOUT.
---
 Library/Homebrew/cmd/uninstall.rb       | 45 +++++++++++++++++--------
 Library/Homebrew/test/test_uninstall.rb | 41 ++++++++++++++++++++--
 Library/Homebrew/test/test_utils.rb     |  4 +++
 Library/Homebrew/utils.rb               |  8 +++++
 4 files changed, 81 insertions(+), 17 deletions(-)

diff --git a/Library/Homebrew/cmd/uninstall.rb b/Library/Homebrew/cmd/uninstall.rb
index d9e6a737780df..d172b02381bce 100644
--- a/Library/Homebrew/cmd/uninstall.rb
+++ b/Library/Homebrew/cmd/uninstall.rb
@@ -28,10 +28,8 @@ def uninstall
       ARGV.kegs.group_by(&:rack)
     end
 
-    if should_check_for_dependents?
-      all_kegs = kegs_by_rack.values.flatten(1)
-      return if check_for_dependents all_kegs
-    end
+    handle_unsatisfied_dependents(kegs_by_rack)
+    return if Homebrew.failed?
 
     kegs_by_rack.each do |rack, kegs|
       if ARGV.force?
@@ -78,28 +76,47 @@ def uninstall
     end
   end
 
-  def should_check_for_dependents?
-    # --ignore-dependencies, to be consistent with install
-    return false if ARGV.include?("--ignore-dependencies")
-    return false if ARGV.homebrew_developer?
-    true
+  def handle_unsatisfied_dependents(kegs_by_rack)
+    return if ARGV.include?("--ignore-dependencies")
+
+    all_kegs = kegs_by_rack.values.flatten(1)
+    check_for_dependents all_kegs
   end
 
   def check_for_dependents(kegs)
     return false unless result = Keg.find_some_installed_dependents(kegs)
 
-    requireds, dependents = result
+    if ARGV.homebrew_developer?
+      dependents_output_for_developers(*result)
+    else
+      dependents_output_for_nondevelopers(*result)
+    end
+
+    true
+  end
 
+  def dependents_output_for_developers(requireds, dependents)
+    msg = requireds.join(", ")
+    msg << (requireds.count == 1 ? " is" : " are")
+    msg << " required by #{dependents.join(", ")}, which "
+    msg << (dependents.count == 1 ? "is" : "are")
+    msg << " currently installed."
+    msg << "\nYou can silence this warning with "
+    msg << "`brew uninstall --ignore-dependencies "
+    msg << "#{requireds.map(&:name).join(" ")}`."
+    opoo msg
+  end
+
+  def dependents_output_for_nondevelopers(requireds, dependents)
     msg = "Refusing to uninstall #{requireds.join(", ")} because "
     msg << (requireds.count == 1 ? "it is" : "they are")
     msg << " required by #{dependents.join(", ")}, which "
     msg << (dependents.count == 1 ? "is" : "are")
     msg << " currently installed."
+    msg << "\nYou can override this and force removal with "
+    msg << "`brew uninstall --ignore-dependencies "
+    msg << "#{requireds.map(&:name).join(" ")}`."
     ofail msg
-    print "You can override this and force removal with "
-    puts "`brew uninstall --ignore-dependencies #{requireds.map(&:name).join(" ")}`."
-
-    true
   end
 
   def rm_pin(rack)
diff --git a/Library/Homebrew/test/test_uninstall.rb b/Library/Homebrew/test/test_uninstall.rb
index a7859b7ad9531..c9b3e0be3e3ca 100644
--- a/Library/Homebrew/test/test_uninstall.rb
+++ b/Library/Homebrew/test/test_uninstall.rb
@@ -2,20 +2,55 @@
 require "cmd/uninstall"
 
 class UninstallTests < Homebrew::TestCase
+  def setup
+    @dependency = formula("dependency") { url "f-1" }
+    @dependent = formula("dependent") do
+      url "f-1"
+      depends_on "dependency"
+    end
+
+    [@dependency, @dependent].each { |f| f.installed_prefix.mkpath }
+
+    tab = Tab.empty
+    tab.tabfile = @dependent.installed_prefix/Tab::FILENAME
+    tab.runtime_dependencies = [
+      { "full_name" => "dependency", "version" => "1" },
+    ]
+    tab.write
+
+    stub_formula_loader @dependency
+    stub_formula_loader @dependent
+  end
+
+  def teardown
+    Homebrew.failed = false
+    [@dependency, @dependent].each { |f| f.rack.rmtree }
+  end
+
+  def handle_unsatisfied_dependents
+    capture_stderr do
+      opts = { @dependency.rack => [Keg.new(@dependency.installed_prefix)] }
+      Homebrew.handle_unsatisfied_dependents(opts)
+    end
+  end
+
   def test_check_for_testball_f2s_when_developer
-    refute_predicate Homebrew, :should_check_for_dependents?
+    assert_match "Warning", handle_unsatisfied_dependents
+    refute_predicate Homebrew, :failed?
   end
 
   def test_check_for_dependents_when_not_developer
     run_as_not_developer do
-      assert_predicate Homebrew, :should_check_for_dependents?
+      assert_match "Error", handle_unsatisfied_dependents
+      assert_predicate Homebrew, :failed?
     end
   end
 
   def test_check_for_dependents_when_ignore_dependencies
     ARGV << "--ignore-dependencies"
     run_as_not_developer do
-      refute_predicate Homebrew, :should_check_for_dependents?
+      assert_empty handle_unsatisfied_dependents
+      refute_predicate Homebrew, :failed?
     end
   ensure
     ARGV.delete("--ignore-dependencies")
diff --git a/Library/Homebrew/test/test_utils.rb b/Library/Homebrew/test/test_utils.rb
index 11332e450ca22..7c0b6f78a1fc5 100644
--- a/Library/Homebrew/test/test_utils.rb
+++ b/Library/Homebrew/test/test_utils.rb
@@ -174,6 +174,10 @@ def test_gzip
     end
   end
 
+  def test_capture_stderr
+    assert_equal "test\n", capture_stderr { $stderr.puts "test" }
+  end
+
   def test_shell_profile
     ENV["SHELL"] = "/bin/sh"
     assert_equal "~/.bash_profile", Utils::Shell.shell_profile
diff --git a/Library/Homebrew/utils.rb b/Library/Homebrew/utils.rb
index fce03f888ff24..68557474defa7 100644
--- a/Library/Homebrew/utils.rb
+++ b/Library/Homebrew/utils.rb
@@ -376,6 +376,14 @@ def ignore_interrupts(opt = nil)
   trap("INT", std_trap)
 end
 
+def capture_stderr
+  old, $stderr = $stderr, StringIO.new
+  yield
+  $stderr.string
+ensure
+  $stderr = old
+end
+
 def nostdout
   if ARGV.verbose?
     yield