fix(phpmd): narrow the UnusedFormalParameter exclusion to */lib/Migration/* - #266
Merged
Merged
Conversation
…on/* to */lib/Migration/* `*/Migration/*` is a path-SEGMENT pattern, not a directory anchor. PDepend compiles an exclude-pattern into an UNANCHORED regex — Input\ExcludePathFilter preg_quote()s the pattern and then turns `\*` into `.*` — so it matches ANY path containing a `/Migration/` segment: `lib/Service/Migration/`, `lib/Command/Migration/`, any future `lib/*/Migration/`. Those are ordinary classes with no interface-mandated signature, and a genuine unused parameter in one of them would never be reported while the run still looked clean. PROBE MATRIX, PHPMD 2.15.0 / PHP 8.4.22, three probe classes on a throwaway copy: probe */Migration/* */lib/Migration/* lib/Migration/… UFP not reported not reported (intended) lib/Migration/… Else (leg 1) reported reported (leg 1 unaffected) lib/Service/… UFP reported reported lib/Service/Migration/… UFP NOT REPORTED reported (the leak) The IMigrationStep exclusion still does exactly its job — a step that cannot drop changeSchema/preSchemaChange/postSchemaChange's three mandated parameters is still spared — and leg 1 still analyses lib/Migration for every other rule. Real numbers: UNCHANGED. A grep for `/Migration/` directories outside lib/Migration/ found none in this repo, so nothing was actually being swallowed. The trap goes regardless. One process note worth recording: the first draft of this comment contained a literal `<-` inside the <description>, which made the ruleset UNPARSEABLE. PHPMD then exited 1 and reported NOTHING — and "no findings printed" is exactly what a clean run looks like to a grep. It was caught only by reading the exit code directly. The XML is validated as part of the check now.
Contributor
Quality Report — ConductionNL/larpingapp @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| check-specs | ✅ | ||||
| test-l10n | ❌ | ||||
| composer | ✅ | ✅ 101/101 | |||
| npm | ✅ | ✅ 537/537 | |||
| PHPUnit | ✅ | ||||
| Newman | ❌ | ||||
| Playwright | ❌ | ||||
| Hydra gates | ❌ |
Quality workflow — 2026-08-05 15:47 UTC
Download the full PDF report from the workflow artifacts.
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.
The trap
*/Migration/*inphpmd-unusedparams.xmlis a path-segment pattern, not a directory anchor. PDepend compiles an exclude-pattern into an unanchored regex —Input\ExcludePathFilter::__constructpreg_quote()s the pattern and then turns\*into.*:So
*/Migration/*matches any path containing a/Migration/segment —lib/Service/Migration/,lib/Command/Migration/, any futurelib/*/Migration/— not just the app'sIMigrationStepimplementations. Those are ordinary classes with no interface-mandated signature, so a genuine unused parameter in one would never be reported and the run would still look clean.Probe matrix — measured, not reasoned
PHPMD 2.15.0 / PHP 8.4.22 inside
nextcloud:latest, three probe classes dropped onto a throwaway copy:*/Migration/**/lib/Migration/*lib/Migration/…— UnusedFormalParameterlib/Migration/…— ElseExpression (leg 1)lib/Service/…— UnusedFormalParameterlib/Service/Migration/…— UnusedFormalParameterThe
IMigrationStepexclusion still does exactly its job: a step that cannot dropchangeSchema/preSchemaChange/postSchemaChange's three mandated parameters is still spared, and leg 1 still analyseslib/Migrationfor every other rule.Numbers: unchanged
A grep for
/Migration/directories outsidelib/Migration/found none in this repo, so nothing was actually being swallowed — as expected. Shipped findings and TRUE findings (measured with every@SuppressWarningsstripped on a throwaway copy) are identical before and after. The trap goes regardless, because the nextlib/Service/Migration/refactor would have walked straight into it.One process note
The first draft of the explanatory comment contained a literal
<-inside the<description>, which made the ruleset unparseable. PHPMD then exited 1 and reported nothing — and "no findings printed" is exactly what a clean run looks like to a grep. It was caught only by reading the exit code directly. The XML is validated as part of the check now.No threshold changed, no rule removed, no baseline added (there is none, and there must not be — PHPMD auto-discovers
phpmd.baseline.xml).🤖 Generated with Claude Code