Remove dead API methods and move generated_hash! to tests#23095
Merged
Conversation
Extracted from the typecheck-deadcode branch (#22733 was the first extraction), which uses Spoom to find code with no static callers. Remove four methods with no callers: - Homebrew::API.cached_formula_json_file_path - Homebrew::API::Cask.all_renames - Homebrew::API::Formula.all_renames - APIHashable#generated_hash! Renames are resolved through Homebrew::API.formula_renames and Homebrew::API.cask_renames, which read the internal API, so the all_renames pair on the JSON API classes is unused. The only remaining references were vestigial RSpec stubs, so drop those too. APIHashable#generated_hash! reverts the global monkeypatches applied by generating_hash!. The generate-*-api commands exit once they are done and never call it, so it is only needed to stop tests leaking global state. Move it to a test helper rather than keeping it in production code.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR removes dead code in the Homebrew::API surface area and relocates APIHashable#generated_hash! into the test suite, keeping production behavior intact while preserving test isolation for API-generation monkeypatches.
Changes:
- Removed unused API methods:
Homebrew::API.cached_formula_json_file_path,Homebrew::API::Formula.all_renames, andHomebrew::API::Cask.all_renames. - Moved
APIHashable#generated_hash!from production (Library/Homebrew/api_hashable.rb) into a new test helper and loaded it viatest/spec_helper.rb. - Updated affected specs to stop stubbing the removed
all_renamesmethods.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| Library/Homebrew/test/support/helper/api_hashable.rb | Adds test-only APIHashable#generated_hash! to revert API-generation monkeypatches. |
| Library/Homebrew/test/spec_helper.rb | Requires the new APIHashable test helper for the test suite. |
| Library/Homebrew/test/formulary_spec.rb | Removes stubbing of deleted Homebrew::API::Formula.all_renames. |
| Library/Homebrew/test/cask/cask_loader/from_api_loader_spec.rb | Removes stubbing of deleted Homebrew::API::Cask.all_renames. |
| Library/Homebrew/api/formula.rb | Deletes dead Homebrew::API::Formula.all_renames. |
| Library/Homebrew/api/cask.rb | Deletes dead Homebrew::API::Cask.all_renames. |
| Library/Homebrew/api.rb | Deletes dead Homebrew::API.cached_formula_json_file_path. |
| Library/Homebrew/api_hashable.rb | Removes APIHashable#generated_hash! from production code. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
carlocab
approved these changes
Jul 14, 2026
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.
Another extraction from my
typecheck-deadcodebranch (following #22733), which runs Spoom's dead code analysis overLibrary/Homebrew. This slice covers theHomebrew::APIfiles.What this removes
Four methods with no callers anywhere:
brewhomebrew-core/homebrew-caskHomebrew::API.cached_formula_json_file_pathHomebrew::API::Cask.all_renamesHomebrew::API::Formula.all_renamesAPIHashable#generated_hash!The GitHub code search for
cached_formula_json_file_pathreturns hits only in vendored copies and forks ofbrewitself, i.e. the definition site, so there are no third-party consumers.Why
all_renamesis deadRenames are resolved through
Homebrew::API.formula_renamesandHomebrew::API.cask_renames, which read the internal API (Homebrew::API::Internal). Those are the methodsFormulary,Cask::CaskLoader,CoreTap,CoreCaskTapandbundleactually call. Theall_renamespair on the JSON API classes is left over from before that path existed, and nothing reaches it. Its only remaining references were two RSpec stubs that stub a method no code under test calls, so those go too.Why
generated_hash!moves to a test helper insteadgenerating_hash!applies global monkeypatches (it swaps outHOMEBREW_PREFIXand setsHOMEandGIT_CONFIG_GLOBAL) so that generated JSON contains placeholders.generated_hash!is the inverse, reverting them.In production nothing ever reverts:
brew generate-formula-api,brew generate-cask-apiandbrew generate-internal-apicallgenerating_hash!and then exit, so the process dies with the monkeypatches still applied. The only callers ofgenerated_hash!are two specs, which need it precisely so that the monkeypatching does not leak into other examples.That makes it a test helper living in production code, so I moved it to
test/support/helper/api_hashable.rbrather than deleting it outright. Behaviour is unchanged, and the two specs still pass.brewcommands to reproduce the bug?brew lgtm(style, typechecking and tests) locally?Claude Code was used to run the dead code analysis, audit each candidate for callers (inside
brew, inhomebrew-coreandhomebrew-caskcheckouts, and via GitHub code search), and prepare the diff. I reviewed the result and verified it locally withbrew lgtm.No new tests are added because this is a removal. The existing specs are the check: they exercise every method touched here, and they were run before and after. The
all_renamesstub removals and thegenerated_hash!move are both covered bytest/formulary_spec.rb,test/cask/cask_loader/from_api_loader_spec.rb,test/cask/cask_spec.rbandtest/service_spec.rb.