Bumps min PHP to ^8.3#59
Conversation
Resolves #53
WalkthroughThe codebase was modernized to require PHP 8.3 or higher, updating composer constraints, test workflows, and code to utilize new PHP features such as enums, typed properties, and strict typing. The ErrorsCollection class was refactored to use modern interfaces and serialization, and tests and traits were updated for compatibility and clarity. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ErrorsCollection
participant ParamsMode
User->>ErrorsCollection: new ErrorsCollection(mode: ParamsMode)
User->>ErrorsCollection: add(error data)
ErrorsCollection->>ParamsMode: Check mode for error handling
User->>ErrorsCollection: toJson()
ErrorsCollection->>User: JSON string (throws JsonException on failure)
Assessment against linked issues
Possibly related issues
Poem
✨ 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 (
|
![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)
tests/ErrorsTraitTest.php (1)
62-62: Fix copy-paste error in method documentation.The comment says "must return array" but the method should return a string.
- 'getErrors without param must return array' + 'getHtmlErrors should return string'
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.github/workflows/on-pull-request.yml(1 hunks)composer.json(1 hunks)src/ErrorsCollection.php(1 hunks)src/ErrorsTrait.php(2 hunks)src/ParamsMode.php(1 hunks)tests/ErrorsCollectionTest.php(4 hunks)tests/ErrorsTraitTest.php(3 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (4)
tests/ErrorsTraitTest.php (2)
src/ErrorsTrait.php (4)
__construct(20-23)getErrors(32-35)getHtmlErrors(50-53)hasError(58-61)src/ErrorsCollection.php (1)
__construct(26-30)
src/ErrorsTrait.php (1)
src/ErrorsCollection.php (3)
ErrorsCollection(14-201)toArray(186-189)getHtml(101-106)
src/ErrorsCollection.php (1)
src/UnsetErrorException.php (1)
UnsetErrorException(7-10)
tests/ErrorsCollectionTest.php (2)
src/ErrorsCollection.php (10)
ErrorsCollection(14-201)add(125-150)offsetGet(50-53)offsetUnset(73-76)isEmpty(83-86)clear(91-94)getHtml(101-106)count(155-158)toArray(186-189)toJson(196-199)src/UnsetErrorException.php (1)
UnsetErrorException(7-10)
🪛 PHPMD (2.15.0)
src/ErrorsTrait.php
42-42: Avoid unused parameters such as '$err_str'. (Unused Code Rules)
(UnusedFormalParameter)
src/ErrorsCollection.php
73-73: Avoid unused parameters such as '$offset'. (Unused Code Rules)
(UnusedFormalParameter)
🔇 Additional comments (16)
composer.json (2)
16-16: PHP version bump looks good and aligns with modernization goals.The update from PHP 5.6 to 8.3 is a significant modernization step that enables the use of modern PHP features like enums, typed properties, and strict typing seen throughout the codebase.
30-34: Good addition of autoload-dev section for test namespace.The PSR-4 autoloading configuration for tests aligns well with the namespace changes in the test files.
.github/workflows/on-pull-request.yml (1)
15-16: Workflow update aligns well with the PHP version bump.Testing both PHP 8.3 (minimum required) and 8.4 (latest stable) provides appropriate coverage for the modernized codebase.
src/ParamsMode.php (1)
1-25: Excellent modernization using PHP 8.1+ enums.The
ParamsModeenum is a great replacement for what were likely integer constants or magic numbers. The strict typing declaration, integer backing, and clear documentation make this a solid implementation that leverages modern PHP features.src/ErrorsTrait.php (3)
15-15: Good addition of strict typing to the errors property.The typed property declaration
ErrorsCollection $errorsimproves type safety and aligns with modern PHP practices.
32-34: Approve the improved method signature with union return type.The addition of parameter and return type hints improves code clarity and type safety. The parameter name change from
$ashtmlto$asHTMLfollows proper camelCase convention.
50-50: Good addition of return type declarations.The explicit return type declarations for
getHtmlErrors(): stringandhasError(): boolimprove type safety and code clarity.Also applies to: 58-58
tests/ErrorsTraitTest.php (3)
3-3: Namespace update aligns with autoload-dev configuration.The namespace change from
Imponeer\Tests\ObjectErrorstoImponeer\ObjectErrors\Testsis consistent with the PSR-4 autoloading configuration added to composer.json.
25-25: Good addition of void return types to test methods.The explicit
voidreturn type declarations modernize the test code and improve type safety.Also applies to: 56-56, 66-66
29-53: Good removal of PHPUnit compatibility checks.Removing the conditional checks for
assertIsStringandassertIsBoolmethods simplifies the code since PHPUnit versions compatible with PHP 8.3+ will have these methods available.src/ErrorsCollection.php (4)
7-14: Modern interface implementation looks good.The replacement of
SerializablewithStringableand the addition ofJsonExceptionimport align well with PHP 8.3+ best practices.
26-30: Excellent use of modern PHP features.The constructor property promotion with readonly modifier is a clean approach for immutable configuration.
73-73: PHPMD warning can be safely ignored.The
$offsetparameter is required by theArrayAccessinterface. Theneverreturn type correctly indicates this method always throws an exception.
196-199: Good improvement in error handling.Using
JSON_THROW_ON_ERRORflag makes JSON encoding failures explicit, which is much better than silent failures.tests/ErrorsCollectionTest.php (2)
148-175: Excellent modernization of data providers.Using
Generatorwith descriptive yield keys and leveragingParamsMode::cases()makes the tests more maintainable and readable.Also applies to: 207-214
15-205: Test modernization is comprehensive and well-executed.The addition of strict typing, use of the
ParamsModeenum, and improved assertions make the tests more robust and maintainable.
| ], | ||
| "type": "library", | ||
| "require-dev": { | ||
| "phpunit/phpunit": "^5.2|^7.0|^8.0", |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Update PHPUnit version constraint for PHP 8.3+ compatibility.
The current PHPUnit constraint ^5.2|^7.0|^8.0 includes very old versions that may not be compatible with PHP 8.3. PHPUnit 5.2 was released in 2016 and doesn't support PHP 8.3.
Update the PHPUnit constraint to support modern PHP versions:
- "phpunit/phpunit": "^5.2|^7.0|^8.0",
+ "phpunit/phpunit": "^9.5|^10.0|^11.0",Let me verify the PHPUnit version compatibility:
🌐 Web query:
What are the PHP version requirements for PHPUnit versions 5.2, 7.0, 8.0, 9.5, 10.0, and 11.0?
💡 Result:
Here are the PHP version requirements for the specified PHPUnit versions:
| PHPUnit Version | PHP Compatibility |
|---|---|
| 5.2 | PHP 5.6 - PHP 7.1 |
| 7.0 | PHP 7.1 - PHP 7.3 |
| 8.0 | PHP 7.2 - PHP 7.4 |
| 9.5 | PHP 7.3 - PHP 8.0 |
| 10.0 | PHP 8.1 or later |
| 11.0 | PHP 8.2 or later |
For detailed information on supported versions, you can refer to the official PHPUnit supported versions page. (phpunit.de)
Citations:
Update PHPUnit constraint for PHP 8.3+ compatibility
The existing constraint
"phpunit/phpunit": "^5.2|^7.0|^8.0",only covers up to PHP 8.0 and includes EOL PHPUnit versions. To ensure coverage on PHP 8.3 (and modern PHP releases) while still testing on PHP 7.3/7.4 if needed, bump to:
File: composer.json
Line: 11
- "phpunit/phpunit": "^5.2|^7.0|^8.0",
+ "phpunit/phpunit": "^9.5|^10.0|^11.0",- ^9.5 → supports PHP 7.3–8.0
- ^10.0 → supports PHP 8.1+
- ^11.0 → supports PHP 8.2+ (and therefore PHP 8.3)
🤖 Prompt for AI Agents
In composer.json at line 11, update the PHPUnit version constraint from
"^5.2|^7.0|^8.0" to include modern supported versions for PHP 8.3 and above.
Replace it with "^9.5|^10.0|^11.0" to cover PHPUnit versions compatible with PHP
7.3 through PHP 8.3+, ensuring testing compatibility with current and future PHP
releases.
| public function setErrors(string $err_str): void | ||
| { | ||
| call_user_func_array([$this->errors, 'add'], func_get_args()); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix misleading parameter declaration in setErrors method.
The method signature declares string $err_str but the implementation uses func_get_args() to accept multiple arguments, making the parameter declaration misleading.
Consider using variadic parameters for clarity:
- public function setErrors(string $err_str): void
+ public function setErrors(string ...$err_strs): void
{
- call_user_func_array([$this->errors, 'add'], func_get_args());
+ call_user_func_array([$this->errors, 'add'], $err_strs);
}Or even better, use the spread operator:
- public function setErrors(string $err_str): void
+ public function setErrors(string ...$err_strs): void
{
- call_user_func_array([$this->errors, 'add'], func_get_args());
+ $this->errors->add(...$err_strs);
}🧰 Tools
🪛 PHPMD (2.15.0)
42-42: Avoid unused parameters such as '$err_str'. (Unused Code Rules)
(UnusedFormalParameter)
🤖 Prompt for AI Agents
In src/ErrorsTrait.php around lines 42 to 45, the setErrors method declares a
single string parameter but internally accepts multiple arguments using
func_get_args(), causing a mismatch. Change the method signature to use a
variadic parameter (e.g., setErrors(string ...$err_str): void) to clearly
indicate it accepts multiple string arguments, and update the implementation to
use the variadic parameter directly instead of func_get_args().
| if (isset($err_data[1]) && ($err_data[1] !== false)) { | ||
| $err_data[0] = sprintf("[%s] %s", $err_data[1], $err_data[0]); | ||
| } | ||
| $this->errors[] = $err_data[0]; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add string cast for consistency.
Other modes cast error data to string, but Mode2AsPrefix doesn't. For consistency and type safety, consider casting here as well.
- $this->errors[] = $err_data[0];
+ $this->errors[] = (string) $err_data[0];📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| $this->errors[] = $err_data[0]; | |
| $this->errors[] = (string) $err_data[0]; |
🤖 Prompt for AI Agents
In src/ErrorsCollection.php at line 144, the error data added to the errors
array is not cast to a string, unlike other modes. To maintain consistency and
type safety, cast $err_data[0] to a string before adding it to $this->errors by
explicitly converting it using (string).
1 participant
Resolves #53
Summary by CodeRabbit
New Features
Refactor
Chores