diff --git a/README.md b/README.md index 3e32c03..0283cd5 100644 --- a/README.md +++ b/README.md @@ -145,4 +145,5 @@ If `composer install` fails (network hiccup, auth issue, missing `composer.json` - 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. - Anonymous classes are skipped. +- 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. - 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. diff --git a/scripts/detect.sh b/scripts/detect.sh index 16b99da..1369789 100755 --- a/scripts/detect.sh +++ b/scripts/detect.sh @@ -40,7 +40,14 @@ mkdir -p "${OUTPUT_DIR}" # All files in scope (added, modified, or deleted) — snapshot.php will silently # ignore missing files, so we can pass the same list for both refs. -mapfile -t changed_files < <(git diff "${BASE_REF}..HEAD" --diff-filter=AMD --name-only -- "${paths_array[@]}" || true) +# +# --no-renames is essential: with git's default rename detection a renamed file +# (e.g. Version.php -> VersionRenamed.php) is reported as a single R entry, which +# --diff-filter=AMD drops entirely, so neither path would reach the snapshotter. +# Disabling rename detection decomposes a rename into a delete (old path) + add +# (new path) — both pass the AMD filter, so the old FQCN is reported as removed +# and the new FQCN as added (there is no rename-pairing). +mapfile -t changed_files < <(git diff "${BASE_REF}..HEAD" --no-renames --diff-filter=AMD --name-only -- "${paths_array[@]}" || true) # Filter out empty entries changed_files=("${changed_files[@]/#/}") diff --git a/scripts/preflight.sh b/scripts/preflight.sh index 47b9bda..37ddb33 100755 --- a/scripts/preflight.sh +++ b/scripts/preflight.sh @@ -29,7 +29,12 @@ done mkdir -p "$(dirname "${OUTPUT_FILE}")" -diff_output=$(git diff "${BASE_REF}..HEAD" --unified=0 -- "${paths_array[@]}" 2>/dev/null || true) +# --no-renames decomposes a renamed file into a full delete + full add, so the +# token scan below sees the old and new content as -/+ lines. Without it, a +# rename that only changes a class's namespace (class name unchanged) shows no +# matching tokens and could wrongly gate the analysis off. Bias stays toward +# "true", consistent with detect.sh. +diff_output=$(git diff "${BASE_REF}..HEAD" --no-renames --unified=0 -- "${paths_array[@]}" 2>/dev/null || true) if [[ -z "${diff_output}" ]]; then echo "false" > "${OUTPUT_FILE}" diff --git a/tests/DetectTest.php b/tests/DetectTest.php index 94c20b7..77e71b3 100644 --- a/tests/DetectTest.php +++ b/tests/DetectTest.php @@ -282,4 +282,39 @@ public function testTopLevelAndNestedFilesAreBothMatched(): void $this->assertStringContainsString('L\\TopLevel::a', $body); $this->assertStringContainsString('L\\Sub\\Nested::b', $body); } + + public function testRenamedClassIsReportedAsRemovedAndAdded(): void + { + // Mirrors composer/composer PR #12919: a file and its class are renamed + // (Version.php/class Version -> VersionRenamed.php/class VersionRenamed). + // The bodies are identical, so git classifies this as a single rename (R) + // entry. detect.sh must decompose it (via --no-renames) into a delete + + // add; otherwise --diff-filter=AMD drops the R entry and neither the old + // nor the new class reaches the snapshotter. + $classBody = "{\n public function major(): int { return 1; }\n public function minor(): int { return 2; }\n public function patch(): int { return 3; }\n}\n"; + $this->commit([ + 'src/Platform/Version.php' => "repoDir . '/src/Platform/Version.php'); + $this->commit([ + 'src/Platform/VersionRenamed.php' => "runDetect(); + + $this->assertStringContainsString('### New API Surface', $rendered); + $this->assertStringContainsString('### Removed API Surface', $rendered); + + // New is rendered before Removed; split so we can assert membership. + $removedAt = strpos($rendered, '### Removed API Surface'); + $newSection = substr($rendered, 0, $removedAt); + $removedSection = substr($rendered, $removedAt); + + $this->assertStringContainsString('L\\Platform\\VersionRenamed', $newSection, 'The renamed-to class should be reported as new API surface.'); + // Backtick-bounded so it does not also match `L\Platform\VersionRenamed`. + $this->assertStringContainsString('`L\\Platform\\Version`', $removedSection, 'The renamed-from class should be reported as removed API surface.'); + } }