Repository navigation
Conversation
…rinter pStmts() always inserted a single newline between statements, even when $this->origTokens was available. This method is used as a plain, non-preserving fallback whenever a containing node lacks an 'origNode' attribute (e.g. because an unrelated ancestor node was replaced), so any blank line that used to separate two still-unmodified statements was silently dropped, even though each statement's own content was still being format-preserved individually. This is exactly the case reported in nikic#803: wrapping an existing class declaration into `return new class extends X { ... };` causes the blank line between the class's own methods to disappear, because the class body ends up printed via pStmts() instead of going through the list format-preservation logic in pArray(). Fix pStmts() to check, for each pair of consecutive statements that still carry a valid 'origNode' attribute, whether the original source between them was pure whitespace containing a blank line -- and if so, re-insert it. If anything else (another statement, inline HTML, a comment) used to sit between them, the old single-newline behavior is kept, since a raw line-number gap alone doesn't tell us a blank line was actually there. Adds a regression test reproducing the exact scenario from nikic#803. Fixes nikic#803 Signed-off-by: Eljees <3.14hell@gmail.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.
Fixes #803.
pStmts()is used as the plain (non-format-preserving) fallback printer for astatement list whenever the containing node doesn't have a usable
origNodeattribute — for example when an unrelated ancestor node was replaced, as in
the reproduction from #803 (wrapping an existing class into
return new class extends X { ... };). It always inserted a single newlinebetween statements, even when
$this->origTokenswas available and theindividual statements themselves still carried valid
origNodeattributes(so their own bodies were still being format-preserved). This silently
dropped any blank line that used to separate two such statements.
Reproduction (before this patch)
Output before the fix (blank line between
up()anddown()lost):Fix
pStmts()now checks, for each pair of consecutive statements that bothstill carry a valid
origNodeattribute, whether the original source textbetween them (via
$this->origTokens) is pure whitespace containing a blankline — and if so, re-inserts it. If anything else used to sit between them
(another statement that was removed, inline HTML, a comment), the existing
single-newline behavior is kept, since a raw line-number gap alone doesn't
tell us a blank line was actually there (this distinction is covered by the
existing
inlineHtml.testfixtures, which continue to pass unchanged).Added
test/code/formatPreservation/blankLineBetweenMethods.test,reproducing the exact scenario from #803.
Testing
vendor/bin/phpunit— 1923 tests, 2797 assertions, all passing (previously1922/1922 on
master; the added test failed for the documented reasonbefore the fix and passes after it). No other test's expected output
changed.
Signed-off-by: Eljees 3.14hell@gmail.com