Merge pull request #14301 from Rylan12/more-on-system-rubocops

Expand `on_system` rubocops

Library/Homebrew/rubocops/cask/on_system_conditionals.rb

@@ -41,6 +41,7 @@ module RuboCop
           end
 
           audit_arch_conditionals(cask_body)
+          audit_macos_version_conditionals(cask_body, recommend_on_system: false)
           simplify_sha256_stanzas
         end
 

Library/Homebrew/rubocops/shared/on_system_conditionals_helper.rb

@@ -88,6 +88,17 @@ module RuboCop
                                  else_method: else_method, else_node: else_node)
           end
         end
+
+        [:arch, :arm?, :intel?].each do |method|
+          hardware_cpu_search(body_node, method: method) do |method_node|
+            # These should already be caught by `if_arch_node_search`
+            next if method_node.parent.source.start_with? "if #{method_node.source}"
+            next if if_node_is_allowed?(method_node, allowed_methods: allowed_methods, allowed_blocks: allowed_blocks)
+
+            offending_node(method_node)
+            problem "Don't use `#{method_node.source}`, use `on_arm` and `on_intel` blocks instead."
+          end
+        end
       end
 
       def audit_base_os_conditionals(body_node, allowed_methods: [], allowed_blocks: [])
@@ -126,6 +137,15 @@ module RuboCop
             if_statement_problem(if_node, "if MacOS.version #{operator} :#{macos_version_option}",
                                  on_system_method_string, autocorrect: autocorrect)
           end
+
+          macos_version_comparison_search(body_node, os_version: macos_version_option) do |method_node|
+            # These should already be caught by `if_macos_version_node_search`
+            next if method_node.parent.source.start_with? "if #{method_node.source}"
+            next if if_node_is_allowed?(method_node, allowed_methods: allowed_methods, allowed_blocks: allowed_blocks)
+
+            offending_node(method_node)
+            problem "Don't use `#{method_node.source}`, use `on_{macos_version}` blocks instead."
+          end
         end
       end
 
@@ -180,6 +200,14 @@ module RuboCop
         (send nil? :on_system (sym :linux) (hash (pair (sym :macos) (sym $_))))
       PATTERN
 
+      def_node_search :hardware_cpu_search, <<~PATTERN
+        (send (const (const nil? :Hardware) :CPU) %method)
+      PATTERN
+
+      def_node_search :macos_version_comparison_search, <<~PATTERN
+        (send (send (const nil? :MacOS) :version) {:== :<= :< :>= :> :!=} (sym %os_version))
+      PATTERN
+
       def_node_search :if_arch_node_search, <<~PATTERN
         $(if (send (const (const nil? :Hardware) :CPU) %arch) _ $_)
       PATTERN

Library/Homebrew/test/rubocops/cask/on_system_conditionals_spec.rb

@@ -262,4 +262,181 @@ describe RuboCop::Cop::Cask::OnSystemConditionals do
       include_examples "does not report any offenses"
     end
   end
+
+  context "when auditing loose `Hardware::CPU` method calls" do
+    context "when there is a `Hardware::CPU.arm?` reference" do
+      let(:source) do
+        <<-CASK.undent
+          cask 'foo' do
+            if Hardware::CPU.arm? && other_condition
+              sha256 "67cdb8a02803ef37fdbf7e0be205863172e41a561ca446cd84f0d7ab35a99d94"
+            else
+              sha256 "8c62a2b791cf5f0da6066a0a4b6e85f62949cd60975da062df44adf887f4370b"
+            end
+          end
+        CASK
+      end
+      let(:expected_offenses) do
+        [{
+          message:  "Don't use `Hardware::CPU.arm?`, use `on_arm` and `on_intel` blocks instead.",
+          severity: :convention,
+          line:     2,
+          column:   5,
+          source:   "Hardware::CPU.arm?",
+        }]
+      end
+
+      include_examples "reports offenses"
+    end
+
+    context "when there is a `Hardware::CPU.intel?` reference" do
+      let(:source) do
+        <<-CASK.undent
+          cask 'foo' do
+            if Hardware::CPU.intel? && other_condition
+              sha256 "67cdb8a02803ef37fdbf7e0be205863172e41a561ca446cd84f0d7ab35a99d94"
+            else
+              sha256 "8c62a2b791cf5f0da6066a0a4b6e85f62949cd60975da062df44adf887f4370b"
+            end
+          end
+        CASK
+      end
+      let(:expected_offenses) do
+        [{
+          message:  "Don't use `Hardware::CPU.intel?`, use `on_arm` and `on_intel` blocks instead.",
+          severity: :convention,
+          line:     2,
+          column:   5,
+          source:   "Hardware::CPU.intel?",
+        }]
+      end
+
+      include_examples "reports offenses"
+    end
+
+    context "when there is a `Hardware::CPU.arch` reference" do
+      let(:source) do
+        <<-CASK.undent
+          cask 'foo' do
+            version "1.2.3"
+            sha256 "67cdb8a02803ef37fdbf7e0be205863172e41a561ca446cd84f0d7ab35a99d94"
+
+            url "https://example.com/foo-\#{version}-\#{Hardware::CPU.arch}.zip"
+          end
+        CASK
+      end
+      let(:expected_offenses) do
+        [{
+          message:  "Don't use `Hardware::CPU.arch`, use `on_arm` and `on_intel` blocks instead.",
+          severity: :convention,
+          line:     5,
+          column:   44,
+          source:   "Hardware::CPU.arch",
+        }]
+      end
+
+      include_examples "reports offenses"
+    end
+  end
+
+  context "when auditing loose `MacOS.version` method calls" do
+    context "when there is a `MacOS.version ==` reference" do
+      let(:source) do
+        <<-CASK.undent
+          cask 'foo' do
+            if MacOS.version == :catalina
+              version "1.0.0"
+            else
+              version "2.0.0"
+            end
+          end
+        CASK
+      end
+      let(:expected_offenses) do
+        [{
+          message:  "Don't use `if MacOS.version == :catalina`, use `on_catalina do` instead.",
+          severity: :convention,
+          line:     2,
+          column:   2,
+          source:   "if MacOS.version == :catalina\n    version \"1.0.0\"\n  else\n    version \"2.0.0\"\n  end",
+        }]
+      end
+
+      include_examples "reports offenses"
+    end
+
+    context "when there is a `MacOS.version <=` reference" do
+      let(:source) do
+        <<-CASK.undent
+          cask 'foo' do
+            if MacOS.version <= :catalina
+              version "1.0.0"
+            else
+              version "2.0.0"
+            end
+          end
+        CASK
+      end
+      let(:expected_offenses) do
+        [{
+          message:  "Don't use `if MacOS.version <= :catalina`, use `on_catalina :or_older do` instead.",
+          severity: :convention,
+          line:     2,
+          column:   2,
+          source:   "if MacOS.version <= :catalina\n    version \"1.0.0\"\n  else\n    version \"2.0.0\"\n  end",
+        }]
+      end
+
+      include_examples "reports offenses"
+    end
+
+    context "when there is a `MacOS.version >=` reference" do
+      let(:source) do
+        <<-CASK.undent
+          cask 'foo' do
+            if MacOS.version >= :catalina
+              version "1.0.0"
+            else
+              version "2.0.0"
+            end
+          end
+        CASK
+      end
+      let(:expected_offenses) do
+        [{
+          message:  "Don't use `if MacOS.version >= :catalina`, use `on_catalina :or_newer do` instead.",
+          severity: :convention,
+          line:     2,
+          column:   2,
+          source:   "if MacOS.version >= :catalina\n    version \"1.0.0\"\n  else\n    version \"2.0.0\"\n  end",
+        }]
+      end
+
+      include_examples "reports offenses"
+    end
+
+    context "when there is a `MacOS.version` reference" do
+      let(:source) do
+        <<-CASK.undent
+          cask 'foo' do
+            version "1.2.3"
+            sha256 "67cdb8a02803ef37fdbf7e0be205863172e41a561ca446cd84f0d7ab35a99d94"
+
+            url "https://example.com/foo-\#{version}-\#{MacOS.version == :monterey}.zip"
+          end
+        CASK
+      end
+      let(:expected_offenses) do
+        [{
+          message:  "Don't use `MacOS.version == :monterey`, use `on_{macos_version}` blocks instead.",
+          severity: :convention,
+          line:     5,
+          column:   44,
+          source:   "MacOS.version == :monterey",
+        }]
+      end
+
+      include_examples "reports offenses"
+    end
+  end
 end

Library/Homebrew/test/support/fixtures/cask/Casks/with-depends-on-macos-failure.rb

@@ -6,7 +6,15 @@ cask "with-depends-on-macos-failure" do
   homepage "https://brew.sh/with-depends-on-macos-failure"
 
   # guarantee a mismatched release
-  depends_on macos: (MacOS.version == :catalina) ? :mojave : :catalina
+  on_mojave :or_older do
+    depends_on macos: :catalina
+  end
+  on_catalina do
+    depends_on macos: :mojave
+  end
+  on_big_sur :or_newer do
+    depends_on macos: :catalina
+  end
 
   app "Caffeine.app"
 end