Integrate php cs fixer into quantum starter project#178
Merged
armanist merged 4 commits intosoftberg:masterfrom Apr 2, 2026
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
💤 Files with no reviewable changes (1)
✅ Files skipped from review due to trivial changes (2)
📝 WalkthroughWalkthroughAdds PHP CS Fixer to the project (dev dependency + Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant Repo as Repository (push)
participant GH as GitHub Actions
participant Composer as Composer
participant Fixer as PHP-CS-Fixer
Dev->>Repo: push branch
Repo->>GH: trigger php.yml workflow
GH->>Composer: composer install
Composer->>Fixer: run vendor/bin/php-cs-fixer check (via script cs:check)
Fixer-->>GH: return pass/fail
alt fail
GH-->>Dev: CI fails (style errors)
else pass
GH-->>Dev: CI passes
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![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 (5)
tests/Helpers/functions.php (1)
68-74: Consider failing fast when directory/file creation fails.
mkdir()andfile_put_contents()can fail silently here, which can make later test failures harder to diagnose.Suggested hardening
- if (!is_dir(uploads_dir())) { - mkdir(uploads_dir(), 0777, true); - } + if (!is_dir(uploads_dir()) && !mkdir(uploads_dir(), 0777, true) && !is_dir(uploads_dir())) { + throw new RuntimeException('Failed to create uploads directory: ' . uploads_dir()); + } - if (!file_exists(uploads_dir() . DS . '.gitkeep')) { - file_put_contents(uploads_dir() . DS . '.gitkeep', ''); - } + if (!file_exists(uploads_dir() . DS . '.gitkeep') && file_put_contents(uploads_dir() . DS . '.gitkeep', '') === false) { + throw new RuntimeException('Failed to create .gitkeep in uploads directory'); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/Helpers/functions.php` around lines 68 - 74, The current setup in tests/Helpers/functions.php uses mkdir() and file_put_contents() without checking their return values, so if uploads_dir() creation or writing the '.gitkeep' file fails the tests will continue with a hidden failure; update the block that checks is_dir(uploads_dir()) and file_exists(uploads_dir() . DS . '.gitkeep') to validate the return values of mkdir() and file_put_contents() and throw or trigger a failure (e.g., throw RuntimeException or call the test framework's fail) when either returns false, including the uploads_dir() path in the error message so failures are immediate and actionable.composer.json (1)
8-8: PHP version constraint doesn't cover all CI-tested versions.The constraint
^7.4 || ^8.0covers PHP 7.4, 8.0, and all 8.x versions. However, it's worth confirming this matches your CI matrix (which tests 7.4, 8.0, 8.1). The constraint is technically correct since^8.0includes 8.1, but consider using^7.4 || ^8.0 || ^8.1for explicit clarity, or simply>=7.4if you intend to support all future PHP versions.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@composer.json` at line 8, Update the PHP platform constraint in composer.json (the "php" version requirement) so it explicitly matches the CI-tested matrix; replace the current value "^7.4 || ^8.0" with either "^7.4 || ^8.0 || ^8.1" for clarity or use ">=7.4" if you intend to support all future PHP releases—edit the "php" field accordingly in composer.json (the string key "php") to reflect the chosen constraint.helpers/functions.php (1)
37-38: Minor inconsistency:preg_quote()delimiter usage.Line 38 correctly adds the
'/'delimiter argument topreg_quote(), but line 37'spreg_match()still usespreg_quote(current_lang())without it. While unlikely to cause issues (language codes typically don't contain/), consider adding the delimiter argument consistently for defensive coding.- if (preg_match('/' . preg_quote(current_lang()) . '/', route_uri())) { + if (preg_match('/' . preg_quote(current_lang(), '/') . '/', route_uri())) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@helpers/functions.php` around lines 37 - 38, In the preg_match call inside the function (using preg_match('/' . preg_quote(current_lang()) . '/', route_uri())), make preg_quote usage consistent by passing the delimiter as the second argument (i.e., preg_quote(current_lang(), '/')), so update the preg_match to use preg_quote(current_lang(), '/') to match the style used in the subsequent preg_replace and avoid delimiter-related edge cases; locate and modify the preg_match line in helpers/functions.php accordingly.shared/Commands/DemoCommand.php (1)
375-383: Guard against empty candidate set beforearray_rand.Line 382 can fail when
$candidatesis empty. Add a defensive check to avoid runtime errors in edge cases.Defensive fix
private function getRandomUserExcept(array $users, string $excludeUuid): array { $candidates = array_filter($users, function ($u) use ($excludeUuid) { return $u['uuid'] !== $excludeUuid; }); $candidates = array_values($candidates); + if (empty($candidates)) { + throw new \RuntimeException('No eligible users found for comment generation.'); + } return $candidates[array_rand($candidates)]; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shared/Commands/DemoCommand.php` around lines 375 - 383, The getRandomUserExcept function can call array_rand on an empty $candidates array; add a defensive guard after building $candidates in getRandomUserExcept(array $users, string $excludeUuid): array — if count($candidates) === 0 then throw a clear exception (e.g., InvalidArgumentException or RuntimeException) with a message like "no candidate users available after excluding {uuid}" (or alternatively return a sensible default if caller expects that), otherwise proceed to array_values and array_rand as before; ensure the thrown exception matches the project's error handling conventions.tests/Unit/shared/Services/PostServiceTest.php (1)
17-19: Prefer non-public constant visibility in this test class.Line 17 and Line 19 expose constants publicly without a clear need. Keeping them internal avoids accidental coupling to test internals.
Proposed tweak
- public const PER_PAGE = 10; - public const CURRENT_PAGE = 1; + private const PER_PAGE = 10; + private const CURRENT_PAGE = 1;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/Unit/shared/Services/PostServiceTest.php` around lines 17 - 19, Change the two public constants in the PostServiceTest class (PER_PAGE and CURRENT_PAGE) to non-public visibility (e.g., private const PER_PAGE = 10; private const CURRENT_PAGE = 1;) and update any internal references inside PostServiceTest to use self::PER_PAGE and self::CURRENT_PAGE instead of external class accessors so the constants remain internal to the test implementation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/php.yml:
- Around line 45-47: The workflow runs the composer script cs:check but the
repository lacks a php-cs-fixer config file, so add a project-specific
configuration file named .php-cs-fixer.php (or .php-cs-fixer.dist.php) that
configures PSR-12 as the baseline, enables strict types
(declare(strict_types=1)), enforces short_array_syntax, ordered_imports and
no_unused_imports rules (and any other project rules like line
endings/whitespace) so php-cs-fixer uses these preferences instead of defaults;
update or confirm the composer cs:check script will use that config when
invoked.
In `@shared/config/modules.php`:
- Around line 3-12: This change introduces behavior by enabling 'Api' and 'Web'
modules (keys 'Api' and 'Web' with 'prefix' and 'enabled'), which breaks the "no
functional changes" rule; revert the file to the prior no-op config (e.g.,
return an empty array) or otherwise remove/disable the 'Api' and 'Web' entries
so no modules are implicitly enabled by this PR, leaving a neutral/default
configuration.
---
Nitpick comments:
In `@composer.json`:
- Line 8: Update the PHP platform constraint in composer.json (the "php" version
requirement) so it explicitly matches the CI-tested matrix; replace the current
value "^7.4 || ^8.0" with either "^7.4 || ^8.0 || ^8.1" for clarity or use
">=7.4" if you intend to support all future PHP releases—edit the "php" field
accordingly in composer.json (the string key "php") to reflect the chosen
constraint.
In `@helpers/functions.php`:
- Around line 37-38: In the preg_match call inside the function (using
preg_match('/' . preg_quote(current_lang()) . '/', route_uri())), make
preg_quote usage consistent by passing the delimiter as the second argument
(i.e., preg_quote(current_lang(), '/')), so update the preg_match to use
preg_quote(current_lang(), '/') to match the style used in the subsequent
preg_replace and avoid delimiter-related edge cases; locate and modify the
preg_match line in helpers/functions.php accordingly.
In `@shared/Commands/DemoCommand.php`:
- Around line 375-383: The getRandomUserExcept function can call array_rand on
an empty $candidates array; add a defensive guard after building $candidates in
getRandomUserExcept(array $users, string $excludeUuid): array — if
count($candidates) === 0 then throw a clear exception (e.g.,
InvalidArgumentException or RuntimeException) with a message like "no candidate
users available after excluding {uuid}" (or alternatively return a sensible
default if caller expects that), otherwise proceed to array_values and
array_rand as before; ensure the thrown exception matches the project's error
handling conventions.
In `@tests/Helpers/functions.php`:
- Around line 68-74: The current setup in tests/Helpers/functions.php uses
mkdir() and file_put_contents() without checking their return values, so if
uploads_dir() creation or writing the '.gitkeep' file fails the tests will
continue with a hidden failure; update the block that checks
is_dir(uploads_dir()) and file_exists(uploads_dir() . DS . '.gitkeep') to
validate the return values of mkdir() and file_put_contents() and throw or
trigger a failure (e.g., throw RuntimeException or call the test framework's
fail) when either returns false, including the uploads_dir() path in the error
message so failures are immediate and actionable.
In `@tests/Unit/shared/Services/PostServiceTest.php`:
- Around line 17-19: Change the two public constants in the PostServiceTest
class (PER_PAGE and CURRENT_PAGE) to non-public visibility (e.g., private const
PER_PAGE = 10; private const CURRENT_PAGE = 1;) and update any internal
references inside PostServiceTest to use self::PER_PAGE and self::CURRENT_PAGE
instead of external class accessors so the constants remain internal to the test
implementation.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: c1171d01-e9a1-4054-b878-50b9b00054f1
📒 Files selected for processing (68)
.github/workflows/php.ymlcomposer.jsonhelpers/functions.phpmigrations/create_table_comments_1698145440.phpmigrations/create_table_posts_1669639752.phpmigrations/create_table_users_1669639740.phppublic/index.phpshared/Commands/CommandValidationTrait.phpshared/Commands/CommentCreateCommand.phpshared/Commands/CommentDeleteCommand.phpshared/Commands/DemoCommand.phpshared/Commands/PostCreateCommand.phpshared/Commands/PostDeleteCommand.phpshared/Commands/PostShowCommand.phpshared/Commands/PostUpdateCommand.phpshared/Commands/UserCreateCommand.phpshared/Commands/UserDeleteCommand.phpshared/Commands/UserShowCommand.phpshared/DTOs/CommentDTO.phpshared/Enums/Role.phpshared/Models/Comment.phpshared/Models/Post.phpshared/Models/User.phpshared/Services/AuthService.phpshared/Services/CommentService.phpshared/Services/PostService.phpshared/Transformers/PostTransformer.phpshared/config/cache.phpshared/config/captcha.phpshared/config/database.phpshared/config/dependencies.phpshared/config/env.phpshared/config/fs.phpshared/config/lang.phpshared/config/logging.phpshared/config/mailer.phpshared/config/modules.phpshared/config/session.phpshared/config/view.phpshared/config/view_cache.phpshared/resources/lang/en/common.phpshared/resources/lang/en/validation.phpshared/resources/lang/es/common.phpshared/resources/lang/es/validation.phptests/Feature/AppTestCase.phptests/Feature/modules/Api/AuthControllerTest.phptests/Feature/modules/Api/CommentControllerTest.phptests/Feature/modules/Api/PostControllerTest.phptests/Feature/modules/Api/PostManagementControllerTest.phptests/Helpers/functions.phptests/Unit/shared/Services/AuthServiceTest.phptests/Unit/shared/Services/CommentServiceTest.phptests/Unit/shared/Services/PostServiceTest.phptests/_root/helpers/functions.phptests/_root/shared/config/app.phptests/_root/shared/config/database.phptests/_root/shared/config/dependencies.phptests/_root/shared/config/env.phptests/_root/shared/config/fs.phptests/_root/shared/config/lang.phptests/_root/shared/config/logging.phptests/_root/shared/config/mailer.phptests/_root/shared/config/view.phptests/_root/shared/resources/lang/en/common.phptests/_root/shared/resources/lang/en/validation.phptests/_root/shared/resources/lang/es/common.phptests/_root/shared/resources/lang/es/validation.phptests/bootstrap.php
💤 Files with no reviewable changes (3)
- migrations/create_table_users_1669639740.php
- migrations/create_table_comments_1698145440.php
- migrations/create_table_posts_1669639752.php
armanist
deleted the
177-Integrate-PHP-CS-Fixer-into-Quantum-Starter-Project
branch
This was referenced Apr 3, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #177
Summary by CodeRabbit
Chores
Bug Fixes