Handle good things like:
```ruby
url "https://example.org/download",
verified: "example.org/download" # This is fine.
```
And bad things like:
```ruby
url "https://example.org/",
verified: "example.org" # This should end with a slash.
```
This was not handled at all during deserialization.
The string argument gets turned into an array internally
but we skip that to preserve all args in the @run_params
variable. That means that we have to handle strings when
deserializing too.
- Specify `strictish: true` in `add_error` to specify that it's not a
super big critical error.
- These will be shown only if `brew audit --strict` is requested.
- Ignore them and don't show them otherwise.
- Part three of issue 15074:
> As a result, I propose that all current cask audit warnings are never
> displayed as warnings but the underlying audit checks turned into
> errors displayed only with --strict (or one of the other relevant
> flags).
- Cask warnings are really noisy and numerous. Let's only show them if
the user passes `--strict` or something implying `--strict`, like
`--new-cask`.
- Additionally remove `display_passes` since we would like silence if
nothing is wrong with the cask, the same as with formula audits.
- Previously this components order cop only checked for correct stanza
order inside `on_*` blocks. This commit extends this cop to also check
for correct stanza order inside `head` and `resource` blocks. This is
a positive change since it standardizes the order of stanzas in all of
the places, making formulae more readable.
- Fixes issue 14017.
- This still doesn't pass `brew readall` for Casks, but it gets us a
little closer since if `url` has a `version` interpolated in it, the
`version` stanza has to come first.
- See https://github.com/Homebrew/homebrew-cask/pull/143201 for the
current failures.
- This, ie Mojave first, is more common in real Casks than the
alternative of newest to oldest ie Ventura first.
- Doing it this way reduces the number of offenses from ~500 to ~200.
- Complaining about only `on_arm` and `on_intel` was too restrictive
since casks can have many `on_system` blocks (`on_#{arch}` and
`on_#{os}`).
- We're a bit of the way there, anyway. Still doesn't support stanza
ordering within blocks, but that's for another time (there's a
separate issue that's been open for a while - 14017).
- These were previously being manually fixed which is time maintainers
could have spent fixing more important problems.
- I don't work with Casks much at all, so I was unsure as to what the
existing "arch" and "on_arch_conditional" parts were, if they're
deprecated or if things were eventually going to migrate to
`on_#{arch}` blocks?
- 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
```
This turns the ability to replace common paths with placeholders
into a mixin that can be used with both Casks and Formulae.
The idea here is to make formula hash generation more consistent.
- 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).
