✨ v6 #47
✨ v6 #47
Conversation
There was a problem hiding this comment.
Pull request overview
This PR represents a major version upgrade (v6) with significant architectural changes including PHP 8.2+ requirement, readonly class adoption, dependency updates, and improved error messaging.
Key Changes:
- Upgraded minimum PHP version from 8.1 to 8.2 and updated dependencies (firebase/php-jwt 6→7, phpstan 1→2)
- Migrated most classes to readonly classes with simplified property declarations
- Added new
Time\Unitenum andCompany\Euidclass with comprehensive test coverage - Improved error messages from French to English with more descriptive text
Reviewed changes
Copilot reviewed 51 out of 51 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| composer.json | Updated PHP requirement to 8.2+ and upgraded firebase/php-jwt to v7, phpstan to v2 |
| phpunit.xml | Modernized configuration with XML declaration, cache directory, and updated coverage syntax |
| component/Abstract/Value/*.php | Migrated to readonly classes with property promotion |
| component/Time/Duration.php | Refactored to use new Unit enum, removed label constants |
| component/Time/Date*.php | Added readonly class declarations |
| component/Time/Unit.php | New enum for time unit labels (singular/plural) |
| component/Company/Euid.php | New class for European Unique Identifier with validation |
| component/Company/Fr/*.php | Updated parameter names from French to English, added readonly |
| component/Money/Price.php | Renamed parameter from 'espaceInsecable' to 'nonBreakingSpace' |
| component/Web/*.php | Migrated to readonly classes where appropriate |
| component/Key/Ssl.php | Added readonly to final class |
| component/Geography/*.php | Added readonly class declarations |
| component/Person/*.php | Added readonly class declarations |
| component/Number/*.php | Added readonly class declarations |
| component/Token/*.php | Added readonly class declarations |
| component/Color/Hexadecimal.php | Removed unused import, added readonly |
| test/Token/JwtTest.php | Updated RSA keys to 2048-bit for improved security |
| test/Key/SslTest.php | Added error handler suppression for expected OpenSSL warnings |
| test/Company/EuidTest.php | Comprehensive test suite for new Euid class |
| test/Time/UnitTest.php | Test suite for new Unit enum |
| test/Abstract/Fixture/Value/*.php | Updated test fixtures to readonly classes |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| readonly class Date | ||
| { | ||
| public readonly int $time; | ||
| public readonly string $date; | ||
| public readonly string $format; | ||
| public int $time; | ||
| public string $date; | ||
| public string $format; |
There was a problem hiding this comment.
The class is declared as readonly, but properties 'time', 'date', and 'format' need to be assigned in the constructor body (lines 33-35 which are outside the diff region). This is incompatible with readonly classes in PHP 8.2+ where properties can only be initialized via promoted constructor parameters or at declaration. Consider removing the readonly keyword from the class declaration or refactoring the initialization logic.
| $result = $this->fixtureInvalid->encrypt('Foo bar'); | ||
| // Set error handler to suppress expected openssl warnings | ||
| set_error_handler(function ($severity, $message, $filename, $lineno) { | ||
| if (strpos($message, 'openssl_private_encrypt') !== false) { |
There was a problem hiding this comment.
Using strpos() for string matching is deprecated in PHP 8+. Consider using str_contains() instead for better readability and performance.
| $result = $this->fixtureInvalid->encrypt(''); | ||
| // Set error handler to suppress expected openssl warnings | ||
| set_error_handler(function ($severity, $message, $filename, $lineno) { | ||
| if (strpos($message, 'openssl_private_encrypt') !== false) { |
There was a problem hiding this comment.
Using strpos() for string matching is deprecated in PHP 8+. Consider using str_contains() instead for better readability and performance.
| ); | ||
| // Set error handler to suppress expected openssl warnings | ||
| set_error_handler(function ($severity, $message, $filename, $lineno) { | ||
| if (strpos($message, 'openssl_public_decrypt') !== false) { |
There was a problem hiding this comment.
Using strpos() for string matching is deprecated in PHP 8+. Consider using str_contains() instead for better readability and performance.
| public function __toString( | ||
| ): string { |
There was a problem hiding this comment.
The opening brace of the __toString method should be on the same line as the function signature for consistency with PHP coding standards. The current formatting places it on a new line, which is inconsistent with typical PSR-12 formatting.
| public readonly int $value | ||
| public int $value | ||
| ) { | ||
| $this->label = $this->buildLabel(); |
There was a problem hiding this comment.
The 'label' property is being assigned in the constructor, which is not allowed for readonly classes in PHP 8.2+. Readonly class properties can only be initialized once via promoted constructor parameters or direct assignment at declaration. Properties that need to be computed cannot be set in the constructor body. This will cause a runtime error.
| throw new NotCompliant('Invalid date interval: from date (' . $from . ') cannot be after to date (' . $to . ')'); | ||
| } | ||
|
|
||
| $this->duration = ($this->from && $this->to) ? new Duration(($this->to->time + Duration::DAY - 1) - $this->from->time) : null; |
There was a problem hiding this comment.
The 'duration' property is being assigned in the constructor body, which is not allowed for readonly classes in PHP 8.2+. Readonly class properties can only be initialized via promoted constructor parameters or at declaration. This will cause a runtime error.
| throw new NotCompliant('Invalid datetime interval: from datetime (' . $from . ') cannot be after to datetime (' . $to . ')'); | ||
| } | ||
|
|
||
| $this->duration = ($this->from && $this->to) ? new Duration($this->to->time - $this->from->time) : null; |
There was a problem hiding this comment.
The 'duration' property is being assigned in the constructor body, which is not allowed for readonly classes in PHP 8.2+. Readonly class properties can only be initialized via promoted constructor parameters or at declaration. This will cause a runtime error.
Co-authored-by: Copilot <[email protected]>
No description provided.