Currently, we silently ignore cases where a formula previously had an
`:all` bottle but now no longer does.
These are most often due to (in order of likelihood):
- bottle reproducibility breakage in `brew`
- new hard-coded `/usr/local` references in text files in a bottle
The former is a bug that should be fixed, while the latter can be fixed
trivally with an `inreplace`.
Let's try to make sure we always do this by making `brew bottle` error
out so that we can catch these instances as they happen rather than
after the fact.
I haven't encountered any cases where a formula previously had an `:all`
bottle but no longer does for reasons other than the two outlined above.
If we do encouter those in the future, we can either:
- update `brew bottle` to skip this check, perhaps with a new flag
- delete the formula's old `:all` bottle before doing `brew bottle` so
it doesn't error
`brew bump-formula-pr` is encountering a type error, as the inferred
return type of `GitHub#check_for_duplicate_pull_requests` doesn't
align with the explicit return type of `#check_pull_requests`:
```
Error: Return value: Expected type T.nilable(T::Array[String]), got
type Module with value T::Private::Types::Void::VOID
Caller: /home/linuxbrew/.linuxbrew/Homebrew/Library/Homebrew/dev-cmd/
bump-formula-pr.rb:137
Definition: /home/linuxbrew/.linuxbrew/Homebrew/Library/Homebrew/
dev-cmd/bump-formula-pr.rb:472 (Homebrew::DevCmd::BumpFormulaPr
#check_pull_requests)
```
This addresses the issue by adding a type signature with a `void`
return type to `#check_for_duplicate_pull_requests` and setting the
return type of `#check_pull_requests` to `void` as well. The return
type from `#check_pull_requests` isn't used, so a `void` return type
is arguably a better reflection of the method's behavior. The
`#check_pull_requests` method in `BumpCaskPr` has a `void` return
type, so this change brings the `BumpFormulaPr` method in line.
- change the messaging depending on how confident we are that we're
actually looking at duplicates i.e. we're not confident without a
version number supplied
- similarly, just warn instead of failing with an error (and no
override) if we're not confident that we're looking at duplicates
because a version wasn't supplied
- change `bump-cask-pr` and `bump-formula-pr` to always check for all
pull requests with the new version number (to allow failing on this)
rather than only checking closed pull requests with a version number
- change `bump` to check for definite/maybe duplicate PRs and only
exit if they are definitely duplicates
- cleanup some variable usage to DRY things up a bit
```shell
$ brew typecheck homebrew/bundle
No sorbet/ directory found. Maybe you want to run 'srb init'?
A type checker for Ruby
Usage:
srb Same as "srb t"
srb (init | initialize) Initializes the `sorbet` directory
srb rbi [options] Manage the `sorbet` directory
srb (t | tc | typecheck) [options] Typechecks the code
Options:
-h, --help View help for this subcommand.
--version Show version.
For full help:
https://sorbet.org
Check https://docs.brew.sh/Typechecking for more information on how to resolve these errors.
```
- Previously I thought that comments were fine to discourage people from
wasting their time trying to bump things that used `undef` that Sorbet
didn't support. But RuboCop is better at this since it'll complain if
the comments are unnecessary.
- Suggested in https://github.com/Homebrew/brew/pull/18018#issuecomment-2283369501.
- I've gone for a mixture of `rubocop:disable` for the files that can't
be `typed: strict` (use of undef, required before everything else, etc)
and `rubocop:todo` for everything else that should be tried to make
strictly typed. There's no functional difference between the two as
`rubocop:todo` is `rubocop:disable` with a different name.
- And I entirely disabled the cop for the docs/ directory since
`typed: strict` isn't going to gain us anything for some Markdown
linting config files.
- This means that now it's easier to track what needs to be done rather
than relying on checklists of files in our big Sorbet issue:
```shell
$ git grep 'typed: true # rubocop:todo Sorbet/StrictSigil' | wc -l
268
```
- And this is confirmed working for new files:
```shell
$ git status
On branch use-rubocop-for-sorbet-strict-sigils
Untracked files:
(use "git add <file>..." to include in what will be committed)
Library/Homebrew/bad.rb
Library/Homebrew/good.rb
nothing added to commit but untracked files present (use "git add" to track)
$ brew style
Offenses:
bad.rb:1:1: C: Sorbet/StrictSigil: Sorbet sigil should be at least strict got true.
^^^^^^^^^^^^^
1340 files inspected, 1 offense detected
```
Per feedback to https://github.com/Homebrew/brew/pull/17806, this
moves some `require` statements in `dev-cmd/contributions.rb` and
`Utils::GitHub` into the methods that need them.
This updates the type signature for `#scan_repositories` to address a
runtime type error and to reflect the actual return type.
The logic in `#scan_repositories` to check for unsupported
repositories leads to a type error, as `#ofail` has a void return
type. To resolve this, I moved the repository verification code into
`#run` (after `repos` is defined but before it's used) and used
`#odie`, so the command will exit early with an error.
While I was at it, I updated the type for the `repos` parameter to
not be `nilable`, as it shouldn't be `nil` based on how we're
handling `repos` in `#run`.
CSV generation is optional, so this moves the related `require` into
the method where `CSV` is used (following a pattern we've used for
other `require` calls throughout `brew`).
This resolves `unitialized constant` errors in `brew contributions`
(`Tap`, `GitHub`) and `Utils::GitHub` (`Utils::Curl`).
This also preemptively adds some requires to `Utils::GitHub` and
`GitHub::API`, to avoid similar errors.
