Skip to content

Conversation

@koriym
Copy link
Member

@koriym koriym commented Nov 11, 2025

Summary

This PR fixes PHP 8.1+ deprecation warnings by adding an explicit return type declaration to the Failure::jsonSerialize() method in the 3.x branch.

Problem

Since PHP 8.1, the JsonSerializable interface requires return type declarations. Without it, the following deprecation warning is triggered:

Return type of Aura\Filter\Failure\Failure::jsonSerialize() should either be compatible 
with JsonSerializable::jsonSerialize(): mixed, or the #[\ReturnTypeWillChange] attribute 
should be used to temporarily suppress the notice

Solution

Add : array return type declaration to the jsonSerialize() method (line 115).

Backward Compatibility

✅ This change is fully backward compatible:

  • Return type declarations are supported since PHP 7.0
  • The 3.x branch already requires PHP 5.6+ per composer.json
  • No breaking changes to method behavior

Testing

The change maintains the same return value and behavior. All existing tests should continue to pass.

Related

  • This fix is already present in the 4.x branch
  • Addresses deprecation warnings for users on PHP 8.1, 8.2, 8.3, and 8.4
  • Resolves issue for projects using aura/filter: ^3.x-dev

Summary by CodeRabbit

Release Notes

  • Chores
    • Updated continuous integration to support PHP 8.2 and 8.3 while removing support for older PHP versions (5.6, 7.0, 7.1)
    • Enhanced code analysis configuration and improved error handling robustness across the codebase

Fix PHP 8.1+ deprecation warning by adding explicit return type
declaration to Failure::jsonSerialize() method.

This change is backward compatible with PHP 7.0+ which already
supports return type declarations.
@coderabbitai
Copy link

coderabbitai bot commented Nov 11, 2025

Walkthrough

Configuration files updated to support PHP 8.2–8.4 with modern CI/CD practices. Source code enhancements include adding return type hints, improving error handling robustness, and exposing internal locators via new protected properties.

Changes

Cohort / File(s) Summary
CI/CD Configuration Updates
\.github/workflows/continuous-integration\.yml, \.scrutinizer\.yml
Upgraded PHP matrix from 5.6–7.1 to 8.2–8.4; replaced deprecated GitHub Actions set-output with GITHUB_OUTPUT; bumped Composer cache action v2→v4; reconfigured Scrutinizer with build section (default-jammy image, PHP 8.4) and removed legacy tools entries.
Type Safety Improvements
src/Failure/Failure\.php
Added explicit array return type hint to jsonSerialize() method for stricter type declaration.
Error Handling Enhancements
src/Rule/AbstractDateTime\.php
Introduced null-safe check (if ($errors && $errors['warnings'])) to prevent warnings when getLastErrors() returns falsy value.
Property Enhancements
src/ValueFilter\.php
Added two new protected properties: $validate_locator (type ValidateLocator) and $sanitize_locator (type SanitizeLocator) to expose internal locators.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Configuration updates are straightforward version/deprecation maintenance
  • Type hint addition is non-breaking and well-scoped
  • Error handling improvement is a localized defensive check
  • No complex logic or structural changes requiring deep analysis

Poem

🐰 The rabbit hops through configs, spry,
PHP 8.4 reaches toward the sky!
Type hints now gleam, errors held tight,
New locators exposed to the light.
Scrutinizer's tools find their rest,
While the workflow runs its very best! ✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change in the pull request - adding a return type declaration to the jsonSerialize() method to fix PHP 8.1+ deprecation warnings.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3bfe2eb and 5735005.

📒 Files selected for processing (5)
  • .github/workflows/continuous-integration.yml (2 hunks)
  • .scrutinizer.yml (1 hunks)
  • src/Failure/Failure.php (1 hunks)
  • src/Rule/AbstractDateTime.php (1 hunks)
  • src/ValueFilter.php (1 hunks)
🔇 Additional comments (8)
.github/workflows/continuous-integration.yml (4)

23-24: LGTM! PHP 8.2 and 8.3 added to test matrix.

Good addition to ensure compatibility with recent PHP versions.


39-39: LGTM! Updated to modern GITHUB_OUTPUT approach.

Correctly replaces the deprecated set-output command with the new GITHUB_OUTPUT environment file approach.


42-42: LGTM! Cache action updated to v4.

Appropriate version bump for the actions/cache action.


17-24: Resolve the PHP version support mismatch between CI matrix and composer.json.

The misalignment is confirmed. Your composer.json declares >=5.6.0, but the CI matrix only tests PHP 7.2–8.3, leaving versions 5.6, 7.0, and 7.1 untested despite being officially supported.

Choose one approach:

  1. Expand CI to include PHP 5.6, 7.0, 7.1 (if you intend to support them)
  2. Update composer.json to >=7.2.0 to match actual tested coverage

Without this alignment, older PHP versions may contain untested compatibility issues.

src/Rule/AbstractDateTime.php (1)

51-51: LGTM! Prevents array access on false.

The guard against falsy values before accessing $errors['warnings'] prevents PHP notices when getLastErrors() returns false and maintains the same behavior for invalid dates.

src/ValueFilter.php (1)

24-40: LGTM! Formalized property declarations with documentation.

Adding explicit property declarations with docblocks improves IDE support and code documentation while maintaining existing behavior.

src/Failure/Failure.php (1)

115-115: LGTM! Fixes PHP 8.1+ deprecation warning.

The explicit : array return type declaration resolves the PHP 8.1+ deprecation notice for JsonSerializable::jsonSerialize(). The return type is correct, matches the actual return value, and is fully backward compatible with PHP 7.0+.

.scrutinizer.yml (1)

1-9: LGTM! Updated to PHP 8.4 environment.

The build configuration now uses PHP 8.4 in the default-jammy (Ubuntu 22.04) image, aligning with the broader modernization effort across CI/CD pipelines.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

- Update actions/cache from v2 to v4
- Replace deprecated set-output command with GITHUB_OUTPUT
PHP 5.6 and 7.0 are no longer supported by Composer 2.x.
These versions reached EOL years ago and cannot run modern CI tools.
- PHP 7.1 is not supported by Composer 2.3+ (requires PHP 7.2.5+)
- Add PHP 8.2 and 8.3 to ensure compatibility with recent versions
@koriym koriym marked this pull request as draft November 11, 2025 17:16
- Add return type declaration to jsonSerialize() method (PHP 8.1+)
- Fix DateTime::getLastErrors() null check (PHP 8+)
- Declare properties in ValueFilter to prevent dynamic property warnings (PHP 8.2+)
@koriym koriym marked this pull request as ready for review November 11, 2025 17:40
- Configure PHP 8.4 environment.
- Add "default-jammy" build image and analysis nodes.
- Remove deprecated tools for streamlined analysis.
@koriym
Copy link
Member Author

koriym commented Nov 11, 2025

@coderabbitai review

@coderabbitai
Copy link

coderabbitai bot commented Nov 11, 2025

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@koriym koriym requested a review from harikt November 12, 2025 01:50
*
*/
public function jsonSerialize()
public function jsonSerialize(): array
Copy link
Member

@harikt harikt Nov 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change will be problematic right ?

@harikt
Copy link
Member

harikt commented Nov 12, 2025

It looks we never have released 3.x . May be we need 4.x or something else ?

@koriym
Copy link
Member Author

koriym commented Nov 12, 2025

May be we need 4.x or something else ?

Yes, let's do that!

@koriym koriym closed this Nov 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants