diff --git a/Library/Homebrew/rubocops/cask/no_overrides.rb b/Library/Homebrew/rubocops/cask/no_overrides.rb index f7cabcac8a..09120e3e4a 100644 --- a/Library/Homebrew/rubocops/cask/no_overrides.rb +++ b/Library/Homebrew/rubocops/cask/no_overrides.rb @@ -17,17 +17,36 @@ module RuboCop def on_cask(cask_block) return if (cask_stanzas = cask_block.toplevel_stanzas).empty? # Skip if there are no `on_*` blocks. - return unless cask_stanzas.any? { |s| ON_SYSTEM_METHODS.include?(s.stanza_name) } + return unless (on_blocks = cask_stanzas.select { |s| ON_SYSTEM_METHODS.include?(s.stanza_name) }).any? cask_stanzas.each do |stanza| - # TODO: We probably only want to disallow `version`, `url`, and `sha256` stanzas being overridden? + # Skip if the stanza is itself an `on_*` block. next unless RuboCop::Cask::Constants::STANZA_ORDER.include?(stanza.stanza_name) # Skip if the stanza we detect is already in an `on_*` block. next if stanza.parent_node.block_type? && ON_SYSTEM_METHODS.include?(stanza.parent_node.method_name) + # Skip if the stanza outside of a block is not also in an `on_*` block. + next if on_system_stanzas(on_blocks).none?(stanza.stanza_name) add_offense(stanza.source_range, message: format(MESSAGE, stanza: stanza.stanza_name)) end end + + def on_system_stanzas(on_system) + names = [] + method_nodes = on_system.map(&:method_node) + method_nodes.each do |node| + next unless node.block_type? + + node.child_nodes.each do |child| + child.each_node(:send) do |send_node| + next if ON_SYSTEM_METHODS.include?(send_node.method_name) + + names << send_node.method_name + end + end + end + names + end end 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 a605b3a883..36f37b4a8e 100644 --- a/Library/Homebrew/test/rubocops/cask/no_overrides_spec.rb +++ b/Library/Homebrew/test/rubocops/cask/no_overrides_spec.rb @@ -38,6 +38,27 @@ describe RuboCop::Cop::Cask::NoOverrides do include_examples "does not report any offenses" end + context "when there's only one difference between the `on_*` blocks" do + let(:source) do + <<~CASK + cask "foo" do + version "1.2.3" + + on_big_sur :or_older do + sha256 "bbb" + url "https://brew.sh/legacy/foo-2.3.4.dmg" + end + on_monterey :or_newer do + sha256 "aaa" + url "https://brew.sh/foo-2.3.4.dmg" + end + end + CASK + end + + include_examples "does not report any offenses" + end + context "when there are top-level standalone stanzas" do let(:source) do <<~CASK @@ -62,15 +83,6 @@ describe RuboCop::Cop::Cask::NoOverrides do line: 2, column: 2, source: "version '2.3.4'", - }, { - message: <<~EOS, - Do not use top-level `url` stanza as the default, add an `on_{system}` block instead. - Use `:or_older` or `:or_newer` to specify a range of macOS versions. - EOS - severity: :convention, - line: 7, - column: 2, - source: "url 'https://brew.sh/foo-2.3.4.dmg'", }] end