Merge pull request #927 from MikeMcQuaid/dev-cmd-rubocop

Fix Library/Homebrew/dev-cmd RuboCop warnings

9 files changed, 114 insertions, 116 deletions

diff --git a/Library/Homebrew/dev-cmd/aspell-dictionaries.rb b/Library/Homebrew/dev-cmd/aspell-dictionaries.rb
index 16ec91835..cbb88ffe5 100644
--- a/Library/Homebrew/dev-cmd/aspell-dictionaries.rb
+++ b/Library/Homebrew/dev-cmd/aspell-dictionaries.rb
@@ -14,8 +14,8 @@ module Homebrew
 
     open("#{dict_url}/0index.html") do |content|
       content.each_line do |line|
-        break if %r{^</table} === line
-        next unless /^<tr><td><a/ === line
+        break if %r{^</table} =~ line
+        next unless /^<tr><td><a/ =~ line
 
         fields = line.split('"')
         lang = fields[1]
diff --git a/Library/Homebrew/dev-cmd/audit.rb b/Library/Homebrew/dev-cmd/audit.rb
index 6d1fa055f..511610a9a 100644
--- a/Library/Homebrew/dev-cmd/audit.rb
+++ b/Library/Homebrew/dev-cmd/audit.rb
@@ -101,20 +101,20 @@ class FormulaText
     @text.split("\n__END__").first
   end
 
-  def has_DATA?
+  def data?
     /^[^#]*\bDATA\b/ =~ @text
   end
 
-  def has_END?
+  def end?
     /^__END__$/ =~ @text
   end
 
-  def has_trailing_newline?
+  def trailing_newline?
     /\Z\n/ =~ @text
   end
 
-  def =~(regex)
-    regex =~ @text
+  def =~(other)
+    other =~ @text
   end
 
   def include?(s)
@@ -152,15 +152,15 @@ class FormulaAuditor
     smake
     sphinx-doc
     swig
-  ]
+  ].freeze
 
   FILEUTILS_METHODS = FileUtils.singleton_methods(false).map { |m| Regexp.escape(m) }.join "|"
 
   def initialize(formula, options = {})
     @formula = formula
-    @new_formula = !!options[:new_formula]
-    @strict = !!options[:strict]
-    @online = !!options[:online]
+    @new_formula = options[:new_formula]
+    @strict = options[:strict]
+    @online = options[:online]
     # Accept precomputed style offense results, for efficiency
     @style_offenses = options[:style_offenses]
     @problems = []
@@ -214,7 +214,7 @@ class FormulaAuditor
       previous_before = previous_pair[0]
       previous_after = previous_pair[1]
     end
-    offset = (previous_after && previous_after[0] && previous_after[0] >= 1) ? previous_after[0] - 1 : 0
+    offset = previous_after && previous_after[0] && previous_after[0] >= 1 ? previous_after[0] - 1 : 0
     present = component_list.map do |regex, name|
       lineno = if reverse
         text.reverse_line_number regex
@@ -229,7 +229,7 @@ class FormulaAuditor
       if reverse
         # scan in the forward direction from the offset
         audit_components(false, [c1, c2]) if c1[0] > c2[0] # at least one more offense
-      elsif c1[0] > c2[0] && (offset == 0 || previous_pair.nil? || (c1[0] + offset) != previous_before[0] || (c2[0] + offset) != previous_after[0])
+      elsif c1[0] > c2[0] && (offset.zero? || previous_pair.nil? || (c1[0] + offset) != previous_before[0] || (c2[0] + offset) != previous_after[0])
         component_problem c1, c2, offset
         no_problem = false
       end
@@ -251,11 +251,11 @@ class FormulaAuditor
                      actual_mode & 0777, wanted_mode & 0777, formula.path)
     end
 
-    if text.has_DATA? && !text.has_END?
+    if text.data? && !text.end?
       problem "'DATA' was found, but no '__END__'"
     end
 
-    if text.has_END? && !text.has_DATA?
+    if text.end? && !text.data?
       problem "'__END__' was found, but 'DATA' is not used"
     end
 
@@ -263,7 +263,7 @@ class FormulaAuditor
       problem "'inreplace ... do' was used for a single substitution (use the non-block form instead)."
     end
 
-    unless text.has_trailing_newline?
+    unless text.trailing_newline?
       problem "File should end with a newline"
     end
 
@@ -328,13 +328,13 @@ class FormulaAuditor
       return
     end
 
-    @@local_official_taps_name_map ||= Tap.select(&:official?).flat_map(&:formula_names).
-      reduce(Hash.new) do |name_map, tap_formula_full_name|
-        tap_formula_name = tap_formula_full_name.split("/").last
-        name_map[tap_formula_name] ||= []
-        name_map[tap_formula_name] << tap_formula_full_name
-        name_map
-      end
+    @@local_official_taps_name_map ||= Tap.select(&:official?).flat_map(&:formula_names)
+                                          .each_with_object({}) do |tap_formula_full_name, name_map|
+      tap_formula_name = tap_formula_full_name.split("/").last
+      name_map[tap_formula_name] ||= []
+      name_map[tap_formula_name] << tap_formula_full_name
+      name_map
+    end
 
     same_name_tap_formulae = @@local_official_taps_name_map[name] || []
 
@@ -396,14 +396,6 @@ class FormulaAuditor
         end
 
         case dep.name
-        when *BUILD_TIME_DEPS
-          next if dep.build? || dep.run?
-          problem <<-EOS.undent
-            #{dep} dependency should be
-              depends_on "#{dep}" => :build
-            Or if it is indeed a runtime dependency
-              depends_on "#{dep}" => :run
-          EOS
         when "git"
           problem "Don't use git as a dependency"
         when "mercurial"
@@ -420,6 +412,9 @@ class FormulaAuditor
           EOS
         when "open-mpi", "mpich"
           problem <<-EOS.undent
+        when *BUILD_TIME_DEPS
+          next if dep.build? || dep.run?
+          problem <<-EOS.undent
             There are multiple conflicting ways to install MPI. Use an MPIRequirement:
               depends_on :mpi => [<lang list>]
             Where <lang list> is a comma delimited list that can include:
@@ -452,10 +447,9 @@ class FormulaAuditor
         problem "Options should begin with with/without. Migrate '--#{o.name}' with `deprecated_option`."
       end
 
-      if o.name =~ /^with(out)?-(?:checks?|tests)$/
-        unless formula.deps.any? { |d| d.name == "check" && (d.optional? || d.recommended?) }
-          problem "Use '--with#{$1}-test' instead of '--#{o.name}'. Migrate '--#{o.name}' with `deprecated_option`."
-        end
+      next unless o.name =~ /^with(out)?-(?:checks?|tests)$/
+      unless formula.deps.any? { |d| d.name == "check" && (d.optional? || d.recommended?) }
+        problem "Use '--with#{$1}-test' instead of '--#{o.name}'. Migrate '--#{o.name}' with `deprecated_option`."
       end
     end
   end
@@ -667,15 +661,14 @@ class FormulaAuditor
 
     attributes.each do |attribute|
       attributes_for_version = attributes_map[attribute][formula.version]
-      if !attributes_for_version.empty?
-        if formula.send(attribute) < attributes_for_version.max
-          problem "#{attribute} should not decrease"
-        end
+      next if attributes_for_version.empty?
+      if formula.send(attribute) < attributes_for_version.max
+        problem "#{attribute} should not decrease"
       end
     end
 
     revision_map = attributes_map[:revision]
-    if formula.revision != 0
+    if formula.revision.nonzero?
       if formula.stable
         if revision_map[formula.stable.version].empty? # check stable spec
           problem "'revision #{formula.revision}' should be removed"
@@ -772,7 +765,7 @@ class FormulaAuditor
 
     # FileUtils is included in Formula
     # encfs modifies a file with this name, so check for some leading characters
-    if line =~ /[^'"\/]FileUtils\.(\w+)/
+    if line =~ %r{[^'"/]FileUtils\.(\w+)}
       problem "Don't need 'FileUtils.' before #{$1}."
     end
 
@@ -1038,7 +1031,7 @@ class FormulaAuditor
   end
 
   def quote_dep(dep)
-    Symbol === dep ? dep.inspect : "'#{dep}'"
+    dep.is_a?(Symbol) ? dep.inspect : "'#{dep}'"
   end
 
   def audit_check_output(output)
diff --git a/Library/Homebrew/dev-cmd/bottle.rb b/Library/Homebrew/dev-cmd/bottle.rb
index 3a8ebefe9..ffdfd088c 100644
--- a/Library/Homebrew/dev-cmd/bottle.rb
+++ b/Library/Homebrew/dev-cmd/bottle.rb
@@ -13,7 +13,7 @@ require "utils/inreplace"
 require "erb"
 require "extend/pathname"
 
-BOTTLE_ERB = <<-EOS
+BOTTLE_ERB = <<-EOS.freeze
   bottle do
     <% if !root_url.start_with?(BottleSpecification::DEFAULT_DOMAIN) %>
     root_url "<%= root_url %>"
@@ -89,15 +89,14 @@ module Homebrew
         end
       end
 
-      if ARGV.verbose? && !text_matches.empty?
-        print_filename string, file
-        text_matches.first(MAXIMUM_STRING_MATCHES).each do |match, offset|
-          puts " #{Tty.gray}-->#{Tty.reset} match '#{match}' at offset #{Tty.em}0x#{offset}#{Tty.reset}"
-        end
+      next unless ARGV.verbose? && !text_matches.empty?
+      print_filename string, file
+      text_matches.first(MAXIMUM_STRING_MATCHES).each do |match, offset|
+        puts " #{Tty.gray}-->#{Tty.reset} match '#{match}' at offset #{Tty.em}0x#{offset}#{Tty.reset}"
+      end
 
-        if text_matches.size > MAXIMUM_STRING_MATCHES
-          puts "Only the first #{MAXIMUM_STRING_MATCHES} matches were output"
-        end
+      if text_matches.size > MAXIMUM_STRING_MATCHES
+        puts "Only the first #{MAXIMUM_STRING_MATCHES} matches were output"
       end
     end
 
@@ -107,10 +106,9 @@ module Homebrew
   def keg_contain_absolute_symlink_starting_with?(string, keg)
     absolute_symlinks_start_with_string = []
     keg.find do |pn|
-      if pn.symlink? && (link = pn.readlink).absolute?
-        if link.to_s.start_with?(string)
-          absolute_symlinks_start_with_string << pn
-        end
+      next unless pn.symlink? && (link = pn.readlink).absolute?
+      if link.to_s.start_with?(string)
+        absolute_symlinks_start_with_string << pn
       end
     end
 
@@ -146,7 +144,7 @@ module Homebrew
       return
     end
 
-    unless Utils::Bottles::built_as? f
+    unless Utils::Bottles.built_as? f
       return ofail "Formula not installed with '--build-bottle': #{f.full_name}"
     end
 
@@ -289,6 +287,9 @@ module Homebrew
     bottle.sha256 sha256 => Utils::Bottles.tag
 
     old_spec = f.bottle_specification
+    p root_url
+    p old_spec.root_url(root_url)
+    p bottle.root_url(root_url)
     if ARGV.include?("--keep-old") && !old_spec.checksums.empty?
       mismatches = [:root_url, :prefix, :cellar, :rebuild].select do |key|
         old_spec.send(key) != bottle.send(key)
@@ -332,7 +333,7 @@ module Homebrew
                 "filename" => filename.to_s,
                 "sha256" => sha256,
               },
-            }
+            },
           },
           "bintray" => {
             "package" => Utils::Bottles::Bintray.package(f.name),
@@ -372,7 +373,7 @@ module Homebrew
       output = bottle_output bottle
 
       if write
-        path = Pathname.new("#{HOMEBREW_REPOSITORY/bottle_hash["formula"]["path"]}")
+        path = Pathname.new((HOMEBREW_REPOSITORY/bottle_hash["formula"]["path"]).to_s)
         update_or_add = nil
 
         Utils::Inreplace.inreplace(path) do |s|
@@ -391,7 +392,7 @@ module Homebrew
                 old_value = old_value_original.to_s.delete ":'\""
                 tag = tag.to_s.delete ":"
 
-                if !tag.empty?
+                unless tag.empty?
                   if !bottle_hash["bottle"]["tags"][tag].to_s.empty?
                     mismatches << "#{key} => #{tag}"
                   else
@@ -403,11 +404,10 @@ module Homebrew
                 value_original = bottle_hash["bottle"][key]
                 value = value_original.to_s
                 next if key == "cellar" && old_value == "any" && value == "any_skip_relocation"
-                if old_value.empty? || value != old_value
-                  old_value = old_value_original.inspect
-                  value = value_original.inspect
-                  mismatches << "#{key}: old: #{old_value}, new: #{value}"
-                end
+                next unless old_value.empty? || value != old_value
+                old_value = old_value_original.inspect
+                value = value_original.inspect
+                mismatches << "#{key}: old: #{old_value}, new: #{value}"
               end
 
               unless mismatches.empty?
@@ -444,7 +444,8 @@ module Homebrew
                     rebuild\ \d+                                                 # rebuild with a number
                   )\n+                                                           # multiple empty lines
                  )+
-               /mx, '\0' + output + "\n")
+               /mx, '\0' + output + "\n"
+              )
             end
             odie "Bottle block addition failed!" unless string
           end
diff --git a/Library/Homebrew/dev-cmd/bump-formula-pr.rb b/Library/Homebrew/dev-cmd/bump-formula-pr.rb
index 9f05dfb8d..38c7559fb 100644
--- a/Library/Homebrew/dev-cmd/bump-formula-pr.rb
+++ b/Library/Homebrew/dev-cmd/bump-formula-pr.rb
@@ -101,7 +101,7 @@ module Homebrew
     old_formula_version = formula_version(formula, requested_spec)
 
     replacement_pairs = []
-    if requested_spec == :stable && formula.revision != 0
+    if requested_spec == :stable && formula.revision.nonzero?
       replacement_pairs << [/^  revision \d+\n(\n(  head "))?/m, "\\2"]
     end
 
diff --git a/Library/Homebrew/dev-cmd/create.rb b/Library/Homebrew/dev-cmd/create.rb
index 9be990318..1667fdfc7 100644
--- a/Library/Homebrew/dev-cmd/create.rb
+++ b/Library/Homebrew/dev-cmd/create.rb
@@ -91,7 +91,7 @@ module Homebrew
 
   def __gets
     gots = $stdin.gets.chomp
-    if gots.empty? then nil else gots end
+    gots.empty? ? nil : gots
   end
 end
 
diff --git a/Library/Homebrew/dev-cmd/edit.rb b/Library/Homebrew/dev-cmd/edit.rb
index ef325b8b6..f80d05861 100644
--- a/Library/Homebrew/dev-cmd/edit.rb
+++ b/Library/Homebrew/dev-cmd/edit.rb
@@ -44,7 +44,9 @@ module Homebrew
 
   def library_folders
     Dir["#{HOMEBREW_LIBRARY}/*"].reject do |d|
-      case File.basename(d) when "LinkedKegs", "Aliases" then true end
+      case File.basename(d)
+      when "LinkedKegs", "Aliases" then true
+      end
     end
   end
 end
diff --git a/Library/Homebrew/dev-cmd/man.rb b/Library/Homebrew/dev-cmd/man.rb
index fdd4bf33d..8b20158ef 100644
--- a/Library/Homebrew/dev-cmd/man.rb
+++ b/Library/Homebrew/dev-cmd/man.rb
@@ -34,15 +34,15 @@ module Homebrew
   end
 
   def path_glob_commands(glob)
-    Pathname.glob(glob).
-      sort_by { |source_file| sort_key_for_path(source_file) }.
-      map { |source_file|
-        source_file.read.lines.
-          grep(/^#:/).
-          map { |line| line.slice(2..-1) }.
-          join
-      }.
-      reject { |s| s.strip.empty? || s.include?("@hide_from_man_page") }
+    Pathname.glob(glob)
+            .sort_by { |source_file| sort_key_for_path(source_file) }
+            .map do |source_file|
+      source_file.read.lines
+                 .grep(/^#:/)
+                 .map { |line| line.slice(2..-1) }
+                 .join
+    end
+            .reject { |s| s.strip.empty? || s.include?("@hide_from_man_page") }
   end
 
   def build_man_page
@@ -51,11 +51,11 @@ module Homebrew
 
     variables[:commands] = path_glob_commands("#{HOMEBREW_LIBRARY_PATH}/cmd/*.{rb,sh}")
     variables[:developer_commands] = path_glob_commands("#{HOMEBREW_LIBRARY_PATH}/dev-cmd/*.{rb,sh}")
-    variables[:maintainers] = (HOMEBREW_REPOSITORY/"README.md").
-      read[/Homebrew's current maintainers are (.*)\./, 1].
-      scan(/\[([^\]]*)\]/).flatten
+    variables[:maintainers] = (HOMEBREW_REPOSITORY/"README.md")
+                              .read[/Homebrew's current maintainers are (.*)\./, 1]
+                              .scan(/\[([^\]]*)\]/).flatten
 
-    ERB.new(template, nil, ">").result(variables.instance_eval{ binding })
+    ERB.new(template, nil, ">").result(variables.instance_eval { binding })
   end
 
   def sort_key_for_path(path)
diff --git a/Library/Homebrew/dev-cmd/pull.rb b/Library/Homebrew/dev-cmd/pull.rb
index e18bf1c2f..a59792281 100644
--- a/Library/Homebrew/dev-cmd/pull.rb
+++ b/Library/Homebrew/dev-cmd/pull.rb
@@ -237,7 +237,7 @@ module Homebrew
 
   private
 
-  def publish_changed_formula_bottles(tap, changed_formulae_names)
+  def publish_changed_formula_bottles(_tap, changed_formulae_names)
     if ENV["HOMEBREW_DISABLE_LOAD_FORMULA"]
       raise "Need to load formulae to publish them!"
     end
@@ -360,7 +360,13 @@ module Homebrew
   def subject_for_bump(formula, old, new)
     if old[:nonexistent]
       # New formula
-      headline_ver = new[:stable] ? new[:stable] : new[:devel] ? new[:devel] : new[:head]
+      headline_ver = if new[:stable]
+        new[:stable]
+      elsif new[:devel]
+        new[:devel]
+      else
+        new[:head]
+      end
       subject = "#{formula.name} #{headline_ver} (new formula)"
     else
       # Update to existing formula
@@ -430,7 +436,7 @@ module Homebrew
       FormulaInfoFromJson.new(Utils::JSON.load(json)[0])
     end
 
-    def bottle_tags()
+    def bottle_tags
       return [] unless info["bottle"]["stable"]
       info["bottle"]["stable"]["files"].keys
     end
@@ -467,7 +473,6 @@ module Homebrew
     end
   end
 
-
   # Bottle info as used internally by pull, with alternate platform support
   class BottleInfo
     # URL of bottle as string
@@ -522,7 +527,7 @@ module Homebrew
         http.use_ssl = true
         retry_count = 0
         http.start do
-          while true do
+          loop do
             req = Net::HTTP::Head.new bottle_info.url
             req.initialize_http_header "User-Agent" => HOMEBREW_USER_AGENT_RUBY
             res = http.request req
@@ -551,7 +556,7 @@ module Homebrew
         max_curl_retries = 1
         retry_count = 0
         # We're in the cache; make sure to force re-download
-        while true do
+        loop do
           begin
             curl url, "-o", filename
             break
diff --git a/Library/Homebrew/dev-cmd/test-bot.rb b/Library/Homebrew/dev-cmd/test-bot.rb
index 7d95d2ebb..b9f659d40 100644
--- a/Library/Homebrew/dev-cmd/test-bot.rb
+++ b/Library/Homebrew/dev-cmd/test-bot.rb
@@ -120,7 +120,7 @@ module Homebrew
 
     if git_url = ENV["UPSTREAM_GIT_URL"] || ENV["GIT_URL"]
       # Also can get tap from Jenkins GIT_URL.
-      url_path = git_url.sub(%r{^https?://github\.com/}, "").chomp("/").sub(%r{\.git$}, "")
+      url_path = git_url.sub(%r{^https?://github\.com/}, "").chomp("/").sub(/\.git$/, "")
       begin
         return Tap.fetch(url_path) if url_path =~ HOMEBREW_TAP_REGEX
       rescue
@@ -190,7 +190,7 @@ module Homebrew
       puts "#{Tty.white}==>#{Tty.red} FAILED#{Tty.reset}" if failed?
     end
 
-    def has_output?
+    def output?
       @output && !@output.empty?
     end
 
@@ -245,7 +245,6 @@ module Homebrew
       @status = $?.success? ? :passed : :failed
       puts_result
 
-
       unless output.empty?
         @output = Homebrew.fix_encoding!(output)
         puts @output if (failed? || @puts_output_on_success) && !verbose
@@ -259,7 +258,7 @@ module Homebrew
   class Test
     attr_reader :log_root, :category, :name, :steps
 
-    def initialize(argument, options={})
+    def initialize(argument, options = {})
       @hash = nil
       @url = nil
       @formulae = []
@@ -277,7 +276,7 @@ module Homebrew
       elsif canonical_formula_name = safe_formula_canonical_name(argument)
         @formulae = [canonical_formula_name]
       else
-        raise ArgumentError.new("#{argument} is not a pull request URL, commit URL or formula name.")
+        raise ArgumentError, "#{argument} is not a pull request URL, commit URL or formula name."
       end
 
       @category = __method__
@@ -404,7 +403,7 @@ module Homebrew
         @short_url = @url.gsub("https://github.com/", "")
         if @short_url.include? "/commit/"
           # 7 characters should be enough for a commit (not 40).
-          @short_url.gsub!(/(commit\/\w{7}).*/, '\1')
+          @short_url.gsub!(%r{(commit/\w{7}).*/, '\1'})
           @name = @short_url
         else
           @name = "#{@short_url}-#{diff_end_sha1}"
@@ -569,7 +568,7 @@ module Homebrew
       dependents -= @formulae
       dependents = dependents.map { |d| Formulary.factory(d) }
 
-      bottled_dependents = dependents.select { |d| d.bottled? }
+      bottled_dependents = dependents.select(&:bottled?)
       testable_dependents = dependents.select { |d| d.bottled? && d.test_defined? }
 
       if (deps | reqs).any? { |d| d.name == "mercurial" && d.build? }
@@ -630,9 +629,9 @@ module Homebrew
           bottle_args << "--skip-relocation" if ARGV.include? "--skip-relocation"
           test "brew", "bottle", *bottle_args
           bottle_step = steps.last
-          if bottle_step.passed? && bottle_step.has_output?
+          if bottle_step.passed? && bottle_step.output?
             bottle_filename =
-              bottle_step.output.gsub(/.*(\.\/\S+#{Utils::Bottles::native_regex}).*/m, '\1')
+              bottle_step.output.gsub(%r{.*(\./\S+#{Utils::Bottles.native_regex}).*/m, '\1'})
             bottle_json_filename = bottle_filename.gsub(/\.(\d+\.)?tar\.gz$/, ".json")
             bottle_merge_args = ["--merge", "--write", "--no-commit", bottle_json_filename]
             bottle_merge_args << "--keep-old" if ARGV.include? "--keep-old"
@@ -666,11 +665,10 @@ module Homebrew
               next if steps.last.failed?
             end
           end
-          if dependent.installed?
-            test "brew", "linkage", "--test", dependent.name
-            if testable_dependents.include? dependent
-              test "brew", "test", "--verbose", dependent.name
-            end
+          next unless dependent.installed?
+          test "brew", "linkage", "--test", dependent.name
+          if testable_dependents.include? dependent
+            test "brew", "test", "--verbose", dependent.name
           end
         end
         test "brew", "uninstall", "--force", formula_name
@@ -795,7 +793,7 @@ module Homebrew
     end
 
     def test(*args)
-      options = Hash === args.last ? args.pop : {}
+      options = args.last.is_a?(Hash) ? args.pop : {}
       options[:repository] = @repository
       step = Step.new self, args, options
       step.run
@@ -934,7 +932,7 @@ module Homebrew
       bintray_repo = bottle_hash["bintray"]["repository"]
       bintray_repo_url = "https://api.bintray.com/packages/homebrew/#{bintray_repo}"
 
-      bottle_hash["bottle"]["tags"].each do |tag, tag_hash|
+      bottle_hash["bottle"]["tags"].each do |_tag, tag_hash|
         filename = tag_hash["filename"]
         if system "curl", "-I", "--silent", "--fail", "--output", "/dev/null",
                   "#{BottleSpecification::DEFAULT_DOMAIN}/#{bintray_repo}/#{filename}"
@@ -973,7 +971,7 @@ module Homebrew
     safe_system "git", "push", "--force", remote, "master:master", "refs/tags/#{git_tag}"
   end
 
-  def sanitize_ARGV_and_ENV
+  def sanitize_argv_and_env
     if Pathname.pwd == HOMEBREW_PREFIX && ARGV.include?("--cleanup")
       odie "cannot use --cleanup from HOMEBREW_PREFIX as it will delete all output."
     end
@@ -1016,7 +1014,7 @@ module Homebrew
   end
 
   def test_bot
-    sanitize_ARGV_and_ENV
+    sanitize_argv_and_env
 
     tap = resolve_test_tap
     # Tap repository if required, this is done before everything else
@@ -1070,19 +1068,18 @@ module Homebrew
           testcase.add_attribute "status", step.status
           testcase.add_attribute "time", step.time
 
-          if step.has_output?
-            output = sanitize_output_for_xml(step.output)
-            cdata = REXML::CData.new output
+          next unless step.output?
+          output = sanitize_output_for_xml(step.output)
+          cdata = REXML::CData.new output
 
-            if step.passed?
-              elem = testcase.add_element "system-out"
-            else
-              elem = testcase.add_element "failure"
-              elem.add_attribute "message", "#{step.status}: #{step.command.join(" ")}"
-            end
-
-            elem << cdata
+          if step.passed?
+            elem = testcase.add_element "system-out"
+          else
+            elem = testcase.add_element "failure"
+            elem.add_attribute "message", "#{step.status}: #{step.command.join(" ")}"
           end
+
+          elem << cdata
         end
       end