- Since moving `comments_hash` to `Stanza`, we've been using the wrong
kind of "comments": the comments for the _stanza_, not the comments
for the entire Cask.
- Add a test to ensure this actually works. There was previously an
infinite loop here due to the bad `comments`, visible in a `StanzaOrder`
cop test, which I speculatively added a failing test for. Turns out
that supporting nested stanza _ordering_ (vs. just grouping) is a
whole separate piece of work (there are multiple TODOs there already),
so I've backed that out and will do that separately.
- Since comment detection is only used in `Stanza`, move it there.
- The `stanzaify` method was only in `CaskBlock` since the other use of
`Stanza.new` was. Since it's only used in one other place, move it to
where it's used.
- A variant of this was an ancient TODO from 2018 (with `if/else` blocks).
- Now in 2023 we have `on_*` blocks within Casks that are very common.
- The most common stanzas present inside `on_*` blocks are `version`,
`sha256` and `url`. So I feel like it's worth keeping a consistent
style for these inside and outside `on_*` blocks.
The preferred method is opt_bin because that works
with the API and is more portable (works between versions).
Also removed the last example from the docs of `bin/"name"`
from the service block section
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 reverts commit b4cd90a3cc47bc2f94e4449fa99b37445b878a5f.
This should never have been merged, given its extraction into PR 15062
had a reasonably long discussion and was decided against in
https://github.com/Homebrew/homebrew-core/pull/126705#discussion_r1149545828,
but I didn't realise I hadn't backed it out of PR 15060 before it was
approved and I merged it.
- 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 `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.
- Otherwise syntax like this, where `#{arch}` is an interpolation,
causes an offense:
```ruby
cask "transcribe" do
arch arm: "_arm"
on_catalina :or_older do
version "8.75.2"
sha256 "f01781100cd3b9987c8f8892145a2eaa358df07b92e10e26f30b6a877f5b352c"
url "https://www.seventhstring.com/xscribe/downmo/transcribe#{version.no_dots}.dmg"
livecheck_version = "10"
end
on_big_sur :or_newer do
livecheck_version = "11"
version "9.21"
sha256 :no_check
url "https://www.seventhstring.com/xscribe/transcribe#{arch}.dmg"
end
[...]
end
```
```
Casks/transcribe.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: "_arm"
^^^^^^^^^^^^^^^^
1 file inspected, 1 offense detected
```
- 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).
