- Fixing the test expected output was unbelievably tedious.
- There's been debate about this setting being `false` but in
https://github.com/Homebrew/brew/pull/15136#issuecomment-1500063225
we decided that it was worth using the default since RuboCop behaviour changed
so we'd have had to do some horrible things to keep it as `false` -
https://github.com/Homebrew/brew/pull/15136#issuecomment-1500037278 -
and multiple maintainers specify the `--display-cop-names` option to
`brew style` themselves since it's clearer what's gone wrong.
- This skips over stanza names that are not overrideable in `on_*`
blocks, with the positive side effect that `on_*` blocks themselves
aren't in the list so we can get rid of another conditional.
- Stanzas overrideable in blocks are defined in `Cask::DSL` by each of
the methods calling `set_unique_stanza`.
- This came up in Cask `simply-fortran`:
```
Scanning /opt/homebrew/Library/Taps/homebrew/homebrew-cask/Casks/simply-fortran.rb
send_node: s(:send, nil, :arch), send_node.parent: s(:begin,
s(:send, nil, :arch)), send_node.parent.parent: (regexp
(str "href=.*?simplyfortran[._-]v?(\\d+(?:\\.\\d+)+)")
(begin
(send nil :arch))
(str "\\.dmg")
(regopt :i))
Casks/simply-fortran.rb:2:3: C: Cask/NoOverrides: Do not use a top-level arch 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.
arch arm: "-arm64", intel: "-x86_64"
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
1 file inspected, 1 offense detected
```
- This passes the previously failing test for `on_*` blocks with
`livecheck` blocks with multiple stanzas inside them (eg `url` and
`strategy`) that weren't being correctly skipped because we weren't
detecting high enough up the ancestry.
- The Cask `little-snitch4` in `Homebrew/homebrew-cask-versions` was
failing and it took me a while to figure out _how_. Add a test for
easier further debugging (and to prevent breakage once the bug is
fixed).
```
❯ brew tests --only=rubocops/cask/no_overrides
Randomized with seed 29917
1 process for 1 spec, ~ 1 spec per process
F
Failures:
1) RuboCop::Cop::Cask::NoOverrides when there are livecheck blocks within `on_*` blocks, ignore their contents does not report any offenses
Failure/Error: DEFAULT_FAILURE_NOTIFIER = lambda { |failure, _opts| raise failure }
expected `[#<RuboCop::Cop::Offense:0x000000012de636c8 @severity=#<RuboCop::Cop::Severity:0x000000012de636a0 @name=:convention>, @location=#<Parser::Source::Range (string) 244...273>, @message="Do not use a top-level `url` stanza as the default. Add it to an `on_{system}` block instead.\nUse `:or_older` or `:or_newer` to specify a range of macOS versions.\n", @cop_name="Cask/NoOverrides", @status=:unsupported, @corrector=nil>].empty?` to be truthy, got false
Shared Example Group: "does not report any offenses" called from ./test/rubocops/cask/no_overrides_spec.rb:77
# ./test/rubocops/cask/shared_examples/cask_cop.rb:24:in `expect_no_offenses'
# ./test/rubocops/cask/shared_examples/cask_cop.rb:7:in `block (2 levels) in <module:CaskCop>'
Took 2 seconds
Tests Failed
```
- The Cask `sip`, to give a random example, was failing this RuboCop
because it has a `livecheck` block within an `on_*` block and the
livecheck block and the top-level Cask both have `url` stanzas. This
is a legitimate use of `livecheck` blocks because the cask software
download URL and the livecheck version check URL are not the same
thing, so let's skip over `livecheck` blocks and their contents.
- I wrote this because I was concerned that I'd done something wrong
since `brew style --only=Cask/NoOverrides` in
`$(brew --repo homebrew/cask)` didn't report any offenses. Turns out
I'd just missed the `.` off the end of the command to target the
current directory! But, the cost of writing the test is sunk now,
and more tests can't hurt?
- In the event that there's only one common stanza within the `on_*`
blocks (eg, `url`) with a generic `version` that doesn't change per-OS,
let's not force adding `version` to each `on_*` block as well.
- 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).
