sorbet: Bump more tests to typed: true#22532
Conversation
6219e70 to
5577f1e
Compare
- `File.delete` expects a `String` or `Pathname`, but the test was passing it a `File`.
- `__dir__` is always a String, but Sorbet can't tell that.
- The `env` test is testing the BuildEnvironment env.
- Follow up to 8862004. - Also slim down the comments added in that commit, as it was too verbose to put everywhere else.
5577f1e to
4a4b06c
Compare
Code Coverage OverviewLanguages: Ruby Ruby / code-coverage/simplecovThe overall coverage in the branch remains at 78%, unchanged from the branch. Show a code coverage summary of the most impacted files.
Updated |
There was a problem hiding this comment.
Pull request overview
This PR continues the Sorbet adoption effort in Homebrew/brew by raising Sorbet strictness in a number of spec files and adding minimal scaffolding (dummy lets and RBI shims) so those tests typecheck cleanly.
Changes:
- Bump multiple RSpec files from
typed: falsetotyped: true(and one totyped: strict) and add Sorbet-friendly dummyletdefaults where needed. - Add/adjust Sorbet annotations in specs (e.g.,
T.must(__dir__),T.bind) to satisfy type inference. - Extend the RSpec shim RBI to include additional Formula DSL methods used in typed specs.
Show a summary per file
| File | Description |
|---|---|
| Library/Homebrew/test/utils/user_spec.rb | Condenses Sorbet-only dummy let commentary to an inline note. |
| Library/Homebrew/test/utils/string_inreplace_extension_spec.rb | Simplifies Sorbet dummy let explanation into an inline comment. |
| Library/Homebrew/test/utils/ruby_check_version_script_spec.rb | Converts multi-line Sorbet dummy let note into an inline comment. |
| Library/Homebrew/test/system_command_result_spec.rb | Bumps to typed: true and adds a Sorbet dummy stdout let for typechecking. |
| Library/Homebrew/test/services/formula_wrapper_spec.rb | Bumps to typed: true and adjusts a stub to satisfy Sorbet typing. |
| Library/Homebrew/test/install_spec.rb | Bumps from typed: true to typed: strict. |
| Library/Homebrew/test/dev-cmd/lgtm_spec.rb | Bumps to typed: true and adds T.must around __dir__ for Sorbet. |
| Library/Homebrew/test/cmd/update_spec.rb | Bumps to typed: true and adds T.must around __dir__ for Sorbet. |
| Library/Homebrew/test/cmd/bundle/remove_subcommand_spec.rb | Bumps to typed: true and introduces Sorbet-only dummy lets for inferred values. |
| Library/Homebrew/test/cleaner_spec.rb | Bumps to typed: true. |
| Library/Homebrew/test/cask/dsl/container_spec.rb | Bumps to typed: true and adds dummy params let for Sorbet. |
| Library/Homebrew/test/cask/dsl_spec.rb | Simplifies Sorbet dummy let explanation into an inline comment. |
| Library/Homebrew/test/cask/depends_on_spec.rb | Bumps to typed: true and adds dummy cask let for Sorbet in top-level subject. |
| Library/Homebrew/test/cask/artifact/zshcompletion_spec.rb | Bumps to typed: true and adds dummy cask_token let for Sorbet. |
| Library/Homebrew/test/cask/artifact/manpage_spec.rb | Bumps to typed: true and adds dummy cask_token let for Sorbet. |
| Library/Homebrew/test/cask/artifact/fishlcompletion_spec.rb | Bumps to typed: true and adds dummy cask_token let for Sorbet. |
| Library/Homebrew/test/cask/artifact/bashcompletion_spec.rb | Bumps to typed: true and adds dummy cask_token let for Sorbet. |
| Library/Homebrew/test/bundle/bundle_spec.rb | Bumps to typed: true and adds dummy brewfile_content let for Sorbet. |
| Library/Homebrew/test/build_environment_spec.rb | Adds a T.bind in a class body to help Sorbet infer the DSL receiver. |
| Library/Homebrew/sorbet/rbi/shims/rspec.rbi | Extends shims to include Formula#keg_only and Formula#skip_clean for typed spec DSL blocks. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Files not reviewed (1)
- Library/Homebrew/sorbet/rbi/shims/rspec.rbi: Language not supported
- Files reviewed: 19/20 changed files
- Comments generated: 1
| tempfile = File.new("/tmp/foo", File::CREAT) | ||
| allow(service).to receive_messages(installed?: true, service_file: Pathname.new(tempfile)) | ||
| allow(service).to receive_messages(installed?: true, service_file: tempfile_path = Pathname.new(tempfile)) | ||
| expect(service.plist?).to be(true) | ||
| File.delete(tempfile) | ||
| File.delete(tempfile_path) |
MikeMcQuaid
left a comment
There was a problem hiding this comment.
Thanks! Same comment as previous PR.
|
|
||
| # Some helper blocks are inferred as Formula instances or class contexts. | ||
| class Formula | ||
| sig { params(reason: T.any(String, Symbol), explanation: String).void } |
There was a problem hiding this comment.
As in the other PR: let's cast to Formula in relevant places and remove this here.
brew lgtm(style, typechecking and tests) with your changes locally?