Skip to content

Lint debt cleanup post-canonical-sync (Phase 2 fleet rollout) #279

Description

@rubenvdlinde

Context

Phase 2 fleet rollout consolidated phpcs/phpmd/psalm/phpstan onto the canonical config from nextcloud-app-template. Pattern from shillinq#299 and decidesk#242.

softwarecatalog has the largest defensive-coding lint debt of any app in the fleet — surfaced by dropping per-app suppressions during the sync. This issue tracks the cleanup work.

Quality gate snapshot after sync

Gate Status
phpcs 15 errors / 442 warnings (pre-existing, not introduced by sync)
psalm 70 errors (pre-existing, not introduced by sync)
phpstan 0 unmatched (206 baselined)
phpmd 124 violations (was 129, 5 cleaned mechanically)

What was dropped in the canonical sync (intentional, do NOT restore)

  • scanDirectories: ../openregister/lib — dropped per fleet directive. phpstan now reports OCA\OpenRegister\ types as unknown but the canonical OCA\OpenRegister\ ignoreErrors family in phpstan.neon covers them. Keeping the scanDirectories was a per-app deviation; the fleet now relies on the broad ignore pattern.
  • 10 categories of phpstan suppressions for defensive-coding patterns (always true, unreachable, is never read, etc.) — all now baselined. These are real refactor debt, not noise.

Cleanup categories (each = one focused PR)

1. phpstan baseline reduction (206 entries)

The baseline has ~10 categories of suppression all originating from defensive coding that's no longer needed (most have null defaults that are then null-checked, but PHPStan can prove the null check is redundant given the assigned type). Plan a series of focused source refactor PRs by file. Largest contributors:

  • lib/Service/ArchiMateImportService.php and ArchiMateExportService.php — heaviest defensive checking, also home to most SimpleXMLElement nullability issues
  • lib/Service/SettingsService.php
  • lib/Service/SoftwareCatalogueService.php
  • lib/Service/SoftwareCatalogue/*Handler.php

2. SimpleXMLElement nullability errors (real bugs)

Multiple ArchiMate import/export sites dereference SimpleXMLElement properties as if non-null. PHPStan flags them; some are actual NPE risks when the XML doesn't conform to expectations. These need real null checks (or assert() if the surrounding contract guarantees the field).

3. phpmd ElseExpression refactor (124 remaining)

Concentrated in 5 service files:

  • ArchiMateImportService.php — 47
  • SettingsService.php — 25
  • ArchiMateExportService.php — 24
  • SoftwareCatalogueService.php — 14
  • ArchiMateService.php — 14

Refactor pattern: early-return (if (!cond) return; ...), pre-assign + conditional override, or ternary where the branches are pure assignments. phpmd has no native baseline, so CI gate stays red until this completes.

4. phpcs errors (15)

Mostly End comment for long condition not found, Function arguments style violations. Many auto-fixable via ./vendor/bin/phpcbf.

5. psalm errors (70)

Need per-file triage. Likely mostly type-narrowing and nullability mirroring the phpstan baselined errors — fixing #1 should drop this number too.

Suggested PR sequence

  1. SimpleXMLElement null-safety in ArchiMateImportService + ArchiMateExportService (real bugs first)
  2. phpmd ElseExpression cleanup file-by-file (5 PRs)
  3. phpstan baseline reduction (4–6 PRs, file-grouped)
  4. phpcs auto-fix + remaining manual fixes
  5. psalm residue

Acceptance criteria

  • phpstan-baseline.neon empty or removed
  • phpmd 0 violations
  • phpcs 0 errors
  • psalm 0 errors
  • All SimpleXMLElement dereferences null-safe

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions