Detect renamed files so renamed classes are reported#2
Merged
Conversation
detect.sh built its changed-file list with `git diff --diff-filter=AMD`. With git's default rename detection, a renamed file (e.g. Version.php -> VersionRenamed.php) is reported as a single rename (R) entry, which the AMD filter drops entirely — so neither the old nor the new path reached the snapshotter and the rename was reported as no change at all (composer/composer#12919). Add `--no-renames` so a rename is decomposed into a delete (old path) + add (new path), both of which pass the AMD filter: the old FQCN is then reported as removed and the new FQCN as added. The snapshot side already ignores paths absent on a given ref, so the same file list works for both. Also add `--no-renames` to preflight.sh for consistency: it makes a rename emit the full old/new content as -/+ lines, so the token scan reliably triggers analysis even for a rename that only changes a namespace. Adds an integration test mirroring composer/composer#12919.
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
1 participant
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.
Problem
On composer/composer#12919 the file/class
src/Composer/Platform/Version.php(class Version) was renamed toVersionRenamed.php(class VersionRenamed). The posted comment reported the in-place edits (Cache/Installer/Runtime) but omitted the rename entirely — neitherComposer\Platform\Version(removed) norComposer\Platform\VersionRenamed(added) appeared.Root cause
scripts/detect.shbuilt its changed-file list withgit diff "${BASE_REF}..HEAD" --diff-filter=AMD --name-only. With git's default rename detection on, a 99%-similar rename is reported as a single rename (R) entry, and--diff-filter=AMDkeeps only A/M/D — droppingRentirely. So neither the old nor the new path ever reached the snapshotter.The snapshot side was already correct:
Snapshotter::snapshot()resolves each path withrealpath()and silently skips paths absent on a given ref, so once both paths are in the (shared) file list, BASE yieldsVersionand HEAD yieldsVersionRenamedon its own.Fix
scripts/detect.sh— add--no-renames, which decomposes a rename into a Delete(old) + Add(new); both pass the existingAMDfilter, so the old FQCN is reported as removed and the new one as added.scripts/preflight.sh— add--no-renamesfor consistency. It makes a rename emit the full old/new content as-/+lines so the token scan reliably triggers analysis, closing a blind spot where a rename that only changes a namespace could wrongly gate analysis off. Preflight stays biased toward "true".tests/DetectTest.php—testRenamedClassIsReportedAsRemovedAndAdded, an integration test mirroring this scenario. It fails on the old code (empty body) and passes with the fix.README.md— note that a rename is reported as removal of the old FQCN + addition of the new one (no rename-pairing).Verification
🤖 Generated with Claude Code