Default $extgstates to an array so the writers can count it (mirrors mpdf/mpdf#2135) - #20
Merged
Merged
Conversation
The property was declared without a default, so it is null on any Mpdf whose constructor has not run. ResourceWriter and BackgroundWriter both reach for it with count(), which is a TypeError on PHP 8. Mirrors mpdf#2135, less its SvgTest rewrite: the deprecation that PR is named after comes from a null $gs subscript, not from $extgstates, and upstream has since fixed that cause in 2dc0381, which gravitypdf already carries. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
Summary
This mirrors mpdf/mpdf#2135, so the fork carries the change while the upstream PR sits open.
$extgstateswas declared with no default, so it isnullon anyMpdfwhose constructor has not run — and bothResourceWriterandBackgroundWriterreach for it withcount(), which is aTypeErroron PHP 8. Giving it the[]every other collection property already has closes that off.That is the whole of the change. The upstream PR pairs it with a rewrite of
SvgTest'sset_up(), which is not carried over: the deprecation named in its title comes from somewhere else entirely, and upstream has since fixed that cause directly. See the note below.Try it
Before:
TypeError: count(): Argument #1 ($value) must be of type Countable|array, null given.After:
int(0).Test plan
testExtgstatesDefaultsToAnArrayintests/Mpdf/ConfigurationTest.phpasserts the property default directly, on an instance built withnewInstanceWithoutConstructor(). It fails ongravitypdf(Failed asserting that null is identical to Array &0 ()) and passes here.composer test— 1063 tests, 2559 assertions, up from 1062/2558.composer csclean.More info — why the SvgTest half of [mpdf#2135](https://github.com/mpdf/pull/2135) is not mirrored
Upstream 2135 is titled after the PHP 8.1+ notice
Using null as an array offset is deprecated, use an empty string instead, raised fromSvg.phpwhere it writes$this->mpdf->extgstates[$gs]['fo'] = true.Adding
= []to$extgstatesdoes not silence that notice. The null in question is$gs, not$extgstates— auto-vivifying an array out of a null property has never been deprecated, but using a null subscript has:$gscame back null becauseSvgTestbuilt itsMpdfasMockery::mock(Mpdf::class)->shouldIgnoreMissing(), so theAddExtGState()call inSvg::ImageSVG()returned null rather than a state number. The upstream PR works around that by swapping the mock for a realnew \Mpdf\Mpdf().Upstream then fixed the same thing at the source in
2dc0381("Fix SvgTest on PHP 8.5 by mock definition"), which teaches the mock to answer that one call:That commit is already in
gravitypdf, so the notice does not occur here, and replacing the mock now would only make every SVG test build a fullMpdffor no gain.var $extgstates = []SvgTestuses a realMpdfAddExtGStatestubbed per2dc0381testExtgstatesDefaultsToAnArray