Merge branch 'master' into audit_line_rubocop_part_4_rebase_attempt_1

Library/.rubocop.yml

@@ -63,26 +63,26 @@ Metrics/AbcSize:
   Max: 250
 
 Metrics/BlockLength:
-  Max: 1250
+  Max: 144
 
 Metrics/ClassLength:
-  Max: 1500
+  Max: 589
 
 Metrics/CyclomaticComplexity:
   Max: 75
 
 Metrics/LineLength:
-  Max: 400
+  Max: 324
 
 Metrics/MethodLength:
-  Max: 250
+  Max: 222
 
 Metrics/ModuleLength:
   CountComments: false
-  Exclude:
+  Max: 367
-    - '**/bin/**/*'
+
-    - '**/cmd/**/*'
+Metrics/ParameterLists:
-    - '**/lib/**/*'
+  CountKeywordArgs: false
 
 Metrics/PerceivedComplexity:
   Max: 100
@@ -94,6 +94,9 @@ Performance/Caller:
 Style/Alias:
   EnforcedStyle: prefer_alias
 
+Style/AsciiComments:
+  Enabled: false
+
 Style/AutoResourceCleanup:
   Enabled: true
 
@@ -119,7 +122,7 @@ Style/Encoding:
   Enabled: true
 
 # use spaces for indentation; detect tabs
-Style/Tab:
+Layout/Tab:
   Enabled: true
 
 # dashes in filenames are typical

Library/Homebrew/.rubocop.yml

@@ -25,14 +25,20 @@ Lint/NestedMethodDefinition:
 Lint/ParenthesesAsGroupedExpression:
   Enabled: true
 
+Metrics/BlockLength:
+  Max: 1250
+
 Metrics/BlockNesting:
   Max: 5
 
-Metrics/ModuleLength:
+Metrics/ClassLength:
-  Max: 360
+  Max: 1226
 
-Metrics/ParameterLists:
+Metrics/LineLength:
-  CountKeywordArgs: false
+  Max: 244
+
+Metrics/MethodLength:
+  Max: 195
 
 # we won't change backward compatible method names
 Naming/MethodName:

Library/Homebrew/brew.rb

@@ -9,7 +9,7 @@ RUBY_VERSION_SPLIT = RUBY_VERSION.split "."
 RUBY_X = RUBY_VERSION_SPLIT[0].to_i
 RUBY_Y = RUBY_VERSION_SPLIT[1].to_i
 if RUBY_X < 2 || (RUBY_X == 2 && RUBY_Y < 3)
-  raise "Homebrew must be run under Ruby 2.3!"
+  raise "Homebrew must be run under Ruby 2.3! You're running #{RUBY_VERSION}."
 end
 
 require "pathname"

Library/Homebrew/cask/lib/hbc/cli/edit.rb

@@ -8,9 +8,6 @@ module Hbc
       end
 
       def run
-        cask = casks.first
-        cask_path = cask.sourcefile_path
-        odebug "Opening editor for Cask #{cask.token}"
         exec_editor cask_path
       rescue CaskUnavailableError => e
         reason = e.reason.empty? ? "" : "#{e.reason} "
@@ -18,6 +15,14 @@ module Hbc
         raise e.class.new(e.token, reason)
       end
 
+      def cask_path
+        casks.first.sourcefile_path
+      rescue CaskInvalidError
+        path = CaskLoader.path(args.first)
+        return path if path.file?
+        raise
+      end
+
       def self.help
         "edits the given Cask"
       end

Library/Homebrew/cask/lib/hbc/container/dmg.rb

@@ -88,7 +88,7 @@ module Hbc
           bomfile.close
 
           Tempfile.open(["", ".list"]) do |filelist|
-            filelist.write(bom_filelist_from_path(mount))
+            filelist.puts(bom_filelist_from_path(mount))
             filelist.close
 
             @command.run!("/usr/bin/mkbom", args: ["-s", "-i", filelist.path, "--", bomfile.path])
@@ -98,16 +98,17 @@ module Hbc
       end
 
       def bom_filelist_from_path(mount)
-        Dir.chdir(mount) do
+        # We need to use `find` here instead of Ruby in order to properly handle
-          Dir.glob("**/*", File::FNM_DOTMATCH).map do |path|
+        # file names containing special characters, such as “e” + “´” vs. “é”.
-            next if skip_path?(Pathname(path))
+        @command.run("/usr/bin/find", args: [".", "-print0"], chdir: mount, print_stderr: false).stdout
-            (path == ".") ? path : path.prepend("./")
+                .split("\0")
-          end.compact.join("\n").concat("\n")
+                .reject { |path| skip_path?(mount, path) }
-        end
+                .join("\n")
       end
 
-      def skip_path?(path)
+      def skip_path?(mount, path)
-        dmg_metadata?(path) || system_dir_symlink?(path)
+        path = Pathname(path.sub(%r{^\./}, ""))
+        dmg_metadata?(path) || system_dir_symlink?(mount, path)
       end
 
       # unnecessary DMG metadata
@@ -130,9 +131,10 @@ module Hbc
         DMG_METADATA_FILES.include?(relative_root.basename.to_s)
       end
 
-      def system_dir_symlink?(path)
+      def system_dir_symlink?(mount, path)
+        full_path = Pathname(mount).join(path)
         # symlinks to system directories (commonly to /Applications)
-        path.symlink? && MacOS.system_dir?(path.readlink)
+        full_path.symlink? && MacOS.system_dir?(full_path.readlink)
       end
 
       def mounts_from_plist(plist)

Library/Homebrew/cask/lib/hbc/system_command.rb

@@ -8,14 +8,14 @@ require "hbc/utils/hash_validator"
 
 module Hbc
   class SystemCommand
-    attr_reader :command
+    extend Predicable
 
-    def self.run(executable, options = {})
+    def self.run(executable, **options)
-      new(executable, options).run!
+      new(executable, **options).run!
     end
 
-    def self.run!(command, options = {})
+    def self.run!(command, **options)
-      run(command, options.merge(must_succeed: true))
+      run(command, **options, must_succeed: true)
     end
 
     def run!
@@ -26,38 +26,49 @@ module Hbc
         case type
         when :stdout
           processed_output[:stdout] << line
-          ohai line.chomp if options[:print_stdout]
+          ohai line.chomp if print_stdout?
         when :stderr
           processed_output[:stderr] << line
-          ohai line.chomp if options[:print_stderr]
+          ohai line.chomp if print_stderr?
         end
       end
 
-      assert_success if options[:must_succeed]
+      assert_success if must_succeed?
       result
     end
 
-    def initialize(executable, options)
+    def initialize(executable, args: [], sudo: false, input: [], print_stdout: false, print_stderr: true, must_succeed: false, **options)
+      executable, *args = Shellwords.shellescape(executable) if args.empty?
+
       @executable = executable
+      @args = args
+      @sudo = sudo
+      @input = input
+      @print_stdout = print_stdout
+      @print_stderr = print_stderr
+      @must_succeed = must_succeed
+      options.extend(HashValidator).assert_valid_keys(:chdir)
       @options = options
-      process_options!
+    end
+
+    def command
+      @command ||= [
+        *sudo_prefix,
+        executable,
+        *args,
+      ].freeze
     end
 
     private
 
-    attr_reader :executable, :options, :processed_output, :processed_status
+    attr_reader :executable, :args, :input, :options, :processed_output, :processed_status
 
-    def process_options!
+    attr_predicate :sudo?, :print_stdout?, :print_stderr?, :must_succeed?
-      options.extend(HashValidator)
+
-             .assert_valid_keys :input, :print_stdout, :print_stderr, :args, :must_succeed, :sudo
+    def sudo_prefix
-      sudo_prefix = %w[/usr/bin/sudo -E --]
+      return [] unless sudo?
-      sudo_prefix = sudo_prefix.insert(1, "-A") unless ENV["SUDO_ASKPASS"].nil?
+      askpass_flags = ENV.key?("SUDO_ASKPASS") ? ["-A"] : []
-      @command = [executable]
+      ["/usr/bin/sudo", *askpass_flags, "-E", "--"]
-      options[:print_stderr] = true    unless options.key?(:print_stderr)
-      @command.unshift(*sudo_prefix)   if  options[:sudo]
-      @command.concat(options[:args])  if  options.key?(:args) && !options[:args].empty?
-      @command[0] = Shellwords.shellescape(@command[0]) if @command.size == 1
-      nil
     end
 
     def assert_success
@@ -77,7 +88,7 @@ module Hbc
 
     def each_output_line(&b)
       raw_stdin, raw_stdout, raw_stderr, raw_wait_thr =
-        Open3.popen3(*expanded_command)
+        Open3.popen3(*expanded_command, **options)
 
       write_input_to(raw_stdin)
       raw_stdin.close_write
@@ -87,7 +98,7 @@ module Hbc
     end
 
     def write_input_to(raw_stdin)
-      [*options[:input]].each { |line| raw_stdin.print line }
+      [*input].each(&raw_stdin.method(:print))
     end
 
     def each_line_from(sources)

Library/Homebrew/cmd/style.rb

@@ -109,7 +109,7 @@ module Homebrew
       args << "--config" << HOMEBREW_LIBRARY_PATH/".rubocop.yml"
       args << HOMEBREW_LIBRARY_PATH
     else
-      args << "--config" << HOMEBREW_LIBRARY/".auditcops.yml"
+      args << "--config" << HOMEBREW_LIBRARY/".rubocop_audit.yml"
       args += files
     end
 

Library/Homebrew/compat/formula.rb

@@ -78,4 +78,9 @@ class Formula
   def startup_plist
     odeprecated "Formula#startup_plist", "Formula#plist"
   end
+
+  def rake(*args)
+    # odeprecated "FileUtils#rake", "system \"rake\""
+    system "rake", *args
+  end
 end

Library/Homebrew/dev-cmd/pull.rb

@@ -560,7 +560,7 @@ module Homebrew
             req = Net::HTTP::Head.new bottle_info.url
             req.initialize_http_header "User-Agent" => HOMEBREW_USER_AGENT_RUBY
             res = http.request req
-            break if res.is_a?(Net::HTTPSuccess)
+            break if res.is_a?(Net::HTTPSuccess) || res.code == "302"
 
             unless res.is_a?(Net::HTTPClientError)
               raise "Failed to find published #{f} bottle at #{url} (#{res.code} #{res.message})!"

Library/Homebrew/extend/fileutils.rb

@@ -101,11 +101,6 @@ module FileUtils
     system Formulary.factory("scons").opt_bin/"scons", *args
   end
 
-  # Run the `rake` from the `ruby` Homebrew is using rather than whatever is in the `PATH`.
-  def rake(*args)
-    system RUBY_BIN/"rake", *args
-  end
-
   # Run `make` 3.81 or newer.
   # Uses the system make on Leopard and newer, and the
   # path to the actually-installed make on Tiger or older.

Library/Homebrew/os/mac.rb

@@ -47,19 +47,11 @@ module OS
     end
 
     def languages
-      return @languages unless @languages.nil?
+      @languages ||= [
-
+        *ARGV.value("language")&.split(","),
-      @languages = Utils.popen_read("defaults", "read", ".GlobalPreferences", "AppleLanguages").scan(/[^ \n"(),]+/)
+        *ENV["HOMEBREW_LANGUAGES"]&.split(","),
-
+        *Open3.capture2("defaults", "read", "-g", "AppleLanguages")[0].scan(/[^ \n"(),]+/),
-      if ENV["HOMEBREW_LANGUAGES"]
+      ].uniq
-        @languages = ENV["HOMEBREW_LANGUAGES"].split(",") + @languages
-      end
-
-      if ARGV.value("language")
-        @languages = ARGV.value("language").split(",") + @languages
-      end
-
-      @languages = @languages.uniq
     end
 
     def language

Library/Homebrew/rubocops/formula_desc_cop.rb

@@ -18,8 +18,14 @@ module RuboCop
             return
           end
 
-          # Check if a formula's desc is too long
+          # Check the formula's desc length. Should be >0 and <80 characters.
           desc = parameters(desc_call).first
+          pure_desc_length = string_content(desc).length
+          if pure_desc_length.zero?
+            problem "The desc (description) should not be an empty string."
+            return
+          end
+
           desc_length = "#{@formula_name}: #{string_content(desc)}".length
           max_desc_length = 80
           return if desc_length <= max_desc_length

Library/Homebrew/rubocops/lines_cop.rb

@@ -1,4 +1,4 @@
-require 'FileUtils'
+require "FileUtils"
 require_relative "./extend/formula_cop"
 
 module RuboCop
@@ -89,7 +89,7 @@ module RuboCop
           end
 
           find_instance_method_call(body_node, :man, :+) do |method|
-            next unless match = regex_match_group(parameters(method).first, %r{man[1-8]})
+            next unless match = regex_match_group(parameters(method).first, /man[1-8]/)
             problem "\"#{method.source}\" should be \"#{match[0]}\""
           end
 
@@ -132,13 +132,13 @@ module RuboCop
 
           find_every_method_call_by_name(body_node, :depends_on).each do |method|
             key, value = destructure_hash(parameters(method).first)
-            next if (key.nil? || value.nil?)
+            next if key.nil? || value.nil?
-            next unless match = regex_match_group(value, %r{(lua|perl|python|ruby)(\d*)})
+            next unless match = regex_match_group(value, /(lua|perl|python|ruby)(\d*)/)
             problem "#{match[1]} modules should be vendored rather than use deprecated #{method.source}`"
           end
 
           find_every_method_call_by_name(body_node, :system).each do |method|
-            next unless match = regex_match_group(parameters(method).first, %r{(env|export)(\s+)?})
+            next unless match = regex_match_group(parameters(method).first, /(env|export)(\s+)?/)
             problem "Use ENV instead of invoking '#{match[1]}' to modify the environment"
           end
 
@@ -163,7 +163,7 @@ module RuboCop
 
           find_instance_method_call(body_node, "ARGV", :include?) do |method|
             param = parameters(method).first
-            next unless match = regex_match_group(param, %r{--(HEAD|devel)})
+            next unless match = regex_match_group(param, /--(HEAD|devel)/)
             problem "Use \"if build.#{match[1].downcase}?\" instead"
           end
 
@@ -240,9 +240,21 @@ module RuboCop
           end
 
           find_every_method_call_by_name(body_node, :assert).each do |method|
-            if method_called?(method, :include?) && !method_called?(method, :!)
+            if method_called_ever?(method, :include?) && !method_called_ever?(method, :!)
               problem "Use `assert_match` instead of `assert ...include?`"
             end
+
+            if method_called_ever?(method, :exist?) && !method_called_ever?(method, :!)
+              problem "Use `assert_predicate <path_to_file>, :exist?` instead of `#{method.source}`"
+            end
+
+            if method_called_ever?(method, :exist?) && method_called_ever?(method, :!)
+              problem "Use `refute_predicate <path_to_file>, :exist?` instead of `#{method.source}`"
+            end
+
+            if method_called_ever?(method, :executable?) && !method_called_ever?(method, :!)
+              problem "Use `assert_predicate <path_to_file>, :executable?` instead of `#{method.source}`"
+            end
           end
 
           find_every_method_call_by_name(body_node, :depends_on).each do |method|
@@ -259,16 +271,15 @@ module RuboCop
 
           find_instance_method_call(body_node, "Dir", :[]) do |method|
             path = parameters(method).first
-            next if !path.str_type?
+            next unless path.str_type?
             next unless match = regex_match_group(path, /^[^\*{},]+$/)
             problem "Dir([\"#{string_content(path)}\"]) is unnecessary; just use \"#{match[0]}\""
           end
 
-
+          fileutils_methods = Regexp.new(FileUtils.singleton_methods(false).map { |m| "(?-mix:^" + Regexp.escape(m) + "$)" }.join("|"))
-          fileUtils_methods= Regexp.new(FileUtils.singleton_methods(false).map { |m| '(?-mix:^'+Regexp.escape(m)+'$)' }.join '|')
           find_every_method_call_by_name(body_node, :system).each do |method|
             param = parameters(method).first
-            next unless match = regex_match_group(param, fileUtils_methods)
+            next unless match = regex_match_group(param, fileutils_methods)
             problem "Use the `#{match}` Ruby method instead of `#{method.source}`"
           end
 
@@ -300,25 +311,25 @@ module RuboCop
 
           find_instance_method_call(body_node, :build, :without?) do |method|
             arg = parameters(method).first
-            next unless match = regex_match_group(arg, %r{-?-?without-(.*)})
+            next unless match = regex_match_group(arg, /-?-?without-(.*)/)
             problem "Don't duplicate 'without': Use `build.without? \"#{match[1]}\"` to check for \"--without-#{match[1]}\""
           end
 
           find_instance_method_call(body_node, :build, :with?) do |method|
             arg = parameters(method).first
-            next unless match = regex_match_group(arg, %r{-?-?with-(.*)})
+            next unless match = regex_match_group(arg, /-?-?with-(.*)/)
             problem "Don't duplicate 'with': Use `build.with? \"#{match[1]}\"` to check for \"--with-#{match[1]}\""
           end
 
           find_instance_method_call(body_node, :build, :include?) do |method|
             arg = parameters(method).first
-            next unless match = regex_match_group(arg, %r{with(out)?-(.*)})
+            next unless match = regex_match_group(arg, /with(out)?-(.*)/)
             problem "Use build.with#{match[1]}? \"#{match[2]}\" instead of build.include? 'with#{match[1]}-#{match[2]}'"
           end
 
           find_instance_method_call(body_node, :build, :include?) do |method|
             arg = parameters(method).first
-            next unless match = regex_match_group(arg, %r{\-\-(.*)})
+            next unless match = regex_match_group(arg, /\-\-(.*)/)
             problem "Reference '#{match[1]}' without dashes"
           end
         end

Library/Homebrew/test/cmd/style_spec.rb

@@ -4,12 +4,12 @@ describe "brew style" do
   around(:each) do |example|
     begin
       FileUtils.ln_s HOMEBREW_LIBRARY_PATH, HOMEBREW_LIBRARY/"Homebrew"
-      FileUtils.ln_s HOMEBREW_LIBRARY_PATH.parent/".rubocop.yml", HOMEBREW_LIBRARY/".auditcops.yml"
+      FileUtils.ln_s HOMEBREW_LIBRARY_PATH.parent/".rubocop.yml", HOMEBREW_LIBRARY/".rubocop_audit.yml"
 
       example.run
     ensure
       FileUtils.rm_f HOMEBREW_LIBRARY/"Homebrew"
-      FileUtils.rm_f HOMEBREW_LIBRARY/".auditcops.yml"
+      FileUtils.rm_f HOMEBREW_LIBRARY/".rubocop_audit.yml"
     end
   end
 

Library/Homebrew/test/rubocops/formula_desc_cop_spec.rb

@@ -27,6 +27,27 @@ describe RuboCop::Cop::FormulaAuditStrict::DescLength do
       end
     end
 
+    it "reports an offense when desc is an empty string" do
+      source = <<-EOS.undent
+        class Foo < Formula
+          url 'http://example.com/foo-1.0.tgz'
+          desc ''
+        end
+      EOS
+
+      msg = "The desc (description) should not be an empty string."
+      expected_offenses = [{ message: msg,
+                             severity: :convention,
+                             line: 3,
+                             column: 2,
+                             source: source }]
+
+      inspect_source(source, "/homebrew-core/Formula/foo.rb")
+      expected_offenses.zip(cop.offenses).each do |expected, actual|
+        expect_offense(expected, actual)
+      end
+    end
+
     it "When desc is too long" do
       source = <<-EOS.undent
         class Foo < Formula

Library/Homebrew/test/rubocops/lines_cop_spec.rb

@@ -70,7 +70,7 @@ describe RuboCop::Cop::FormulaAudit::ClassInheritance do
                               column: 10,
                               source: source }]
 
-      inspect_source(source, '/homebrew-core/Formula/foo.rb')
+      inspect_source(source, "/homebrew-core/Formula/foo.rb")
 
       expected_offenses.zip(cop.offenses).each do |expected, actual|
         expect_offense(expected, actual)
@@ -535,6 +535,72 @@ describe RuboCop::Cop::FormulaAudit::Miscellaneous do
       end
     end
 
+    it "assert ...exist? without a negation" do
+      source = <<-EOS.undent
+        class Foo < Formula
+          desc "foo"
+          url 'http://example.com/foo-1.0.tgz'
+          assert File.exist? "default.ini"
+        end
+      EOS
+
+      expected_offenses = [{  message: 'Use `assert_predicate <path_to_file>, :exist?` instead of `assert File.exist? "default.ini"`',
+                              severity: :convention,
+                              line: 4,
+                              column: 9,
+                              source: source }]
+
+      inspect_source(source)
+
+      expected_offenses.zip(cop.offenses).each do |expected, actual|
+        expect_offense(expected, actual)
+      end
+    end
+
+    it "assert ...exist? with a negation" do
+      source = <<-EOS.undent
+        class Foo < Formula
+          desc "foo"
+          url 'http://example.com/foo-1.0.tgz'
+          assert !File.exist?("default.ini")
+        end
+      EOS
+
+      expected_offenses = [{  message: 'Use `refute_predicate <path_to_file>, :exist?` instead of `assert !File.exist?("default.ini")`',
+                              severity: :convention,
+                              line: 4,
+                              column: 9,
+                              source: source }]
+
+      inspect_source(source)
+
+      expected_offenses.zip(cop.offenses).each do |expected, actual|
+        expect_offense(expected, actual)
+      end
+    end
+
+    it "assert ...executable? without a negation" do
+      source = <<-EOS.undent
+        class Foo < Formula
+          desc "foo"
+          url 'http://example.com/foo-1.0.tgz'
+          assert File.executable? f
+        end
+      EOS
+
+      expected_offenses = [{  message: "Use `assert_predicate <path_to_file>, :executable?` instead of `assert File.executable? f`",
+                              severity: :convention,
+                              line: 4,
+                              column: 9,
+                              source: source }]
+
+      inspect_source(source)
+
+      expected_offenses.zip(cop.offenses).each do |expected, actual|
+        expect_offense(expected, actual)
+      end
+    end
+
     it "depends_on with an instance as an argument" do
       source = <<-EOS.undent
         class Foo < Formula

docs/External-Commands.md

@@ -44,12 +44,12 @@ Note they are largely untested, and as always, be careful about running untested
 
 ### brew-livecheck
 Check if there is a new upstream version of a formula.
-See the [`README`](https://github.com/youtux/homebrew-livecheck/blob/master/README.md) for more info and usage.
+See the [`README`](https://github.com/Homebrew/homebrew-livecheck/blob/master/README.md) for more info and usage.
 
 Install using:
 
 ```sh
-brew tap youtux/livecheck
+brew tap homebrew/livecheck
 ```
 
 ### brew-gem