Skip to content

Comments

linkage_cache_store: typed: strict#21594

Open
costajohnt wants to merge 3 commits intoHomebrew:mainfrom
costajohnt:linkage-cache-store-strict
Open

linkage_cache_store: typed: strict#21594
costajohnt wants to merge 3 commits intoHomebrew:mainfrom
costajohnt:linkage-cache-store-strict

Conversation

@costajohnt
Copy link

@costajohnt costajohnt commented Feb 19, 2026

Summary

  • Bump linkage_cache_store.rb from typed: true to typed: strict
  • Add sig annotations to all 6 methods (initialize, keg_exists?, update!, fetch, delete!, fetch_hash_values)
  • Add T.let for @keg_path instance variable in initialize
  • Make CacheStore properly abstract (abstract!) and remove pseudo-abstract *args methods that used raise NotImplementedError. This resolves the Sorbet runtime override shape validation that occurs when subclass methods have typed signatures with fixed parameters while the parent used *args.
  • Fix type mismatch in keg.rb where Pathname was passed to LinkageCacheStore.new instead of String (surfaced by the new sig annotation)

Contributes to #17297

AI/LLM Disclosure

This PR was developed with assistance from Claude (Opus 4.6) via Claude Code. All code has been reviewed and understood. I can address review comments directly.

Test plan

  • brew typecheck — no errors
  • brew style Library/Homebrew/cache_store.rb Library/Homebrew/linkage_cache_store.rb Library/Homebrew/keg.rb — no offenses
  • brew tests --only=linkage_cache_store — 7 examples, 0 failures
  • brew tests --only=cache_store — 11 examples, 0 failures
  • brew tests --only=description_cache_store — 7 examples, 0 failures
  • brew tests --only=keg — 33 examples, 0 failures
  • All above tests pass with HOMEBREW_SORBET_RUNTIME=1 (simulating CI environment)

@costajohnt
Copy link
Author

The syntax CI failure is not caused by our code changes — it's caused by a bug in the cache-homebrew-prefix action that was recently introduced in #21565.

What's happening: The cache-homebrew-prefix action restores a cached copy of the entire Homebrew prefix (/home/linuxbrew/.linuxbrew), which includes the Homebrew/ source tree. This overwrites the PR code that setup-homebrew previously checked out, causing brew typecheck to run against stale (cached) source code.

Verification: The merge ref (refs/pull/21594/merge) contains the correct fix (path.to_s), but CI reports the old code (path) because the cache restore overwrites it:

keg.rb:318: Expected `String` but found `Pathname` for argument `keg_path`
     318 |      LinkageCacheStore.new(path, db).delete!

Filed: Homebrew/actions#796

costajohnt and others added 3 commits February 21, 2026 21:32
Add Sorbet `sig` annotations to all methods and `T.let` for instance
variables to enable `typed: strict` in `linkage_cache_store.rb`.

Contributes to Homebrew#17297

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The `CacheStore` base class used `*args` with `raise NotImplementedError`
as a pseudo-abstract pattern. When subclasses add Sorbet `sig`
annotations, the Sorbet runtime's override shape validation rejects the
incompatible signatures (child methods have fixed params, parent has
`*args`).

Replace this pattern with Sorbet's proper `abstract!` declaration and
remove the pseudo-abstract methods entirely. `CacheStore` is never
instantiated directly - all callers use concrete subclasses.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Pass `path.to_s` instead of `path` (a Pathname) to
`LinkageCacheStore.new`, matching the pattern used in
`linkage_checker.rb`. The sig on `initialize` correctly requires a
String since the value is used as a JSON-serializable database key.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@costajohnt costajohnt force-pushed the linkage-cache-store-strict branch from e640fd0 to 1a89ca4 Compare February 22, 2026 05:33
Copy link
Member

@MikeMcQuaid MikeMcQuaid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work so far! One question then good to merge.

#
# @param hash_values [Hash] hash containing KVPs of { :type => Hash }
# @return [nil]
sig { params(hash_values: T::Hash[Symbol, T.untyped]).void }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you see if this or any other T.untyped in this file can use T.anything instead?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants