bundle: narrow types#22799
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR tightens Sorbet typing across brew bundle’s checking logic by replacing broad Object usage with more specific types (notably Dsl::Entry) and by standardizing check outputs as arrays of error strings. This improves type inference through the bundle checker pipeline and reduces the need for explicit casts.
Changes:
- Standardize
check/find_actionablereturn types toT::Array[String](error messages), including makingexit_early_checkproduce the same kind of output asfull_check. - Convert
Homebrew::Bundle::Checker::CheckResultfrom an untypedStructto a typedT::Struct. - Narrow many
entriesparameters fromT::Array[Object]toT::Array[Dsl::Entry], removing redundantT.castusage.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| Library/Homebrew/bundle/tap.rb | Narrows entries typing for tap checks to Dsl::Entry. |
| Library/Homebrew/bundle/package_type.rb | Standardizes check output to string errors and narrows entries typing. |
| Library/Homebrew/bundle/extensions/winget.rb | Narrows entries typing in format_checkable. |
| Library/Homebrew/bundle/extensions/uv.rb | Narrows entries typing in format_checkable. |
| Library/Homebrew/bundle/extensions/mac_app_store.rb | Narrows entries typing; updates early-exit check to return string errors. |
| Library/Homebrew/bundle/extensions/flatpak.rb | Narrows entries typing and removes casts. |
| Library/Homebrew/bundle/extensions/extension.rb | Narrows entries typing and standardizes check return type to string errors. |
| Library/Homebrew/bundle/checker.rb | Introduces typed CheckResult and narrows all checker error arrays to String. |
| Library/Homebrew/bundle/brew.rb | Narrows entries typing and standardizes actionable results as string errors. |
| Library/Homebrew/bundle/brew_services.rb | Narrows entries typing in format_checkable. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Updates some Object usage to more specific types. Some changes include: - Change `exit_early_check` to return String errors so that it has the same return signature as `full_check` - Make `CheckResult` into a `T::Struct` for better type checking - `entries` consist of `Dsl::Entry` which avoids having to `T.cast`
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.
brewcommands to reproduce the bug?brew lgtm(style, typechecking and tests) locally?Updates some Object usage to more specific types. Some changes include:
exit_early_checkto return String errors so that it has the same return signature asfull_checkCheckResultinto aT::Structfor better type checkingentriesconsist ofDsl::Entrywhich avoids having toT.castThis allows Sorbet to follow types better.
Some usage of Object needs to remain due to odd inheritance hacks we have for different "packages". Will require a refactor to get rid of these.