Skip to content

Widen docblocks to the types the code accepts, and raise PHPStan to level 4 - #258

Merged
harikt merged 2 commits into
6.xfrom
docblock-widen-actual-types
Jul 30, 2026
Merged

Widen docblocks to the types the code accepts, and raise PHPStan to level 4#258
harikt merged 2 commits into
6.xfrom
docblock-widen-actual-types

Conversation

@harikt

@harikt harikt commented Jul 30, 2026

Copy link
Copy Markdown
Member

Follow-on to #257. Docblocks only; no code changes, no behaviour change.

Each widening was checked by building the SQL rather than read off the
signature:

Update::set(col, null)                -> UPDATE "t" SET "c" = NULL
Mysql onDuplicateKeyUpdate(col, null) -> ON DUPLICATE KEY UPDATE `c` = NULL
Select::where(Closure)                -> WHERE ( x = 1 )
Select::having(Closure)               -> HAVING ( n > 1 OR n < 0 )
Select::leftJoin('u')                 -> LEFT JOIN "u"

So @param string $value becomes string|null where passing null renders
SQL NULL, @param string $cond becomes string|Closure on the WHERE and
HAVING family, and the 10 join conditions become string|null -- they
default to null and a join with no condition is a supported call.

Joins do NOT take a closure (preg_split(): Argument #2 must be of type string, Closure given), so those stay string, not string|Closure. The
distinction is why each case was built rather than swept.

Level 3 -> 4, with three ignores in phpstan.neon.dist:

  • addClauseCondClosure() empties a clause, hands the query to a closure,
    then reads the clause back. PHPStan does not model mutation through the
    closure, so it reports the test as always true and the rest as unreachable.
    The code is correct and is what makes where(function ($q) { ... })
    produce grouped parentheses.
  • getCond() asserts an array key is an int, which PHP guarantees once the
    string case has returned. Kept as a deliberate guard.

Each ignore carries a count, so it suppresses only the occurrences it
describes -- verified by injecting an unreachable statement elsewhere in
AbstractQuery.php and confirming the run fails.

Checks unchanged: 1303 unit, 132 integration, 24 doc examples.

Summary by CodeRabbit

  • Documentation
    • Updated PHPDoc to reflect that condition parameters may accept strings, closures, or null where supported.
    • Updated value parameter documentation to allow null, documenting that it is represented as SQL NULL.
    • Improved consistency of PHPDoc across query building, joins, where clauses, insert/update, and conflict-handling APIs.
  • Chores
    • Tweaked static analysis configuration and refined suppression/scoping notes (no runtime behavior changes).

@coderabbitai

coderabbitai Bot commented Jul 30, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 5a2e197c-6fea-4b54-8164-408bd6bb27c2

📥 Commits

Reviewing files that changed from the base of the PR and between 3d07ca9 and 4cc7816.

📒 Files selected for processing (14)
  • phpstan.neon.dist
  • src/AbstractDmlQuery.php
  • src/AbstractQuery.php
  • src/Common/Insert.php
  • src/Common/LateralJoinTrait.php
  • src/Common/OnConflictUpdateInterface.php
  • src/Common/OnConflictUpdateTrait.php
  • src/Common/Select.php
  • src/Common/SelectInterface.php
  • src/Common/Update.php
  • src/Common/ValuesInterface.php
  • src/Common/WhereInterface.php
  • src/Common/WhereTrait.php
  • src/Mysql/Insert.php

📝 Walkthrough

Walkthrough

PHPDoc annotations across query, mutation, join, where, and having APIs were widened to match existing nullable and closure inputs. PHPStan analysis for src was raised to level 4 with targeted suppressions for AbstractQuery.php.

Changes

Query type documentation

Layer / File(s) Summary
Nullable value parameter documentation
src/AbstractDmlQuery.php, src/Common/{Insert,Update,ValuesInterface,OnConflictUpdateInterface,OnConflictUpdateTrait}.php, src/Mysql/Insert.php
Mutation and insert-related PHPDoc value parameters now accept `string
Condition parameter documentation
src/AbstractQuery.php, src/Common/{LateralJoinTrait,Select,SelectInterface,WhereInterface,WhereTrait}.php
Join conditions now document `string
PHPStan analysis configuration
phpstan.neon.dist
PHPStan runs at level 4 for src, with targeted suppressions added for AbstractQuery.php.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the two main changes: broader docblocks and a PHPStan level increase.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch

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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@phpstan.neon.dist`:
- Around line 19-25: Update the counted ignores for booleanNot.alwaysTrue and
deadCode.unreachable in phpstan.neon.dist to include message/messages patterns
that match only the intended diagnostics in src/AbstractQuery.php.
Alternatively, replace each file-scoped suppression with an inline ignore at its
exact diagnostic location, ensuring unrelated diagnostics cannot consume the
allowed counts.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 09eaaf18-5ee3-4f04-a533-9d4f9923e99b

📥 Commits

Reviewing files that changed from the base of the PR and between 3d07ca9 and f53540c.

📒 Files selected for processing (14)
  • phpstan.neon.dist
  • src/AbstractDmlQuery.php
  • src/AbstractQuery.php
  • src/Common/Insert.php
  • src/Common/LateralJoinTrait.php
  • src/Common/OnConflictUpdateInterface.php
  • src/Common/OnConflictUpdateTrait.php
  • src/Common/Select.php
  • src/Common/SelectInterface.php
  • src/Common/Update.php
  • src/Common/ValuesInterface.php
  • src/Common/WhereInterface.php
  • src/Common/WhereTrait.php
  • src/Mysql/Insert.php

Comment thread phpstan.neon.dist Outdated
@auraphp auraphp deleted a comment from coderabbitai Bot Jul 30, 2026
@auraphp auraphp deleted a comment from coderabbitai Bot Jul 30, 2026
@auraphp auraphp deleted a comment from coderabbitai Bot Jul 30, 2026
@auraphp auraphp deleted a comment from coderabbitai Bot Jul 30, 2026
@harikt
harikt merged commit e02c4e0 into 6.x Jul 30, 2026
20 checks passed
@harikt
harikt deleted the docblock-widen-actual-types branch July 30, 2026 17:51
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