diff --git a/Library/Homebrew/rubocops/cask/on_system_conditionals.rb b/Library/Homebrew/rubocops/cask/on_system_conditionals.rb index cb3abd0a79..0887c8d54e 100644 --- a/Library/Homebrew/rubocops/cask/on_system_conditionals.rb +++ b/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 diff --git a/Library/Homebrew/rubocops/shared/on_system_conditionals_helper.rb b/Library/Homebrew/rubocops/shared/on_system_conditionals_helper.rb index f8cfafaceb..7763756615 100644 --- a/Library/Homebrew/rubocops/shared/on_system_conditionals_helper.rb +++ b/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 diff --git a/Library/Homebrew/test/rubocops/cask/on_system_conditionals_spec.rb b/Library/Homebrew/test/rubocops/cask/on_system_conditionals_spec.rb index a815225b27..2864a83a6d 100644 --- a/Library/Homebrew/test/rubocops/cask/on_system_conditionals_spec.rb +++ b/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 diff --git a/Library/Homebrew/test/support/fixtures/cask/Casks/with-depends-on-macos-failure.rb b/Library/Homebrew/test/support/fixtures/cask/Casks/with-depends-on-macos-failure.rb index 9dbddb789e..f7d2061796 100644 --- a/Library/Homebrew/test/support/fixtures/cask/Casks/with-depends-on-macos-failure.rb +++ b/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