Update screens for output that changed with PHP itself - #5720
Update screens for output that changed with PHP itself#5720KentarouTakeda wants to merge 13 commits into
Conversation
086ead5 to
c043d6b
Compare
Since PHP 8.0 "+" and "-" bind tighter than ".", so the concatenation example no longer prints -1. Its comments were written for the old grouping and stayed behind; the example below it, kept non-interactive, carries that history. Since PHP 7.3 a constant is allowed on the left of instanceof and evaluates to false instead of raising a fatal error; the prose above that example already says so.
Reading a missing array key is a warning rather than a notice since PHP 8.0, so the sentence introducing that example moves with the screen. Since PHP 8.2 the ValueError from BackedEnum::from() no longer quotes the enum name.
Both pages used array_merge() to show that reflection reports no return type for internal functions. Since PHP 8.0 it does report one, so the examples demonstrated nothing and the paragraph warning about built-ins was no longer true. A method with a tentative return type is the case that still reports none, so the examples use one and point at the accessors that return it.
The dump of Exception has drifted across several releases rather than one: property types and defaults are printed differently, tentative return types appeared, and the method count was stale. Regenerated from PHP 8.5 instead of reconstructed.
Since PHP 8.0 an anonymous class is named after the class it extends. The note further down that page still showed the PHP 7 form, captured on 3v4l.org rather than from the example; the NUL byte the name contains is invisible in a screen, so the note now says so. Since PHP 8.2 var_export() writes class names fully qualified, with a leading backslash.
Non-strict detection follows the documented rule since PHP 8.3, so this string now yields ASCII rather than UTF-8. The comment goes with it: it named UTF-8 as the closer match, and the two candidates are in fact equally distant here, so it should not name a winner at all.
The non-strict change landed in 8.3.0, so 8.2 still prints the old result and one screen cannot cover both. The basic example is split too: its screen already showed only the 8.3 behaviour.
| @@ -391,9 +391,13 @@ echo (("x minus one equals " . $x) - 1) . ", or so I hope\n"; | |||
| &example.outputs; | |||
| <screen> | |||
| <![CDATA[ | |||
| -1, or so I hope | |||
| -1, or so I hope | |||
| Fatal error: Uncaught TypeError: Unsupported operand types: string - int | |||
| x minus one equals 3, or so I hope | |||
| x minus one equals 3, or so I hope | |||
|
|
|||
| Fatal error: Uncaught TypeError: Unsupported operand types: string - int in script:10 | |||
| Stack trace: | |||
| #0 {main} | |||
| thrown in script on line 10 | |||
| ]]> | |||
There was a problem hiding this comment.
I have not looked at the rest of the changes in this PR yet. If the behaviour changed, then we need to maintain both outputs. In this case, the example is only applicable to PHP 7 (it looks like the output was incorrect even with PHP 7), but we need to keep the example until PHP 9 is released. So if you want to show that this behaviour changed in PHP 8, first you need to fix the example so that it makes sense for PHP 7, and then add a note that as of PHP 8, this has been fixed and is no longer an issue. You should not change the code comments as they are correct and point out the pitfall. The output should include the warnings too. There should also be a note about the deprecation in PHP 7.4.
The correct output:
Warning: A non-numeric value encountered in /in/mFYgu on line 4
-1, or so I hope
x minus one equals 3, or so I hope
Warning: A non-numeric value encountered in /in/mFYgu on line 10
-1, or so I hope
Although this warning has been there only since PHP 7.1.
We could make the example represent the current state, but that would be incorrect in this case. We do not want to preserve this example going forward. It only exists to warn people of this historical quirk. So in my opinion, the most beneficial is to show what it was like between 7.1 and 7.4 and add a note for 7.4 and 8.0 changes.
There was a problem hiding this comment.
I just saw that there is another example underneath which shows what it was before. I don't think that's right. Both code blocks are the same. This needs to be merged as I said above.
There was a problem hiding this comment.
Merged — the current-behaviour example is gone, and this one keeps its original comments untouched.
The output includes the warnings now. Measured: 7.1, 7.2 and 7.3 all print the two A non-numeric value encountered warnings, and the screen that was here matched PHP 7.0 exactly, the one 7.x that doesn't warn. Hence &example.outputs.71; rather than &example.outputs.7;. The 7.4 deprecation is in a note below, as you asked.
One thing I did differently: the 8.0 change is a second screen rather than only a note, following your "if the behaviour changed, then we need to maintain both outputs". It also shows that the spelled-out grouping now throws, so the third line never runs. Say the word and it goes back to prose.
I kept annotations="non-interactive" on the example — Run code overwrites the first screen in place, caption included, so on a split example the older screen disappears on the first click. More on that in a comment on the PR.
There was a problem hiding this comment.
BTW are doing all of it with Claude? If so it would be nice if you gave it the proper attribution like in 0009ebd
There was a problem hiding this comment.
Yes. I'll add the trailer from here on — thanks for the pointer.
c043d6b to
35185c4
Compare
docbook-cs de7e4fd tightened which elements SimparaSniff accepts, and the sentence this PR rewrites is inline-only.
| bool(false) | ||
| bool(false) | ||
| PHP Fatal error: instanceof expects an object instance, constant given | ||
| bool(false) |
There was a problem hiding this comment.
Ok, but we need to retain the error message. Perhaps split it into two outputs per version? Or just add a code comment on var_dump(FALSE instanceof stdClass); saying it would throw with this error message?
There was a problem hiding this comment.
Took the code comment. A split would mislead here: prior to 7.3 this is a compile-time error, so 7.2 prints the fatal line and nothing else — none of the three bool(false) above it ever run. The screen that was here, three bools followed by the fatal error, was never any version's output.
That's why the comment says "so nothing above ran". The 7.3 boundary itself is already covered by the example directly below, which uses &example.outputs.73;.
|
Following up on "if the behaviour changed, then we need to maintain both outputs" — since you hadn't looked at the rest of the PR yet, I went back over every hunk with one question: was the screen that was there an actual capture of a version the manual still covers? Every answer below is measured, not guessed. Split, because there was a real output to keep
Not split, because the old screen was no version's output
One thing worth knowing about the split pattern Run code replaces the first screen in place, caption included ( It is visible today on language.types.array.php: press Run code and the "PHP 8.0" screen becomes "PHP 8.5.0" carrying the 8.1 output, leaving two identical outputs under different labels. The markup there is fine; it is the runner that assumes one screen per example. Every example I split in this PR is therefore marked |
Follow-up to #5718, from the same audit: the examples where PHP's own behaviour moved and the
<screen>was never updated. One commit per kind of change.+/-vs.precedence (8.0), constants left ofinstanceof(7.3)BackedEnum::from()ValueError no longer quotes the enum name (8.2)var_export()leading backslash (8.2)mb_detect_encoding()non-strict detection (8.3)ReflectionClass::__toString()regenerated rather than attributed to one change; theExceptiondump has drifted across several releasesWhere a corrected screen would leave the page contradicting itself, the prose or comment moves with it: the
foreach"notice" sentence, two comments in the precedence example, themb_detect_encoding()comment, and the anonymous class note. The two reflection examples needed replacing outright — they usedarray_merge()to show that internals report no return type, which it now does, so a method with a tentative return type takes its place.Pages to check with Run code
Press Run code and the output that appears is what this PR puts in the
<screen>. The static one still on these pages is the "before" side of the diff, until the manual is rebuilt.P.S. — after the review on #5718, every example in this PR was re-run on 8.2, 8.3, 8.4 and 8.5 rather than trusted to Run code, which is one wasm build of one version.
That turned up one case:
mb_detect_encoding()'s non-strict change landed in 8.3.0, so 8.2 still prints the old result and a single screen cannot cover both. Both examples on that page are now split with&example.outputs.82;/&example.outputs.83;— the basic example was already showing only the 8.3 behaviour, so it is split too rather than leaving one page inconsistent with itself.Everything else agrees across 8.2–8.5, including the line numbers in the two
Fatal error:screens and theReflectionClass::__toString()dump ofException.