Modernize for PHP 8.1+ and PHPUnit 11#2
Conversation
Drop PHP 7.x support, require PHP 8.1+. Update PHPUnit from ^7 to ^11 (fixes CVE-2026-24765 Dependabot alert). Modernize source with typed constants and trailing commas. Rewrite tests with data providers and proper assertions. Add phpunit.xml.dist, CI workflow for PRs, and Packagist badges to README. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR modernizes the library for newer PHP/PHPUnit versions by updating platform constraints, refreshing the implementation and test suite, and adding CI + PHPUnit configuration.
Changes:
- Bump requirements to PHP 8.1+ (plus
ext-mbstring) and upgrade PHPUnit to^11. - Modernize
StripAccents+ expand/rewrite tests using PHPUnit attributes and more explicit assertions. - Add PHPUnit config (
phpunit.xml.dist) and a GitHub Actions CI workflow to run tests across multiple PHP versions.
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
src/StripAccents.php |
Modernized implementation and constants; reformatted stripNonPrint() signature. |
tests/StripAccentsTest.php |
Rewritten tests with a data provider + added coverage for edge cases/ligatures/stripNonPrint(). |
phpunit.xml.dist |
New PHPUnit 11 configuration (bootstrap, cache dir, source include). |
composer.json |
Drops PHP 7.x, requires PHP 8.1+ and ext-mbstring, upgrades PHPUnit dev requirement. |
README.md |
Updated docs with badges and modern examples (incl. named args). |
.gitignore |
Ignores PHPUnit cache directory. |
.github/workflows/ci.yml |
Adds CI job running PHPUnit in a PHP version matrix. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // "ABC AEY " | ||
|
|
||
| // You can specify any encoding supported by htmlentities() as a second parameter | ||
| echo StripAccents::strip("A strïng with àccénts", "iso-8859-1"); | ||
| echo StripAccents::stripNonPrint("ABC ÀÈÝ 是我这", replaceWith: "-"); | ||
| // "ABC AEY ---" |
There was a problem hiding this comment.
The example output for stripNonPrint(..., replaceWith: "-") appears incorrect for the current implementation: the regex runs byte-wise (no /u modifier), so each 3-byte CJK character becomes 3 replacements (the tests assert 9 - total). Update the README example to match actual behavior, or clarify/document a character-wise replacement approach if that’s the intent.
| branches: [master] | ||
| pull_request: | ||
| branches: [master] |
There was a problem hiding this comment.
The workflow is restricted to branches: [master]. If the default branch is main (as suggested by the PR diff base), CI won’t run for pushes/PRs to the default branch. Consider switching this filter to the actual default branch name or removing the branch filter to ensure CI runs on all PRs.
| branches: [master] | |
| pull_request: | |
| branches: [master] | |
| branches: [main] | |
| pull_request: | |
| branches: [main] |
|
|
||
| strategy: | ||
| matrix: | ||
| php-version: ['8.1', '8.2', '8.3', '8.4'] |
There was a problem hiding this comment.
CI runs PHP 8.1/8.2, but the code in this PR uses typed class constants (PHP 8.3+). Either drop 8.1/8.2 from the matrix or adjust the code/composer constraints so the matrix matches the supported PHP versions.
| php-version: ['8.1', '8.2', '8.3', '8.4'] | |
| php-version: ['8.3', '8.4'] |
| composer require codeinc/strip-accents | ||
| ``` | ||
|
|
||
| Requires PHP 8.1 or later. |
There was a problem hiding this comment.
README requirement statement is incomplete/inconsistent with composer.json: the package now requires ext-mbstring, but the README only mentions the PHP version. Also, if typed class constants remain, the minimum PHP version becomes 8.3+. Please align the README requirements with the actual composer/platform constraints.
| Requires PHP 8.1 or later. | |
| Requires PHP 8.3 or later with the `mbstring` extension enabled. |
| class StripAccents | ||
| { | ||
| public const DEFAULT_ENCODING = 'utf-8'; | ||
| private const string DEFAULT_ENCODING = 'utf-8'; |
There was a problem hiding this comment.
private const string DEFAULT_ENCODING uses typed class constants, which are not supported in PHP 8.1/8.2. With the current composer.json requirement (php >=8.1) and CI matrix including 8.1/8.2, this will cause a parse error on those versions. Either remove the constant type declarations, or bump the minimum PHP version (and update README/CI accordingly).
| private const string DEFAULT_ENCODING = 'utf-8'; | |
| private const DEFAULT_ENCODING = 'utf-8'; |
| 'I', 'O', 'O', 'O', 'O', 'O', 'U', 'U', 'U', 'U', 'Y', 'a', 'a', 'a', | ||
| 'a', 'a', 'a', 'c', 'e', 'e', 'e', 'e', 'i', 'i', 'i', 'i', 'e', 'o', | ||
| 'o', 'o', 'o', 'o', 'u', 'u', 'u', 'u', 'y', 'y' | ||
| private const array ACCENT_MAP = [ |
There was a problem hiding this comment.
private const array ACCENT_MAP uses typed class constants, which are not supported in PHP 8.1/8.2. Since the PR targets PHP 8.1+ (and CI tests 8.1/8.2), this will make the test suite fail to even parse on those versions. Either remove the type from the constant or raise the project’s minimum PHP version and align CI/README.
| private const array ACCENT_MAP = [ | |
| private const ACCENT_MAP = [ |
Remove typed class constants (PHP 8.3+ only) to maintain PHP 8.1+ compatibility. Document mbstring requirement and correct stripNonPrint example output in README. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
PHPUnit 11 requires PHP 8.2+, so drop 8.1 from the matrix and bump the composer constraint accordingly. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Keep backward compatibility with existing consumers that may reference StripAccents::DEFAULT_ENCODING directly. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
ext-mbstring^7to^11(resolves Dependabot alert #1 — CVE-2026-24765)#[DataProvider]attributes,assertSameassertions, and full coverage ofstrip(),stripNonPrint(), ligatures, edge casesphpunit.xml.distconfigurationTest plan
🤖 Generated with Claude Code