Merge pull request #22385 from Homebrew/rubocop-todo-cleanup

Remove RuboCop TODO suppressions

Author: MikeMcQuaid

.github/workflows/vendor-gems.yml

@@ -87,7 +87,7 @@ jobs:
       - name: Update RubyGems licence metadata
         working-directory: ${{ steps.set-up-homebrew.outputs.repository-path }}
         run: |
-          BUNDLE_WITH="$(brew ruby -- -e 'require "utils/gems"; puts Homebrew.valid_gem_groups.join(":")')"
+          BUNDLE_WITH="$(brew ruby -- -e 'require "utils/gem_setup"; puts Homebrew.valid_gem_groups.join(":")')"
           export BUNDLE_WITH
           licensed cache --config .github/licensed.yml
           licensed status --config .github/licensed.yml

.github/workflows/vendor-version.yml

@@ -62,7 +62,7 @@ jobs:
               git check-ignore --quiet "${gem_dir}" || exit_code=$?
               if (( exit_code != 0 )); then
                 if (( exit_code == 1 )); then
-                  echo "::error::VENDOR_VERSION needs bumping in utils/gems.rb" >&2
+                  echo "::error::VENDOR_VERSION needs bumping in utils/gem_setup.rb" >&2
                 else
                   echo "::error::git check-ignore failed" >&2
                 fi

Library/Homebrew/.rubocop.yml

@@ -1,19 +1,8 @@
 inherit_from:
   - ../.rubocop.yml
 
-Bundler/GemFilename:
-  Enabled: false
-
 Homebrew/MoveToExtendOS:
   Enabled: true
-  Exclude:
-    - "{test,requirements}/**/*"
-    - "os.rb"
-
-# We don't use Sorbet for RSpec tests so let's disable this there.
-Sorbet/BlockMethodDefinition:
-  Exclude:
-    - test/**/*
 
 # Want to preserve our own API for these methods for now.
 Naming/PredicatePrefix:
@@ -111,11 +100,6 @@ Style/Documentation:
     - version.rb
     - tap.rb
 
-Homebrew/NegateInclude:
-  Exclude:
-    # YARD runs stand-alone.
-    - yard/docstring_parser.rb
-
 Style/DocumentationMethod:
   Include:
     - "formula.rb"
@@ -124,8 +108,6 @@ Style/DocumentationMethod:
 # No Include needed: the cop self-selects by finding @api public comments.
 Homebrew/PublicApiDocumentation:
   Enabled: true
-  Exclude:
-    - "vendor/**/*"
 
 # Ensure cookbook-referenced methods are annotated @api public.
 # No Include needed: the cop self-selects from cookbook rubydoc links.

Library/Homebrew/dev-cmd/sh.rb

@@ -4,7 +4,7 @@
 require "abstract_command"
 require "extend/ENV"
 require "formula"
-require "utils/gems"
+require "utils/gem_setup"
 require "utils/shell"
 
 module Homebrew

Library/Homebrew/ignorable.rb

@@ -22,16 +22,13 @@ def ignore
 
   sig { params(blk: T.nilable(T.proc.void)).void }
   def self.hook_raise(&blk)
-    # TODO: migrate away from this inline class here, they don't play nicely with
-    #       Sorbet, when we migrate to `typed: strict`
-    # rubocop:todo Sorbet/BlockMethodDefinition
     Object.class_eval do
       alias_method :original_raise, :raise
 
-      sig { params(args: T.anything).void }
-      def raise(*args)
+      # `define_method` keeps Sorbet happy inside this `class_eval` block.
+      define_method(:raise) do |*args|
         callcc do |continuation|
-          super
+          super(*args)
         # Handle all possible exceptions.
         rescue Exception => e # rubocop:disable Lint/RescueException
           unless e.is_a?(ScriptError)
@@ -44,7 +41,6 @@ def raise(*args)
 
       alias_method :fail, :raise
     end
-    # rubocop:enable Sorbet/BlockMethodDefinition
 
     return unless block_given?
 

Library/Homebrew/rubocops/move_to_extend_os.rb

@@ -26,6 +26,11 @@ def extend_offense_message(extend_os, os_method)
         sig { params(node: RuboCop::AST::Node).void }
         def on_send(node)
           file_path = processed_source.file_path
+          # The OS loader, requirements and tests need direct host checks; this
+          # cop is for portable production code that should live under `extend/os`.
+          return if file_path.match?(%r{(?:\A|/)Library/Homebrew/(?:requirements|test)/}) ||
+                    file_path.match?(%r{(?:\A|/)Library/Homebrew/os\.rb\z})
+
           if file_path.include?("extend/os/mac/")
             add_offense(node, message: extend_offense_message("mac", "mac")) if os_mac?(node)
             add_offense(node, message: extend_offense_message("mac", "linux")) if os_linux?(node)

Library/Homebrew/standalone/init.rb

@@ -61,7 +61,7 @@ def self.from_rubylibdir(feature)
 HOMEBREW_BUNDLER_VERSION = ENV.fetch("HOMEBREW_BUNDLER_VERSION").freeze
 ENV["BUNDLER_VERSION"] = HOMEBREW_BUNDLER_VERSION
 
-require_relative "../utils/gems"
+require_relative "../utils/gem_setup"
 Homebrew.setup_gem_environment!(setup_path: false)
 
 # Install gems for Rubies we don't vendor for.

Library/Homebrew/test/.rubocop.yml

@@ -15,3 +15,7 @@ RSpec/ContextWording:
     - for
     - which
     - to
+
+# RSpec helper methods typecheck better as regular methods.
+Sorbet/BlockMethodDefinition:
+  Enabled: false

Library/Homebrew/test/rubocops/move_to_extend_os_spec.rb

@@ -22,6 +22,24 @@
     RUBY
   end
 
+  it "allows `OS.linux?` in requirements" do
+    expect_no_offenses(<<~RUBY, "Library/Homebrew/requirements/linux_requirement.rb")
+      OS.linux?
+    RUBY
+  end
+
+  it "allows `OS.mac?` in tests" do
+    expect_no_offenses(<<~RUBY, "Library/Homebrew/test/example_spec.rb")
+      OS.mac?
+    RUBY
+  end
+
+  it "allows OS checks in the OS loader" do
+    expect_no_offenses(<<~RUBY, "Library/Homebrew/os.rb")
+      OS.mac?
+    RUBY
+  end
+
   context "when in extend/os/mac" do
     it "registers an offense when using `OS.linux?`" do
       expect_offense(<<~RUBY, "Library/Homebrew/extend/os/mac/foo.rb")