From 7bdae9d34f7d9b558b167856a0f9accd8d48ec38 Mon Sep 17 00:00:00 2001 From: Sebastian Mendel Date: Thu, 23 Jul 2026 18:53:55 +0200 Subject: [PATCH 1/2] docs(static-analysis): warn DEAD_CODE Rector rewrites can change intent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit RemoveDefaultArgumentValueRector strips a trailing arg equal to its default even when the value is meaningful (e.g. a 0 passed to disable a floor that otherwise defaults to a configured value) — pass it by name to preserve it. Verify lossy-set rewrites against intent rather than applying blind. Signed-off-by: Sebastian Mendel --- .../references/static-analysis-tools.md | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/skills/php-modernization/references/static-analysis-tools.md b/skills/php-modernization/references/static-analysis-tools.md index b25d90f..410f2c1 100644 --- a/skills/php-modernization/references/static-analysis-tools.md +++ b/skills/php-modernization/references/static-analysis-tools.md @@ -373,6 +373,21 @@ for what's actually available. run: vendor/bin/rector process --dry-run --ansi ``` +### DEAD_CODE rewrites can change intent — verify, don't auto-apply + +`SetList::DEAD_CODE` includes `RemoveDefaultArgumentValueRector`, which strips a +trailing argument whose value equals the parameter's default. That is wrong when +the argument is *meaningful* and only coincidentally equals a default — e.g. +`verifyChain($from, $to, 0)` where the third parameter defaults to a configured +value and `0` is passed to disable it: Rector rewrites the call to +`verifyChain()`, silently restoring the floor. Pass such an argument **by name** +(`verifyChain(minEpoch: 0)`) — the rule leaves named arguments alone. (The same +set also rewrites `$x !== null` to `$x instanceof T` on a `?T` return; that one +is a safe equivalent — apply it and add the import.) + +A rewrite from a lossy set (`DEAD_CODE`, `CODE_QUALITY`) is a suggestion to +verify against intent, not an edit to apply blind. + ## PHP-CS-Fixer (Coding Style) Enforces coding standards automatically. From b17b237fdbeebc559f2ec556c43494150f45221b Mon Sep 17 00:00:00 2001 From: Sebastian Mendel Date: Thu, 23 Jul 2026 18:53:55 +0200 Subject: [PATCH 2/2] docs(php-cs-fixer): add pre-push usage gotchas (cache, unused-imports) The .php-cs-fixer.cache masks violations locally (run --using-cache=no for a reliable pre-push check); no_unused_imports deletes an import added before its first usage across staged edits, which CI then fails with class-not-found. Both cost CI round-trips when unknown. Signed-off-by: Sebastian Mendel --- .../references/php-cs-fixer-deprecations.md | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/skills/php-modernization/references/php-cs-fixer-deprecations.md b/skills/php-modernization/references/php-cs-fixer-deprecations.md index d2d752f..5c2ed90 100644 --- a/skills/php-modernization/references/php-cs-fixer-deprecations.md +++ b/skills/php-modernization/references/php-cs-fixer-deprecations.md @@ -78,3 +78,19 @@ The verify scripts should detect: - `@PSR12` / `@PSR12:risky` (deprecated by `@PER-CS`). - Any rule-set name that produces a "Detected deprecations" warning on dry-run (run the detection command above). + +## Pre-push usage gotchas + +Two php-cs-fixer behaviours make a local run disagree with CI: + +- **The cache masks violations.** php-cs-fixer records clean files in + `.php-cs-fixer.cache`; a later run then reports a file clean while CI (fresh + checkout, no cache) fails the Code Style job on it — often an + `ordered_class_elements` reorder that a newer fixer version flags. For a + reliable pre-push check, run with `--using-cache=no`. (The CI fixer version can + also be newer than local; cache-off catches most of that gap.) +- **`no_unused_imports` deletes an import you added before its first use.** When + a `use X;` is added in one edit and its usage (a new method or test) in a later + edit, with a fixer run in between, the import is stripped as unused and does not + come back — CI then dies with `Class "…\X" not found`. After adding code that + references a newly-imported class, re-verify the import is still present.