fix(quality): phpmd DevelopmentCodeFragment could never fire on namespaced code - #16
Merged
Merged
Conversation
…paced code Every Conduction repo enables rulesets/design.xml/DevelopmentCodeFragment, and it has never reported anything in any of them. The cause is a config gap, not a phpmd bug: PDepend resolves an unqualified call inside a namespaced file to the current-namespace-qualified image, so `var_dump($x)` written inside `namespace OCA\MyApp\Service;` reaches the rule as `OCA\MyApp\Service\var_dump` and never matches the `unwanted-functions` list. All of our production PHP is namespaced, so with the default the rule is dead. The rule's own `ignore-namespaces` property is the switch. This mirrors the configuration already merged in openregister (ConductionNL/openregister#2286). Proof, phpmd 2.15.0 / PHP 8.3.32, against this repo's own phpmd.xml: namespaced probe class calling var_dump() -> exit 2, DevelopmentCodeFragment same class with the call removed -> exit 0, no finding Before the change the identical namespaced probe exited 0. Blast radius on this repo: measured 0 new findings over the scanned path on the base branch, with a per-run positive control (dropping the namespaced probe into the same extracted tree does produce exit 2, so the zero is a true zero). Nothing is baselined or suppressed here. Note: this repo does not run the shared quality workflow, so phpmd is not executed in CI here. This change fixes `composer check:strict` when run locally and stops the broken configuration propagating; it does not by itself add a CI gate.
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 hole
Every Conduction repo enables
rulesets/design.xml/DevelopmentCodeFragment— therule that is supposed to stop
var_dump()/print_r()/var_export()shippingin production code. It has never reported anything, in any repo.
The cause is a config gap, not a phpmd bug. PDepend resolves an unqualified
call inside a namespaced file to the current-namespace-qualified image, so:
…which never matches the rule's
unwanted-functionslist. All of our productionPHP is namespaced, so with the default (
ignore-namespaces=false) the rule isdead everywhere. The fleet has had no
composer check:strictprotectionagainst shipped debug helpers.
The rule's own
ignore-namespacesproperty is the switch. This PR copies theconfiguration already merged in openregister (
ConductionNL/openregister#2286).Proof it now fires
phpmd 2.15.0 / PHP 8.3.32, run against this repo's own
phpmd.xml, using abyte-identical pair of namespaced probe classes:
var_dump()DevelopmentCodeFragmentBoth directions are checked deliberately: a repair that only shows "something
failed" cannot distinguish a working rule from a noisy one.
Blast radius: 0 new findings
Measured, not assumed. The rule was run in isolation over this repo's scanned
path at the base branch before the flip: 0 findings.
The zero carries a per-run positive control — the namespaced probe was dropped
into the same extracted tree and did produce
exit 2, so this is a true zero andnot a harness that silently analysed nothing. An independent
git grepforvar_dump|print_r|dd|dumpacrosslib/agrees. Nothing is baselined orsuppressed by this PR.
Note on overlap with hydra gate 2
hydra-gate-forbidden-patternsindependently grepslib/forvar_dump / die / error_log / print_r / dd / dump, and is the broader control —it also catches calls made outside a method body, which
DevelopmentCodeFragment(a MethodAware/FunctionAware rule) structurally cannot see. That gate is why the
measured count is 0: it has been holding this line alone. This rule is the
composer check:stricthalf of the same guard. Keep both.This repo does not call the shared quality workflow, so
composer phpmdis not executed in CI here. This change fixes
composer check:strictwhen runlocally and stops the broken configuration propagating — it does not by itself
add a CI gate. Wiring one up is separate follow-up work.