Merge pull request #20075 from Homebrew/no-nested-depends_on

rubocops/cask/no_overrides: avoid nested `depends_on macos:`

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 `%<stanza>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 `%<stanza>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

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

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

@@ -1,20 +1,18 @@
 cask "with-depends-on-macos-failure" do
+  # guarantee a mismatched release
+  on_big_sur :or_older do
     version "1.2.3"
     sha256 "67cdb8a02803ef37fdbf7e0be205863172e41a561ca446cd84f0d7ab35a99d94"
-
-  # guarantee a mismatched release
-  on_mojave :or_older do
-    depends_on macos: :catalina
   end
-  on_catalina do
+  on_ventura :or_newer do
-    depends_on macos: :mojave
+    version "1.2.3"
-  end
+    sha256 "67cdb8a02803ef37fdbf7e0be205863172e41a561ca446cd84f0d7ab35a99d94"
-  on_big_sur :or_newer do
-    depends_on macos: :catalina
   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