Skip to content

Commit a49fdcf

Browse files
committed
Detect renamed files so renamed classes are reported
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.
1 parent c630883 commit a49fdcf

4 files changed

Lines changed: 50 additions & 2 deletions

File tree

README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,4 +145,5 @@ If `composer install` fails (network hiccup, auth issue, missing `composer.json`
145145

146146
- Trait usage is not factored into the introduction-point check. A class that newly applies a trait will see the trait's methods reported as new on the class.
147147
- Anonymous classes are skipped.
148+
- Renaming a class (or moving it to another namespace) is reported as a removal of the old fully-qualified name plus an addition of the new one — there is no rename-pairing heuristic.
148149
- When a parent class or implemented interface can't be resolved (no `install-dependencies`, or a private repository, or a transient install failure), the introduction-point check defaults to "introduced here" — i.e. methods will be reported even if they actually came from an unresolved parent. This is the conservative direction: rather over-report than miss real API additions.

scripts/detect.sh

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,14 @@ mkdir -p "${OUTPUT_DIR}"
4040

4141
# All files in scope (added, modified, or deleted) — snapshot.php will silently
4242
# ignore missing files, so we can pass the same list for both refs.
43-
mapfile -t changed_files < <(git diff "${BASE_REF}..HEAD" --diff-filter=AMD --name-only -- "${paths_array[@]}" || true)
43+
#
44+
# --no-renames is essential: with git's default rename detection a renamed file
45+
# (e.g. Version.php -> VersionRenamed.php) is reported as a single R entry, which
46+
# --diff-filter=AMD drops entirely, so neither path would reach the snapshotter.
47+
# Disabling rename detection decomposes a rename into a delete (old path) + add
48+
# (new path) — both pass the AMD filter, so the old FQCN is reported as removed
49+
# and the new FQCN as added (there is no rename-pairing).
50+
mapfile -t changed_files < <(git diff "${BASE_REF}..HEAD" --no-renames --diff-filter=AMD --name-only -- "${paths_array[@]}" || true)
4451

4552
# Filter out empty entries
4653
changed_files=("${changed_files[@]/#/}")

scripts/preflight.sh

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,12 @@ done
2929

3030
mkdir -p "$(dirname "${OUTPUT_FILE}")"
3131

32-
diff_output=$(git diff "${BASE_REF}..HEAD" --unified=0 -- "${paths_array[@]}" 2>/dev/null || true)
32+
# --no-renames decomposes a renamed file into a full delete + full add, so the
33+
# token scan below sees the old and new content as -/+ lines. Without it, a
34+
# rename that only changes a class's namespace (class name unchanged) shows no
35+
# matching tokens and could wrongly gate the analysis off. Bias stays toward
36+
# "true", consistent with detect.sh.
37+
diff_output=$(git diff "${BASE_REF}..HEAD" --no-renames --unified=0 -- "${paths_array[@]}" 2>/dev/null || true)
3338

3439
if [[ -z "${diff_output}" ]]; then
3540
echo "false" > "${OUTPUT_FILE}"

tests/DetectTest.php

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -282,4 +282,39 @@ public function testTopLevelAndNestedFilesAreBothMatched(): void
282282
$this->assertStringContainsString('L\\TopLevel::a', $body);
283283
$this->assertStringContainsString('L\\Sub\\Nested::b', $body);
284284
}
285+
286+
public function testRenamedClassIsReportedAsRemovedAndAdded(): void
287+
{
288+
// Mirrors composer/composer PR #12919: a file and its class are renamed
289+
// (Version.php/class Version -> VersionRenamed.php/class VersionRenamed).
290+
// The bodies are identical, so git classifies this as a single rename (R)
291+
// entry. detect.sh must decompose it (via --no-renames) into a delete +
292+
// add; otherwise --diff-filter=AMD drops the R entry and neither the old
293+
// nor the new class reaches the snapshotter.
294+
$classBody = "{\n public function major(): int { return 1; }\n public function minor(): int { return 2; }\n public function patch(): int { return 3; }\n}\n";
295+
$this->commit([
296+
'src/Platform/Version.php' => "<?php\nnamespace L\\Platform;\nclass Version " . $classBody,
297+
], 'base');
298+
299+
// The commit() helper only writes files; remove the old one ourselves so
300+
// `git add -A` stages the deletion alongside the new file.
301+
unlink($this->repoDir . '/src/Platform/Version.php');
302+
$this->commit([
303+
'src/Platform/VersionRenamed.php' => "<?php\nnamespace L\\Platform;\nclass VersionRenamed " . $classBody,
304+
], 'rename Version to VersionRenamed');
305+
306+
$rendered = $this->runDetect();
307+
308+
$this->assertStringContainsString('### New API Surface', $rendered);
309+
$this->assertStringContainsString('### Removed API Surface', $rendered);
310+
311+
// New is rendered before Removed; split so we can assert membership.
312+
$removedAt = strpos($rendered, '### Removed API Surface');
313+
$newSection = substr($rendered, 0, $removedAt);
314+
$removedSection = substr($rendered, $removedAt);
315+
316+
$this->assertStringContainsString('L\\Platform\\VersionRenamed', $newSection, 'The renamed-to class should be reported as new API surface.');
317+
// Backtick-bounded so it does not also match `L\Platform\VersionRenamed`.
318+
$this->assertStringContainsString('`L\\Platform\\Version`', $removedSection, 'The renamed-from class should be reported as removed API surface.');
319+
}
285320
}

0 commit comments

Comments
 (0)