feat(checkpoints): detect self-referencing class constants (PM-44) - #82
Merged
Merged
Conversation
Bulk-extracting a repeated literal into a constant — the usual php:S1192 fix — also rewrites the line that declares the constant when run as a blind replace-all, producing `const X = self::X`. That is fatal at class-load time and PHPStan level 10 reports nothing, because the engine raises it only when the class is actually loaded. A change set can pass every analyzer and still be broken. Adds PM-44 (mechanical) plus an anti-pattern section covering the two rules that make the transform safe: exclude the declaration line, and verify by executing rather than analyzing. The check enumerates existing source directories before grepping. Passing absent directories to `grep -q` makes it exit 2, and negating 2 yields 0 — the check would have silently passed on a real violation in any repo lacking one of src/, Classes/, tests/, Tests/. Verified in both directions: exit 1 on `const P = self::P`, exit 0 on a legitimate alias `const B = self::A` and on a repo with no source directories. Found via /retro after the pattern reached a nr-vault test file and was caught only by the functional run. Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
Contributor
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
|
github-actions
Bot
added
documentation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
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
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.
Summary
Adds mechanical checkpoint
PM-44forconst X = self::X— a fatal-at-load self-referencing constant that no static analyzer reports — plus the anti-pattern section describing the transform that produces it.Came from
/retrosession on 2026-07-30:3bbf68c3-a30c-4e1e-af38-c97eaf711150Finding: routed to
checkpointrather than prose because the pattern is a one-line regex — a gate that fails the build outranks a rule asking for care.Cannot declare self-referencing constantat runtime, after a scripted extraction of a repeated test payload into a constant. PHPStan level 10 passed the change; only the functional run caught it.const SMALL_PAYLOAD = '{...}'intoconst SMALL_PAYLOAD = self::SMALL_PAYLOAD. The engine raises this only when the class is loaded, so analyzers cannot see it.checkpoints.yamlidPM-44.Change
checkpoints.yaml:PM-44, mechanical, severityerrorreferences/migration-strategies.md: anti-pattern section under "Anti-Patterns to Avoid", noting the hazard generalizes to any language's extract-to-constant transformNote on the check's shape
The command enumerates existing source directories before grepping. My first version passed all four unconditionally, which is subtly broken:
grep -qexits 2 when a directory is missing, and negating 2 yields 0 — the check would have silently passed on a real violation in any repo lacking one ofsrc/,Classes/,tests/,Tests/. ExistingPM-37/PM-39avoid this by ending their pipelines ingrep -q ., so no other checkpoint needed the same fix.Test plan
validate-skill.sh— 0 errors;checkpoints.yamlparses and the command survives YAML escaping intactconst P = self::P→ exit 1const A = 'x'; const B = self::A;→ exit 0Tests/as well asClasses/→ exit 1