Skip to content

[Caching] Fix cross-file cache false negative - invalidate on ancestor change (#8493) - #8783

Closed
TomasVotruba wants to merge 2 commits into
mainfrom
reproduce-8493-cache-dependency
Closed

TomasVotruba wants to merge 2 commits into
mainfrom
reproduce-8493-cache-dependency

Conversation

@TomasVotruba

@TomasVotruba TomasVotruba commented Oct 23, 2026 •

Copy link
Copy Markdown
Member

Fixes #8493.

A child class's refactoring result can depend on its parent/interface/trait (e.g. AddReturnTypeDeclarationBasedOnParentClassMethodRector, ReturnTypeFromStrictTypedCallRector, AddOverrideAttributeToOverriddenMethodsRector). The cache keyed each file only by its own contents, so when an ancestor changed and the child did not, the child's entry still looked valid - it was skipped and the transformation it now needed was silently missed.

Fix

  • FileDependencyResolver resolves the ancestry files (parents, interfaces, traits) a file relies on, via reflection. Best-effort: resolving ancestors can throw on malformed/unresolvable classes, and a miss there only skips a cache optimisation, so it is caught and ignored.
  • When a clean file is cached, ChangedFilesDetector also stores the content hash of each ancestry file under a separate key. hasFileChanged() reports the file as changed when its own hash matches but any stored ancestor hash no longer does (or the ancestor file disappeared).

Scope

Covers class-ancestry dependencies, which are the cases reported in the issue. It does not track arbitrary cross-file dependencies (a called function's file, a referenced constant); that would need a full dependency graph and is out of scope here.

Tests

  • CacheFileDependencyTest - end-to-end: a cached child is reconsidered after its parent gains a return type.
  • FileDependencyResolverTest - unit: the resolver returns the parent file, and nothing for an ancestor-less class.
  • Full suite green.

Note

Extra reflection + ancestor-hash work runs per clean cached file, so warm runs do slightly more work. Correctness-first; happy to run a full warm-run A/B against main if wanted before merge.

@TomasVotruba TomasVotruba changed the title [Tests] Add reproducer for cross-file cache false negative (#8493) [Caching] Fix cross-file cache false negative - invalidate on ancestor change (#8493) Oct 23, 2026
@TomasVotruba
TomasVotruba deleted the reproduce-8493-cache-dependency branch October 23, 2026 09:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

No dependency tracking - reproducible false negative

1 participant