Merge pull request #7168 from MikeMcQuaid/rubocop-tweaks

rubocop: adjust rules.

Library/.rubocop.yml

@@ -21,10 +21,6 @@ Layout/HashAlignment:
 Layout/ArgumentAlignment:
   Enabled: false
 
-# favour parens-less DSL-style arguments
-Lint/AmbiguousOperator:
-  Enabled: false
-
 # this is a bit less "floaty"
 Layout/CaseIndentation:
   EnforcedStyle: end
@@ -37,8 +33,7 @@ Layout/EndAlignment:
 Layout/SpaceAroundOperators:
   Enabled: false
 
-# Auto-correct is broken (https://github.com/rubocop-hq/rubocop/issues/6258)
+# layout is not configurable (https://github.com/rubocop-hq/rubocop/issues/6254).
-# and layout is not configurable (https://github.com/rubocop-hq/rubocop/issues/6254).
 Layout/RescueEnsureAlignment:
   Enabled: false
 
@@ -51,15 +46,6 @@ Lint/AmbiguousBlockAssociation:
 Lint/AmbiguousRegexpLiteral:
   Enabled: false
 
-# assignment in conditions are useful sometimes
-# TODO: add parentheses for these and remove
-Lint/AssignmentInCondition:
-  Enabled: false
-
-# we output how to use interpolated strings too often
-Lint/InterpolationCheck:
-  Enabled: false
-
 # so many of these in formulae and can't be autocorrected
 Lint/ParenthesesAsGroupedExpression:
   Enabled: false
@@ -84,11 +70,6 @@ Layout/LineLength:
   # ignore manpage comments and long single-line strings
   IgnoredPatterns: ['#: ', ' url "', ' mirror "', ' plist_options :']
 
-# our current conditional style is established
-# TODO: enable this when possible
-Style/ConditionalAssignment:
-  Enabled: false
-
 # most of our APIs are internal so don't require docs
 Style/Documentation:
  Enabled: false
@@ -97,7 +78,7 @@ Style/Documentation:
 Style/FrozenStringLiteralComment:
   Enabled: false
 
-# so many of these in formulae and can't be autocorrected
+# potential for errors in formulae too high with this
 Style/GuardClause:
   Enabled: false
 
@@ -110,6 +91,14 @@ Style/HashSyntax:
   - '**/lib/**/*.rb'
   - '**/spec/**/*.rb'
 
+# autocorrectable and more readable
+Style/HashEachMethods:
+  Enabled: true
+Style/HashTransformKeys:
+  Enabled: true
+Style/HashTransformValues:
+  Enabled: true
+
 # ruby style guide favorite
 Style/StringLiterals:
   EnforcedStyle: double_quotes

Library/Homebrew/.rubocop.yml

@@ -22,6 +22,10 @@ Layout/MultilineMethodCallIndentation:
 Lint/AmbiguousRegexpLiteral:
   Enabled: true
 
+# TODO: add parentheses for these and remove
+Lint/AssignmentInCondition:
+  Enabled: false
+
 # `formula do` uses nested method definitions
 Lint/NestedMethodDefinition:
   Exclude:

Library/Homebrew/cask/audit.rb

@@ -309,10 +309,10 @@ module Cask
       appcast_contents, = curl_output("--compressed", "--user-agent", HOMEBREW_USER_AGENT_FAKE_SAFARI, "--location",
                                       "--globoff", "--max-time", "5", appcast_stanza)
       version_stanza = cask.version.to_s
-      if cask.appcast.configuration.blank?
+      adjusted_version_stanza = if cask.appcast.configuration.blank?
-        adjusted_version_stanza = version_stanza.split(",")[0].split("-")[0].split("_")[0]
+        version_stanza.split(",")[0].split("-")[0].split("_")[0]
       else
-        adjusted_version_stanza = cask.appcast.configuration
+        cask.appcast.configuration
       end
       return if appcast_contents.include? adjusted_version_stanza
 

Library/Homebrew/cask/cmd/list.rb

@@ -47,7 +47,7 @@ module Cask
         elsif versions?
           puts installed_casks.map(&self.class.method(:format_versioned))
         elsif full_name?
-          puts installed_casks.map(&:full_name).sort &tap_and_name_comparison
+          puts installed_casks.map(&:full_name).sort(&tap_and_name_comparison)
         elsif !installed_casks.empty?
           puts Formatter.columns(installed_casks.map(&:to_s))
         end

Library/Homebrew/cli/parser.rb

@@ -103,10 +103,10 @@ module Homebrew
       end
 
       def flag(*names, description: nil, required_for: nil, depends_on: nil)
-        if names.any? { |name| name.end_with? "=" }
+        required = if names.any? { |name| name.end_with? "=" }
-          required = OptionParser::REQUIRED_ARGUMENT
+          OptionParser::REQUIRED_ARGUMENT
         else
-          required = OptionParser::OPTIONAL_ARGUMENT
+          OptionParser::OPTIONAL_ARGUMENT
         end
         names.map! { |name| name.chomp "=" }
         description = option_to_description(*names) if description.nil?

Library/Homebrew/cmd/gist-logs.rb

@@ -63,10 +63,10 @@ module Homebrew
     end
 
     # Description formatted to work well as page title when viewing gist
-    if f.core_formula?
+    descr = if f.core_formula?
-      descr = "#{f.name} on #{OS_VERSION} - Homebrew build logs"
+      "#{f.name} on #{OS_VERSION} - Homebrew build logs"
     else
-      descr = "#{f.name} (#{f.full_name}) on #{OS_VERSION} - Homebrew build logs"
+      "#{f.name} (#{f.full_name}) on #{OS_VERSION} - Homebrew build logs"
     end
     url = create_gist(files, descr)
 

Library/Homebrew/cmd/sh.rb

@@ -40,10 +40,10 @@ module Homebrew
       # superenv stopped adding brew's bin but generally users will want it
       ENV["PATH"] = PATH.new(ENV["PATH"]).insert(1, HOMEBREW_PREFIX/"bin")
     end
-    if ENV["SHELL"].include?("zsh")
+    ENV["PS1"] = if ENV["SHELL"].include?("zsh")
-      ENV["PS1"] = "brew %B%F{green}~%f%b$ "
+      "brew %B%F{green}~%f%b$ "
     else
-      ENV["PS1"] = 'brew \[\033[1;32m\]\w\[\033[0m\]$ '
+      'brew \[\033[1;32m\]\w\[\033[0m\]$ '
     end
     ENV["VERBOSE"] = "1"
     puts <<~EOS

Library/Homebrew/cmd/tap-info.rb

@@ -27,10 +27,10 @@ module Homebrew
   def tap_info
     tap_info_args.parse
 
-    if args.installed?
+    taps = if args.installed?
-      taps = Tap
+      Tap
     else
-      taps = args.named.sort.map do |name|
+      args.named.sort.map do |name|
         Tap.fetch(name)
       end
     end

Library/Homebrew/cmd/update-report.rb

@@ -243,10 +243,10 @@ class Reporter
       new_name = tap.formula_renames[old_name]
       next unless new_name
 
-      if tap.core_tap?
+      new_full_name = if tap.core_tap?
-        new_full_name = new_name
+        new_name
       else
-        new_full_name = "#{tap}/#{new_name}"
+        "#{tap}/#{new_name}"
       end
 
       renamed_formulae << [old_full_name, new_full_name] if @report[:A].include? new_full_name
@@ -257,10 +257,10 @@ class Reporter
       old_name = tap.formula_renames.key(new_name)
       next unless old_name
 
-      if tap.core_tap?
+      old_full_name = if tap.core_tap?
-        old_full_name = old_name
+        old_name
       else
-        old_full_name = "#{tap}/#{old_name}"
+        "#{tap}/#{old_name}"
       end
 
       renamed_formulae << [old_full_name, new_full_name]

Library/Homebrew/dev-cmd/bottle.rb

@@ -306,10 +306,10 @@ module Homebrew
 
         ohai "Detecting if #{filename} is relocatable..." if bottle_path.size > 1 * 1024 * 1024
 
-        if Homebrew.default_prefix?(prefix)
+        prefix_check = if Homebrew.default_prefix?(prefix)
-          prefix_check = File.join(prefix, "opt")
+          File.join(prefix, "opt")
         else
-          prefix_check = prefix
+          prefix
         end
 
         # Ignore matches to source code, which is not required at run time.

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

@@ -275,18 +275,18 @@ module Homebrew
 
     if forced_version && forced_version != "0"
       if requested_spec == :stable
-        if File.read(formula.path).include?("version \"#{old_formula_version}\"")
+        replacement_pairs << if File.read(formula.path).include?("version \"#{old_formula_version}\"")
-          replacement_pairs << [
+          [
             old_formula_version.to_s,
             forced_version,
           ]
         elsif new_mirrors
-          replacement_pairs << [
+          [
             /^( +)(mirror \"#{Regexp.escape(new_mirrors.last)}\"\n)/m,
             "\\1\\2\\1version \"#{forced_version}\"\n",
           ]
         else
-          replacement_pairs << [
+          [
             /^( +)(url \"#{Regexp.escape(new_url)}\"\n)/m,
             "\\1\\2\\1version \"#{forced_version}\"\n",
           ]

Library/Homebrew/dev-cmd/bump-revision.rb

@@ -45,14 +45,14 @@ module Homebrew
         [checksum.hash_type, checksum.hexdigest]
       end
 
-      if hash_type
+      old = if hash_type
         # insert replacement revision after hash
-        old = <<~EOS
+        <<~EOS
           #{hash_type} "#{old_hash}"
         EOS
       else
         # insert replacement revision after :revision
-        old = <<~EOS
+        <<~EOS
           :revision => "#{formula_spec.specs[:revision]}"
         EOS
       end

Library/Homebrew/dev-cmd/pull.rb

@@ -354,10 +354,10 @@ module Homebrew
       patch_args = []
       # Normally we don't want whitespace errors, but squashing them can break
       # patches so an option is provided to skip this step.
-      if @args.ignore_whitespace? || @args.clean?
+      patch_args << if @args.ignore_whitespace? || @args.clean?
-        patch_args << "--whitespace=nowarn"
+        "--whitespace=nowarn"
       else
-        patch_args << "--whitespace=fix"
+        "--whitespace=fix"
       end
 
       # Fall back to three-way merge if patch does not apply cleanly

Library/Homebrew/extend/ENV/shared.rb

@@ -76,10 +76,10 @@ module SharedEnvExtension
     value = value.to_s
     Array(keys).each do |key|
       old = self[key]
-      if old.nil? || old.empty?
+      self[key] = if old.nil? || old.empty?
-        self[key] = value
+        value
       else
-        self[key] = value + separator + old
+        value + separator + old
       end
     end
   end

Library/Homebrew/extend/os/mac/extend/ENV/super.rb

@@ -54,10 +54,10 @@ module Superenv
   def homebrew_extra_library_paths
     paths = []
     if compiler == :llvm_clang
-      if !MacOS.sdk_path_if_needed
+      paths << if !MacOS.sdk_path_if_needed
-        paths << "/usr/lib"
+        "/usr/lib"
       else
-        paths << "#{MacOS.sdk_path}/usr/lib"
+        "#{MacOS.sdk_path}/usr/lib"
       end
       paths << Formula["llvm"].opt_lib.to_s
     end

Library/Homebrew/formula_creator.rb

@@ -27,10 +27,10 @@ module Homebrew
         end
       end
       update_path
-      if @version
+      @version = if @version
-        @version = Version.create(@version)
+        Version.create(@version)
       else
-        @version = Version.detect(url, {})
+        Version.detect(url, {})
       end
     end
 

Library/Homebrew/patch.rb

@@ -38,11 +38,11 @@ module Patch
       {}
     end.each_pair do |strip, urls|
       Array(urls).each do |url|
-        case url
+        patch = case url
         when :DATA
-          patch = DATAPatch.new(strip)
+          DATAPatch.new(strip)
         else
-          patch = LegacyPatch.new(strip, url)
+          LegacyPatch.new(strip, url)
         end
         patches << patch
       end

Library/Homebrew/requirements/java_requirement.rb

@@ -46,10 +46,10 @@ class JavaRequirement < Requirement
 
   def display_s
     if @version
-      if exact_version?
+      op = if exact_version?
-        op = "="
+        "="
       else
-        op = ">="
+        ">="
       end
       "#{name} #{op} #{version_without_plus}"
     else

Library/Homebrew/rubocops/class.rb

@@ -49,9 +49,10 @@ module RuboCop
           lambda do |corrector|
             case node.type
             when :str, :dstr
+              # Rubocop: intentionally outputted non-interpolated strings
               corrector.replace(node.source_range,
                                 node.source.to_s.sub(%r{(/usr/local/(s?bin))},
-                                                     '#{\2}'))
+                                                     '#{\2}')) # rubocop:disable Lint/InterpolationCheck
             when :int
               corrector.remove(
                 range_with_surrounding_comma(

Library/Homebrew/rubocops/extend/formula.rb

@@ -40,10 +40,10 @@ module RuboCop
 
         node_begin_pos = start_column(node)
         line_begin_pos = line_start_column(node)
-        if node_begin_pos == line_begin_pos
+        @column = if node_begin_pos == line_begin_pos
-          @column = node_begin_pos + match_object.begin(0) - line_begin_pos
+          node_begin_pos + match_object.begin(0) - line_begin_pos
         else
-          @column = node_begin_pos + match_object.begin(0) - line_begin_pos + 1
+          node_begin_pos + match_object.begin(0) - line_begin_pos + 1
         end
         @length = match_object.to_s.length
         @line_no = line_number(node)
@@ -200,10 +200,10 @@ module RuboCop
       # Returns true if given dependency name and dependency type exist in given dependency method call node.
       # TODO: Add case where key of hash is an array
       def depends_on_name_type?(node, name = nil, type = :required)
-        if name
+        name_match = if name
-          name_match = false
+          false
         else
-          name_match = true # Match only by type when name is nil
+          true # Match only by type when name is nil
         end
 
         case type

Library/Homebrew/style.rb

@@ -24,10 +24,10 @@ module Homebrew
       args = %w[
         --force-exclusion
       ]
-      if fix
+      args << if fix
-        args << "--auto-correct"
+        "--auto-correct"
       else
-        args << "--parallel"
+        "--parallel"
       end
 
       args += ["--extra-details", "--display-cop-names"] if Homebrew.args.verbose?

Library/Homebrew/tab.rb

@@ -81,10 +81,10 @@ class Tab < OpenStruct
 
     if attributes["source"]["spec"].nil?
       version = PkgVersion.parse path.to_s.split("/").second_to_last
-      if version.head?
+      attributes["source"]["spec"] = if version.head?
-        attributes["source"]["spec"] = "head"
+        "head"
       else
-        attributes["source"]["spec"] = "stable"
+        "stable"
       end
     end
 
@@ -360,10 +360,10 @@ class Tab < OpenStruct
 
   def to_s
     s = []
-    if poured_from_bottle
+    s << if poured_from_bottle
-      s << "Poured from bottle"
+      "Poured from bottle"
     else
-      s << "Built from source"
+      "Built from source"
     end
 
     s << Time.at(time).strftime("on %Y-%m-%d at %H:%M:%S") if time

Library/Homebrew/test/dev-cmd/audit_spec.rb

@@ -187,6 +187,8 @@ module Homebrew
       end
     end
 
+    # Intentionally outputted non-interpolated strings
+    # rubocop:disable Lint/InterpolationCheck
     describe "#line_problems" do
       specify "pkgshare" do
         fa = formula_auditor "foo", <<~RUBY, strict: true
@@ -237,6 +239,7 @@ module Homebrew
           .to eq('Use pkgshare instead of (share/"foolibc++")')
       end
     end
+    # rubocop:enable Lint/InterpolationCheck
 
     describe "#audit_github_repository" do
       specify "#audit_github_repository when HOMEBREW_NO_GITHUB_API is set" do

Library/Homebrew/utils/bottles.rb

@@ -46,10 +46,10 @@ module Utils
         name = receipt_file_path.split("/").first
         tap = Tab.from_file_content(receipt_file, "#{bottle_file}/#{receipt_file_path}").tap
 
-        if tap.nil? || tap.core_tap?
+        full_name = if tap.nil? || tap.core_tap?
-          full_name = name
+          name
         else
-          full_name = "#{tap}/#{name}"
+          "#{tap}/#{name}"
         end
 
         [name, full_name]

Library/Homebrew/utils/github.rb

@@ -48,13 +48,13 @@ module GitHub
     def initialize(github_message)
       @github_message = github_message
       message = +"GitHub #{github_message}:"
-      if ENV["HOMEBREW_GITHUB_API_TOKEN"]
+      message << if ENV["HOMEBREW_GITHUB_API_TOKEN"]
-        message << <<~EOS
+        <<~EOS
           HOMEBREW_GITHUB_API_TOKEN may be invalid or expired; check:
             #{Formatter.url("https://github.com/settings/tokens")}
         EOS
       else
-        message << <<~EOS
+        <<~EOS
           The GitHub credentials in the macOS keychain may be invalid.
           Clear them with:
             printf "protocol=https\\nhost=github.com\\n" | git credential-osxkeychain erase