- Use `sort_by` to sort the array, rather than comparing each element
to the next.
- This doesn't error with complaints about clobbering at all when run on
`homebrew/cask`, hurray. And it also handles interpolations correctly,
rather than ignoring them.
Co-authored-by: Bevan Kay <email@bevankay.me>
- Interpolating the version into a path is a common pattern, but the interpolations
trip up the alphabetization autocorrect quite spectacularly, so let's
ignore them (for now?).
Without this, there's an infinite loop on `brew style --fix` if you have
a `if Hardware::CPU.arm?` in a `postflight` block where it will change
back and forward between `if` and `on_os` syntax forever.
- Part of issue 16323.
- Previously this was being done manually by Cask maintainers.
- While we're here, enforce that the `zap trash` path is not in `[]` if
it only contains a single element.
- This is buggy on actual Casks, hence the draft PR.
- Thanks to Markus on Slack for saying "the cop should only apply to the
content of the blocks, or more specifically only to stanzas that are
direct children of cask or on_* blocks", which made me realize that
I was overcomplicating things.
- Now that we detect correct stanza _grouping_ within `on_*` blocks in
Casks (PR 15211), correct stanza _ordering_ in `on_*` blocks was the
next logical step. For example, `url` has to come after `version` and
`sha256` in an `on_macos` or `on_intel` block for consistency with the
top-level stanza order we enforce elsewhere.
- Still not doing the nested `on_os` inside `on_arch`, that felt
excessive for an edge case that isn't present in any actual real
Casks we have. I removed the test with that specific TODO.
- 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.
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 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.
