Upgrade to Laravel 12 and PHP 8.2+ with improved type hints#41
Conversation
Upgrade the framework and dev toolchain to the latest stable Laravel, keeping the existing application skeleton (Http/Console kernels, bootstrap/app.php, config/app.php providers), which remains fully supported in Laravel 11/12. Dependencies (composer.json): - php: ^8.1 -> ^8.2 - laravel/framework: ^10.4 -> ^12.0 - laravel/sanctum: ^3.2 -> ^4.0 - laravel/ui: ^4.2 -> ^4.6 - laravel/tinker, laravel/sail, guzzlehttp/guzzle bumped - phpunit/phpunit: ^9.5 -> ^11.5 - nunomaduro/larastan -> larastan/larastan ^3.0 (phpstan 2) - nunomaduro/collision: ^7.0 -> ^8.6 - fakerphp/faker, mockery/mockery, spatie/laravel-ignition bumped Tooling config: - phpunit.xml: migrate to PHPUnit 11 schema (coverage <include> -> <source>, drop removed processUncoveredFiles) - phpstan.neon.dist: use larastan/larastan extension, drop removed checkMissingIterableValueType, set treatPhpDocTypesAsCertain: false - CI: matrix on PHP 8.2/8.3/8.4, actions/checkout@v4, setup-php@v2 Code fixes for Laravel 12 / stricter static analysis: - Resources: toArray() PHPDoc matches the native array return type - User model: $fillable/$hidden typed as list<string> - Models: explicit BelongsTo return type on laravel_user() relations - Form requests: cast route parameters to int before assigning to int FKs Verified: 23 tests pass (PHPUnit 11 / PHP 8.4), PHPStan level 5 clean, all migrations run, app boots (45 routes + config cache). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BS9oQ2ynXc33SUyduSrciw
|
Warning Review limit reached
Next review available in: 20 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
ウォークスルーPHP 8.2/8.3/8.4 の CI マトリクスを追加し、 変更内容依存関係更新・型安全性改善・CI マトリクス対応
推定コードレビュー工数🎯 2 (Simple) | ⏱️ ~10 minutes ポエム
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
…trix The lock was resolved on PHP 8.4, which pulled Symfony 8.1 components (require php >=8.4.1). `composer install` then failed on the PHP 8.2 and 8.3 CI jobs with "lock file does not contain a compatible set of packages", while only 8.4 passed. Set config.platform.php to 8.2.0 (the require floor) and re-resolve, which downgrades symfony/clock, css-selector, event-dispatcher, string, translation and yaml from v8.1 to v7.4 — the Symfony 7 line Laravel 12 is built on. The lock now satisfies php 8.2.0, so it installs cleanly on 8.2/8.3/8.4. Verified: composer why-not php 8.2.0 is clean, 23 tests pass, PHPStan level 5 clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BS9oQ2ynXc33SUyduSrciw
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dbbe9f815c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
app/Http/Requests/User/ModifyRequest.php (1)
27-33: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueルートパラメータの int キャストは型安全性向上に有効です。
modifyUser()でidを(int)キャストしてUserモデルに代入する変更は、下流のModifyActionでwhere('id', '=', $user->id)として利用される型安全性を高めます。ただし、同じクラス内の
authorize()（Line 13）とrules()（Line 22）でも同じ$this->route()->parameter('id')を利用していますが、こちらはキャストされていません。authorize()ではGate::allows()に渡される値、rules()ではユニーク制約の除外 ID として文字列連結に利用されるため、型の一貫性が損なわれています。可能であれば、同じルートパラメータに対する型扱いをクラス内で統一することを検討してください。
public function authorize(Gate $gate) : bool { - $auth = $gate->allows('modify-user', $this->route()->parameter('id')); + $id = (int) $this->route()->parameter('id'); + $auth = $gate->allows('modify-user', $id); return $auth; }'email'=>'filled|unique:users,email,'.$this->route()->parameter('id').',id', + 'email'=>'filled|unique:users,email,'.(int) $this->route()->parameter('id').',id',🤖 Prompt for 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. In `@app/Http/Requests/User/ModifyRequest.php` around lines 27 - 33, The route parameter type handling is inconsistent in User/ModifyRequest: modifyUser() now casts the route id to int, but authorize() and rules() still use the raw $this->route()->parameter('id'). Update the id handling in authorize() and rules() to use the same integer-cast value (preferably via a shared helper or local variable in ModifyRequest) so Gate::allows() and the unique-rule exclusion ID are treated consistently with modifyUser().
🤖 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 @.github/workflows/laravel.yml:
- Line 20: The checkout step in the workflow should disable credential
persistence because this job does not need to push back to the repository.
Update the actions/checkout usage in the Laravel workflow to set
persist-credentials to false so the GITHUB_TOKEN is not stored in .git/config.
Use the existing checkout step as the target and keep the rest of the job
unchanged.
In `@app/Http/Requests/SquidAllowedIp/CreateRequest.php`:
- Line 34: The user_id route parameter in CreateRequest is being cast with
(int), which turns non-numeric input into 0 instead of rejecting it; add numeric
validation to the route binding or FormRequest so invalid values are blocked.
Update the CreateRequest validation rules or the route definition that feeds
$this->route()->parameter('user_id') to enforce integer-only input, then keep
the assignment to $squidAllowedIp->user_id relying on validated data.
---
Nitpick comments:
In `@app/Http/Requests/User/ModifyRequest.php`:
- Around line 27-33: The route parameter type handling is inconsistent in
User/ModifyRequest: modifyUser() now casts the route id to int, but authorize()
and rules() still use the raw $this->route()->parameter('id'). Update the id
handling in authorize() and rules() to use the same integer-cast value
(preferably via a shared helper or local variable in ModifyRequest) so
Gate::allows() and the unique-rule exclusion ID are treated consistently with
modifyUser().
🪄 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
Run ID: 3ecef87b-d505-4561-8487-44aee6e3b245
⛔ Files ignored due to path filters (1)
composer.lockis excluded by!**/*.lock
📒 Files selected for processing (16)
.github/workflows/laravel.ymlapp/Http/Requests/SquidAllowedIp/CreateRequest.phpapp/Http/Requests/SquidUser/CreateRequest.phpapp/Http/Requests/SquidUser/ModifyRequest.phpapp/Http/Requests/User/ModifyRequest.phpapp/Http/Resources/SquidAllowedIpCollection.phpapp/Http/Resources/SquidAllowedIpResource.phpapp/Http/Resources/SquidUserCollection.phpapp/Http/Resources/SquidUserResource.phpapp/Http/Resources/UserCollection.phpapp/Models/SquidAllowedIp.phpapp/Models/SquidUser.phpapp/Models/User.phpcomposer.jsonphpstan.neon.distphpunit.xml
| { | ||
| $squidAllowedIp = new SquidAllowedIp($this->validated()); | ||
| $squidAllowedIp->user_id = $this->route()->parameter('user_id'); | ||
| $squidAllowedIp->user_id = (int) $this->route()->parameter('user_id'); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
echo "== route definitions mentioning user_id =="
fd -t f . routes | xargs -r rg -n -C2 "user_id|whereNumber|Route::"
echo
echo "== request files using route()->parameter('user_id') =="
fd -t f 'CreateRequest.php' app/Http/Requests | xargs -r rg -n -C2 "route\(\)->parameter\('user_id'\)|\(int\)\s*\$this->route\(\)->parameter\('user_id'\)"Repository: 39ff/squid-db-auth-web
Length of output: 6596
🏁 Script executed:
#!/bin/bash
set -eu
echo "== app/Http/Requests/SquidAllowedIp/CreateRequest.php =="
cat -n app/Http/Requests/SquidAllowedIp/CreateRequest.php
echo
echo "== routes/web.php relevant section =="
sed -n '27,43p' routes/web.php
echo
echo "== routes/api.php relevant section =="
sed -n '24,45p' routes/api.phpRepository: 39ff/squid-db-auth-web
Length of output: 4083
user_id に数値制約を追加してください
route() の値を (int) で入れているため、非数値が 0 になります。whereNumber('user_id') か FormRequest 側の整数検証を追加してください。
🤖 Prompt for 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.
In `@app/Http/Requests/SquidAllowedIp/CreateRequest.php` at line 34, The user_id
route parameter in CreateRequest is being cast with (int), which turns
non-numeric input into 0 instead of rejecting it; add numeric validation to the
route binding or FormRequest so invalid values are blocked. Update the
CreateRequest validation rules or the route definition that feeds
$this->route()->parameter('user_id') to enforce integer-only input, then keep
the assignment to $squidAllowedIp->user_id relying on validated data.
The laravel-tests job only reads the repo, installs dependencies and runs tests — it never pushes back. Set persist-credentials: false on actions/checkout so the GITHUB_TOKEN is not written to .git/config, where a later compromised step (e.g. pulling third-party composer deps) could read it. Addresses the zizmor "artipacked" warning from the PR review. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BS9oQ2ynXc33SUyduSrciw
2 participants
Summary
This PR upgrades the project to Laravel 12 and PHP 8.2 as minimum versions, along with corresponding updates to all dependencies. Additionally, it improves type safety throughout the codebase by adding explicit return type declarations and modernizing PHPDoc annotations.
Key Changes
Dependency Updates
^8.1to^8.2^10.4to^12.0^3.2to^4.0^4.2to^4.6^7.2to^7.9^11.5, Collision to^8.6, and other dev toolsnunomaduro/larastantolarastan/larastan^3.0Type Safety Improvements
BelongsToreturn type declarations to relationship methods inSquidAllowedIpandSquidUsermodelsarray<int, string>tolist<string>inUsermodel for better type precisionarray|\Illuminate\Contracts\Support\Arrayable|JsonSerializabletoarray<string, mixed>orarray<int|string, mixed>JsonSerializableimports from resource classesConfiguration Updates
larastan/larastanpackage path<source>element instead of deprecated<coverage>elementNotable Implementation Details
https://claude.ai/code/session_01BS9oQ2ynXc33SUyduSrciw
Summary by CodeRabbit
新機能
不具合修正
Chores