Update dependency and PHP version for enhanced compatibility #74
Conversation
Upgraded PHP version from 8.1 to 8.3 in both static-analysis and coding-standards workflows. This ensures compatibility with the latest features and improvements in PHP 8.3.
Extended version constraints for "rize/uri-template" and "doctrine/annotations" to include newer versions. This ensures better compatibility with projects using updated library versions while maintaining support for older ones.
|
Warning Rate limit exceeded@koriym has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 8 minutes and 16 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
WalkthroughThis pull request involves updating PHP versions in GitHub Actions workflows and expanding dependency version constraints in the Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
![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
🔭 Outside diff range comments (1)
composer.json (1)
Line range hint
10-10: Update PHP version constraint to match workflow changes.The PHP version constraint
"^7.4 || ^8.0"should be updated to explicitly include PHP 8.3 since the workflows are being updated to use it.Apply this change:
- "php": "^7.4 || ^8.0", + "php": "^7.4 || ^8.0 || ^8.1 || ^8.2 || ^8.3",🧰 Tools
🪛 GitHub Actions: Static Analysis
[error] Unknown symbol 'Doctrine\Common\Annotations\NamedArgumentConstructorAnnotation' found without corresponding dependency declaration
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/coding-standards.yml(1 hunks).github/workflows/static-analysis.yml(1 hunks)composer.json(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- .github/workflows/coding-standards.yml
🧰 Additional context used
🪛 GitHub Actions: Static Analysis
composer.json
[error] Unknown symbol 'Doctrine\Common\Annotations\NamedArgumentConstructorAnnotation' found without corresponding dependency declaration
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: cs / Coding Standards
- GitHub Check: cs / Coding Standards
🔇 Additional comments (1)
composer.json (1)
18-18: Verify compatibility with rize/uri-template v0.4.While expanding version constraints improves flexibility, we should verify there are no breaking changes between v0.3.4 and v0.4 that could affect the codebase.
Let's check the changelog and usage:
✅ Verification successful
Version 0.4 is safe to use but requires PHP 8.1+
The update only adds explicit nullable types for PHP 8.4 compatibility with no breaking changes to the API. Our simple URI template usage pattern is fully compatible with v0.4. Just ensure your PHP version is 8.1 or higher.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for breaking changes in rize/uri-template v0.4 # Search for URI template usage in the codebase echo "Searching for URI template usage patterns..." rg -l "UriTemplate|uri-template" --type php # Fetch changelog to check for breaking changes echo "Checking changelog for breaking changes..." curl -s https://raw.githubusercontent.com/rize/UriTemplate/master/CHANGELOG.md || echo "No changelog found"Length of output: 362
Script:
#!/bin/bash # Check how UriTemplate is used in the codebase echo "=== URI Template Usage ===" cat src-files/uri_template.php echo -e "\n=== Package Info ===" # Get package details from Packagist curl -s https://packagist.org/packages/rize/uri-template.json | jq -r '.package.versions["0.4.0"] | {version, description, time, require}' # Try to fetch README for BC promise info echo -e "\n=== README Info ===" curl -s https://raw.githubusercontent.com/rize/UriTemplate/0.4.0/README.mdLength of output: 9416
🧰 Tools
🪛 GitHub Actions: Static Analysis
[error] Unknown symbol 'Doctrine\Common\Annotations\NamedArgumentConstructorAnnotation' found without corresponding dependency declaration
This update adds the NamedArgumentConstructor annotation to several classes, improving consistency across annotation definitions. It also removes the previously implemented `NamedArgumentConstructorAnnotation` interface where it was no longer necessary.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/Annotation/AuraSqlQueryConfig.php (1)
Line range hint
1-1: Document the annotation system changes in UPGRADE.md.These changes represent a shift from interface-based to attribute-based named arguments, which is part of modernizing the codebase for newer PHP versions. Consider:
- Adding an UPGRADE.md file to document this architectural change
- Specifying minimum required versions of doctrine/annotations
- Including examples of both annotation and attribute usage for backward compatibility
Would you like me to help draft the UPGRADE.md content?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/Annotation/AuraSqlQueryConfig.php(1 hunks)src/Annotation/PagerViewOption.php(1 hunks)src/Annotation/Read.php(1 hunks)src/Annotation/Transactional.php(1 hunks)
🔇 Additional comments (5)
src/Annotation/Transactional.php (2)
13-16: LGTM: Migration to @NamedArgumentConstructor annotation.The removal of
NamedArgumentConstructorAnnotationinterface in favor of@NamedArgumentConstructorannotation aligns with Doctrine's recommended approach for handling named arguments.
Line range hint
19-22: Consider addressing the deprecated property.The
$valueproperty is marked as deprecated but still in use. Consider creating a follow-up issue to track its removal and provide migration guidance for users.Would you like me to help create an issue to track the deprecation and outline a migration plan?
src/Annotation/Read.php (1)
8-8: LGTM: Consistent annotation updates.The changes maintain consistency with other annotation classes while preserving the Qualifier functionality.
Also applies to: 16-16, 19-19
src/Annotation/PagerViewOption.php (1)
8-8: LGTM: Consistent annotation updates.The changes maintain consistency with other annotation classes while preserving the Qualifier functionality.
Also applies to: 16-16, 19-19
src/Annotation/AuraSqlQueryConfig.php (1)
8-8: LGTM: Consistent annotation updates.The changes maintain consistency with other annotation classes while preserving proper type hints and nullability.
Also applies to: 16-16, 19-19
This change adds PHPDoc to explicitly define the type of the `$invocation` parameter in the `getConnection` method. It improves code readability and helps with static analysis tools.
Ensure the closure explicitly declares its return type as `Select` for better type safety and clarity. This change improves code readability and aligns with strict type-checking practices.
Upgraded multiple dependencies to their latest versions as specified in composer.lock. This includes updates to "amphp/amp", "doctrine/instantiator", "nikic/php-parser", and others, bringing improvements, fixes, and compatibility changes. Ensure codebase compatibility with the changes before deploying.
This update includes the NamedArgumentConstructor attribute to enhance compatibility with named arguments in PHP. The `Write` class no longer implements the `NamedArgumentConstructorAnnotation` interface, simplifying the implementation. Adjustments maintain consistency and follow modern PHP conventions.
Replaced `NamedArgumentConstructorAnnotation` with `NamedArgumentConstructor` and adjusted syntax for consistency across annotations. This change ensures compatibility with the updated Doctrine annotations library.
Changed the Psalm static analysis command to use `--show-info=false`. This ensures a cleaner output by suppressing informational messages during analysis.
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
src-deprecated/Annotation/Write.php (1)
Remove unused Write annotation class
The
Writeannotation class insrc-deprecated/Annotation/Write.phpis not used anywhere in the codebase. The correct annotation to use isWriteConnection(found insrc/Annotation/WriteConnection.php).
- Remove the deprecated class in the next major version
- Update the deprecation notice to:
@deprecated since 2.x.x, will be removed in 3.0.0. Use WriteConnection annotation/attribute instead.🔗 Analysis chain
Line range hint
1-29: Clarify deprecation status and migration path.The file contains a vague deprecation notice ("No one using?"). Since this is in a deprecated directory and the PR is updating for PHP 8.3 compatibility:
- Document when this class will be removed
- Provide the recommended alternative or migration path for users
Let's check if this class is still being used:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for usages of the Write annotation/attribute rg -l "Write\(" --type php rg -l "#\[Write" --type phpLength of output: 266
Script:
#!/bin/bash # Check actual usage in core files echo "=== Core Usage ===" rg "Write\(" -A 5 -B 5 src/AuraSqlConnectionInterceptor.php src/AuraSqlReplicationDbProvider.php src/AuraSqlMasterDbInterceptor.php echo -e "\n=== Test Usage ===" rg "#\[Write" -A 5 -B 5 tests/Fake/FakeModel.php echo -e "\n=== Related Files ===" # Look for potential alternatives or documentation fd -e php -e md . | rg -i "write|query|sql"Length of output: 4545
🧹 Nitpick comments (1)
src/AuraSqlConnectionInterceptor.php (1)
51-51: LGTM! Consider adding return type documentation.The addition of the generic type parameter
MethodInvocation<object>improves type safety and static analysis capabilities. This is particularly beneficial with PHP 8.3's enhanced type system.Consider adding the return type to the PHPDoc for completeness:
- /** @param MethodInvocation<object> $invocation */ + /** + * @param MethodInvocation<object> $invocation + * @return ExtendedPdoInterface + */
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
vendor-bin/tools/composer.lockis excluded by!**/*.lock
📒 Files selected for processing (8)
composer.json(2 hunks)src-deprecated/Annotation/Write.php(1 hunks)src/Annotation/AuraSqlQueryConfig.php(1 hunks)src/Annotation/PagerViewOption.php(1 hunks)src/Annotation/Read.php(1 hunks)src/Annotation/Transactional.php(1 hunks)src/AuraSqlConnectionInterceptor.php(1 hunks)src/Pagerfanta/AuraSqlQueryPager.php(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- src/Annotation/Transactional.php
- src/Annotation/Read.php
- src/Annotation/PagerViewOption.php
- composer.json
- src/Annotation/AuraSqlQueryConfig.php
🔇 Additional comments (2)
src/Pagerfanta/AuraSqlQueryPager.php (1)
72-72: Great addition of return type for improved type safety!The explicit return type
: Selecton the closure enhances code clarity and enables better static analysis. This change aligns well with PHP 8.3's emphasis on type safety.src-deprecated/Annotation/Write.php (1)
16-16: LGTM! Addition of @NamedArgumentConstructor annotation.The addition of
@NamedArgumentConstructoraligns with modern PHP practices and prepares the code for PHP 8.3 compatibility.
This commit removes redundant `NamedArgumentConstructorAnnotation` imports and updates the `AuraSqlConfig` class to no longer implement it. These changes reflect the cleanup of deprecated or unnecessary dependencies in the codebase.
Extended version constraints for "rize/uri-" and "doctrine/annotations" to ensure compatibility with various library versions. This update allows better support for both older and newer libraries.
Summary by CodeRabbit
rize/uri-templateanddoctrine/annotationsin project dependencies.@NamedArgumentConstructorannotation to several classes, enhancing support for named arguments in constructors.