rubocops/cask: Enforce the use of on_{system} blocks

- As discussed in
  https://github.com/Homebrew/brew/pull/14976#issuecomment-1474544569
  and further comments, this is needed because in order to enforce the
  order of `on_{arch,system}` blocks we need to have everything
  consistently within one of those blocks.
- We previously allowed overrides where the top-level `version` stanza
  would be the default, unless on an OS that had an `on_system` block
  with a `version` specified. But this breaks down when we try to order
  the `on_system` blocks because if a `url` at the top-level has a
  `version` interpolated in it, then the `version` stanza needs to be
  above the `url` stanza. But it could be that `version` is OS-specific.
- Let's stop allowing overrides and require that everything be in an
  `on_system` block. This will make it easier to enforce the order of
  `on_system` blocks in a future PR (14976).
This commit is contained in:
Issy Long 2023-03-19 17:31:32 +00:00
parent a8b1e2c18d
commit 9cc046bc60
No known key found for this signature in database
GPG Key ID: 8247C390DADC67D4
3 changed files with 96 additions and 0 deletions

View File

@ -0,0 +1,31 @@
# typed: true
# frozen_string_literal: true
module RuboCop
module Cop
module Cask
class NoOverrides < Base
extend T::Sig
include CaskHelp
MESSAGE = <<~EOS
Do not use top-level `%<stanza>s` stanza as the default, add an `on_{system}` block instead.
Use `:or_older` or `:or_newer` to specify a range of macOS versions.
EOS
def on_cask(cask_block)
return if cask_block.toplevel_stanzas.empty?
cask_block.toplevel_stanzas.each do |stanza|
# TODO: We probably only want to disallow `version`, `url`, and `sha256` stanzas being overridden?
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? && stanza.parent_node.method_name.to_s.start_with?("on_")
add_offense(stanza.source_range, message: format(MESSAGE, stanza: stanza.stanza_name))
end
end
end
end
end
end

View File

@ -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"

View File

@ -0,0 +1,64 @@
# 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 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 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 top-level `version` 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: 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
include_examples "reports offenses"
end
end