Skip to content

chore(#122): fix real Psalm type issues + enable findUnusedCode (PR B)#134

Merged
Jaggob merged 1 commit into
mainfrom
chore/122-psalm-real-fixes
Jun 9, 2026
Merged

chore(#122): fix real Psalm type issues + enable findUnusedCode (PR B)#134
Jaggob merged 1 commit into
mainfrom
chore/122-psalm-real-fixes

Conversation

@Jaggob

@Jaggob Jaggob commented Jun 9, 2026

Copy link
Copy Markdown
Collaborator

PR B of the Psalm baseline burndown (#122). Companion to the already-merged PR A (#133, noise reduction). Brings the type-issue baseline to zero and turns on findUnusedCode.

Type fixes (all 34 real findings)

  • 8 listeners: @template-implements IEventListener<Event> (MissingTemplateParam)
  • LifecycleService restore-result @return shapes reconciled (pad_id? added) so buildSkippedResult assigns cleanly
  • PadCreationService::createFromUrl: @return gains snapshot_warning_code?; dropped the action-closure : array hint so withCreateRollback's templated return carries the precise shape
  • ExternalPadExportFetcher::getPublicTextFromResolvedExternalPad: @param widened to the full resolved shape (origin/pad_id were being rejected)
  • Viewer/PublicViewer ErrorMapper: parenthesized the callable(): (A|B) return union (precedence bug)
  • EtherpadHealthCheckService: latency math uses a float literal (strict binary operands)
  • ConsistencyCheckService / AdminController: dropped redundant is_array guards
  • PublicShareUrlBuilder: removed genuinely-dead '' branches (path is already trimmed)
  • Targeted @psalm-suppress with rationale where Psalm can't model runtime: OCP\Image (not in nextcloud/ocp stubs), the Files-app LoadAdditionalScriptsEvent, the curl write-callback mutation of $sizeExceeded, and basename('/') === ''

findUnusedCode

  • Enabled in psalm.xml
  • Entry points (controllers, listeners, migrations, settings, jobs, hooks, preview, Application) annotated @psalm-api so Psalm roots traversal from them
  • Removed real dead code surfaced by it: 5 unused local inits, 1 unused param (buildProtectedRestorePadName), 1 unnecessary @var
  • Remaining findings are NC DI / entry-point false positives (service constructors with no literal new; repair-step internals reached only via run()). Captured in psalm-baseline.xml so new dead code is still caught.

Verification

  • composer psalm ✅ green (0 unbaselined issues)
  • PHPUnit ✅ 409 tests, 1677 assertions
  • Playwright e2e ✅ 23/23 against the Hamal test server

PR B of the Psalm baseline burndown. Fixes all 34 real type findings so
the type-issue baseline drops to zero, then turns on findUnusedCode.

Type fixes:
- 8 IEventListener listeners get @template-implements IEventListener<Event>
- LifecycleService restore-result @return shapes reconciled (add pad_id?)
- PadCreationService::createFromUrl @return gains snapshot_warning_code?;
  withCreateRollback action closure return hint dropped so the templated
  return type carries the precise shape
- ExternalPadExportFetcher::getPublicTextFromResolvedExternalPad @param
  widened to the full resolved shape (origin/pad_id were rejected)
- Viewer/PublicViewer ErrorMapper: parenthesize callable return union
- EtherpadHealthCheckService latency math uses float literal
- ConsistencyCheckService / AdminController: drop redundant is_array guards
- PublicShareUrlBuilder: remove genuinely-dead '' branches (trimmed path)
- Preview/Application/PadFileCreator/ExternalPadExportFetcher: targeted
  @psalm-suppress with rationale where Psalm can't model runtime (OCP\Image
  not in ocp stubs, Files-app event, curl write-callback mutation,
  basename('/') === '')

findUnusedCode:
- Enabled in psalm.xml
- Entry points (controllers, listeners, migrations, settings, jobs, hooks,
  preview, Application) annotated @psalm-api so Psalm roots from them
- Removed real dead code surfaced: 5 unused local inits, 1 unused param
  (buildProtectedRestorePadName), 1 unnecessary @var
- Remaining findings are NC DI/entry-point false positives (constructors
  with no literal `new`, repair-step internals reached only via run());
  captured in psalm-baseline.xml so new dead code is still caught

composer psalm green; 409 PHPUnit tests pass.
@Jaggob Jaggob merged commit 7b81860 into main Jun 9, 2026
13 checks passed
@Jaggob Jaggob deleted the chore/122-psalm-real-fixes branch June 9, 2026 09:13
Jaggob added a commit that referenced this pull request Jun 9, 2026
Bump version to 1.1.0-alpha.3 across appinfo/info.xml, package.json, and
package-lock.json (consistency guard green); regenerate js/ license
sidecars with the new version (bundles unchanged). Add the alpha.3
CHANGELOG section summarising the security hardening (#105, #102, #110),
the Playwright e2e suite (#54), the Psalm burndown + findUnusedCode
(#82, #122, #133, #134), and the supporting refactors/tooling.
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