Remove unneeded T.let #21297
Merged
Remove unneeded T.let #21297
Conversation
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
When constructor argument is used directly (as opposed to a method call on the argument), Sorbet can infer the type[^1]. This allows avoiding the unnecessary function call to `T.let` which is run even when runtime checks are disabled for users. [^1]: https://sorbet.org/docs/type-assertions#prefer-method-signatures-over-type-assertions
Contributor
There was a problem hiding this comment.
Pull request overview
This PR removes unnecessary T.let type assertions in constructor methods throughout the Homebrew codebase. When constructor parameters are assigned directly to instance variables and have type signatures in the method's sig block, Sorbet can infer types without explicit T.let calls, eliminating unnecessary runtime overhead.
- Removes redundant
T.letcalls from 21 files across various Homebrew components - Maintains type safety through existing method signatures
- Improves runtime performance by avoiding unnecessary function calls
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| Library/Homebrew/utils/shebang.rb | Removed T.let for regex, max_length, and replacement parameters in RewriteInfo constructor |
| Library/Homebrew/utils/github/artifacts.rb | Removed T.let for token parameter in GitHubArtifactDownloadStrategy constructor |
| Library/Homebrew/utils/github/api.rb | Removed T.let for reset parameter in RateLimitExceededError constructor |
| Library/Homebrew/unpack_strategy.rb | Removed T.let for merge_xattrs parameter in UnpackStrategy constructor |
| Library/Homebrew/test_runner_formula.rb | Removed T.let for formula and eval_all parameters in TestRunnerFormula constructor |
| Library/Homebrew/software_spec.rb | Removed T.let for flags parameter in SoftwareSpec constructor |
| Library/Homebrew/services/formula_wrapper.rb | Removed T.let for formula parameter and output/success/type parameters in nested classes |
| Library/Homebrew/rubocops/cask/ast/stanza.rb | Removed T.let for method_node and all_comments parameters in Stanza constructor |
| Library/Homebrew/pypi_packages.rb | Removed T.let for package_name, extra_packages, exclude_packages, and dependencies parameters |
| Library/Homebrew/pkg_version.rb | Removed T.let for version and revision parameters in PkgVersion constructor |
| Library/Homebrew/os/linux/elf.rb | Removed T.let for path parameter in ELF constructor |
| Library/Homebrew/mktemp.rb | Removed T.let for retain_in_cache parameter in Mktemp constructor |
| Library/Homebrew/github_runner_matrix.rb | Removed T.let for testing_formulae, deleted_formulae, all_supported, and dependent_matrix parameters |
| Library/Homebrew/formula_info.rb | Removed T.let for info parameter in FormulaInfo constructor |
| Library/Homebrew/formula.rb | Removed T.let for name, unresolved_path, alias_path, and force_bottle parameters in Formula constructor |
| Library/Homebrew/diagnostic.rb | Removed T.let for verbose parameter in Checks constructor |
| Library/Homebrew/cask_dependent.rb | Removed T.let for cask parameter in CaskDependent::Requirement constructor |
| Library/Homebrew/cask/url.rb | Removed T.let for caller_location parameter in URL constructor |
| Library/Homebrew/cask/dsl/base.rb | Removed T.let for cask parameter in DSL::Base constructor |
| Library/Homebrew/build_options.rb | Removed T.let for args and options parameters in BuildOptions constructor |
| Library/Homebrew/build.rb | Removed T.let for args parameter in Build constructor |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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?When constructor argument is used directly (as opposed to a method call on the argument), Sorbet can infer the type1. This allows avoiding the unnecessary function call to
T.letwhich is run even when runtime checks are disabled for users.Footnotes
https://sorbet.org/docs/type-assertions#prefer-method-signatures-over-type-assertions ↩