Modernise package for PHP 8.2+#150
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Modernizes the library for PHP 8 by adding strict typing across the codebase, updating tooling/configuration, and adjusting tests/documentation accordingly.
Changes:
- Raised minimum PHP version to 8.0 and added PHPStan config/baseline wiring.
- Added type declarations and return types across Captcha/Phrase builders and image handling.
- Updated PHPUnit config and adjusted tests/docs plus ignored generated artifacts.
Reviewed changes
Copilot reviewed 11 out of 13 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/CaptchaBuilderTest.php | Updates tests for typed properties/return types; adjusts output file assertions. |
| src/Gregwar/Captcha/PhraseBuilderInterface.php | Adds strict parameter/return types to the phrase builder contract. |
| src/Gregwar/Captcha/PhraseBuilder.php | Implements typed properties and typed method signatures. |
| src/Gregwar/Captcha/ImageFileHandler.php | Adds typed properties/methods and GD image typing; changes GC logic. |
| src/Gregwar/Captcha/CaptchaBuilderInterface.php | Adds typed, modern method signatures and doc annotations. |
| src/Gregwar/Captcha/CaptchaBuilder.php | Broad typing refactor; adds temp dir usage and OCR helper changes. |
| phpunit.xml.dist | Updates PHPUnit config to modern schema/coverage config. |
| phpstan.neon.dist | Removes old distributed PHPStan config. |
| phpstan.neon | Adds new PHPStan config (level 8 + baseline include). |
| composer.json | Raises PHP requirement and adds PHPStan dev dependency. |
| README.md | Updates examples/wording to match modern PHP syntax. |
| .gitignore | Ignores test-generated image outputs and tool caches/lockfile. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Php8.0 issues
* tempp always exists here * col is always an int * add match for createBackgroundImageFromType
What do you mean? Link? |
In phpstorm it mentioned php 8.0 when i was updating the code, i'll need to look it up again, but will confirm |
https://php.watch/versions/8.0/gdimage Here we go, what i mean is that gdimage replaces the resource type. |
bytestream
left a comment
There was a problem hiding this comment.
Thanks for your efforts.
|
@lachlan-00 just need to fix those PHPstan errors then I'm happy to merge 👍🏼 |
* read function value min max checking * Distort width and height check explicit int<> typing
No problem @bytestream last commit 9d4bd7e addresses 8.5 issues.
Let me know if you need anything else! |
|
Thanks for persevering with this @lachlan-00. Your efforts are greatly appreciated 👍🏼 |
|
no problem again! I wasn't expecting to do much but i think it's a useful library to keep updated and Ampache will start using it to replace our archaic captcha that is VERY machine readable. |
The current code is referencing php8.0 GdImage so I've raised it from 5.3 -> 8.2
static analysis and tests complete successfully.
composer.json, and addedphpstan/phpstanas a dev dependency for static analysis.ImageFileHandlerclassphpstan.neon.distconfiguration with higher analysis levelphpunit.xml.distto use the latest schema and improved code coverage configuration.CaptchaBuilderto use strict property and method typing, PHP 8+ features (union types, constructor property promotion), and improved null safety.README.md.