Merge branch 'master' into audit_line_rubocop_part_4_rebase_attempt_1

10 files changed, 138 insertions, 130 deletions

diff --git a/Library/Homebrew/dev-cmd/aspell-dictionaries.rb b/Library/Homebrew/dev-cmd/aspell-dictionaries.rb
index 955e41b01..ab0e66d2b 100644
--- a/Library/Homebrew/dev-cmd/aspell-dictionaries.rb
+++ b/Library/Homebrew/dev-cmd/aspell-dictionaries.rb
@@ -27,13 +27,12 @@ module Homebrew
       end
     end
 
-    languages.inject([]) do |resources, (lang, path)|
+    languages.each do |lang, path|
       r = Resource.new(lang)
       r.owner = Formulary.factory("aspell")
       r.url "#{dict_url}/#{path}"
       r.mirror "#{dict_mirror}/#{path}"
-      resources << r
-    end.each(&:fetch).each do |r|
+      r.fetch
       puts <<-EOS
         option "with-lang-#{r.name}", "Install #{r.name} dictionary"
         resource "#{r.name}" do
diff --git a/Library/Homebrew/dev-cmd/audit.rb b/Library/Homebrew/dev-cmd/audit.rb
index f8236e6a6..b58672043 100644
--- a/Library/Homebrew/dev-cmd/audit.rb
+++ b/Library/Homebrew/dev-cmd/audit.rb
@@ -53,7 +53,8 @@ module Homebrew
   module_function
 
   def audit
-    Homebrew.inject_dump_stats!(FormulaAuditor, /^audit_/) if ARGV.switch? "D"
+    inject_dump_stats!(FormulaAuditor, /^audit_/) if ARGV.switch? "D"
+    Homebrew.auditing = true
 
     formula_count = 0
     problem_count = 0
@@ -201,19 +202,24 @@ class FormulaAuditor
     @specs = %w[stable devel head].map { |s| formula.send(s) }.compact
   end
 
-  def self.check_http_content(url, name, user_agents: [:default])
+  def self.check_http_content(url, user_agents: [:default], check_content: false, strict: false, require_http: false)
     return unless url.start_with? "http"
 
     details = nil
     user_agent = nil
-    hash_needed = url.start_with?("http:") && name != "curl"
+    hash_needed = url.start_with?("http:") && !require_http
     user_agents.each do |ua|
       details = http_content_headers_and_checksum(url, hash_needed: hash_needed, user_agent: ua)
       user_agent = ua
       break if details[:status].to_s.start_with?("2")
     end
 
-    return "The URL #{url} is not reachable" unless details[:status]
+    unless details[:status]
+      # Hack around https://github.com/Homebrew/brew/issues/3199
+      return if MacOS.version == :el_capitan
+      return "The URL #{url} is not reachable"
+    end
+
     unless details[:status].start_with? "2"
       return "The URL #{url} is not reachable (HTTP status code #{details[:status]})"
     end
@@ -236,18 +242,40 @@ class FormulaAuditor
       details[:content_length] == secure_details[:content_length]
     file_match = details[:file_hash] == secure_details[:file_hash]
 
-    return if !etag_match && !content_length_match && !file_match
-    "The URL #{url} could use HTTPS rather than HTTP"
+    if etag_match || content_length_match || file_match
+      return "The URL #{url} should use HTTPS rather than HTTP"
+    end
+
+    return unless check_content
+
+    no_protocol_file_contents = %r{https?:\\?/\\?/}
+    details[:file] = details[:file].gsub(no_protocol_file_contents, "/")
+    secure_details[:file] = secure_details[:file].gsub(no_protocol_file_contents, "/")
+
+    # Check for the same content after removing all protocols
+    if details[:file] == secure_details[:file]
+      return "The URL #{url} should use HTTPS rather than HTTP"
+    end
+
+    return unless strict
+
+    # Same size, different content after normalization
+    # (typical causes: Generated ID, Timestamp, Unix time)
+    if details[:file].length == secure_details[:file].length
+      return "The URL #{url} may be able to use HTTPS rather than HTTP. Please verify it in a browser."
+    end
+
+    lenratio = (100 * secure_details[:file].length / details[:file].length).to_i
+    return unless (90..110).cover?(lenratio)
+    "The URL #{url} may be able to use HTTPS rather than HTTP. Please verify it in a browser."
   end
 
   def self.http_content_headers_and_checksum(url, hash_needed: false, user_agent: :default)
     max_time = hash_needed ? "600" : "25"
-    args = curl_args(
-      extra_args: ["--connect-timeout", "15", "--include", "--max-time", max_time, url],
-      show_output: true,
-      user_agent: user_agent,
+    output, = curl_output(
+      "--connect-timeout", "15", "--include", "--max-time", max_time, "--location", url,
+      user_agent: user_agent
     )
-    output = Open3.popen3(*args) { |_, stdout, _, _| stdout.read }
 
     status_code = :unknown
     while status_code == :unknown || status_code.to_s.start_with?("3")
@@ -262,6 +290,7 @@ class FormulaAuditor
       etag: headers[%r{ETag: ([wW]\/)?"(([^"]|\\")*)"}, 2],
       content_length: headers[/Content-Length: (\d+)/, 1],
       file_hash: output_hash,
+      file: output,
     }
   end
 
@@ -329,7 +358,8 @@ class FormulaAuditor
       end
       valid_alias_names = [alias_name_major, alias_name_major_minor]
 
-      if formula.tap && !formula.tap.core_tap?
+      unless formula.tap&.core_tap?
+        versioned_aliases.map! { |a| "#{formula.tap}/#{a}" }
         valid_alias_names.map! { |a| "#{formula.tap}/#{a}" }
       end
 
@@ -357,31 +387,17 @@ class FormulaAuditor
     end
   end
 
-  def audit_class
-    if @strict
-      unless formula.test_defined?
-        problem "A `test do` test block should be added"
-      end
-    end
-
-    classes = %w[GithubGistFormula ScriptFileFormula AmazonWebServicesFormula]
-    klass = classes.find do |c|
-      Object.const_defined?(c) && formula.class < Object.const_get(c)
-    end
-
-    problem "#{klass} is deprecated, use Formula instead" if klass
+  def self.aliases
+    # core aliases + tap alias names + tap alias full name
+    @aliases ||= Formula.aliases + Formula.tap_aliases
   end
 
-  # core aliases + tap alias names + tap alias full name
-  @@aliases ||= Formula.aliases + Formula.tap_aliases
-
   def audit_formula_name
     return unless @strict
     # skip for non-official taps
     return if formula.tap.nil? || !formula.tap.official?
 
     name = formula.name
-    full_name = formula.full_name
 
     if Homebrew::MissingFormula.blacklisted_reason(name)
       problem "'#{name}' is blacklisted."
@@ -397,33 +413,10 @@ class FormulaAuditor
       return
     end
 
-    if !formula.core_formula? && Formula.core_names.include?(name)
-      problem "Formula name conflicts with existing core formula."
-      return
-    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
+    return if formula.core_formula?
+    return unless Formula.core_names.include?(name)
 
-    same_name_tap_formulae = @@local_official_taps_name_map[name] || []
-
-    if @online
-      Homebrew.search_taps(name).each do |tap_formula_full_name|
-        tap_formula_name = tap_formula_full_name.split("/").last
-        next if tap_formula_name != name
-        same_name_tap_formulae << tap_formula_full_name
-      end
-    end
-
-    same_name_tap_formulae.delete(full_name)
-
-    return if same_name_tap_formulae.empty?
-    problem "Formula name conflicts with #{same_name_tap_formulae.join ", "}"
+    problem "Formula name conflicts with existing core formula."
   end
 
   def audit_deps
@@ -451,7 +444,7 @@ class FormulaAuditor
           problem "Dependency '#{dep.name}' was renamed; use new name '#{dep_f.name}'."
         end
 
-        if @@aliases.include?(dep.name) &&
+        if self.class.aliases.include?(dep.name) &&
            (dep_f.core_formula? || !dep_f.versioned_formula?)
           problem "Dependency '#{dep.name}' is an alias; use the canonical name '#{dep.to_formula.full_name}'."
         end
@@ -462,16 +455,16 @@ class FormulaAuditor
           problem "Dependency '#{dep.name}' may be unnecessary as it is provided by macOS; try to build this formula without it."
         end
 
-        dep.options.reject do |opt|
-          next true if dep_f.option_defined?(opt)
-          dep_f.requirements.detect do |r|
+        dep.options.each do |opt|
+          next if dep_f.option_defined?(opt)
+          next if dep_f.requirements.detect do |r|
             if r.recommended?
               opt.name == "with-#{r.name}"
             elsif r.optional?
               opt.name == "without-#{r.name}"
             end
           end
-        end.each do |opt|
+
           problem "Dependency #{dep} does not define option #{opt.name.inspect}"
         end
 
@@ -564,10 +557,11 @@ class FormulaAuditor
 
     return unless @online
 
-    return unless DevelopmentTools.curl_handles_most_https_homepages?
+    return unless DevelopmentTools.curl_handles_most_https_certificates?
     if http_content_problem = FormulaAuditor.check_http_content(homepage,
-                                               formula.name,
-                                               user_agents: [:browser, :default])
+                                               user_agents: [:browser, :default],
+                                               check_content: true,
+                                               strict: @strict)
       problem http_content_problem
     end
   end
@@ -598,7 +592,8 @@ class FormulaAuditor
     return if metadata.nil?
 
     problem "GitHub fork (not canonical repository)" if metadata["fork"]
-    if (metadata["forks_count"] < 20) && (metadata["subscribers_count"] < 20) &&
+    if formula&.tap&.core_tap? &&
+       (metadata["forks_count"] < 20) && (metadata["subscribers_count"] < 20) &&
        (metadata["stargazers_count"] < 50)
       problem "GitHub repository not notable enough (<20 forks, <20 watchers and <50 stars)"
     end
@@ -617,13 +612,14 @@ class FormulaAuditor
     end
 
     %w[Stable Devel HEAD].each do |name|
-      next unless spec = formula.send(name.downcase)
+      spec_name = name.downcase.to_sym
+      next unless spec = formula.send(spec_name)
 
-      ra = ResourceAuditor.new(spec, online: @online, strict: @strict).audit
+      ra = ResourceAuditor.new(spec, spec_name, online: @online, strict: @strict).audit
       problems.concat ra.problems.map { |problem| "#{name}: #{problem}" }
 
       spec.resources.each_value do |resource|
-        ra = ResourceAuditor.new(resource, online: @online, strict: @strict).audit
+        ra = ResourceAuditor.new(resource, spec_name, online: @online, strict: @strict).audit
         problems.concat ra.problems.map { |problem|
           "#{name} resource #{resource.name.inspect}: #{problem}"
         }
@@ -688,7 +684,7 @@ class FormulaAuditor
     end
 
     stable = formula.stable
-    case stable && stable.url
+    case stable&.url
     when /[\d\._-](alpha|beta|rc\d)/
       matched = Regexp.last_match(1)
       version_prefix = stable.version.to_s.sub(/\d+$/, "")
@@ -855,7 +851,7 @@ class FormulaAuditor
   def audit_reverse_migration
     # Only enforce for new formula being re-added to core and official taps
     return unless @strict
-    return unless formula.tap && formula.tap.official?
+    return unless formula.tap&.official?
     return unless formula.tap.tap_migrations.key?(formula.name)
 
     problem <<-EOS.undent
@@ -918,10 +914,10 @@ class FormulaAuditor
 end
 
 class ResourceAuditor
-  attr_reader :problems
-  attr_reader :version, :checksum, :using, :specs, :url, :mirrors, :name
+  attr_reader :name, :version, :checksum, :url, :mirrors, :using, :specs, :owner
+  attr_reader :spec_name, :problems
 
-  def initialize(resource, options = {})
+  def initialize(resource, spec_name, options = {})
     @name     = resource.name
     @version  = resource.version
     @checksum = resource.checksum
@@ -929,9 +925,11 @@ class ResourceAuditor
     @mirrors  = resource.mirrors
     @using    = resource.using
     @specs    = resource.specs
-    @online   = options[:online]
-    @strict   = options[:strict]
-    @problems = []
+    @owner    = resource.owner
+    @spec_name = spec_name
+    @online    = options[:online]
+    @strict    = options[:strict]
+    @problems  = []
   end
 
   def audit
@@ -1005,11 +1003,29 @@ class ResourceAuditor
     problem "Redundant :using value in URL"
   end
 
+  def self.curl_openssl_and_deps
+    @curl_openssl_and_deps ||= begin
+      formulae_names = ["curl", "openssl"]
+      formulae_names += formulae_names.flat_map do |f|
+        Formula[f].recursive_dependencies.map(&:name)
+      end
+      formulae_names.uniq
+    rescue FormulaUnavailableError
+      []
+    end
+  end
+
   def audit_urls
     urls = [url] + mirrors
 
-    if name == "curl" && !urls.find { |u| u.start_with?("http://") }
-      problem "should always include at least one HTTP url"
+    curl_openssl_or_deps = ResourceAuditor.curl_openssl_and_deps.include?(owner.name)
+
+    if spec_name == :stable && curl_openssl_or_deps
+      problem "should not use xz tarballs" if url.end_with?(".xz")
+
+      unless urls.find { |u| u.start_with?("http://") }
+        problem "should always include at least one HTTP mirror"
+      end
     end
 
     return unless @online
@@ -1021,7 +1037,7 @@ class ResourceAuditor
         # A `brew mirror`'ed URL is usually not yet reachable at the time of
         # pull request.
         next if url =~ %r{^https://dl.bintray.com/homebrew/mirror/}
-        if http_content_problem = FormulaAuditor.check_http_content(url, name)
+        if http_content_problem = FormulaAuditor.check_http_content(url, require_http: curl_openssl_or_deps)
           problem http_content_problem
         end
       elsif strategy <= GitDownloadStrategy
@@ -1030,6 +1046,7 @@ class ResourceAuditor
         end
       elsif strategy <= SubversionDownloadStrategy
         next unless DevelopmentTools.subversion_handles_most_https_certificates?
+        next unless Utils.svn_available?
         unless Utils.svn_remote_exists url
           problem "The URL #{url} is not a valid svn URL"
         end
diff --git a/Library/Homebrew/dev-cmd/bottle.rb b/Library/Homebrew/dev-cmd/bottle.rb
index d8aefc4c0..fb862c773 100644
--- a/Library/Homebrew/dev-cmd/bottle.rb
+++ b/Library/Homebrew/dev-cmd/bottle.rb
@@ -47,7 +47,7 @@ BOTTLE_ERB = <<-EOS.freeze
     <% elsif cellar != BottleSpecification::DEFAULT_CELLAR %>
     cellar "<%= cellar %>"
     <% end %>
-    <% if rebuild > 0 %>
+    <% if rebuild.positive? %>
     rebuild <%= rebuild %>
     <% end %>
     <% checksums.each do |checksum_type, checksum_values| %>
@@ -75,7 +75,7 @@ module Homebrew
 
       @put_filenames ||= []
 
-      return if @put_filenames.include? filename
+      return if @put_filenames.include?(filename)
 
       puts Formatter.error(filename.to_s)
       @put_filenames << filename
@@ -84,8 +84,7 @@ module Homebrew
     result = false
 
     keg.each_unique_file_matching(string) do |file|
-      # skip document file.
-      next if Metafiles::EXTENSIONS.include? file.extname
+      next if Metafiles::EXTENSIONS.include?(file.extname) # Skip document files.
 
       linked_libraries = Keg.file_linked_libraries(file, string)
       result ||= !linked_libraries.empty?
@@ -156,9 +155,7 @@ module Homebrew
       return ofail "Formula not installed or up-to-date: #{f.full_name}"
     end
 
-    tap = f.tap
-
-    unless tap
+    unless tap = f.tap
       unless ARGV.include?("--force-core-tap")
         return ofail "Formula not from core or any taps: #{f.full_name}"
       end
@@ -186,7 +183,7 @@ module Homebrew
       ohai "Determining #{f.full_name} bottle rebuild..."
       versions = FormulaVersions.new(f)
       rebuilds = versions.bottle_version_map("origin/master")[f.pkg_version]
-      rebuilds.pop if rebuilds.last.to_i > 0
+      rebuilds.pop if rebuilds.last.to_i.positive?
       rebuild = rebuilds.empty? ? 0 : rebuilds.max.to_i + 1
     end
 
@@ -283,7 +280,7 @@ module Homebrew
         raise
       ensure
         ignore_interrupts do
-          original_tab.write if original_tab
+          original_tab&.write
           unless ARGV.include? "--skip-relocation"
             keg.replace_placeholders_with_locations changed_files
           end
diff --git a/Library/Homebrew/dev-cmd/bump-formula-pr.rb b/Library/Homebrew/dev-cmd/bump-formula-pr.rb
index 1c56749a3..87d8274cc 100644
--- a/Library/Homebrew/dev-cmd/bump-formula-pr.rb
+++ b/Library/Homebrew/dev-cmd/bump-formula-pr.rb
@@ -89,7 +89,8 @@ module Homebrew
 
   def check_for_duplicate_pull_requests(formula)
     pull_requests = fetch_pull_requests(formula)
-    return unless pull_requests && !pull_requests.empty?
+    return unless pull_requests
+    return if pull_requests.empty?
     duplicates_message = <<-EOS.undent
       These open pull requests may be duplicates:
       #{pull_requests.map { |pr| "#{pr["title"]} #{pr["html_url"]}" }.join("\n")}
@@ -124,7 +125,7 @@ module Homebrew
       Formula.each do |f|
         if is_devel && f.devel && f.devel.url && f.devel.url.match(base_url)
           guesses << f
-        elsif f.stable && f.stable.url && f.stable.url.match(base_url)
+        elsif f.stable&.url && f.stable.url.match(base_url)
           guesses << f
         end
       end
@@ -176,7 +177,10 @@ module Homebrew
       rsrc.version = forced_version if forced_version
       odie "No version specified!" unless rsrc.version
       rsrc_path = rsrc.fetch
-      if Utils.popen_read("/usr/bin/tar", "-tf", rsrc_path) =~ %r{/.*\.}
+      gnu_tar_gtar_path = HOMEBREW_PREFIX/"opt/gnu-tar/bin/gtar"
+      gnu_tar_gtar = gnu_tar_gtar_path if gnu_tar_gtar_path.executable?
+      tar = which("gtar") || gnu_tar_gtar || which("tar")
+      if Utils.popen_read(tar, "-tf", rsrc_path) =~ %r{/.*\.}
         new_hash = rsrc_path.sha256
       elsif new_url.include? ".tar"
         odie "#{formula}: no url/#{hash_type} specified!"
@@ -293,9 +297,7 @@ module Homebrew
         ohai "git fetch --unshallow origin" if shallow
         ohai "git checkout --no-track -b #{branch} origin/master"
         ohai "git commit --no-edit --verbose --message='#{formula.name} #{new_formula_version}#{devel_message}' -- #{formula.path}"
-        ohai "hub fork --no-remote"
-        ohai "hub fork"
-        ohai "hub fork (to read $HUB_REMOTE)"
+        ohai "hub fork # read $HUB_REMOTE"
         ohai "git push --set-upstream $HUB_REMOTE #{branch}:#{branch}"
         ohai "hub pull-request --browse -m '#{formula.name} #{new_formula_version}#{devel_message}'"
         ohai "git checkout -"
@@ -305,9 +307,9 @@ module Homebrew
         safe_system "git", "commit", "--no-edit", "--verbose",
           "--message=#{formula.name} #{new_formula_version}#{devel_message}",
           "--", formula.path
-        safe_system "hub", "fork", "--no-remote"
-        quiet_system "hub", "fork"
-        remote = Utils.popen_read("hub fork 2>&1")[/fatal: remote (.+) already exists\./, 1]
+        remote = Utils.popen_read("hub fork 2>&1")[/remote:? (\S+)/, 1]
+        # repeat for hub 2.2 backwards compatibility:
+        remote = Utils.popen_read("hub fork 2>&1")[/remote:? (\S+)/, 1] if remote.to_s.empty?
         odie "cannot get remote from 'hub'!" if remote.to_s.empty?
         safe_system "git", "push", "--set-upstream", remote, "#{branch}:#{branch}"
         pr_message = <<-EOS.undent
diff --git a/Library/Homebrew/dev-cmd/man.rb b/Library/Homebrew/dev-cmd/man.rb
index 472bb7c2b..b2bb3c8c3 100644
--- a/Library/Homebrew/dev-cmd/man.rb
+++ b/Library/Homebrew/dev-cmd/man.rb
@@ -48,12 +48,9 @@ module Homebrew
   def path_glob_commands(glob)
     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") }
+            .map(&:read).map(&:lines)
+            .map { |lines| lines.grep(/^#:/).map { |line| line.slice(2..-1) }.join }
+            .reject { |s| s.strip.empty? || s.include?("@hide_from_man_page") }
   end
 
   def build_man_page
diff --git a/Library/Homebrew/dev-cmd/mirror.rb b/Library/Homebrew/dev-cmd/mirror.rb
index e2492203d..6445bc34c 100644
--- a/Library/Homebrew/dev-cmd/mirror.rb
+++ b/Library/Homebrew/dev-cmd/mirror.rb
@@ -25,9 +25,9 @@ module Homebrew
            "public_download_numbers": true,
            "public_stats": true}
         EOS
-        curl "--silent", "--fail", "-u#{bintray_user}:#{bintray_key}",
-             "-H", "Content-Type: application/json",
-             "-d", package_blob, bintray_repo_url
+        curl "--silent", "--fail", "--user", "#{bintray_user}:#{bintray_key}",
+             "--header", "Content-Type: application/json",
+             "--data", package_blob, bintray_repo_url
         puts
       end
 
@@ -40,8 +40,8 @@ module Homebrew
       content_url = "https://api.bintray.com/content/homebrew/mirror"
       content_url += "/#{bintray_package}/#{f.pkg_version}/#{filename}"
       content_url += "?publish=1"
-      curl "--silent", "--fail", "-u#{bintray_user}:#{bintray_key}",
-           "-T", download, content_url
+      curl "--silent", "--fail", "--user", "#{bintray_user}:#{bintray_key}",
+           "--upload-file", download, content_url
       puts
       ohai "Mirrored #{filename}!"
     end
diff --git a/Library/Homebrew/dev-cmd/pull.rb b/Library/Homebrew/dev-cmd/pull.rb
index 9681bb2bc..8cb270303 100644
--- a/Library/Homebrew/dev-cmd/pull.rb
+++ b/Library/Homebrew/dev-cmd/pull.rb
@@ -69,13 +69,13 @@ module Homebrew
     tap = nil
 
     ARGV.named.each do |arg|
-      if arg.to_i > 0
+      if arg.to_i.positive?
         issue = arg
         url = "https://github.com/Homebrew/homebrew-core/pull/#{arg}"
         tap = CoreTap.instance
       elsif (testing_match = arg.match %r{/job/Homebrew.*Testing/(\d+)/})
         tap = ARGV.value("tap")
-        tap = if tap && tap.start_with?("homebrew/")
+        tap = if tap&.start_with?("homebrew/")
           Tap.fetch("homebrew", tap.strip_prefix("homebrew/"))
         elsif tap
           odie "Tap option did not start with \"homebrew/\": #{tap}"
@@ -154,8 +154,8 @@ module Homebrew
 
         begin
           f = Formula[name]
-        # Make sure we catch syntax errors.
-        rescue Exception
+        rescue Exception # rubocop:disable Lint/RescueException
+          # Make sure we catch syntax errors.
           next
         end
 
@@ -228,7 +228,7 @@ module Homebrew
           "https://github.com/BrewTestBot/homebrew-#{tap.repo}/compare/homebrew:master...pr-#{issue}"
         end
 
-        curl "--silent", "--fail", "-o", "/dev/null", "-I", bottle_commit_url
+        curl "--silent", "--fail", "--output", "/dev/null", "--head", bottle_commit_url
 
         safe_system "git", "checkout", "--quiet", "-B", bottle_branch, orig_revision
         pull_patch bottle_commit_url, "bottle commit"
@@ -303,7 +303,7 @@ module Homebrew
       extra_msg = @description ? "(#{@description})" : nil
       ohai "Fetching patch #{extra_msg}"
       puts "Patch: #{patch_url}"
-      curl patch_url, "-s", "-o", patchpath
+      curl_download patch_url, to: patchpath
     end
 
     def apply_patch
@@ -350,7 +350,7 @@ module Homebrew
       files << Regexp.last_match(1) if line =~ %r{^\+\+\+ b/(.*)}
     end
     files.each do |file|
-      if tap && tap.formula_file?(file)
+      if tap&.formula_file?(file)
         formula_name = File.basename(file, ".rb")
         formulae << formula_name unless formulae.include?(formula_name)
       else
@@ -433,10 +433,10 @@ module Homebrew
     end
     version = info.pkg_version
     ohai "Publishing on Bintray: #{package} #{version}"
-    curl "-w", '\n', "--silent", "--fail",
-         "-u#{creds[:user]}:#{creds[:key]}", "-X", "POST",
-         "-H", "Content-Type: application/json",
-         "-d", '{"publish_wait_for_secs": 0}',
+    curl "--write-out", '\n', "--silent", "--fail",
+         "--user", "#{creds[:user]}:#{creds[:key]}", "--request", "POST",
+         "--header", "Content-Type: application/json",
+         "--data", '{"publish_wait_for_secs": 0}',
          "https://api.bintray.com/content/homebrew/#{repo}/#{package}/#{version}/publish"
     true
   rescue => e
@@ -587,7 +587,7 @@ module Homebrew
         # We're in the cache; make sure to force re-download
         loop do
           begin
-            curl url, "-o", filename
+            curl_download url, continue_at: 0, to: filename
             break
           rescue
             if retry_count >= max_curl_retries
@@ -606,7 +606,7 @@ module Homebrew
   end
 
   def check_bintray_mirror(name, url)
-    headers = curl_output("--connect-timeout", "15", "--head", url)[0]
+    headers, = curl_output("--connect-timeout", "15", "--location", "--head", url)
     status_code = headers.scan(%r{^HTTP\/.* (\d+)}).last.first
     return if status_code.start_with?("2")
     opoo "The Bintray mirror #{url} is not reachable (HTTP status code #{status_code})."
diff --git a/Library/Homebrew/dev-cmd/release-notes.rb b/Library/Homebrew/dev-cmd/release-notes.rb
index e578869bf..496023956 100644
--- a/Library/Homebrew/dev-cmd/release-notes.rb
+++ b/Library/Homebrew/dev-cmd/release-notes.rb
@@ -10,10 +10,8 @@ module Homebrew
 
   def release_notes
     previous_tag = ARGV.named.first
-    unless previous_tag
-      previous_tag = Utils.popen_read("git tag --list --sort=-version:refname")
+    previous_tag ||= Utils.popen_read("git tag --list --sort=-version:refname")
                           .lines.first.chomp
-    end
     odie "Could not find any previous tags!" unless previous_tag
 
     end_ref = ARGV.named[1] || "origin/master"
diff --git a/Library/Homebrew/dev-cmd/test.rb b/Library/Homebrew/dev-cmd/test.rb
index c678171ac..6622a8c25 100644
--- a/Library/Homebrew/dev-cmd/test.rb
+++ b/Library/Homebrew/dev-cmd/test.rb
@@ -65,8 +65,6 @@ module Homebrew
           args << "--devel"
         end
 
-        Sandbox.print_sandbox_message if Sandbox.test?
-
         Utils.safe_fork do
           if Sandbox.test?
             sandbox = Sandbox.new
@@ -86,7 +84,7 @@ module Homebrew
       rescue ::Test::Unit::AssertionFailedError => e
         ofail "#{f.full_name}: failed"
         puts e.message
-      rescue Exception => e
+      rescue Exception => e # rubocop:disable Lint/RescueException
         ofail "#{f.full_name}: failed"
         puts e, e.backtrace
       ensure
diff --git a/Library/Homebrew/dev-cmd/update-test.rb b/Library/Homebrew/dev-cmd/update-test.rb
index aa7fe6a92..1f1cdbeed 100644
--- a/Library/Homebrew/dev-cmd/update-test.rb
+++ b/Library/Homebrew/dev-cmd/update-test.rb
@@ -81,7 +81,7 @@ module Homebrew
       safe_system "git", "reset", "--hard", start_commit
 
       # update ENV["PATH"]
-      ENV["PATH"] = "#{curdir}/bin:/usr/local/bin:/usr/bin:/bin"
+      ENV["PATH"] = PATH.new(ENV["PATH"]).prepend(curdir/"bin")
 
       # run brew update
       oh1 "Running brew update..."