diff --git a/Library/Homebrew/rubocops/cask/on_system_conditionals.rb b/Library/Homebrew/rubocops/cask/on_system_conditionals.rb index 1981db0550..0887c8d54e 100644 --- a/Library/Homebrew/rubocops/cask/on_system_conditionals.rb +++ b/Library/Homebrew/rubocops/cask/on_system_conditionals.rb @@ -41,7 +41,7 @@ module RuboCop end audit_arch_conditionals(cask_body) - audit_macos_version_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 f3faed9156..7763756615 100644 --- a/Library/Homebrew/rubocops/shared/on_system_conditionals_helper.rb +++ b/Library/Homebrew/rubocops/shared/on_system_conditionals_helper.rb @@ -91,6 +91,8 @@ module RuboCop [: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) @@ -137,6 +139,8 @@ module RuboCop 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) 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 1003a7c482..2864a83a6d 100644 --- a/Library/Homebrew/test/rubocops/cask/on_system_conditionals_spec.rb +++ b/Library/Homebrew/test/rubocops/cask/on_system_conditionals_spec.rb @@ -338,4 +338,105 @@ describe RuboCop::Cop::Cask::OnSystemConditionals do 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