From ea8152757e4ddde54ea83eb03d6071c689b70078 Mon Sep 17 00:00:00 2001 From: Eric Knibbe Date: Tue, 10 Jun 2025 13:51:06 -0400 Subject: [PATCH] rubocops/cask/no_overrides: avoid nested `depends_on macos:` --- .../Homebrew/rubocops/cask/no_overrides.rb | 20 +++++++--- .../test/rubocops/cask/no_overrides_spec.rb | 39 +++++++++++++++++++ .../Casks/with-depends-on-macos-failure.rb | 18 ++++----- 3 files changed, 62 insertions(+), 15 deletions(-) diff --git a/Library/Homebrew/rubocops/cask/no_overrides.rb b/Library/Homebrew/rubocops/cask/no_overrides.rb index 6f313f41ed..1eeb1e1df8 100644 --- a/Library/Homebrew/rubocops/cask/no_overrides.rb +++ b/Library/Homebrew/rubocops/cask/no_overrides.rb @@ -11,14 +11,14 @@ module RuboCop # TODO: Update this list if new stanzas are added to `Cask::DSL` that call `set_unique_stanza`. OVERRIDABLE_METHODS = [ :appcast, :arch, :auto_updates, :conflicts_with, :container, - :desc, :homepage, :sha256, :url, :version + :desc, :homepage, :os, :sha256, :url, :version ].freeze - MESSAGE = "Do not use a top-level `%s` stanza as the default. " \ - "Add it to an `on_{system}` block instead. " \ - "Use `:or_older` or `:or_newer` to specify a range of macOS versions." sig { override.params(cask_block: RuboCop::Cask::AST::CaskBlock).void } def on_cask(cask_block) + message = "Do not use a top-level `%s` stanza as the default. " \ + "Add it to an `on_{system}` block instead. " \ + "Use `:or_older` or `:or_newer` to specify a range of macOS versions." cask_stanzas = cask_block.toplevel_stanzas return if (on_blocks = on_system_methods(cask_stanzas)).none? @@ -31,12 +31,14 @@ module RuboCop # Skip if the stanza outside of a block is not also in an `on_*` block. next unless stanzas_in_blocks.include?(stanza.stanza_name) - add_offense(stanza.source_range, message: format(MESSAGE, stanza: stanza.stanza_name)) + add_offense(stanza.source_range, message: format(message, stanza: stanza.stanza_name)) end end sig { params(on_system: T::Array[RuboCop::Cask::AST::Stanza]).returns(T::Set[Symbol]) } def on_system_stanzas(on_system) + message = "Do not use a `depends_on macos:` stanza inside an `on_{system}` block. " \ + "Add it once to specify the oldest macOS supported by any version in the cask." names = T.let(Set.new, T::Set[Symbol]) method_nodes = on_system.map(&:method_node) method_nodes.select(&:block_type?).each do |node| @@ -51,6 +53,14 @@ module RuboCop end next if RuboCop::Cask::Constants::ON_SYSTEM_METHODS.include?(send_node.method_name) + if send_node.method_name == :depends_on && + send_node.arguments.first.pairs.any? { |a| a.key.value == :macos } && + OnSystemConditionalsHelper::ON_SYSTEM_OPTIONS.map do |m| + :"on_#{m}" + end.include?(T.cast(node, RuboCop::AST::BlockNode).method_name) + add_offense(send_node.source_range, message:) + end + names.add(send_node.method_name) end end diff --git a/Library/Homebrew/test/rubocops/cask/no_overrides_spec.rb b/Library/Homebrew/test/rubocops/cask/no_overrides_spec.rb index 3b68c05a32..ba7de8e7b4 100644 --- a/Library/Homebrew/test/rubocops/cask/no_overrides_spec.rb +++ b/Library/Homebrew/test/rubocops/cask/no_overrides_spec.rb @@ -173,4 +173,43 @@ RSpec.describe RuboCop::Cop::Cask::NoOverrides, :config do end CASK end + + it "accepts when there is a top-level `depends_on macos:` stanza" do + expect_no_offenses <<~CASK + cask 'foo' do + version '1.2.3' + url 'https://brew.sh/foo.pkg' + + depends_on macos: ">= :sequoia" + + name 'Foo' + end + CASK + end + + it "reports an offense when `on_*` blocks contain a `depends_on macos:` stanza" do + expect_offense <<~CASK + cask 'foo' do + version '1.2.3' + + on_sequoia :or_newer do + sha256 "aaa" + url "https://brew.sh/foo-mac.dmg" + + depends_on macos: ">= :sequoia" + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Do not use a `depends_on macos:` stanza inside an `on_{system}` block. Add it once to specify the oldest macOS supported by any version in the cask. + end + + on_arm do + sha256 "bbb" + url "https://brew.sh/foo-arm.dmg" + + depends_on macos: ">= :sequoia" + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Do not use a `depends_on macos:` stanza inside an `on_{system}` block. Add it once to specify the oldest macOS supported by any version in the cask. + end + + name 'Foo' + end + CASK + 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 bbde7db9c3..a65cde70d0 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 @@ -1,20 +1,18 @@ cask "with-depends-on-macos-failure" do - version "1.2.3" - sha256 "67cdb8a02803ef37fdbf7e0be205863172e41a561ca446cd84f0d7ab35a99d94" - # guarantee a mismatched release - on_mojave :or_older do - depends_on macos: :catalina + on_big_sur :or_older do + version "1.2.3" + sha256 "67cdb8a02803ef37fdbf7e0be205863172e41a561ca446cd84f0d7ab35a99d94" end - on_catalina do - depends_on macos: :mojave - end - on_big_sur :or_newer do - depends_on macos: :catalina + on_ventura :or_newer do + version "1.2.3" + sha256 "67cdb8a02803ef37fdbf7e0be205863172e41a561ca446cd84f0d7ab35a99d94" end url "file://#{TEST_FIXTURE_DIR}/cask/caffeine.zip" homepage "https://brew.sh/with-depends-on-macos-failure" + depends_on macos: :monterey + app "Caffeine.app" end