Merge pull request #3183 from MikeMcQuaid/rubocop-upgrade

Rubocop: 0.50.0 and Ruby 2.3

Library/.rubocop.yml

@@ -1,5 +1,5 @@
 AllCops:
-  TargetRubyVersion: 2.0
+  TargetRubyVersion: 2.3
   Exclude:
     - '**/Casks/**/*'
     - '**/vendor/**/*'
@@ -119,7 +119,7 @@ Style/Encoding:
   Enabled: true
 
 # dashes in filenames are typical
-Style/FileName:
+Naming/FileName:
   Regex: !ruby/regexp /^[\w\@\-\+\.]+(\.rb)?$/
 
 # falsely flags e.g. curl formatting arguments as format strings
@@ -189,8 +189,25 @@ Style/TrailingCommaInArguments:
   EnforcedStyleForMultiline: comma
 
 # we have too many variables like sha256 where this harms readability
-Style/VariableNumber:
+Naming/VariableNumber:
   Enabled: false
 
 Style/WordArray:
   MinSize: 4
+
+# we want to add this slowly and manually
+Style/FrozenStringLiteralComment:
+  Enabled: false
+
+# generally rescuing StandardError is fine
+Lint/RescueWithoutErrorClass:
+  Enabled: false
+
+# implicitly allow EOS as we use it everywhere
+Naming/HeredocDelimiterNaming:
+  Blacklist:
+    - END, EOD, EOF
+
+# we output how to use interpolated strings too often
+Lint/InterpolationCheck:
+  Enabled: false

Library/Homebrew/.rubocop.yml

@@ -42,12 +42,12 @@ Style/HashSyntax:
   EnforcedStyle: ruby19_no_mixed_keys
 
 # we won't change backward compatible method names
-Style/MethodName:
+Naming/MethodName:
   Exclude:
     - 'compat/**/*'
 
 # we won't change backward compatible predicate names
-Style/PredicateName:
+Naming/PredicateName:
   Exclude:
     - 'compat/**/*'
   NameWhitelist: is_32_bit?, is_64_bit?

Library/Homebrew/.rubocop_todo.yml

@@ -81,7 +81,7 @@ Security/MarshalLoad:
     - 'utils/fork.rb'
 
 # Offense count: 1
-Style/AccessorMethodName:
+Naming/AccessorMethodName:
   Exclude:
     - 'extend/ENV/super.rb'
 
@@ -136,10 +136,3 @@ Style/MutableConstant:
     - 'formulary.rb'
     - 'tab.rb'
     - 'tap.rb'
-
-# Offense count: 8
-Style/OpMethod:
-  Exclude:
-    - 'dependencies.rb'
-    - 'install_renamed.rb'
-    - 'options.rb'

Library/Homebrew/brew.rb

@@ -105,18 +105,16 @@
     possible_tap = OFFICIAL_CMD_TAPS.find { |_, cmds| cmds.include?(cmd) }
     possible_tap = Tap.fetch(possible_tap.first) if possible_tap
 
-    if possible_tap && !possible_tap.installed?
-      brew_uid = HOMEBREW_BREW_FILE.stat.uid
-      tap_commands = []
-      if Process.uid.zero? && !brew_uid.zero?
-        tap_commands += %W[/usr/bin/sudo -u ##{brew_uid}]
-      end
-      tap_commands += %W[#{HOMEBREW_BREW_FILE} tap #{possible_tap}]
-      safe_system(*tap_commands)
-      exec HOMEBREW_BREW_FILE, cmd, *ARGV
-    else
-      odie "Unknown command: #{cmd}"
+    odie "Unknown command: #{cmd}" if !possible_tap || possible_tap.installed?
+
+    brew_uid = HOMEBREW_BREW_FILE.stat.uid
+    tap_commands = []
+    if Process.uid.zero? && !brew_uid.zero?
+      tap_commands += %W[/usr/bin/sudo -u ##{brew_uid}]
     end
+    tap_commands += %W[#{HOMEBREW_BREW_FILE} tap #{possible_tap}]
+    safe_system(*tap_commands)
+    exec HOMEBREW_BREW_FILE, cmd, *ARGV
   end
 rescue UsageError => e
   require "cmd/help"

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

@@ -56,7 +56,7 @@ def cask(header_token, **options, &block)
 
     class FromURILoader < FromPathLoader
       def self.can_load?(ref)
-        ref.to_s.match?(::URI.regexp)
+        ref.to_s.match?(::URI::DEFAULT_PARSER.make_regexp)
       end
 
       attr_reader :url

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

@@ -16,7 +16,7 @@ def extract
 
       def target_file
         return @path.basename if @nested
-        URI.decode(File.basename(@cask.url.path))
+        CGI.unescape(File.basename(@cask.url.path))
       end
     end
   end

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

@@ -161,7 +161,7 @@ def container(*args)
         begin
           DSL::Container.new(*args).tap do |container|
             # TODO: remove this backward-compatibility section after removing nested_container
-            if container && container.nested
+            if container&.nested
               artifacts[:nested_container] << Artifact::NestedContainer.new(cask, container.nested)
             end
           end

Library/Homebrew/cask/lib/hbc/dsl/version.rb

@@ -49,7 +49,7 @@ def conversion_method_name(left_divider, right_divider)
         end
       end
 
-      DIVIDERS.keys.each do |divider|
+      DIVIDERS.each_key do |divider|
         define_divider_methods(divider)
       end
 

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

@@ -159,7 +159,7 @@ def extract_primary_container
       odebug "Extracting primary container"
 
       FileUtils.mkdir_p @cask.staged_path
-      container = if @cask.container && @cask.container.type
+      container = if @cask.container&.type
         Container.from_type(@cask.container.type)
       else
         Container.for_path(@downloaded_path, @command)
@@ -361,7 +361,7 @@ def save_caskfile
 
       savedir = @cask.metadata_subdir("Casks", timestamp: :now, create: true)
       FileUtils.copy @cask.sourcefile_path, savedir
-      old_savedir.rmtree unless old_savedir.nil?
+      old_savedir&.rmtree
     end
 
     def uninstall

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

@@ -61,7 +61,7 @@ def process_options!
     end
 
     def assert_success
-      return if processed_status && processed_status.success?
+      return if processed_status&.success?
       raise CaskCommandFailedError.new(command, processed_output[:stdout], processed_output[:stderr], processed_status)
     end
 

Library/Homebrew/caveats.rb

@@ -163,7 +163,7 @@ def elisp_caveats
 
   def plist_caveats
     s = []
-    if f.plist || (keg && keg.plist_installed?)
+    if f.plist || (keg&.plist_installed?)
       plist_domain = f.plist_path.basename(".plist")
 
       # we readlink because this path probably doesn't exist since caveats

Library/Homebrew/cmd/prune.rb

@@ -55,7 +55,7 @@ def prune
       else
         n, d = ObserverPathnameExtension.counts
         print "Pruned #{n} symbolic links "
-        print "and #{d} directories " if d > 0
+        print "and #{d} directories " if d.positive?
         puts "from #{HOMEBREW_PREFIX}"
       end
     end

Library/Homebrew/cmd/search.rb

@@ -67,7 +67,7 @@ def search
 
         ohai "Searching blacklisted, migrated and deleted formulae..."
         if reason = Homebrew::MissingFormula.reason(query, silent: true)
-          if count > 0
+          if count.positive?
             puts
             puts "If you meant #{query.inspect} specifically:"
           end

Library/Homebrew/cmd/tap-info.rb

@@ -64,10 +64,10 @@ def print_tap_info(taps)
         if tap.installed?
           info += tap.pinned? ? "pinned" : "unpinned"
           info += ", private" if tap.private?
-          if (formula_count = tap.formula_files.size) > 0
+          if (formula_count = tap.formula_files.size).positive?
             info += ", #{Formatter.pluralize(formula_count, "formula")}"
           end
-          if (command_count = tap.command_files.size) > 0
+          if (command_count = tap.command_files.size).positive?
             info += ", #{Formatter.pluralize(command_count, "command")}"
           end
           info += ", no formulae/commands" if (formula_count + command_count).zero?

Library/Homebrew/cmd/unlinkapps.rb

@@ -77,7 +77,7 @@ def unlinkapps_from_dir(target_dir, opts = {})
   def unlinkapps_unlink?(target_app, opts = {})
     # Skip non-symlinks and symlinks that don't point into the Homebrew prefix.
     app = target_app.readlink.to_s if target_app.symlink?
-    return false unless app && app.start_with?(*UNLINKAPPS_PREFIXES)
+    return false unless app&.start_with?(*UNLINKAPPS_PREFIXES)
 
     if opts.fetch(:prune, false)
       !File.exist?(app) # Remove only broken symlinks in prune mode.

Library/Homebrew/constants.rb

@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 # RuboCop version used for `brew style` and `brew cask style`
-HOMEBREW_RUBOCOP_VERSION = "0.49.1".freeze
-HOMEBREW_RUBOCOP_CASK_VERSION = "~> 0.13.1".freeze # has to be updated when RuboCop version changes
+HOMEBREW_RUBOCOP_VERSION = "0.50.0"
+HOMEBREW_RUBOCOP_CASK_VERSION = "~> 0.14.2" # has to be updated when RuboCop version changes

Library/Homebrew/debrew.rb

@@ -57,7 +57,7 @@ def self.choose
         input.chomp!
 
         i = input.to_i
-        if i > 0
+        if i.positive?
           choice = menu.entries[i - 1]
         else
           possible = menu.entries.find_all { |e| e.name.start_with?(input) }

Library/Homebrew/debrew/irb.rb

@@ -16,7 +16,7 @@ def start_within(binding)
       workspace = WorkSpace.new(binding)
       irb = Irb.new(workspace)
 
-      @CONF[:IRB_RC].call(irb.context) if @CONF[:IRB_RC]
+      @CONF[:IRB_RC]&.call(irb.context)
       @CONF[:MAIN_CONTEXT] = irb.context
 
       trap("SIGINT") do

Library/Homebrew/dependency.rb

@@ -51,7 +51,7 @@ def missing_options(inherited_options)
   end
 
   def modify_build_environment
-    env_proc.call unless env_proc.nil?
+    env_proc&.call
   end
 
   def inspect

Library/Homebrew/dev-cmd/audit.rb

@@ -358,7 +358,7 @@ def audit_file
       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
@@ -707,7 +707,7 @@ def audit_specs
     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+$/, "")
@@ -1018,7 +1018,7 @@ def line_problems(line, _lineno)
   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

Library/Homebrew/dev-cmd/bottle.rb

@@ -47,7 +47,7 @@
     <% elsif cellar != BottleSpecification::DEFAULT_CELLAR %>
     cellar "<%= cellar %>"
     <% end %>
-    <% if rebuild > 0 %>
+    <% if rebuild.positive? %>
     rebuild <%= rebuild %>
     <% end %>
     <% checksums.each do |checksum_type, checksum_values| %>
@@ -186,7 +186,7 @@ def bottle_formula(f)
       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 +283,7 @@ def bottle_formula(f)
         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

Library/Homebrew/dev-cmd/bump-formula-pr.rb

@@ -89,7 +89,7 @@ def fetch_pull_requests(formula)
 
   def check_for_duplicate_pull_requests(formula)
     pull_requests = fetch_pull_requests(formula)
-    return unless pull_requests && !pull_requests.empty?
+    return unless 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 +124,7 @@ def bump_formula_pr
       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

Library/Homebrew/dev-cmd/pull.rb

@@ -69,13 +69,13 @@ def pull
     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}"
@@ -350,7 +350,7 @@ def files_changed_in_patch(patchfile, tap)
       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

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"

Library/Homebrew/diagnostic.rb

@@ -522,7 +522,7 @@ def check_for_gettext
         homebrew_owned = @found.all? do |path|
           Pathname.new(path).realpath.to_s.start_with? "#{HOMEBREW_CELLAR}/gettext"
         end
-        return if gettext && gettext.linked_keg.directory? && homebrew_owned
+        return if gettext&.linked_keg&.directory? && homebrew_owned
 
         inject_file_list @found, <<-EOS.undent
           gettext files detected at a system prefix.
@@ -540,7 +540,7 @@ def check_for_iconv
         rescue
           nil
         end
-        if libiconv && libiconv.linked_keg.directory?
+        if libiconv&.linked_keg&.directory?
           unless libiconv.keg_only?
             <<-EOS.undent
               A libiconv formula is installed and linked.

Library/Homebrew/exceptions.rb

@@ -416,7 +416,7 @@ def dump
 
     puts
 
-    if issues && !issues.empty?
+    unless issues&.empty?
       puts "These open issues may also help:"
       puts issues.map { |i| "#{i["title"]} #{i["html_url"]}" }.join("\n")
     end