Merge pull request #14163 from apainintheneck/create-cop-for-os-checks

Add cop for `OS.linux?` and `OS.mac?` usage

Library/.rubocop.yml

@@ -55,6 +55,12 @@ FormulaAudit:
 FormulaAuditStrict:
   Enabled: true
 
+Homebrew/MoveToExtendOS:
+  Exclude:
+    - "Homebrew/{extend,test,requirements}/**/*"
+    - "Taps/**/*"
+    - "Homebrew/os.rb"
+
 # enable all Homebrew custom cops
 Homebrew:
   Enabled: true

Library/Homebrew/cask/cmd/install.rb

@@ -68,7 +68,8 @@ module Cask
         zap: nil,
         dry_run: nil
       )
-        odie "Installing casks is supported only on macOS" unless OS.mac?
+        # TODO: Refactor and move to extend/os
+        odie "Installing casks is supported only on macOS" unless OS.mac? # rubocop:disable Homebrew/MoveToExtendOS
 
         options = {
           verbose:        verbose,

Library/Homebrew/cleanup.rb

@@ -405,28 +405,12 @@ module Homebrew
     end
 
     def cleanup_portable_ruby
-      rubies = [which("ruby"), which("ruby", ORIGINAL_PATHS)].compact
-      system_ruby = Pathname.new("/usr/bin/ruby")
-      rubies << system_ruby if system_ruby.exist?
-
-      use_system_ruby = if Homebrew::EnvConfig.force_vendor_ruby?
-        false
-      elsif OS.mac?
-        ENV["HOMEBREW_MACOS_SYSTEM_RUBY_NEW_ENOUGH"].present?
-      else
-        check_ruby_version = HOMEBREW_LIBRARY_PATH/"utils/ruby_check_version_script.rb"
-        rubies.uniq.any? do |ruby|
-          quiet_system ruby, "--enable-frozen-string-literal", "--disable=gems,did_you_mean,rubyopt",
-                       check_ruby_version, HOMEBREW_REQUIRED_RUBY_VERSION
-        end
-      end
-
       vendor_dir = HOMEBREW_LIBRARY/"Homebrew/vendor"
       portable_ruby_latest_version = (vendor_dir/"portable-ruby-version").read.chomp
 
       portable_rubies_to_remove = []
       Pathname.glob(vendor_dir/"portable-ruby/*.*").select(&:directory?).each do |path|
-        next if !use_system_ruby && portable_ruby_latest_version == path.basename.to_s
+        next if !use_system_ruby? && portable_ruby_latest_version == path.basename.to_s
 
         portable_rubies_to_remove << path
       end
@@ -445,6 +429,8 @@ module Homebrew
       end
     end
 
+    def use_system_ruby?; end
+
     def cleanup_bootsnap
       bootsnap = cache/"bootsnap"
       return unless bootsnap.exist?
@@ -615,3 +601,5 @@ module Homebrew
     end
   end
 end
+
+require "extend/os/cleanup"

Library/Homebrew/cmd/update-report.rb

@@ -293,7 +293,8 @@ module Homebrew
   end
 
   def migrate_gcc_dependents_if_needed
-    return if OS.mac?
+    # TODO: Refactor and move to extend/os
+    return if OS.mac? # rubocop:disable Homebrew/MoveToExtendOS
     return if Settings.read("gcc-rpaths.fixed") == "true"
 
     Formula.installed.each do |formula|

Library/Homebrew/default_prefix.rb

@@ -4,7 +4,8 @@
 require "simulate_system"
 
 module Homebrew
-  DEFAULT_PREFIX, DEFAULT_REPOSITORY = if OS.mac? && Hardware::CPU.arm?
+  # TODO: Refactor and move to extend/os
+  DEFAULT_PREFIX, DEFAULT_REPOSITORY = if OS.mac? && Hardware::CPU.arm? # rubocop:disable Homebrew/MoveToExtendOS
     [HOMEBREW_MACOS_ARM_DEFAULT_PREFIX, HOMEBREW_MACOS_ARM_DEFAULT_REPOSITORY]
   elsif Homebrew::SimulateSystem.simulating_or_running_on_linux?
     [HOMEBREW_LINUX_DEFAULT_PREFIX, HOMEBREW_LINUX_DEFAULT_REPOSITORY]

Library/Homebrew/dev-cmd/bottle.rb

@@ -247,7 +247,8 @@ module Homebrew
       "--pax-option", "globexthdr.name=/GlobalHead.%n,exthdr.name=%d/PaxHeaders/%f,delete=atime,delete=ctime"
     ].freeze
 
-    return ["tar", gnutar_args].freeze if OS.linux?
+    # TODO: Refactor and move to extend/os
+    return ["tar", gnutar_args].freeze if OS.linux? # rubocop:disable Homebrew/MoveToExtendOS
 
     # Use gnu-tar on macOS as it can be set up for reproducibility better than libarchive.
     begin
@@ -275,6 +276,8 @@ module Homebrew
       ignores << %r{#{cellar_regex}/#{go_regex}/[\d.]+/libexec}
     end
 
+    # TODO: Refactor and move to extend/os
+    # rubocop:disable Homebrew/MoveToExtendOS
     ignores << case f.name
     # On Linux, GCC installation can be moved so long as the whole directory tree is moved together:
     # https://gcc-help.gcc.gnu.narkive.com/GnwuCA7l/moving-gcc-from-the-installation-path-is-it-allowed.
@@ -284,6 +287,7 @@ module Homebrew
     when Version.formula_optionally_versioned_regex(:binutils)
       %r{#{cellar_regex}/binutils} if OS.linux?
     end
+    # rubocop:enable Homebrew/MoveToExtendOS
 
     ignores.compact
   end
@@ -409,7 +413,8 @@ module Homebrew
           # Set the times for reproducible bottles.
           if file.symlink?
             # Need to make symlink permissions consistent on macOS and Linux
-            File.lchmod 0777, file if OS.mac?
+            # TODO: Refactor and move to extend/os
+            File.lchmod 0777, file if OS.mac? # rubocop:disable Homebrew/MoveToExtendOS
             File.lutime(tab.source_modified_time, tab.source_modified_time, file)
           else
             file.utime(tab.source_modified_time, tab.source_modified_time)

Library/Homebrew/dev-cmd/tests.rb

@@ -157,6 +157,8 @@ module Homebrew
         --require spec_helper
       ]
 
+      # TODO: Refactor and move to extend/os
+      # rubocop:disable Homebrew/MoveToExtendOS
       unless OS.mac?
         bundle_args << "--tag" << "~needs_macos" << "--tag" << "~cask"
         files = files.grep_v(%r{^test/(os/mac|cask)(/.*|_spec\.rb)$})
@@ -166,6 +168,7 @@ module Homebrew
         bundle_args << "--tag" << "~needs_linux"
         files = files.grep_v(%r{^test/os/linux(/.*|_spec\.rb)$})
       end
+      # rubocop:enable Homebrew/MoveToExtendOS
 
       puts "Randomized with seed #{seed}"
 

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

@@ -65,7 +65,8 @@ module Homebrew
           tags = if (HOMEBREW_REPOSITORY/".git/shallow").exist?
             safe_system "git", "fetch", "--tags", "--depth=1"
             Utils.popen_read("git", "tag", "--list", "--sort=-version:refname")
-          elsif OS.linux?
+          # TODO: Refactor and move to extend/os
+          elsif OS.linux? # rubocop:disable Homebrew/MoveToExtendOS
             Utils.popen_read("git tag --list | sort -rV")
           end
         end

Library/Homebrew/extend/os/cleanup.rb

@@ -0,0 +1,8 @@
+# typed: false
+# frozen_string_literal: true
+
+if OS.mac?
+  require "extend/os/mac/cleanup"
+elsif OS.linux?
+  require "extend/os/linux/cleanup"
+end

Library/Homebrew/extend/os/formula_installer.rb

@@ -0,0 +1,8 @@
+# typed: strict
+# frozen_string_literal: true
+
+if OS.mac?
+  require "extend/os/mac/formula_installer"
+elsif OS.linux?
+  require "extend/os/linux/formula_installer"
+end

Library/Homebrew/extend/os/linux/cleanup.rb

@@ -0,0 +1,22 @@
+# typed: false
+# frozen_string_literal: true
+
+module Homebrew
+  class Cleanup
+    undef use_system_ruby?
+
+    def use_system_ruby?
+      return false if Homebrew::EnvConfig.force_vendor_ruby?
+
+      rubies = [which("ruby"), which("ruby", ORIGINAL_PATHS)].compact
+      system_ruby = Pathname.new("/usr/bin/ruby")
+      rubies << system_ruby if system_ruby.exist?
+
+      check_ruby_version = HOMEBREW_LIBRARY_PATH/"utils/ruby_check_version_script.rb"
+      rubies.uniq.any? do |ruby|
+        quiet_system ruby, "--enable-frozen-string-literal", "--disable=gems,did_you_mean,rubyopt",
+                     check_ruby_version, HOMEBREW_REQUIRED_RUBY_VERSION
+      end
+    end
+  end
+end

Library/Homebrew/extend/os/linux/formula_installer.rb

@@ -0,0 +1,12 @@
+# typed: true
+# frozen_string_literal: true
+
+class FormulaInstaller
+  undef fresh_install?
+
+  sig { params(formula: Formula).returns(T.nilable(T::Boolean)) }
+  def fresh_install?(formula)
+    !Homebrew::EnvConfig.developer? &&
+      (!installed_as_dependency? || !formula.any_version_installed?)
+  end
+end

Library/Homebrew/extend/os/linux/simulate_system.rb

@@ -0,0 +1,29 @@
+# typed: true
+# frozen_string_literal: true
+
+module Homebrew
+  class SimulateSystem
+    class << self
+      undef os
+      undef simulating_or_running_on_linux?
+      undef current_os
+
+      sig { returns(T.nilable(Symbol)) }
+      def os
+        return :macos if @os.blank? && Homebrew::EnvConfig.simulate_macos_on_linux?
+
+        @os
+      end
+
+      sig { returns(T::Boolean) }
+      def simulating_or_running_on_linux?
+        os.blank? || os == :linux
+      end
+
+      sig { returns(Symbol) }
+      def current_os
+        os || :linux
+      end
+    end
+  end
+end

Library/Homebrew/extend/os/mac/cleanup.rb

@@ -0,0 +1,14 @@
+# typed: false
+# frozen_string_literal: true
+
+module Homebrew
+  class Cleanup
+    undef use_system_ruby?
+
+    def use_system_ruby?
+      return false if Homebrew::EnvConfig.force_vendor_ruby?
+
+      ENV["HOMEBREW_MACOS_SYSTEM_RUBY_NEW_ENOUGH"].present?
+    end
+  end
+end

Library/Homebrew/extend/os/mac/formula_installer.rb

@@ -0,0 +1,12 @@
+# typed: true
+# frozen_string_literal: true
+
+class FormulaInstaller
+  undef fresh_install?
+
+  sig { params(formula: Formula).returns(T.nilable(T::Boolean)) }
+  def fresh_install?(formula)
+    !Homebrew::EnvConfig.developer? && !OS::Mac.version.outdated_release? &&
+      (!installed_as_dependency? || !formula.any_version_installed?)
+  end
+end

Library/Homebrew/extend/os/mac/sandbox.rb

@@ -0,0 +1,9 @@
+# typed: strict
+# frozen_string_literal: true
+
+class Sandbox
+  sig { returns(T::Boolean) }
+  def self.available?
+    File.executable?(SANDBOX_EXEC)
+  end
+end

Library/Homebrew/extend/os/mac/simulate_system.rb

@@ -0,0 +1,21 @@
+# typed: true
+# frozen_string_literal: true
+
+module Homebrew
+  class SimulateSystem
+    class << self
+      undef simulating_or_running_on_macos?
+      undef current_os
+
+      sig { returns(T::Boolean) }
+      def simulating_or_running_on_macos?
+        os.blank? || [:macos, *MacOSVersions::SYMBOLS.keys].include?(os)
+      end
+
+      sig { returns(Symbol) }
+      def current_os
+        os || MacOS.version.to_sym
+      end
+    end
+  end
+end

Library/Homebrew/extend/os/sandbox.rb

@@ -0,0 +1,4 @@
+# typed: strict
+# frozen_string_literal: true
+
+require "extend/os/mac/sandbox" if OS.mac?

Library/Homebrew/extend/os/simulate_system.rb

@@ -0,0 +1,8 @@
+# typed: strict
+# frozen_string_literal: true
+
+if OS.mac?
+  require "extend/os/mac/simulate_system"
+elsif OS.linux?
+  require "extend/os/linux/simulate_system"
+end

Library/Homebrew/extend/os/unpack_strategy/zip.rb

@@ -0,0 +1,4 @@
+# typed: strict
+# frozen_string_literal: true
+
+require "extend/os/mac/unpack_strategy/zip" if OS.mac?

Library/Homebrew/formula_cellar_checks.rb

@@ -319,7 +319,8 @@ module FormulaCellarChecks
   def check_binary_arches(formula)
     return unless formula.prefix.directory?
     # There is no `binary_executable_or_library_files` method for the generic OS
-    return if !OS.mac? && !OS.linux?
+    # TODO: Refactor and move to extend/os
+    return if !OS.mac? && !OS.linux? # rubocop:disable Homebrew/MoveToExtendOS
 
     keg = Keg.new(formula.prefix)
     mismatches = {}

Library/Homebrew/formula_installer.rb

@@ -258,9 +258,7 @@ class FormulaInstaller
       # don't want to complain about no bottle available if doing an
       # upgrade/reinstall/dependency install (but do in the case the bottle
       # check fails)
-      elsif !Homebrew::EnvConfig.developer? &&
-            (!installed_as_dependency? || !formula.any_version_installed?) &&
-            (!OS.mac? || !OS::Mac.version.outdated_release?)
+      elsif fresh_install?(formula)
         <<~EOS
           #{formula}: no bottle available!
         EOS
@@ -343,6 +341,11 @@ class FormulaInstaller
           "#{formula.full_name} requires the latest version of pinned dependencies"
   end
 
+  sig { params(_formula: Formula).returns(T.nilable(T::Boolean)) }
+  def fresh_install?(_formula)
+    false
+  end
+
   sig { void }
   def install_fetch_deps
     return if @compute_dependencies.blank?

Library/Homebrew/rubocops/all.rb

@@ -14,6 +14,7 @@ end
 
 require_relative "io_read"
 require_relative "shell_commands"
+require_relative "platform"
 
 require_relative "formula_desc"
 require_relative "components_order"

Library/Homebrew/rubocops/platform.rb

@@ -0,0 +1,25 @@
+# typed: false
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    module Homebrew
+      # This cop ensures that platform specific code ends up in `extend/os`.
+      #
+      # @api private
+      class MoveToExtendOS < Base
+        MSG = "Move `OS.linux?` and `OS.mac?` calls to `extend/os`."
+
+        def_node_matcher :os_check?, <<~PATTERN
+          (send (const nil? :OS) {:mac? | :linux?})
+        PATTERN
+
+        def on_send(node)
+          return unless os_check?(node)
+
+          add_offense(node)
+        end
+      end
+    end
+  end
+end

Library/Homebrew/sandbox.rb

@@ -17,7 +17,7 @@ class Sandbox
 
   sig { returns(T::Boolean) }
   def self.available?
-    OS.mac? && File.executable?(SANDBOX_EXEC)
+    false
   end
 
   sig { void }
@@ -256,3 +256,5 @@ class Sandbox
   end
   private_constant :SandboxProfile
 end
+
+require "extend/os/sandbox"

Library/Homebrew/simulate_system.rb

@@ -9,14 +9,7 @@ module Homebrew
     class << self
       extend T::Sig
 
-      attr_reader :arch
-
-      sig { returns(T.nilable(Symbol)) }
-      def os
-        return :macos if @os.blank? && !OS.mac? && Homebrew::EnvConfig.simulate_macos_on_linux?
-
-        @os
-      end
+      attr_reader :arch, :os
 
       sig { params(new_os: Symbol).void }
       def os=(new_os)
@@ -40,15 +33,11 @@ module Homebrew
 
       sig { returns(T::Boolean) }
       def simulating_or_running_on_macos?
-        return OS.mac? if os.blank?
-
         [:macos, *MacOSVersions::SYMBOLS.keys].include?(os)
       end
 
       sig { returns(T::Boolean) }
       def simulating_or_running_on_linux?
-        return OS.linux? if os.blank?
-
         os == :linux
       end
 
@@ -59,11 +48,10 @@ module Homebrew
 
       sig { returns(Symbol) }
       def current_os
-        return T.must(os) if os.present?
-        return :linux if OS.linux?
+        os || :generic
+      end
+    end
+  end
+end
 
-        MacOS.version.to_sym
-      end
-    end
-  end
-end
+require "extend/os/simulate_system"

Library/Homebrew/test/rubocops/platform_spec.rb

@@ -0,0 +1,22 @@
+# typed: false
+# frozen_string_literal: true
+
+require "rubocops/platform"
+
+describe RuboCop::Cop::Homebrew::MoveToExtendOS do
+  subject(:cop) { described_class.new }
+
+  it "registers an offense when using `OS.linux?`" do
+    expect_offense(<<~RUBY)
+      OS.linux?
+      ^^^^^^^^^ Move `OS.linux?` and `OS.mac?` calls to `extend/os`.
+    RUBY
+  end
+
+  it "registers an offense when using `OS.mac?`" do
+    expect_offense(<<~RUBY)
+      OS.mac?
+      ^^^^^^^ Move `OS.linux?` and `OS.mac?` calls to `extend/os`.
+    RUBY
+  end
+end

Library/Homebrew/unpack_strategy/zip.rb

@@ -49,4 +49,4 @@ module UnpackStrategy
   end
 end
 
-require "extend/os/mac/unpack_strategy/zip" if OS.mac?
+require "extend/os/unpack_strategy/zip"