diff --git a/Library/.rubocop.yml b/Library/.rubocop.yml index ed31de5954..c51d02a4bd 100644 --- a/Library/.rubocop.yml +++ b/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: - - '**/bin/**/*' - - '**/cmd/**/*' - - '**/lib/**/*' + Max: 367 + +Metrics/ParameterLists: + 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 diff --git a/Library/.auditcops.yml b/Library/.rubocop_audit.yml similarity index 100% rename from Library/.auditcops.yml rename to Library/.rubocop_audit.yml diff --git a/Library/Homebrew/.rubocop.yml b/Library/Homebrew/.rubocop.yml index 1434686432..0e1fb2d041 100644 --- a/Library/Homebrew/.rubocop.yml +++ b/Library/Homebrew/.rubocop.yml @@ -25,14 +25,20 @@ Lint/NestedMethodDefinition: Lint/ParenthesesAsGroupedExpression: Enabled: true +Metrics/BlockLength: + Max: 1250 + Metrics/BlockNesting: Max: 5 -Metrics/ModuleLength: - Max: 360 +Metrics/ClassLength: + Max: 1226 -Metrics/ParameterLists: - CountKeywordArgs: false +Metrics/LineLength: + Max: 244 + +Metrics/MethodLength: + Max: 195 # we won't change backward compatible method names Naming/MethodName: diff --git a/Library/Homebrew/brew.rb b/Library/Homebrew/brew.rb index 86b40a79da..8c5386612c 100644 --- a/Library/Homebrew/brew.rb +++ b/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" diff --git a/Library/Homebrew/cask/lib/hbc/cli/edit.rb b/Library/Homebrew/cask/lib/hbc/cli/edit.rb index 8bce81c523..693edcd515 100644 --- a/Library/Homebrew/cask/lib/hbc/cli/edit.rb +++ b/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 diff --git a/Library/Homebrew/cask/lib/hbc/container/dmg.rb b/Library/Homebrew/cask/lib/hbc/container/dmg.rb index 1d172a4b7b..c0e43f68a0 100644 --- a/Library/Homebrew/cask/lib/hbc/container/dmg.rb +++ b/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 - Dir.glob("**/*", File::FNM_DOTMATCH).map do |path| - next if skip_path?(Pathname(path)) - (path == ".") ? path : path.prepend("./") - end.compact.join("\n").concat("\n") - end + # We need to use `find` here instead of Ruby in order to properly handle + # file names containing special characters, such as “e” + “´” vs. “é”. + @command.run("/usr/bin/find", args: [".", "-print0"], chdir: mount, print_stderr: false).stdout + .split("\0") + .reject { |path| skip_path?(mount, path) } + .join("\n") end - def skip_path?(path) - dmg_metadata?(path) || system_dir_symlink?(path) + def skip_path?(mount, 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) diff --git a/Library/Homebrew/cask/lib/hbc/system_command.rb b/Library/Homebrew/cask/lib/hbc/system_command.rb index be083c29ef..9ce3de907b 100644 --- a/Library/Homebrew/cask/lib/hbc/system_command.rb +++ b/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 = {}) - new(executable, options).run! + def self.run(executable, **options) + new(executable, **options).run! end - def self.run!(command, options = {}) - run(command, options.merge(must_succeed: true)) + def self.run!(command, **options) + 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! - options.extend(HashValidator) - .assert_valid_keys :input, :print_stdout, :print_stderr, :args, :must_succeed, :sudo - sudo_prefix = %w[/usr/bin/sudo -E --] - sudo_prefix = sudo_prefix.insert(1, "-A") unless ENV["SUDO_ASKPASS"].nil? - @command = [executable] - 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 + attr_predicate :sudo?, :print_stdout?, :print_stderr?, :must_succeed? + + def sudo_prefix + return [] unless sudo? + askpass_flags = ENV.key?("SUDO_ASKPASS") ? ["-A"] : [] + ["/usr/bin/sudo", *askpass_flags, "-E", "--"] 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) diff --git a/Library/Homebrew/cmd/style.rb b/Library/Homebrew/cmd/style.rb index 58f430a27d..89484d67d9 100644 --- a/Library/Homebrew/cmd/style.rb +++ b/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 diff --git a/Library/Homebrew/compat/formula.rb b/Library/Homebrew/compat/formula.rb index 853a387069..57ab84a760 100644 --- a/Library/Homebrew/compat/formula.rb +++ b/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 diff --git a/Library/Homebrew/dev-cmd/pull.rb b/Library/Homebrew/dev-cmd/pull.rb index 8cb2703035..aa3c9a9d71 100644 --- a/Library/Homebrew/dev-cmd/pull.rb +++ b/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})!" diff --git a/Library/Homebrew/extend/fileutils.rb b/Library/Homebrew/extend/fileutils.rb index ed5bfe6c33..34ef3869fe 100644 --- a/Library/Homebrew/extend/fileutils.rb +++ b/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. diff --git a/Library/Homebrew/os/mac.rb b/Library/Homebrew/os/mac.rb index 853f75140f..9dbb252e4c 100644 --- a/Library/Homebrew/os/mac.rb +++ b/Library/Homebrew/os/mac.rb @@ -47,19 +47,11 @@ module OS end def languages - return @languages unless @languages.nil? - - @languages = Utils.popen_read("defaults", "read", ".GlobalPreferences", "AppleLanguages").scan(/[^ \n"(),]+/) - - if ENV["HOMEBREW_LANGUAGES"] - @languages = ENV["HOMEBREW_LANGUAGES"].split(",") + @languages - end - - if ARGV.value("language") - @languages = ARGV.value("language").split(",") + @languages - end - - @languages = @languages.uniq + @languages ||= [ + *ARGV.value("language")&.split(","), + *ENV["HOMEBREW_LANGUAGES"]&.split(","), + *Open3.capture2("defaults", "read", "-g", "AppleLanguages")[0].scan(/[^ \n"(),]+/), + ].uniq end def language diff --git a/Library/Homebrew/rubocops/formula_desc_cop.rb b/Library/Homebrew/rubocops/formula_desc_cop.rb index 2b613c9b49..2ef60303dc 100644 --- a/Library/Homebrew/rubocops/formula_desc_cop.rb +++ b/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 diff --git a/Library/Homebrew/rubocops/lines_cop.rb b/Library/Homebrew/rubocops/lines_cop.rb index 97bc45bc01..0491845b5d 100644 --- a/Library/Homebrew/rubocops/lines_cop.rb +++ b/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 unless match = regex_match_group(value, %r{(lua|perl|python|ruby)(\d*)}) + next if key.nil? || value.nil? + 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 , :exist?` instead of `#{method.source}`" + end + + if method_called_ever?(method, :exist?) && method_called_ever?(method, :!) + problem "Use `refute_predicate , :exist?` instead of `#{method.source}`" + end + + if method_called_ever?(method, :executable?) && !method_called_ever?(method, :!) + problem "Use `assert_predicate , :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 diff --git a/Library/Homebrew/test/cmd/style_spec.rb b/Library/Homebrew/test/cmd/style_spec.rb index 4701036f10..9bc8fcab15 100644 --- a/Library/Homebrew/test/cmd/style_spec.rb +++ b/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 diff --git a/Library/Homebrew/test/rubocops/formula_desc_cop_spec.rb b/Library/Homebrew/test/rubocops/formula_desc_cop_spec.rb index ac8893e180..4816c3b269 100644 --- a/Library/Homebrew/test/rubocops/formula_desc_cop_spec.rb +++ b/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 diff --git a/Library/Homebrew/test/rubocops/lines_cop_spec.rb b/Library/Homebrew/test/rubocops/lines_cop_spec.rb index 962827ca3a..df40857216 100644 --- a/Library/Homebrew/test/rubocops/lines_cop_spec.rb +++ b/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 , :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 , :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 , :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 diff --git a/docs/External-Commands.md b/docs/External-Commands.md index e1178ceda1..881a1293a2 100644 --- a/docs/External-Commands.md +++ b/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