sorbet: Less RBI duplication to successfully typecheck RSpec tests#22540
Merged
Conversation
5a192b5 to
cb184f4
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR reduces Sorbet RBI duplication for RSpec test typechecking by moving helper method type information into the real test helper modules and tightening a number of test/helper sigils and bindings so typed specs typecheck cleanly.
Changes:
- Refactors the RSpec shim RBI to include typed helper modules instead of duplicating method signatures directly in
rspec.rbi. - Adds/strengthens Sorbet signatures (
sig) andtyped:levels in test helper modules, and updates specs with targetedT.bindcalls in DSL blocks. - Updates Tapioca require mappings and RuboCop RBIs/trimming allowlist to include
CopHelper.
Show a summary per file
| File | Description |
|---|---|
| Library/Homebrew/test/utils/analytics_spec.rb | Adds T.bind inside formula DSL blocks for better static typing. |
| Library/Homebrew/test/test_bot/formulae_spec.rb | Removes unsafe casts and adds T.bind to formula definitions. |
| Library/Homebrew/test/support/helper/subcommand.rb | Adds sigs for subcommand helpers and types Bundle::SubcommandContext. |
| Library/Homebrew/test/support/helper/spec/shared_context/integration_test.rb | Moves integration helpers into a typed module and includes it from the shared context. |
| Library/Homebrew/test/support/helper/mktmpdir.rb | Makes the helper typed: strict and adds a generic return sig. |
| Library/Homebrew/test/support/helper/formula.rb | Makes the helper typed: strict and tightens signatures for formula helpers. |
| Library/Homebrew/test/support/helper/cask.rb | Makes the helper typed: strict and adds a sig for stub_cask_loader. |
| Library/Homebrew/test/resource_spec.rb | Adds T.bind inside patch DSL block for correct receiver typing. |
| Library/Homebrew/test/messages_spec.rb | Adds T.bind to formula DSL blocks used by the tests. |
| Library/Homebrew/test/language/node_spec.rb | Adds T.bind to a formula DSL block. |
| Library/Homebrew/test/language/java_spec.rb | Adds T.bind to a formula DSL block. |
| Library/Homebrew/test/install_spec.rb | Upgrades file to typed: strict. |
| Library/Homebrew/test/dev-cmd/bump_spec.rb | Adds T.bind to a formula DSL block. |
| Library/Homebrew/test/description_cache_store_spec.rb | Adds T.bind to a formula DSL block. |
| Library/Homebrew/test/cmd/reinstall_spec.rb | Adds T.bind to a formula DSL block. |
| Library/Homebrew/test/cmd/install_spec.rb | Adds T.bind to multiple formula DSL blocks. |
| Library/Homebrew/test/cmd/home_spec.rb | Adds T.bind to a formula DSL block. |
| Library/Homebrew/test/cmd/bundle/dump_subcommand_spec.rb | Adds T.bind to a stubbed formula DSL block. |
| Library/Homebrew/test/cmd/bundle/check_subcommand_spec.rb | Adds T.bind to stubbed formula DSL blocks. |
| Library/Homebrew/test/cmd/--cache_spec.rb | Upgrades file from typed: false to typed: true. |
| Library/Homebrew/test/cli/named_args_spec.rb | Adds T.bind to formula DSL blocks. |
| Library/Homebrew/test/bundle/skipper_spec.rb | Adds T.bind to a stubbed formula DSL block. |
| Library/Homebrew/test/bundle/mac_app_store_spec.rb | Adds T.bind to a stubbed formula DSL block. |
| Library/Homebrew/test/bottle_filename_spec.rb | Adds T.bind to a formula DSL block. |
| Library/Homebrew/sorbet/tapioca/require.rb | Ensures additional requires load RuboCop RSpec helpers needed for RBIs. |
| Library/Homebrew/sorbet/rbi/shims/rspec.rbi | Removes duplicated method stubs and includes typed helper modules instead. |
| Library/Homebrew/sorbet/rbi/gems/rubocop@1.86.2.rbi | Adds CopHelper definitions and adjusts some Rainbow presenter types. |
| Library/Homebrew/dev-cmd/typecheck.rb | Updates RuboCop RBI trimming allowlist to keep CopHelper. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Files not reviewed (2)
- Library/Homebrew/sorbet/rbi/gems/rubocop@1.86.2.rbi: Language not supported
- Library/Homebrew/sorbet/rbi/shims/rspec.rbi: Language not supported
- Files reviewed: 26/28 changed files
- Comments generated: 1
6 tasks
- Don't duplicate all of `formula.rbi` in the `rspec.rbi` shim because it's a terrible pattern. - I also noticed a supposedly typechecked file `test/test_bot/formulae_spec.rb` that was definitely cheating through the use of `T.bind(self, T.untyped)` and `T.unsafe`. So, follow the same `T.bind(self, T.class_of(Formula))` pattern in test files to make sure we're getting proper type checking in `formula` blocks.
- Instead of duplicating all the methods in `rspec.rbi`, transfer the type sigs to the actual helper method definitions.
- Rather than duplicating all of the methods in `rspec.rbi` and `test/support/helper/spec/shared_context/integration_test.rb`, we can move the methods into a test helper module and include that in both places.
- `CopHelper` is a RuboCop module, so include it in the generated RuboCop RBI rather than hand-writing it.
cb184f4 to
14e04fc
Compare
Code Coverage OverviewLanguages: Ruby Ruby / code-coverage/simplecovThe overall coverage in the branch is 75%. The coverage in the branch is 78%. Show a code coverage summary of the most impacted files.
Updated |
- To teach Sorbet about RSpec's `alias_matcher` method without having to hand-write the RBI shim. - Use `extend` where the others are `include` here because `alias_matcher` is a class method, not an instance method. - Hence we can bump a bunch more tests to Sorbet `typed: true`.
f01b0fe to
f1f6e9f
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
brew lgtm(style, typechecking and tests) with your changes locally?GPT-5.3-Codex (Medium) for commit c2c1e92 because it was quite tedious.
rspec.rbi#22531 (review) as the duplication was getting extremely bad.