Add workflow job for phpstan #70
Conversation
📝 WalkthroughWalkthroughConsolidates CI linting into a new GitHub Actions workflow and adds PHPStan configuration; additionally adjusts several files to add/remove PHPStan suppression comments and reorders an HTTP POST call in one action. No runtime API or public signatures changed. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
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 |
Boy132
force-pushed
the
phpstan-workflow
branch
from
3599952 to
d7acfe6
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
.github/workflows/lint.yml (1)
29-29: Consider managing dependency versions more maintainably.The workflow specifies exact version constraints for all dependencies. While this ensures reproducibility, it requires manual updates when new versions are released. Consider:
- Using broader version ranges (e.g.,
^1.0instead of^1.15.3)- Maintaining these versions in a separate configuration file or repository variable
- Using Dependabot to automatically update dependency versions
This approach reduces maintenance overhead while still ensuring compatible versions are used.
Also applies to: 52-52, 55-55, 58-58
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/lint.yml.github/workflows/pint.ymlphpstan.neon
💤 Files with no reviewable changes (1)
- .github/workflows/pint.yml
🧰 Additional context used
🪛 actionlint (1.7.9)
.github/workflows/lint.yml
16-16: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
39-39: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🔇 Additional comments (4)
.github/workflows/lint.yml (3)
1-8: Well-structured workflow with appropriate triggers.The workflow is cleanly organized with two independent jobs (Pint and PHPStan) that can run in parallel. The trigger configuration for pushes to
mainand pull requests is appropriate for continuous integration.Also applies to: 10-12, 34-36
26-26: The workflow's use ofcomposer init --no-interactionis appropriate. Since the repository has no committedcomposer.jsonfile, the init command creates a minimal configuration needed for the CI jobs to install and run PHP linting tools (Laravel Pint and Phpstan). This is a standard pattern for isolated linting environments and does not overwrite any existing project configuration.
21-21: No action required — PHP 8.4 is well-supported for your CI environment.PHP 8.4 is available in
shivammathur/setup-php@v2(since v2.32.0), and all stated dependencies explicitly support it: Laravel 12 supports PHP 8.2–8.4, Larastan 3 requires PHP 8.2+, and Filament 4 requires PHP 8.2+. No compatibility concerns with your specified dependency versions.phpstan.neon (1)
1-2: No issues identified. The phpstan.neon configuration correctly includes the Larastan extension, and Larastan 3.4 is a stable version with no breaking changes. The configuration is compatible and properly set up.
Boy132
force-pushed
the
phpstan-workflow
branch
from
54ca1d4 to
187dcb6
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
player-counter/src/Filament/Server/Pages/PlayersPage.php (2)
43-43: Consider documenting the relationship instead of suppressing the warning.While the suppression silences PHPStan, it reduces static analysis effectiveness. A better approach would be to add proper PHPDoc annotations to the
Eggmodel to document thegameQueryrelationship:/** * @return \Illuminate\Database\Eloquent\Relations\HasOne<GameQuery> */ public function gameQuery()Alternatively, if you're using larastan, ensure it's properly configured to detect Laravel relationships automatically.
75-75: Consider documenting the relationship property instead of suppressing warnings.Similar to the method access on line 43, these property suppressions can be avoided by adding a
@propertyannotation to theEggmodel:/** * @property-read GameQuery|null $gameQuery */ class Egg extends ModelThis allows PHPStan to understand the property access pattern without needing inline suppressions, improving type safety and static analysis coverage.
Also applies to: 108-108
subdomains/src/Models/CloudflareDomain.php (1)
37-40: PHPStan suppression is appropriate for the Laravel HTTP facade macro.The
Http::cloudflare()macro is properly registered inSubdomainsPluginProvider::boot()and the@phpstan-ignore staticMethod.notFounddirective correctly suppresses the static analysis error, since PHPStan cannot detect dynamically registered macros.For future improvements, consider adding PHPStan stubs to configure this macro in the type system. While the current approach works, stubs would eliminate the need for inline suppressions and improve IDE support. The project's phpstan.neon can be extended with a stubs path when stubs are created.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
.github/workflows/lint.yml.github/workflows/pint.ymlbilling/src/Filament/App/Widgets/ProductWidget.phpbilling/src/Models/Coupon.phpbilling/src/Models/Order.phpbilling/src/Models/Product.phpbilling/src/Models/ProductPrice.phppastefox-share/src/Filament/Components/Actions/UploadLogsAction.phpphpstan.neonplayer-counter/database/Seeders/PlayerCounterSeeder.phpplayer-counter/src/Filament/Server/Pages/PlayersPage.phpplayer-counter/src/Filament/Server/Widgets/ServerPlayerWidget.phpsubdomains/src/Filament/Server/Resources/Subdomains/SubdomainResource.phpsubdomains/src/Models/CloudflareDomain.phpsubdomains/src/Models/Subdomain.phpuser-creatable-servers/src/Filament/Server/Pages/ServerResourcePage.phpuser-creatable-servers/src/Models/UserResourceLimits.php
💤 Files with no reviewable changes (1)
- .github/workflows/pint.yml
✅ Files skipped from review due to trivial changes (5)
- billing/src/Models/ProductPrice.php
- subdomains/src/Filament/Server/Resources/Subdomains/SubdomainResource.php
- billing/src/Models/Product.php
- player-counter/src/Filament/Server/Widgets/ServerPlayerWidget.php
- billing/src/Models/Coupon.php
🚧 Files skipped from review as they are similar to previous changes (2)
- .github/workflows/lint.yml
- phpstan.neon
🔇 Additional comments (8)
player-counter/database/Seeders/PlayerCounterSeeder.php (1)
35-38: Appropriate suppression for Laravel seeder pattern.The PHPStan ignore directive correctly addresses a false positive. The
$this->commandproperty is nullable in Laravel seeders (only set when run via CLI), so the defensive check on line 36 is necessary to prevent null pointer exceptions.pastefox-share/src/Filament/Components/Actions/UploadLogsAction.php (1)
76-81: LGTM! Improved method chain ordering.The reordering of
->post()to occur after->throw()is correct and follows Laravel HTTP client best practices. Configuration methods (timeout,connectTimeout,throw) should precede the request execution method (post), which improves both readability and type safety for static analysis tools like PHPStan.user-creatable-servers/src/Filament/Server/Pages/ServerResourcePage.php (1)
165-166: LGTM! PHPStan suppression removal is appropriate.The removal of the PHPStan ignore directive is correct. The
@varannotation on line 165 provides sufficient type information for static analysis to understand thatapp(DaemonServerRepository::class)returns aDaemonServerRepositoryinstance.subdomains/src/Models/Subdomain.php (1)
71-79: PHPStan suppressions applied consistently across all Cloudflare operations.The PHPStan ignore directives are appropriately applied to all three Cloudflare HTTP operations (POST, PATCH, DELETE), ensuring consistent handling of the
Http::cloudflare()macro across the codebase.As noted in the previous file, consider using PHPStan configuration or stubs for a more maintainable approach in the future.
Also applies to: 98-106, 113-114
user-creatable-servers/src/Models/UserResourceLimits.php (1)
150-151: The @var annotation provides sufficient type information for static analysis.The
@var ServerCreationService $servicetype hint on line 150 is properly formatted, andServerCreationServiceis correctly imported at the top of the file (line 9). The removal of the PHPStan suppression comment aligns with standard Laravel patterns, where theapp()helper with a class string argument is understood by LaraSTan to return the specified type.billing/src/Models/Order.php (2)
231-231: Consistent service resolution pattern.The removal of the PHPStan suppression here is consistent with the other service container resolution patterns in this file. PHPStan should be able to infer the return type from the
handle()method signature.
78-78: Good cleanup of PHPStan suppressions.Removing these suppressions confirms that PHPStan is properly configured via Larastan to understand Laravel's service container resolution patterns. The explicit
@vardocblocks on lines 101 and 114 provide adequate type information for the StripeClient instances, and Larastan handles the remaining app() calls correctly.billing/src/Filament/App/Widgets/ProductWidget.php (1)
25-25: Verify the necessity of this PHPStan suppression.The property
$viewis explicitly initialized with a string value, and this is the only occurrence of aproperty.defaultValuesuppression in the codebase. Since Larastan is configured to handle Laravel/Filament patterns, consider whether this suppression masks a false positive that could be better resolved through PHPStan configuration or a Filament-specific extension rather than a blanket ignore comment.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.