Align declared PHP support with what is tested, and fix PHP 9 fatals - #23
Open
Matt (matt-evervault) wants to merge 4 commits into
Open
Align declared PHP support with what is tested, and fix PHP 9 fatals#23Matt (matt-evervault) wants to merge 4 commits into
Matt (matt-evervault) wants to merge 4 commits into
Conversation
|
Vaultkeeper Commands Mention
You can also request |
🦋 Changeset detectedLatest commit: fae1347 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
The previous commit narrowed CI to PHP 8.4/8.5 but left the manifest, test toolchain and source claiming support for PHP 7. This closes that gap and fixes the deprecations CI was hiding. - composer.json: php ^7.1|^8.0 -> ^8.4, declare ext-gmp directly, and bump phpunit to ^13 (9.6 is EOL and untested on 8.4+). - phpunit.xml: was written against the PHPUnit 10+ schema while the lock pinned 9.6, so every CI run printed a validation warning and silently ignored all four displayDetails* flags. Rebuilt on the 13 schema with bootstrap, a named testsuite, and failOnDirectDeprecation. - CI: setup-php's production ini sets error_reporting to E_ALL & ~E_DEPRECATED, which is why the EvervaultError deprecation never appeared in a green build. Force E_ALL, add gmp, and replace the deprecated ::set-output with $GITHUB_OUTPUT. - EvervaultError: implicitly-nullable $previous is a PHP 9 fatal. Now ?\Throwable, which also matches \Exception::__construct. - EvervaultUtils: parse_url()['host'] warned and passed null into str_ends_with() (a TypeError in PHP 9) for hostless URLs. - Evervault/EvervaultHttp: guard the unconditional $options['async'] and json_decode property reads that warn under PHP 8. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The new error_reporting=E_ALL plus failOnDirectDeprecation caught this on the first run: curl_close() is deprecated as of 8.5 because it has had no effect since 8.0, when curl handles became objects freed by the GC. The 8.4 job passed and 8.5 reported 14 passing tests with 3 deprecations. Removing the calls is a no-op at runtime. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Matt (matt-evervault)
left a comment
Author
There was a problem hiding this comment.
I've reviewed this output from Claude and cleaned up the PR description a bit
Matt (matt-evervault)
marked this pull request as ready for review
August 13, 2026 09:15
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Migrates the SDK to currently-supported PHP versions.
Why each change
composer.json— the declared floor was inaccurate"php": "^7.1|^8.0"had not been true sincestr_ends_with()(PHP 8.0+) was introduced intoEvervaultUtils::isDecryptionDomain(). A PHP 7 consumer could install the package and then hit a fatal at runtime — the worst failure mode, because Composer is the layer that is supposed to prevent exactly that.Raised to
^8.4to match the CI matrix. This is a breaking change for consumers on 8.1–8.3, so a changeset is included (minor, i.e.0.2.0under semver-0 convention).ext-gmpis now declared directly. The SDK's entire crypto path runs through GMP viaparagonie/ecc, but it was only ever satisfied as a transitive requirement — meaning a consumer without GMP would get a confusing error attributed to a dependency instead of a clear platform requirement up front.phpunit/phpunit ^9.0→^13.0— the config and the tool disagreedThis one was actively costing us signal.
phpunit.xmlhad been written against the PHPUnit 10+ schema (all fourdisplayDetailsOnTestsThatTrigger*attributes), but the lockfile pinned 9.6.34, which doesn't understand them. Every CI run printed:So the settings intended to surface deprecations were being silently discarded, in a green build, for however long the mismatch existed. PHPUnit 9 is also EOL and unsupported on 8.4+.
phpunit.xmlis rebuilt on the PHPUnit 13 schema and now also has abootstrap, a namedEndToEndtestsuite, and a<source>block. The<source>block matters: it is what lets PHPUnit tell a deprecation in our code from one in a dependency, which is what makesfailOnDirectDeprecation="true"safe to enable — our own deprecations fail the build, a dependency's are only displayed.No test changes were needed (no data providers, and
setUpBeforeClass(): voidwas already correct).CI was configured to hide the very thing we needed to see
shivammathur/setup-phpdefaults toini-file: production, which setserror_reporting = E_ALL & ~E_DEPRECATED. That is the reason theEvervaultErrordeprecation below never appeared in a passing build despite running on 8.4 and 8.5. Now pinned toerror_reporting=E_ALL.Also in the workflow:
::set-output→$GITHUB_OUTPUT. GitHub deprecated the workflow-command form and it warns on every run.gmptoextensions, matching the new manifest requirement rather than relying on it being present by default.phpunit-versions: ['latest']matrix axis. It fed into nothing — dependencies come from the lockfile — so it only put a misleading "latest" in job names while actually running whatever the lock pinned.--testsuite EndToEndinstead of a bare path, so the suite definition lives in one place.Two latent PHP 9 fatals
Both are deprecations on 8.4 and hard errors in PHP 9:
EvervaultError::__construct()—\Exception $previous = nullis an implicitly-nullable parameter. Now?\Throwable $previous = null, which additionally fixes a real narrowing bug: the signature rejected\Errorand other non-Exceptionthrowables that its own parent\Exception::__construct()accepts, sonew EvervaultError($msg, 0, $someError)wouldTypeError.EvervaultUtils::isDecryptionDomain()—parse_url($domain)['host']emittedUndefined array key "host"for a URL without a host and then passednullintostr_ends_with(), which is deprecated now and aTypeErrorin PHP 9. Replaced withparse_url($domain, PHP_URL_HOST)plus an explicit early return. Wildcard matching, exact matching and non-matching all verified unchanged.Three PHP 8 warning-severity reads
These were notices in PHP 7 and are warnings in PHP 8; with
failOnWarningnow on, they would become test failures, so they are fixed rather than left to bite later.Evervault::run()read$options['async']unconditionally. The parameter default is['version' => null, 'async' => false], but a caller passing['version' => 2]replaces the default array wholesale, soasyncgoes missing and the read warns. Now!empty().EvervaultHttp::_handleApiResponse()read->codeand->detailoffjson_decode()without checking it succeeded — a non-JSON or empty 403 body (a gateway error page, say) producedAttempt to read property on null. Now guarded on both fields, so a malformed body degrades to the generic permissions message instead of warning. Verified against a local server returning real 403s: a well-formed body still yields the exactdetailstring thattestEncrytWithDataRoleForbiddingDecryptionasserts.EvervaultCrypto::_createV2Aad()threw an unqualifiedExceptionfrom insidenamespace Evervault, which resolves to the non-existentEvervault\Exception— so if that branch were ever reached it would raiseError: Class "Evervault\Exception" not foundinstead of the intended error. NowEvervaultError. (The branch is currently unreachable, since$versionNumberis a literal1; left in place rather than deleted.)curl_close()in the tests — found by this PR's own changeWorth calling out, because it is the justification for the whole exercise. The moment
error_reporting=E_ALLandfailOnDirectDeprecationwent in, CI went red on its first run with something nobody had seen before:curl_close()has been a no-op since PHP 8.0, when curl handles became objects freed by the garbage collector, and 8.5 deprecates it outright. Three test helpers were still calling it. The 8.4 job passed and only 8.5 flagged it — this is exactly the class of 8.5-and-later problem the previous configuration was structurally incapable of reporting, since it ran on 8.5 withE_DEPRECATEDmasked and a config file the test runner rejected. Removing the calls is a runtime no-op.Housekeeping
flake.nixgainsgmpexplicitly — it worked only because nixpkgs enables it in the default extension set..gitignoregains.phpunit.cache, the cache directory PHPUnit 13 is now configured to use.Reviewer notes
^8.4floor is the one judgement call here. It is a breaking change for consumers on 8.1–8.3. The alternative was^8.2(everything still receiving security fixes as of Aug 2026) with a dualphpunit ^11 || ^13constraint and a four-version matrix.^8.4was chosen to match the matrix this branch already committed to; say the word and I'll widen it.isDecryptionDomainand theEvervaultCryptoexception class were not in the original triage list — I picked them up because enablingfailOnWarning/failOnDirectDeprecationwhile leaving known warning paths in the code would be inconsistent. Easy to drop if you'd rather keep this PR narrower.failOnNotice/failOnWarningdo raise the strictness of a suite that talks to a live API. If that turns out to be flaky in practice, those two are the dials to turn down —failOnDirectDeprecationis the one carrying the PHP 9 value.🤖 Generated with Claude Code