chore: upgrade PHP to 8.1, add spiral/code-style, config and CI for code style, update psalm #11
Conversation
WalkthroughThe changes update PHP version requirements across documentation and configuration, refactor PHP-CS-Fixer setup to use a builder pattern, and adjust GitHub Actions workflows for code style and testing. Additional ignore rules and code formatting improvements are included, with no changes to public APIs or logic. Changes
Possibly related PRs
Suggested reviewers
Poem
Note ⚡️ AI Code Reviews for VS Code, Cursor, WindsurfCodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback. Note ⚡️ Faster reviews with cachingCodeRabbit now supports caching for code and dependencies, helping speed up reviews. This means quicker feedback, reduced wait times, and a smoother review experience overall. Cached data is encrypted and stored securely. This feature will be automatically enabled for all accounts on May 30th. To opt out, configure 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (11)
💤 Files with no reviewable changes (2)
✅ Files skipped from review due to trivial changes (4)
🚧 Files skipped from review as they are similar to previous changes (4)
🔇 Additional comments (2)
✨ 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
novikoff-vvs
force-pushed
the
chore/php81-spiral-cs-ci-psalm
branch
2 times, most recently
from
224648b to
4f9635a
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: 0
🧹 Nitpick comments (1)
composer.json (1)
37-39: Standardize php-cs-fixer binary path in Composer scripts
Thecs:diffandcs:fixscripts currently invokephp-cs-fixerwithout thevendor/bin/prefix, which may lead to execution failures if the binary isn't globally installed.
Apply this diff to align with thecsscript invocation:- "cs:diff": "php-cs-fixer fix --dry-run -v --diff", - "cs:fix": "php-cs-fixer fix -v" + "cs:diff": "vendor/bin/php-cs-fixer fix --dry-run -v --diff", + "cs:fix": "vendor/bin/php-cs-fixer fix -v"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (9)
.github/workflows/cs.yml(1 hunks).github/workflows/phpunit.yml(1 hunks).gitignore(1 hunks).php-cs-fixer.dist.php(1 hunks)README.md(1 hunks)composer.json(2 hunks)src/Exception/RequiredVersionException.php(1 hunks)src/Exception/RoadrunnerNotInstalledException.php(1 hunks)src/Exception/VersionCheckerException.php(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
src/Exception/RequiredVersionException.php (1)
src/Exception/VersionCheckerException.php (1)
VersionCheckerException(7-7)
src/Exception/RoadrunnerNotInstalledException.php (1)
src/Exception/VersionCheckerException.php (1)
VersionCheckerException(7-7)
🔇 Additional comments (12)
src/Exception/VersionCheckerException.php (1)
7-7: Consistent single-line class declaration.The
VersionCheckerExceptionhas been refactored to a one-line declaration with an empty body, aligning with the repository’s new code style. No behavior or inheritance changes.src/Exception/RoadrunnerNotInstalledException.php (1)
7-7: Single-line formatting for exception class.The
RoadrunnerNotInstalledExceptionnow uses a concise one-line declaration, matching the format of other exception classes. Functionality and inheritance remain unchanged.src/Exception/RequiredVersionException.php (1)
7-7: Simplified declaration for consistency.The
RequiredVersionExceptionclass declaration has been converted to a single line to standardize formatting across exception classes. No change in logic or behavior.README.md (1)
15-15: Version requirement properly updated.The minimum PHP version requirement has been correctly updated from 8.0+ to 8.1+ in the README documentation, which aligns with the PR's objective of upgrading the minimum required PHP version.
.github/workflows/phpunit.yml (1)
16-16: PHP version matrix correctly updated.The PHP versions for testing have been updated to only include PHP 8.1, 8.2, and 8.3, removing 8.0. This change is consistent with the updated minimum PHP version requirement.
.gitignore (1)
13-13: Appropriate cache file added to gitignore.The addition of
/runtime/php-cs-fixer.cacheto .gitignore properly prevents the PHP-CS-Fixer cache files from being tracked in version control, which is a best practice when introducing code style tooling..github/workflows/cs.yml (3)
4-4: Branch filter updated to check all branches.The branch filter has been changed from a specific branch to all branches (
'*'), ensuring code style checks are performed on all pushed branches.
6-6: Workflow name updated for clarity.The workflow name has been changed to "Check Code Style" which better reflects its purpose and improves readability in GitHub Actions UI.
9-9: Job identifier renamed to match its function.The job identifier has been renamed from "psalm" to "cs-check", which better reflects the actual purpose of the job (checking code style rather than running Psalm).
.php-cs-fixer.dist.php (1)
5-12: Code style configuration simplified with builder pattern.The PHP-CS-Fixer configuration has been refactored to use Spiral's CodeStyle Builder pattern, which provides a cleaner and more maintainable approach. This implementation properly includes the source directory and the configuration file itself for formatting checks.
The use of the builder pattern is a good improvement as it:
- Reduces boilerplate code
- Leverages standardized configuration from the spiral/code-style package
- Makes future configuration changes easier to manage
composer.json (2)
12-16: Upgrade minimum PHP requirement to 8.1
Updating the PHP version constraint to^8.1ensures compatibility with the new language features and aligns with the updated code style tooling and CI workflows.
18-23: Introduce consistent code style and static analysis tools
Addingfriendsofphp/php-cs-fixer,phpcompatibility/php-compatibility,spiral/code-style, andvimeo/psalmtorequire-devaligns with the CI configuration for automated linting and static checks. Please verify that the corresponding configuration files (.php-cs-fixer.dist.php,psalm.xml) are in place and up to date.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
src/VersionChecker.php (2)
42-42: Changed empty check to explicit null checkThe change from
empty($version)to$version == nullchanges the validation behavior. Whileempty()would return true for null, empty strings, and "0", the null check is more specific and only catches null values.Given the parameter is typed as
@param non-empty-string|null, you might want to ensure empty strings are still properly validated elsewhere, or add a separate check liketrim($version) === ''if empty strings should be rejected.
46-46: Same empty vs null check change as aboveThis is the same change from
empty($version)to$version == null. Consider whether allowing empty strings here is intentional or if additional validation is needed.src/Version/Installed.php (2)
45-45: Changed empty check to explicit null checkSimilar to changes in VersionChecker.php, this changes from
!empty(self::$cachedVersion)toself::$cachedVersion != null. The null check is more specific and would allow empty strings to pass through where empty() would have caught them.Since the cache variable is typed as
@var non-empty-string|null, ensure proper validation happens when values are initially assigned to the cache.
97-99: Consider updating this check for consistencyFor consistency with the changes made elsewhere in this file and in VersionChecker.php, consider updating this condition from
isset($matches[1]) && !empty($matches[1])toisset($matches[1]) && $matches[1] != null.-if (isset($matches[1]) && !empty($matches[1])) { +if (isset($matches[1]) && $matches[1] != null) { return $matches[1]; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (2)
src/Version/Installed.php(1 hunks)src/VersionChecker.php(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
src/VersionChecker.php (2)
src/Version/Required.php (1)
getRequiredVersion(30-41)src/Version/RequiredInterface.php (1)
getRequiredVersion(12-12)
src/Version/Installed.php (1)
tests/src/Unit/Version/InstalledTest.php (2)
getVersionFromEnv(60-75)getVersionFromConsoleCommand(77-96)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: psalm / PHP 8.3-ubuntu-latest
🔇 Additional comments (3)
src/VersionChecker.php (1)
113-113: Improved array key existence checkThe change from
!empty($matches[0])toisset($matches[0]) && $matches[0] != nullis better as it first checks if the array key exists before checking its value. This prevents potential PHP notices if the regex doesn't match anything and is a good defensive programming practice.src/Version/Installed.php (2)
49-52: Improved version retrieval patternThis refactoring improves the code structure by:
- Assigning the result to a local variable first
- Checking if the variable is not null
- Only then caching the value and returning it
This is cleaner and ensures the cache is only updated when a valid value is found.
54-57: Similar improvement to version retrieval patternSame improvement as above for the console command version retrieval. This consistent pattern makes the code more maintainable and ensures the cached version is only set when a valid value is found.
Fix Psalm errors by refining null and empty checks and improving type safety chore: upgrade PHP to 8.1, add spiral/code-style, config and CI for code style, update psalm
novikoff-vvs
force-pushed
the
chore/php81-spiral-cs-ci-psalm
branch
from
603c6b9 to
b951626
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/cs.yml (1)
17-18: Optimize dependency installation with caching.
Consider adding a Composer cache step usingactions/cache@v3to reduce CI runtime:- uses: actions/cache@v3 with: path: vendor key: ${{ runner.os }}-composer-${{ hashFiles('composer.lock') }}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (11)
.github/workflows/cs.yml(1 hunks).github/workflows/phpunit.yml(1 hunks).gitignore(1 hunks).php-cs-fixer.dist.php(1 hunks)README.md(1 hunks)composer.json(2 hunks)src/Exception/RequiredVersionException.php(1 hunks)src/Exception/RoadrunnerNotInstalledException.php(1 hunks)src/Exception/VersionCheckerException.php(1 hunks)src/Version/Installed.php(1 hunks)src/VersionChecker.php(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/Exception/VersionCheckerException.php
🚧 Files skipped from review as they are similar to previous changes (9)
- README.md
- src/Exception/RoadrunnerNotInstalledException.php
- .github/workflows/phpunit.yml
- src/Exception/RequiredVersionException.php
- .gitignore
- src/VersionChecker.php
- src/Version/Installed.php
- composer.json
- .php-cs-fixer.dist.php
🧰 Additional context used
🪛 actionlint (1.7.7)
.github/workflows/cs.yml
10-10: 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 (5)
.github/workflows/cs.yml (5)
1-3: Workflow name and triggers are correctly updated.
The name "PHP-CS-Fixer Check" clearly describes the job, and triggering on bothpushandpull_requestensures style checks on all code changes.
6-9: Job definition and runner selection are appropriate.
Defining thephpcsfixerjob onubuntu-latestprovides a stable environment with PHP 8.1 support.
10-10: Checkout action version is appropriate.
Usingactions/checkout@v3aligns with the current major release. The actionlint warning about it being “too old” is a false positive.🧰 Tools
🪛 actionlint (1.7.7)
10-10: 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)
12-15: PHP setup matches project requirements.
The use ofshivammathur/setup-php@v2withphp-version: '8.1'correctly aligns with the updated minimum PHP version incomposer.jsonand documentation.
20-22: Non-destructive code style check via dry-run.
Runningphp-cs-fixer fix --dry-run -v --difffails the job on violations without modifying files, which is CI best practice.
novikoff-vvs
force-pushed
the
chore/php81-spiral-cs-ci-psalm
branch
from
380ba5e to
263fb1b
Compare
| 'ordered_imports' => ['sort_algorithm' => 'alpha'], | ||
| 'fully_qualified_strict_types' => true, | ||
| 'no_unused_imports' => true, |
There was a problem hiding this comment.
Let's move it into https://github.com/spiral/code-style
chore: upgrade PHP to 8.1, add spiral/code-style, config and CI for code style, update psalm
This PR upgrades the minimum required PHP version to 8.1, adds spiral/code-style for consistent code formatting, includes a code style configuration file, and sets up CI to ensure code style checks are automatically enforced. Additionally, Psalm has been updated to the latest version to improve static analysis accuracy.
Summary by CodeRabbit