doctor: Move cask doctor checks to brew diagnostics

Library/Homebrew/cask/cmd/doctor.rb

@@ -2,12 +2,11 @@
 
 require "system_config"
 require "cask/checkable"
+require "diagnostic"
 
 module Cask
   class Cmd
     class Doctor < AbstractCommand
-      include Checkable
-
       def initialize(*)
         super
         return if args.empty?
@@ -15,219 +14,24 @@ module Cask
         raise ArgumentError, "#{self.class.command_name} does not take arguments."
       end
 
-      def success?
-        !(errors? || warnings?)
-      end
-
       def summary_header
         "Cask's Doctor Checkup"
       end
 
       def run
-        check_software_versions
+        success = true
-        check_xattr
-        check_quarantine_support
-        check_install_location
-        check_staging_location
-        check_taps
-        check_load_path
-        check_environment_variables
 
-        puts summary unless success?
+        checks = Homebrew::Diagnostic::Checks.new true
-        raise CaskError, "There are some problems with your setup." unless success?
+        checks.cask_checks.each do |check|
-      end
+          out = checks.send(check)
 
-      def check_software_versions
+          if out.present?
-        ohai "Homebrew Version", HOMEBREW_VERSION
+            success = false
-        ohai "macOS", MacOS.full_version
+            puts out
-        ohai "SIP", self.class.check_sip
-        ohai "Java", SystemConfig.describe_java
-      end
-
-      # This could be done by calling into Homebrew, but the situation
-      # where `brew doctor` is needed is precisely the situation where such
-      # things are less dependable.
-      def check_install_location
-        ohai "Homebrew Cask Install Location"
-
-        locations = Dir.glob(HOMEBREW_CELLAR.join("brew-cask", "*")).reverse
-        if locations.empty?
-          puts self.class.none_string
-        else
-          locations.map do |l|
-            add_error "Legacy install at #{l}. Run `brew uninstall --force brew-cask`."
-            puts l
-          end
-        end
-      end
-
-      def check_staging_location
-        ohai "Homebrew Cask Staging Location"
-
-        path = Caskroom.path
-
-        if path.exist? && !path.writable?
-          add_error "The staging path #{user_tilde(path.to_s)} is not writable by the current user."
-          add_error "To fix, run \'sudo chown -R $(whoami):staff #{user_tilde(path.to_s)}'"
-        end
-
-        puts user_tilde(path.to_s)
-      end
-
-      def check_taps
-        default_tap = Tap.default_cask_tap
-        alt_taps = Tap.select { |t| t.cask_dir.exist? && t != default_tap }
-
-        ohai "Homebrew Cask Taps:"
-        [default_tap, *alt_taps].each do |tap|
-          if tap.path.blank?
-            puts none_string
-          else
-            puts "#{tap.path} (#{cask_count_for_tap(tap)})"
-          end
-        end
-      end
-
-      def check_load_path
-        ohai "Contents of $LOAD_PATH"
-        paths = $LOAD_PATH.map(&method(:user_tilde))
-
-        if paths.empty?
-          puts none_string
-          add_error "$LOAD_PATH is empty"
-        else
-          puts paths
           end
         end
 
-      def check_environment_variables
+        raise CaskError, "There are some problems with your setup." unless success
-        ohai "Environment Variables"
-
-        environment_variables = %w[
-          RUBYLIB
-          RUBYOPT
-          RUBYPATH
-          RBENV_VERSION
-          CHRUBY_VERSION
-          GEM_HOME
-          GEM_PATH
-          BUNDLE_PATH
-          PATH
-          SHELL
-          HOMEBREW_CASK_OPTS
-        ]
-
-        locale_variables = ENV.keys.grep(/^(?:LC_\S+|LANG|LANGUAGE)\Z/).sort
-
-        (locale_variables + environment_variables).sort.each(&method(:render_env_var))
-      end
-
-      def check_xattr
-        ohai "xattr issues"
-        result = system_command "/usr/bin/xattr"
-
-        if result.status.success?
-          puts none_string
-        elsif result.stderr.include? "ImportError: No module named pkg_resources"
-          result = system_command "/usr/bin/python", "--version"
-
-          if result.stdout.include? "Python 2.7"
-            add_error "Your Python installation has a broken version of setuptools."
-            add_error "To fix, reinstall macOS or run 'sudo /usr/bin/python -m pip install -I setuptools'."
-          else
-            add_error "The system Python version is wrong."
-            add_error "To fix, run 'defaults write com.apple.versioner.python Version 2.7'."
-          end
-        elsif result.stderr.include? "pkg_resources.DistributionNotFound"
-          add_error "Your Python installation is unable to find xattr."
-        else
-          add_error "unknown xattr error: #{result.stderr.split("\n").last}"
-        end
-      end
-
-      def check_quarantine_support
-        ohai "Gatekeeper support"
-
-        case Quarantine.check_quarantine_support
-        when :quarantine_available
-          puts "Enabled"
-        when :xattr_broken
-          add_error "There's not a working version of xattr."
-        when :no_swift
-          add_error "Swift is not available on this system."
-        when :no_quarantine
-          add_error "This feature requires the macOS 10.10 SDK or higher."
-        else
-          onoe "Unknown support status"
-        end
-      end
-
-      def user_tilde(path)
-        self.class.user_tilde(path)
-      end
-
-      def cask_count_for_tap(tap)
-        self.class.cask_count_for_tap(tap)
-      end
-
-      def none_string
-        self.class.none_string
-      end
-
-      def render_env_var(var)
-        self.class.render_env_var(var)
-      end
-
-      def self.check_sip
-        csrutil = "/usr/bin/csrutil"
-        return "N/A" unless File.executable?(csrutil)
-
-        Open3.capture2(csrutil, "status")
-             .first
-             .gsub("This is an unsupported configuration, likely to break in " \
-                   "the future and leave your machine in an unknown state.", "")
-             .gsub("System Integrity Protection status: ", "")
-             .delete("\t\.")
-             .capitalize
-             .strip
-      end
-
-      def self.locale_variables
-        ENV.keys.grep(/^(?:LC_\S+|LANG|LANGUAGE)\Z/).sort
-      end
-
-      def self.none_string
-        "<NONE>"
-      end
-
-      def self.error_string(string = "Error")
-        Formatter.error("(#{string})")
-      end
-
-      def self.alt_taps
-        Tap.select { |t| t.cask_dir.exist? && t != Tap.default_cask_tap }
-      end
-
-      def self.cask_count_for_tap(tap)
-        cask_count = begin
-          tap.cask_files.count
-        rescue
-          add_error "Unable to read from Tap: #{tap.path}"
-          0
-        end
-
-        "#{cask_count} #{"cask".pluralize(cask_count)}"
-      end
-
-      def self.render_env_var(var)
-        return unless ENV.key?(var)
-
-        var = %Q(#{var}="#{ENV[var]}")
-        puts user_tilde(var)
-      end
-
-      def self.user_tilde(path)
-        path.gsub(ENV["HOME"], "~")
       end
 
       def self.help

Library/Homebrew/cmd/doctor.rb

@@ -2,6 +2,7 @@
 
 require "diagnostic"
 require "cli/parser"
+require "cask/caskroom"
 
 module Homebrew
   module_function
@@ -32,11 +33,11 @@ module Homebrew
 
     inject_dump_stats!(Diagnostic::Checks, /^check_*/) if args.audit_debug?
 
-    checks = Diagnostic::Checks.new
+    checks = Diagnostic::Checks.new args.verbose?
 
     if args.list_checks?
       puts checks.all.sort
-      exit
+      return
     end
 
     if args.no_named?
@@ -45,6 +46,7 @@ module Homebrew
         check_missing_deps
       ]
       methods = (checks.all.sort - slow_checks) + slow_checks
+      methods -= checks.cask_checks if Cask::Caskroom.casks.blank?
     else
       methods = args.named
     end

Library/Homebrew/diagnostic.rb

@@ -7,6 +7,9 @@ require "formulary"
 require "version"
 require "development_tools"
 require "utils/shell"
+require "system_config"
+require "cask/caskroom"
+require "cask/quarantine"
 
 module Homebrew
   module Diagnostic
@@ -61,6 +64,10 @@ module Homebrew
     end
 
     class Checks
+      def initialize(verbose = true)
+        @verbose = verbose
+      end
+
       ############# HELPERS
       # Finds files in `HOMEBREW_PREFIX` *and* /usr/local.
       # Specify paths relative to a prefix, e.g. "include/foo.h".
@@ -75,6 +82,18 @@ module Homebrew
         list.reduce(string.dup) { |acc, elem| acc << "  #{elem}\n" }
             .freeze
       end
+
+      def user_tilde(path)
+        path.gsub(ENV["HOME"], "~")
+      end
+
+      def none_string
+        "<NONE>"
+      end
+
+      def add_info(*args)
+        ohai(*args) if @verbose
+      end
       ############# END HELPERS
 
       def fatal_preinstall_checks
@@ -854,9 +873,160 @@ module Homebrew
         EOS
       end
 
+      def check_cask_software_versions
+        add_info "Homebrew Version", HOMEBREW_VERSION
+        add_info "macOS", MacOS.full_version
+        add_info "SIP", begin
+          csrutil = "/usr/bin/csrutil"
+          if File.executable?(csrutil)
+            Open3.capture2(csrutil, "status")
+                 .first
+                 .gsub("This is an unsupported configuration, likely to break in " \
+                   "the future and leave your machine in an unknown state.", "")
+                 .gsub("System Integrity Protection status: ", "")
+                 .delete("\t\.")
+                 .capitalize
+                 .strip
+          else
+            "N/A"
+          end
+        end
+        add_info "Java", SystemConfig.describe_java
+
+        nil
+      end
+
+      # This could be done by calling into Homebrew, but the situation
+      # where `brew doctor` is needed is precisely the situation where such
+      # things are less dependable.
+      def check_cask_install_location
+        locations = Dir.glob(HOMEBREW_CELLAR.join("brew-cask", "*")).reverse
+        return if locations.empty?
+
+        locations.map do |l|
+          "Legacy install at #{l}. Run `brew uninstall --force brew-cask`."
+        end.join "\n"
+      end
+
+      def check_cask_staging_location
+        path = Cask::Caskroom.path
+
+        add_info "Homebrew Cask Staging Location", user_tilde(path.to_s)
+
+        return unless path.exist? && !path.writable?
+
+        <<~EOS
+          The staging path #{user_tilde(path.to_s)} is not writable by the current user.
+          To fix, run \'sudo chown -R $(whoami):staff #{user_tilde(path.to_s)}'
+        EOS
+      end
+
+      def check_cask_taps
+        default_tap = Tap.default_cask_tap
+        alt_taps = Tap.select { |t| t.cask_dir.exist? && t != default_tap }
+
+        error_tap_paths = []
+
+        add_info "Homebrew Cask Taps:", ([default_tap, *alt_taps].map do |tap|
+          if tap.path.blank?
+            none_string
+          else
+            cask_count = begin
+                tap.cask_files.count
+              rescue
+                error_tap_paths << tap.path
+                0
+              end
+
+            "#{tap.path} (#{cask_count} #{"cask".pluralize(cask_count)})"
+          end
+        end)
+
+        taps = "tap".pluralize error_tap_paths.count
+        "Unable to read from cask #{taps}: #{error_tap_paths.to_sentence}" if error_tap_paths.present?
+      end
+
+      def check_cask_load_path
+        paths = $LOAD_PATH.map(&method(:user_tilde))
+
+        add_info "$LOAD_PATHS", paths.presence || none_string
+
+        "$LOAD_PATH is empty" if paths.blank?
+      end
+
+      def check_cask_environment_variables
+        environment_variables = %w[
+          RUBYLIB
+          RUBYOPT
+          RUBYPATH
+          RBENV_VERSION
+          CHRUBY_VERSION
+          GEM_HOME
+          GEM_PATH
+          BUNDLE_PATH
+          PATH
+          SHELL
+          HOMEBREW_CASK_OPTS
+        ]
+
+        locale_variables = ENV.keys.grep(/^(?:LC_\S+|LANG|LANGUAGE)\Z/).sort
+
+        add_info "Cask Environment Variables:", ((locale_variables + environment_variables).sort.each do |var|
+          next unless ENV.key?(var)
+
+          var = %Q(#{var}="#{ENV[var]}")
+          user_tilde(var)
+        end)
+      end
+
+      def check_cask_xattr
+        result = system_command "/usr/bin/xattr"
+
+        return if result.status.success?
+
+        if result.stderr.include? "ImportError: No module named pkg_resources"
+          result = system_command "/usr/bin/python", "--version"
+
+          if result.stdout.include? "Python 2.7"
+            <<~EOS
+              Your Python installation has a broken version of setuptools.
+              To fix, reinstall macOS or run 'sudo /usr/bin/python -m pip install -I setuptools'.
+            EOS
+          else
+            <<~EOS
+              The system Python version is wrong.
+              To fix, run 'defaults write com.apple.versioner.python Version 2.7'.
+            EOS
+          end
+        elsif result.stderr.include? "pkg_resources.DistributionNotFound"
+          "Your Python installation is unable to find xattr."
+        else
+          "unknown xattr error: #{result.stderr.split("\n").last}"
+        end
+      end
+
+      def check_cask_quarantine_support
+        case Cask::Quarantine.check_quarantine_support
+        when :quarantine_available
+          nil
+        when :xattr_broken
+          "There's not a working version of xattr."
+        when :no_swift
+          "Swift is not available on this system."
+        when :no_quarantine
+          "This feature requires the macOS 10.10 SDK or higher."
+        else
+          "Unknown support status"
+        end
+      end
+
       def all
         methods.map(&:to_s).grep(/^check_/)
       end
+
+      def cask_checks
+        all.grep(/^check_cask_/)
+      end
     end
   end
 end