- 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
```
Fixes
TypeError: Parameter 'before': Expected type T.nilable(T.any(Regexp, String)), got type Pathname with value #<Pathname:/opt/homebrew/Cellar/php@8.1/8.1.21/lib/php>
Caller: /opt/homebrew/Library/Homebrew/formula.rb:2484
Definition: /opt/homebrew/Library/Homebrew/utils/inreplace.rb:48
https://github.com/Homebrew/homebrew-core/actions/runs/5635089949/job/15266014851#step:3:1737
- Sorbet gives preference to class methods over methods defined in
included modules, hence Sorbet was unavailable to resolve the
definition of the gsub! method.
- The gsub! method in StringInreplaceExtension conflicts with the definition in String.
- This PR refactors the call to the gsub! method so that a custom object
is exposed instead of a string.
Adjust the rules based on the current codebase. Remove various enable,
disables and default values that are unnecessary. Add more comments
explaining why. Make minor changes needed to enable a few more rules.
When the first parameter to inreplace was an array, and the replacement
failed, InreplaceError would end up crashing with an undefined method
exception because the order of operations resulted in super not being
passed the value of the entire inject block.
`any?` is not the opposite of `empty?`. Besides the case that
`[false, nil].any?` will return false, `any?`(O(n)) has much worse
performance than `empty?`(O(1)).
Provides feature parity between the block and non-block forms of
inreplace by creating a four-argument version of the non-block form,
where the fourth argument is an optional Boolean value, defaulting to
true, which specifies whether a failed inreplace should cause an
InreplaceError error to be raised. The fourth argument is passed along
to StringInreplaceExtension#gsub!, which already supports an optional
audit_result argument.
This resolves the Catch-22 that single replacements aren't permissible
in the block form (in that they now cause `brew audit` to complain), but
the audit_result argument is not available in the non-block form.
Closes#552.
Signed-off-by: ilovezfs <ilovezfs@icloud.com>
