Skip to content

fix(quality): the psalm and phpmd findings my container refactor introduced - #3534

Merged
rubenvdlinde merged 1 commit into
developmentfrom
fix/psalm-and-phpmd-after-the-locator-refactor
Sep 8, 2026
Merged

fix(quality): the psalm and phpmd findings my container refactor introduced#3534
rubenvdlinde merged 1 commit into
developmentfrom
fix/psalm-and-phpmd-after-the-locator-refactor

Conversation

@rubenvdlinde

Copy link
Copy Markdown
Contributor

Why

The development push run for #3532 went red on PHP Quality (psalm) and PHP Quality (phpmd). Both findings are mine. psalm was green on the five commits before that merge and red on it.

Both were invisible to #3532's own checks because that PR ran psalm and phpmd per touched file rather than over the tree, and the findings only exist at tree scope.

psalm, five errors

Four are UnusedBaselineEntry: twenty baselined UnusedParam entries across DestructionCheckJob, DestructionExecutionJob, DeckCardService and DeckLinkService that the refactor made unnecessary, because those parameters are used now. Removed from psalm-baseline.xml.

The fifth is UndefinedClass for \OC_App::loadApp() in ImportHandler. The call predates this work, but psalm never reached it: the enclosing branch was only reachable through a container lookup psalm could not type, and injecting IAppManager made the branch analysable. OC_App is a Nextcloud server internal with no OCP equivalent for loadApp(), and it exists whenever that branch can run. Suppressed on the method docblock with the reason, which is the pattern the repo already uses for SecurityException and Transliterator.

UnusedBaselineEntry only ever appears on a full psalm run. A per-file run cannot produce it, because psalm has to see the whole baseline used up before it can call an entry extra. That is why #3532 was green locally and red on CI.

phpmd, one error

SearchTrailMapper crossed the 1000-line ExcessiveClassLength threshold, at 1015. It gained clearAllLogs() in #3532 because the admin endpoint had always called that method and it never existed, so the endpoint threw an \Error for an undefined method. Suppressed with that reason, the idiom 74 other classes in this repo already use, rather than adding a baseline entry.

Verified locally

Check Result
psalm --memory-limit=2G, full tree No errors found, exit 0
phpmd lib both rulesets, with baseline exit 0, no output
phpcs --standard=phpcs.xml on both touched files exit 0
phpunit tests/Unit --no-coverage 19,454 tests, exit 0, peak 454 MB

🤖 Generated with Claude Code

…oduced

The development push run for #3532 went red on psalm and phpmd. Both are mine,
and both were invisible to that PR's own checks because it ran psalm and phpmd
per touched file rather than over the tree.

- psalm: five errors. Four are UnusedBaselineEntry, twenty baselined UnusedParam
  entries the refactor made unnecessary because the parameters are used now.
  Removed. The fifth is OC_App::loadApp() in ImportHandler, a server internal
  psalm cannot see: it was unreachable before, and injecting IAppManager made
  psalm analyse the branch. Suppressed on the method docblock with the reason.
  UnusedBaselineEntry only ever appears on a FULL psalm run, never on a per-file
  one, which is why the PR's checks were green.
- phpmd: SearchTrailMapper crossed the 1000-line ExcessiveClassLength threshold
  at 1015 when it gained the clearAllLogs() the admin endpoint had always called
  and that never existed. Suppressed with that reason, the idiom 74 other classes
  in this repo already use.

Verified: psalm full run exit 0, both phpmd rulesets exit 0, phpcs exit 0 on both
touched files, 19,454 unit tests exit 0 at peak 454 MB.
@rubenvdlinde
rubenvdlinde merged commit 139dcdc into development Sep 8, 2026
36 of 37 checks passed
@rubenvdlinde
rubenvdlinde deleted the fix/psalm-and-phpmd-after-the-locator-refactor branch September 8, 2026 10:08
@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/openregister @ 7bcd29f

Check PHP Vue Security License Tests
lint
phpcs
phpmd
psalm
phpstan
phpmetrics
eslint
stylelint
build
check-specs
test-l10n
test-l10n-parity
format
check-schema-l10n
check-l10n-js
composer ✅ 174/174
npm ✅ 543/543
app:check-code ⏭️
info.xml
REUSE
PHPUnit
Newman
Playwright ⏭️ deferred — runs on the promotion into beta/main, not on a pull request into development
Hydra gates

Quality workflow — 2026-09-08 10:22 UTC

Download the full PDF report from the workflow artifacts.

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