Skip to content

fix(quality): phpmd DevelopmentCodeFragment could never fire on namespaced code - #419

Merged
rubenvdlinde merged 1 commit into
developmentfrom
fix/phpmd-ignore-namespaces
Aug 3, 2026
Merged

fix(quality): phpmd DevelopmentCodeFragment could never fire on namespaced code#419
rubenvdlinde merged 1 commit into
developmentfrom
fix/phpmd-ignore-namespaces

Conversation

@rubenvdlinde

Copy link
Copy Markdown
Contributor

The hole

Every Conduction repo enables rulesets/design.xml/DevelopmentCodeFragment — the
rule that is supposed to stop var_dump() / print_r() / var_export() shipping
in 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:

namespace OCA\MyApp\Service;
...
var_dump($x);          // reaches the rule as OCA\MyApp\Service\var_dump

…which never matches the rule's unwanted-functions list. All of our production
PHP is namespaced, so with the default (ignore-namespaces=false) the rule is
dead everywhere. The fleet has had no composer check:strict protection
against shipped debug helpers
.

The rule's own ignore-namespaces property is the switch. This PR copies the
configuration 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 a
byte-identical pair of namespaced probe classes:

probe before after
namespaced class calling var_dump() exit 0, no finding exit 2, DevelopmentCodeFragment
same class, call removed exit 0 exit 0, no finding

Both 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 and
not a harness that silently analysed nothing. An independent git grep for
var_dump|print_r|dd|dump across lib/ agrees. Nothing is baselined or
suppressed by this PR.

Note on overlap with hydra gate 2

hydra-gate-forbidden-patterns independently greps lib/ for
var_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:strict half of the same guard. Keep both.

Also: the composer script guard

This PR also drops the if [ -f vendor/bin/phpmd ] wrapper from the phpmd
script. A file-presence guard that echoes and returns 0 is the same silent-pass
defect as the || echo '... skipping' it replaced — a phpmd that failed to
install reads as a phpmd that found nothing.

phpmd/phpmd is a declared require-dev and the shared quality workflow runs
composer install before the gate, so this is a no-op whenever the tool is
present
and a loud failure when it is not. The shared workflow already states
this rule for its own require_script check (ConductionNL/.github#121).

psalm / phpstan / test:* in the same file still carry the guard. Left
alone deliberately — not measured in this pass.

…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.

Also drops the `if [ -f vendor/bin/phpmd ]` wrapper from the composer script.
A file-presence guard that echoes and returns 0 is the same silent-pass defect
as the `|| echo ... skipping` it replaced, just in a new costume: a phpmd that
failed to install reads as a phpmd that found nothing. phpmd/phpmd is a
declared require-dev and the shared quality workflow runs
`composer install` before the gate, so removing the guard is a no-op whenever
the tool is present and a loud failure when it is not - which is the point.
The shared workflow already states this rule explicitly for its own
`require_script` check (ConductionNL/.github#121).
@github-actions

github-actions Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/softwarecatalog @ 324f61a

Check PHP Vue Security License Tests
lint
phpcs
phpmd
psalm
phpstan
phpmetrics
eslint
stylelint
composer ✅ 121/121
npm ✅ 849/849
PHPUnit ⏭️
Newman ⏭️
Playwright ⏭️

Quality workflow — 2026-08-03 08:36 UTC

Download the full PDF report from the workflow artifacts.

@rubenvdlinde
rubenvdlinde merged commit 8d905c9 into development Aug 3, 2026
20 of 25 checks passed
@rubenvdlinde
rubenvdlinde deleted the fix/phpmd-ignore-namespaces branch August 3, 2026 09:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant