Skip to content

Fix PHP 8.5 Deprecations/Warnings and Modernize CI (Keeps PHP 7.3)#161

Open
jonathanmaron wants to merge 3 commits into
DragonBe:masterfrom
jonathanmaron:php85
Open

Fix PHP 8.5 Deprecations/Warnings and Modernize CI (Keeps PHP 7.3)#161
jonathanmaron wants to merge 3 commits into
DragonBe:masterfrom
jonathanmaron:php85

Conversation

@jonathanmaron

Copy link
Copy Markdown
Contributor

Makes the package run cleanly on PHP 8.5 without dropping PHP 7.3 support (composer.json unchanged). No public API or behavior change for valid input.

  • ValidatorIE: replace $checkChar++ (increment on non-numeric string, deprecated in PHP 8.3) with chr(ord($checkChar) + 1).
  • ViesTest: guard ReflectionMethod::setAccessible() (deprecated in 8.5, no-op since 8.1) behind PHP_VERSION_ID < 80100 — still required on < 8.1.
  • ValidatorSK: add ! ctype_digit($vatNumber) so malformed input returns false before the modulo instead of emitting a PHP 8 warning (+ regression test).
  • CI: PHP 7.4–8.5 matrix plus a non-blocking 8.6 leg; actions/*@v5; per-PHP-version cache key; fail-fast: false.

Verified: OK (252 tests, 450 assertions), all actions are green, zero deprecations/warnings from src/ or tests/ on 8.5; PHPCompatibility 7.3-8.5 and PSR-12 clean.

ValidatorIE mapped a checksum value to a letter by incrementing the string
'A' in a loop; incrementing a non-numeric string is deprecated as of PHP 8.3
(str_increment() is the suggested replacement but only exists on 8.3+).
Replace the increment with chr(ord($checkChar) + 1), which is identical for
the A-V range reached here and valid on PHP 7.3.

ViesTest called ReflectionMethod::setAccessible(true), which has had no effect
since PHP 8.1 and is deprecated in 8.5, but is still required on PHP < 8.1 to
invoke the private addOptionalArguments() via reflection. Guard each of the
five calls behind PHP_VERSION_ID < 80100 so it runs only where it has an
effect.

No language feature newer than 7.3 is used and composer.json's "php": ">=7.3"
is unchanged.
ValidatorSK::validate() gated only on length, first digit and third digit,
then ran $vatNumber % 11 (and bcmod) on the raw string. A 10-char input that
passed the gate but contained a non-digit (e.g. '222222222A') reached the
modulo and triggered 'Warning: A non-numeric value encountered' on PHP 8.x,
which breaks callers that escalate warnings to exceptions.

Add ! ctype_digit($vatNumber) to the gate so non-numeric input returns false
before any arithmetic. ctype_digit() is available on all supported PHP
versions and ext-ctype is already required; valid all-digit numbers are
unaffected, so there is no behavior change for valid input. Add a regression
test asserting the malformed input is invalid and raises no warning.
- Expand the test matrix to PHP 7.4-8.5, plus a non-blocking experimental 8.6
  leg (continue-on-error).
- Bump actions/checkout and actions/cache to v5 (Node 24): v2 is auto-failed
  by GitHub and v4 runs on the deprecated Node 20 runtime.
- Fix the Composer cache key. It hashed composer.lock, which is gitignored and
  absent in CI, so the key collapsed to a constant shared across every PHP
  version and one vendor/ was reused for all of them (crashing quality steps).
  Make the key per-PHP-version and hash the tracked composer.json instead.
- Add fail-fast: false so each PHP version reports independently.
@jonathanmaron

Copy link
Copy Markdown
Contributor Author

Hi Michelangelo,

I hope you're doing well! I wanted to reach out to you about pull request #161 (#161).

I recently migrated several web applications to PHP 8.5, and this is the last package for which I've had to maintain a fork to avoid deprecation notices. The changes in this PR are minimal. Merging it would allow me to drop my fork and depend on the upstream package again.

When you have a moment, would you mind taking a look? I'd really appreciate it.

Thank you so much for maintaining this library!

Best regards,

Jonathan

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.

1 participant