diff --git a/Library/Homebrew/rubocops/cask/no_overrides.rb b/Library/Homebrew/rubocops/cask/no_overrides.rb new file mode 100644 index 0000000000..8a72402cf1 --- /dev/null +++ b/Library/Homebrew/rubocops/cask/no_overrides.rb @@ -0,0 +1,79 @@ +# typed: true +# frozen_string_literal: true + +module RuboCop + module Cop + module Cask + class NoOverrides < Base + extend T::Sig + include CaskHelp + + ON_SYSTEM_METHODS = RuboCop::Cask::Constants::ON_SYSTEM_METHODS + # These stanzas can be overridden by `on_*` blocks, so take them into account. + # TODO: Update this list if new stanzas are added to `Cask::DSL` that call `set_unique_stanza`. + OVERRIDEABLE_METHODS = [ + :appcast, :arch, :auto_updates, :conflicts_with, :container, + :desc, :homepage, :sha256, :url, :version + ].freeze + MESSAGE = <<~EOS + 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. + EOS + + def on_cask(cask_block) + cask_stanzas = cask_block.toplevel_stanzas + + # Skip if there are no `on_*` blocks. + return unless (on_blocks = cask_stanzas.select { |s| ON_SYSTEM_METHODS.include?(s.stanza_name) }).any? + + stanzas_in_blocks = on_system_stanzas(on_blocks) + + cask_stanzas.each do |stanza| + # Skip if the stanza is not allowed to be overridden. + next unless OVERRIDEABLE_METHODS.include?(stanza.stanza_name) + # 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)) + end + end + + def on_system_stanzas(on_system) + names = Set.new + 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| + # Skip (nested) livecheck blocks as its `url` is different to a download `url`. + next if send_node.method_name == :livecheck || inside_livecheck_block?(send_node) + # Skip string interpolations. + if send_node.ancestors.drop_while { |a| !a.begin_type? }.any? { |a| a.dstr_type? || a.regexp_type? } + next + end + next if ON_SYSTEM_METHODS.include?(send_node.method_name) + + names.add(send_node.method_name) + end + end + end + names + end + + def inside_livecheck_block?(node) + single_stanza_livecheck_block?(node) || multi_stanza_livecheck_block?(node) + end + + def single_stanza_livecheck_block?(node) + node.parent.block_type? && node.parent.method_name == :livecheck + end + + def multi_stanza_livecheck_block?(node) + grandparent_node = node.parent.parent + node.parent.begin_type? && grandparent_node.block_type? && grandparent_node.method_name == :livecheck + end + end + end + end +end diff --git a/Library/Homebrew/rubocops/rubocop-cask.rb b/Library/Homebrew/rubocops/rubocop-cask.rb index 2a3a9cefd3..78fb978b32 100644 --- a/Library/Homebrew/rubocops/rubocop-cask.rb +++ b/Library/Homebrew/rubocops/rubocop-cask.rb @@ -15,6 +15,7 @@ require_relative "cask/mixin/on_url_stanza" require_relative "cask/desc" require_relative "cask/homepage_url_trailing_slash" require_relative "cask/no_dsl_version" +require_relative "cask/no_overrides" require_relative "cask/on_system_conditionals" require_relative "cask/stanza_order" require_relative "cask/stanza_grouping" diff --git a/Library/Homebrew/test/rubocops/cask/no_overrides_spec.rb b/Library/Homebrew/test/rubocops/cask/no_overrides_spec.rb new file mode 100644 index 0000000000..4227b90dd3 --- /dev/null +++ b/Library/Homebrew/test/rubocops/cask/no_overrides_spec.rb @@ -0,0 +1,255 @@ +# typed: false +# frozen_string_literal: true + +require "rubocops/rubocop-cask" +require "test/rubocops/cask/shared_examples/cask_cop" + +describe RuboCop::Cop::Cask::NoOverrides do + include CaskCop + + subject(:cop) { described_class.new } + + context "when there are no on_system blocks" do + let(:source) do + <<~CASK + cask 'foo' do + version '1.2.3' + url 'https://brew.sh/foo.pkg' + + name 'Foo' + end + CASK + end + + include_examples "does not report any offenses" + end + + context "when there are no top-level standalone stanzas" do + let(:source) do + <<~CASK + cask 'foo' do + on_mojave :or_later do + version :latest + end + end + CASK + end + + include_examples "does not report any offenses" + end + + context "when there are top-level stanzas also in `on_*` blocks that should not override" do + let(:source) do + <<~CASK + cask 'foo' do + version '1.2.3' + + on_arm do + binary "foo-\#{version}-arm64" + end + + app "foo-\#{version}.app" + + binary "foo-\#{version}" + end + CASK + end + + include_examples "does not report any offenses" + end + + context "when there are `arch` variables in the `url` in the `on_*` blocks" do + let(:source) do + <<~CASK + cask 'foo' do + arch arm: "arm64", intel: "x86" + version '1.2.3' + on_mojave :or_later do + url "https://brew.sh/foo-\#{version}-\#{arch}.pkg" + sha256 "aaa" + end + end + CASK + end + + include_examples "does not report any offenses" + end + + context "when there are `version` interpolations in `on_*` blocks with methods called on them" do + let(:source) do + <<~CASK + cask 'foo' do + version 0.99,123.3 + + on_mojave :or_later do + url "https://brew.sh/foo-\#{version.csv.first}-\#{version.csv.second}.pkg" + end + end + CASK + end + + include_examples "does not report any offenses" + end + + context "when there are `arch` interpolations in regexps in `on_*` blocks" do + let(:source) do + <<~CASK + cask 'foo' do + arch arm: "arm64", intel: "x86" + + version 0.99,123.3 + + on_mojave :or_later do + url "https://brew.sh/foo-\#{arch}-\#{version.csv.first}-\#{version.csv.last}.pkg" + + livecheck do + url "https://brew.sh/foo/releases.html" + regex(/href=.*?foo[._-]v?(\d+(?:.\d+)+)-\#{arch}.pkg/i) + end + end + end + CASK + end + + include_examples "does not report any offenses" + end + + context "when there are single-line livecheck blocks within `on_*` blocks, ignore their contents" do + let(:source) do + <<~CASK + cask 'foo' do + on_intel do + livecheck do + url 'https://brew.sh/foo' # Livecheck should be allowed since it's a different "kind" of URL. + end + version '1.2.3' + end + on_arm do + version '2.3.4' + end + + url 'https://brew.sh/foo.pkg' + sha256 "bbb" + end + CASK + end + + include_examples "does not report any offenses" + end + + context "when there are multi-line livecheck blocks within `on_*` blocks, ignore their contents" do + let(:source) do + <<~CASK + cask 'foo' do + on_intel do + livecheck do + url 'https://brew.sh/foo' # Livecheck should be allowed since it's a different "kind" of URL. + strategy :sparkle + end + version '1.2.3' + end + on_arm do + version '2.3.4' + end + + url 'https://brew.sh/foo.pkg' + sha256 "bbb" + end + CASK + end + + 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 multiple differences between the `on_*` blocks" do + let(:source) do + <<~CASK + cask "foo" do + version "1.2.3" + sha256 "aaa" + url "https://brew.sh/foo-2.3.4.dmg" + + on_big_sur :or_older do + sha256 "bbb" + url "https://brew.sh/legacy/foo-2.3.4.dmg" + end + end + CASK + end + + let(:expected_offenses) do + [{ + message: <<~EOS, + Do not use a top-level `sha256` 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. + EOS + severity: :convention, + line: 3, + column: 2, + source: "sha256 \"aaa\"", + }, { + message: <<~EOS, + Do not use a top-level `url` 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. + EOS + severity: :convention, + line: 4, + column: 2, + source: "url \"https://brew.sh/foo-2.3.4.dmg\"", + }] + end + + include_examples "reports offenses" + end + + context "when there are top-level standalone stanzas" do + let(:source) do + <<~CASK + cask 'foo' do + version '2.3.4' + on_mojave :or_older do + version '1.2.3' + end + + url 'https://brew.sh/foo-2.3.4.dmg' + end + CASK + end + + let(:expected_offenses) do + [{ + message: <<~EOS, + Do not use a top-level `version` 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. + EOS + severity: :convention, + line: 2, + column: 2, + source: "version '2.3.4'", + }] + end + + include_examples "reports offenses" + end +end diff --git a/Library/Homebrew/test/support/fixtures/cask/Casks/multiple-versions.rb b/Library/Homebrew/test/support/fixtures/cask/Casks/multiple-versions.rb index 02a9e28fc1..fea39877d6 100644 --- a/Library/Homebrew/test/support/fixtures/cask/Casks/multiple-versions.rb +++ b/Library/Homebrew/test/support/fixtures/cask/Casks/multiple-versions.rb @@ -2,16 +2,17 @@ cask "multiple-versions" do arch arm: "arm", intel: "intel" platform = on_arch_conditional arm: "darwin-arm64", intel: "darwin" - version "1.2.3" - sha256 "67cdb8a02803ef37fdbf7e0be205863172e41a561ca446cd84f0d7ab35a99d94" - + on_catalina :or_older do + version "1.0.0" + sha256 "1866dfa833b123bb8fe7fa7185ebf24d28d300d0643d75798bc23730af734216" + end on_big_sur do version "1.2.0" sha256 "8c62a2b791cf5f0da6066a0a4b6e85f62949cd60975da062df44adf887f4370b" end - on_catalina :or_older do - version "1.0.0" - sha256 "1866dfa833b123bb8fe7fa7185ebf24d28d300d0643d75798bc23730af734216" + on_monterey :or_newer do + version "1.2.3" + sha256 "67cdb8a02803ef37fdbf7e0be205863172e41a561ca446cd84f0d7ab35a99d94" end url "file://#{TEST_FIXTURE_DIR}/cask/caffeine/#{platform}/#{version}/#{arch}.zip"