Skip to content

refactor(types): eliminate the PHPStan baseline (level max → 0)#48

Merged
dotMavriQ merged 1 commit into
mainfrom
chore/phpstan-baseline-zero
Jun 5, 2026
Merged

refactor(types): eliminate the PHPStan baseline (level max → 0)#48
dotMavriQ merged 1 commit into
mainfrom
chore/phpstan-baseline-zero

Conversation

@dotMavriQ

@dotMavriQ dotMavriQ commented Jun 5, 2026

Copy link
Copy Markdown
Owner

What

Clears the last 15 entries of `phpstan-baseline.neon` and deletes the file (plus its `includes` line in `phpstan.neon`). The codebase is now clean at `level: max` with zero suppressed debt. Closes out the #39 epic.

The baseline was already down from ~1340 → 15 via the earlier #46 / Rector (#47) passes; this pays off the remainder.

The 15 entries, in two clusters

Auth (3) — VerifyEmailController
`User` already used the `MustVerifyEmail` trait methods and the verify flow is tested (asserts the `Verified` event fires), but it never implemented the interface — so the `verified` middleware was a silent no-op. `User` now `implements MustVerifyEmail`, the event + method calls type-check, and the controller uses a plain null-guard (no suppressions).

  • ⚠️ Behavior note: the `verified` middleware now enforces across protected routes. Harmless today — registration is disabled and all users are verified — but called out explicitly.

Index raw SQL (12)

  • orderByRaw("\"$sortBy\" $sortDir NULLS LAST") was duplicated across Album/BoardGame/Concert/Game indexes → extracted into WithIndexFiltering::applyNullsLastOrder() (DRY), with one documented @phpstan-ignore for the whitelist-validated identifier.
  • orWhereRaw in WithAccentInsensitiveSearchone documented @phpstan-ignore for the hardcoded column identifier (PHPStan attributed this to all 8 using-classes; fixing the trait clears all 8).

Verification

  • composer stan[OK] No errors (level max, no baseline)
  • composer lint (Pint) → pass
  • php artisan test91 passed (incl. all EmailVerification + sort-sanitization/index tests)

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes

    • Fixed email verification process to properly mark emails as verified.
    • Improved sorting behavior when displaying items with missing data across albums, board games, concerts, and games sections.
  • Tests

    • Enhanced static analysis configuration to enforce stricter code quality standards.

The baseline is down to its last 15 entries; clear them and delete the
file so the codebase is clean at level: max with no suppressed debt.

- VerifyEmailController: User now implements MustVerifyEmail (it already
  used the trait's methods and the verify flow is tested), so the event
  + method calls type-check. Controller uses a plain null-guard. The
  'verified' middleware becomes enforcing — a no-op today since
  registration is disabled and all users are verified.
- Indexes: extract the duplicated NULLS-LAST orderByRaw into a shared
  WithIndexFiltering::applyNullsLastOrder() helper (DRY across Album/
  Board Game/Concert/Game), with one documented ignore for the
  whitelist-validated identifier.
- WithAccentInsensitiveSearch: one documented ignore for the hardcoded
  column identifier in orWhereRaw.
- Drop phpstan-baseline.neon and its include from phpstan.neon.

Closes #39, #41, #42, #43, #44, #45.
@qodo-code-review

Copy link
Copy Markdown

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

@coderabbitai

coderabbitai Bot commented Jun 5, 2026

Copy link
Copy Markdown

Review Change Stack

Note

.coderabbit.yaml has unrecognized properties

CodeRabbit is using all valid settings from your configuration. Unrecognized properties (listed below) have been ignored and may indicate typos or deprecated fields that can be removed.

⚠️ Parsing warnings (1)
Validation error: Unrecognized key: "tools"
⚙️ Configuration instructions
  • Please see the configuration documentation for more information.
  • You can also validate your configuration using the online YAML validator.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: c599645d-ffc1-4575-92ac-1d13c21e0064

📥 Commits

Reviewing files that changed from the base of the PR and between 154c32b and 81e27ee.

📒 Files selected for processing (10)
  • app/Http/Controllers/Auth/VerifyEmailController.php
  • app/Livewire/Albums/AlbumIndex.php
  • app/Livewire/BoardGames/BoardGameIndex.php
  • app/Livewire/Concerns/WithAccentInsensitiveSearch.php
  • app/Livewire/Concerns/WithIndexFiltering.php
  • app/Livewire/Concerts/ConcertIndex.php
  • app/Livewire/Games/GameIndex.php
  • app/Models/User.php
  • phpstan-baseline.neon
  • phpstan.neon
💤 Files with no reviewable changes (2)
  • phpstan-baseline.neon
  • phpstan.neon

📝 Walkthrough

Walkthrough

This PR enforces email verification on the User model, extracts duplicated nulls-last ordering logic into a reusable helper trait method, wires that helper across four Livewire index components, and removes all PHPStan baseline suppressions by fixing the underlying type issues and adding targeted ignore comments.

Changes

Type Safety & Query Ordering Refactor

Layer / File(s) Summary
Email Verification Contract & Guard
app/Models/User.php, app/Http/Controllers/Auth/VerifyEmailController.php
User declares MustVerifyEmail implementation. VerifyEmailController caches the authenticated user and redirect URL, guards against null or already-verified users, and only dispatches the Verified event after calling markEmailAsVerified().
Nulls-Last Ordering Helper
app/Livewire/Concerns/WithIndexFiltering.php
New applyNullsLastOrder() helper normalizes sort direction, quotes column identifiers, and applies PostgreSQL NULLS LAST semantics via orderByRaw. Imports Builder and Model for proper typing.
Apply Nulls-Last Across Index Components
app/Livewire/Albums/AlbumIndex.php, app/Livewire/BoardGames/BoardGameIndex.php, app/Livewire/Concerts/ConcertIndex.php, app/Livewire/Games/GameIndex.php
Four index render methods replace inline orderByRaw(... NULLS LAST) calls with delegated applyNullsLastOrder() for nullable columns (rating, year, bgg_rating, plays, year_published, event_date).
PHPStan Baseline & Suppressions
phpstan.neon, phpstan-baseline.neon, app/Livewire/Concerns/WithAccentInsensitiveSearch.php
Removes phpstan-baseline.neon from includes and deletes 91 baseline error suppressions. Adds @phpstan-ignore comment in WithAccentInsensitiveSearch for safe type coercion.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related issues

  • #39: This PR directly implements the code fixes and removes PHPStan baseline suppressions targeted by the "burn down the PHPStan baseline" effort, addressing the same files (VerifyEmailController, Livewire index components, WithIndexFiltering, WithAccentInsensitiveSearch).

Poem

🔐 Email now verified, nulls sorted last,
PHPStan baselines relegated to the past.
One helper born to tame the order,
Four indices dance across the border.
Type safety wins the day! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 37.50% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main objective: eliminating the PHPStan baseline to reach level max with zero suppressed violations, which is the core purpose across all file changes.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch chore/phpstan-baseline-zero

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@dotMavriQ dotMavriQ merged commit dfa2962 into main Jun 5, 2026
4 of 5 checks passed
@dotMavriQ dotMavriQ deleted the chore/phpstan-baseline-zero branch June 5, 2026 14:04
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