From 8e2bee8ae6126863992c70c4032a9c0c96b588c3 Mon Sep 17 00:00:00 2001 From: Luke Arms Date: Wed, 8 Jan 2025 12:21:21 +1100 Subject: [PATCH] Fix incorrect formatting in alternative syntax constructs - Fix issue where assignments in alternative syntax constructs are not aligned when `align-data` is enabled - Fix issue where alignment is incorrectly applied to default value assignments in parameter lists with no leading newline when `align-data` is enabled - Fix issue where alternative syntax constructs may not be indented correctly when `align-lists` is enabled - Fix issue where blank lines may not be added before `yield` statements in alternative syntax constructs when `blank-before-return` is enabled - Fix issue where blank lines before and after comments are incorrectly suppressed in alternative syntax constructs --- docs/Rules.md | 4 +- src/Rule/AlignData.php | 16 +- src/Rule/AlignLists.php | 22 +- src/Rule/BlankBeforeReturn.php | 5 +- src/Rule/HangingIndentation.php | 2 +- src/Rule/PreserveNewlines.php | 10 +- src/Rule/Preset/WordPress.php | 2 +- .../preset/wordpress/loop.out | 2 +- .../preset/wordpress/meta-boxes.out | 2 +- tests/unit/Rule/AlignDataTest.php | 236 +++++++++++++++++- tests/unit/Rule/AlignListsTest.php | 78 ++++++ 11 files changed, 355 insertions(+), 24 deletions(-) diff --git a/docs/Rules.md b/docs/Rules.md index be3e14a8..772f4158 100644 --- a/docs/Rules.md +++ b/docs/Rules.md @@ -456,11 +456,11 @@ C++- and shell-style comments on their own line after a comment beside code are When they appear in the same scope, a callback is registered to align consecutive: -- assignment operators +- assignment operators (except as noted below) - `=>` delimiters in array syntax (except as noted below) - `=>` delimiters in `match` expressions -If the open bracket of an array is not followed by a newline and neither `AlignLists` nor `StrictLists` are enabled, its `=>` delimiters are ignored. +If the open bracket of an array or parameter list is not followed by a newline and neither `AlignLists` nor `StrictLists` are enabled, its `=>` delimiters or assignment operators are ignored. ### `AlignChains` (2) diff --git a/src/Rule/AlignData.php b/src/Rule/AlignData.php index 493fa8c1..f1b77a5a 100644 --- a/src/Rule/AlignData.php +++ b/src/Rule/AlignData.php @@ -61,13 +61,13 @@ public function boot(): void * When they appear in the same scope, a callback is registered to align * consecutive: * - * - assignment operators + * - assignment operators (except as noted below) * - `=>` delimiters in array syntax (except as noted below) * - `=>` delimiters in `match` expressions * - * If the open bracket of an array is not followed by a newline and neither - * `AlignLists` nor `StrictLists` are enabled, its `=>` delimiters are - * ignored. + * If the open bracket of an array or parameter list is not followed by a + * newline and neither `AlignLists` nor `StrictLists` are enabled, its `=>` + * delimiters or assignment operators are ignored. * * @prettyphp-callback Assignment operators are aligned unless the * formatter's `MaxAssignmentPadding` property is not `null` and would be @@ -112,7 +112,13 @@ public function processBlock(array $lines): void ( !$token->Parent || $token->Parent->Flags & TokenFlag::STRUCTURAL_BRACE - || $token->Parent->isParameterList() + || $token->Parent->id === T_COLON + || ( + $token->Parent->isParameterList() && ( + $this->ListRuleEnabled + || $token->Parent->hasNewlineBeforeNextCode() + ) + ) ) // Ignore assignment operators after the first: // - in the statement diff --git a/src/Rule/AlignLists.php b/src/Rule/AlignLists.php index af2fc6d0..1d49ef97 100644 --- a/src/Rule/AlignLists.php +++ b/src/Rule/AlignLists.php @@ -117,16 +117,30 @@ static function () use ( if ($parent->CloseBracket) { // `]` after `2` -> `]` after `5` while (($adjacent = $to->lastSiblingBeforeNewline()) !== $to && !( - $adjacent->id === \T_OPEN_BRACE - && $adjacent->Flags & TokenFlag::STRUCTURAL_BRACE + ( + ( + $adjacent->id === \T_OPEN_BRACE + && $adjacent->Flags & TokenFlag::STRUCTURAL_BRACE + ) || ( + $adjacent->id === T_COLON + && $adjacent->CloseBracket + ) + ) && $adjacent->Depth <= $parent->Depth )) { $to = $adjacent; } // `]` after `5` -> `+` -> `)` after `11` while (($adjacent = $to->adjacentBeforeNewline()) && !( - $adjacent->id === \T_OPEN_BRACE - && $adjacent->Flags & TokenFlag::STRUCTURAL_BRACE + ( + ( + $adjacent->id === \T_OPEN_BRACE + && $adjacent->Flags & TokenFlag::STRUCTURAL_BRACE + ) || ( + $adjacent->id === T_COLON + && $adjacent->CloseBracket + ) + ) && $adjacent->Depth <= $parent->Depth )) { $to = TokenUtil::getOperatorEndExpression($adjacent); diff --git a/src/Rule/BlankBeforeReturn.php b/src/Rule/BlankBeforeReturn.php index 11468a6d..ba438337 100644 --- a/src/Rule/BlankBeforeReturn.php +++ b/src/Rule/BlankBeforeReturn.php @@ -55,7 +55,10 @@ public function processTokens(array $tokens): void if ( $token->Statement !== $token || ( $token->Parent - && !($token->Parent->Flags & TokenFlag::STRUCTURAL_BRACE) + && !( + $token->Parent->Flags & TokenFlag::STRUCTURAL_BRACE + || $token->Parent->id === T_COLON + ) ) ) { continue; diff --git a/src/Rule/HangingIndentation.php b/src/Rule/HangingIndentation.php index b0b17672..9280bc6d 100644 --- a/src/Rule/HangingIndentation.php +++ b/src/Rule/HangingIndentation.php @@ -110,7 +110,7 @@ public function processTokens(array $tokens): void ) ) || ( $token->id === \T_COLON - && $token->isColonAltSyntaxDelimiter() + && $token->CloseBracket ) || $token->id === \T_OPEN_UNENCLOSED; $flags = 0; if ($token->hasNewlineBeforeNextCode()) { diff --git a/src/Rule/PreserveNewlines.php b/src/Rule/PreserveNewlines.php index cf26784d..d8fb471c 100644 --- a/src/Rule/PreserveNewlines.php +++ b/src/Rule/PreserveNewlines.php @@ -246,8 +246,10 @@ private function maybePreserveNewlineAfter( && $prevCode->Parent === $parent && $prevCode->EndStatement !== $prevCode ) || ( - $parent - && !($parent->Flags & TokenFlag::STRUCTURAL_BRACE) + $parent && !( + $parent->Flags & TokenFlag::STRUCTURAL_BRACE + || $parent->id === T_COLON + ) ) ) ) || ( @@ -258,8 +260,10 @@ private function maybePreserveNewlineAfter( && $prevCode->EndStatement !== $prevCode ) || ( $next->Parent - && !($next->Parent->Flags & TokenFlag::STRUCTURAL_BRACE) && !( + $next->Parent->Flags & TokenFlag::STRUCTURAL_BRACE + || $next->Parent->id === T_COLON + ) && !( $next->Parent->id === \T_OPEN_BRACE && $next->Parent->isMatchOpenBrace() && ($prevCode = $next->PrevCode) diff --git a/src/Rule/Preset/WordPress.php b/src/Rule/Preset/WordPress.php index 39bb43ce..528ac490 100644 --- a/src/Rule/Preset/WordPress.php +++ b/src/Rule/Preset/WordPress.php @@ -136,7 +136,7 @@ public function processTokens(array $tokens): void } if ($token->id === \T_COLON) { - if (!$token->isColonAltSyntaxDelimiter()) { + if (!$token->CloseBracket) { continue; } $token->applyWhitespace(Space::SPACE_BEFORE); diff --git a/tests/fixtures/App/PrettyPHPCommand/preset/wordpress/loop.out b/tests/fixtures/App/PrettyPHPCommand/preset/wordpress/loop.out index 94cf4835..12c90c8b 100644 --- a/tests/fixtures/App/PrettyPHPCommand/preset/wordpress/loop.out +++ b/tests/fixtures/App/PrettyPHPCommand/preset/wordpress/loop.out @@ -78,7 +78,7 @@ while ( have_posts() ) : $images = twentyten_get_gallery_images(); if ( $images ) : $total_images = count( $images ); - $image = reset( $images ); + $image = reset( $images ); ?>
diff --git a/tests/fixtures/App/PrettyPHPCommand/preset/wordpress/meta-boxes.out b/tests/fixtures/App/PrettyPHPCommand/preset/wordpress/meta-boxes.out index 2d6945fc..f50068bf 100644 --- a/tests/fixtures/App/PrettyPHPCommand/preset/wordpress/meta-boxes.out +++ b/tests/fixtures/App/PrettyPHPCommand/preset/wordpress/meta-boxes.out @@ -1023,7 +1023,7 @@ function page_attributes_meta_box( $post ) { * @param WP_Post $post The current post. */ $dropdown_args = apply_filters( 'page_attributes_dropdown_pages_args', $dropdown_args, $post ); - $pages = wp_dropdown_pages( $dropdown_args ); + $pages = wp_dropdown_pages( $dropdown_args ); if ( ! empty( $pages ) ) : ?>

diff --git a/tests/unit/Rule/AlignDataTest.php b/tests/unit/Rule/AlignDataTest.php index 3071358b..02b8d9d1 100644 --- a/tests/unit/Rule/AlignDataTest.php +++ b/tests/unit/Rule/AlignDataTest.php @@ -3,24 +3,250 @@ namespace Lkrms\PrettyPHP\Tests\Rule; use Lkrms\PrettyPHP\Rule\AlignData; -use Lkrms\PrettyPHP\Rule\PreserveOneLineStatements; +use Lkrms\PrettyPHP\Rule\AlignLists; +use Lkrms\PrettyPHP\Rule\StrictLists; use Lkrms\PrettyPHP\Tests\TestCase; +use Lkrms\PrettyPHP\Formatter; +use Lkrms\PrettyPHP\FormatterBuilder as FormatterB; final class AlignDataTest extends TestCase { /** * @dataProvider outputProvider + * + * @param Formatter|FormatterB $formatter */ - public function testOutput(string $expected, string $code): void + public function testOutput(string $expected, string $code, $formatter): void { - $this->assertCodeFormatIs($expected, $code, [AlignData::class, PreserveOneLineStatements::class]); + $this->assertFormatterOutputIs($expected, $code, $formatter); } /** - * @return array + * @return array */ public static function outputProvider(): array { - return []; + $formatterB = Formatter::build() + ->enable([AlignData::class]); + $formatter = $formatterB->build(); + + $input1 = <<<'PHP' + 0, + 'foo' => 1, 'bar' => 2, + 'quux' => 3, 'b' => 4); +array('a' => 0, + 'foo' => 1, 'bar' => 2, + 'quux' => 3, 'b' => 4,); + +['a' => 0, + 'foo' => 1, 'bar' => 2, + 'quux' => 3, 'b' => 4]; +['a' => 0, + 'foo' => 1, 'bar' => 2, + 'quux' => 3, 'b' => 4,]; + +function ($a = 0, + $foo = 1, $bar = 2, + $qux = 3, $b = 4) {}; +fn($a = 0, + $foo = 1, $bar = 2, + $quux = 3, $b = 4) => null; +PHP; + + return [ + [ + <<<'PHP' + 0, + 'foo' => 1, 'bar' => 2, + 'quux' => 3, 'b' => 4); +array( + 'a' => 0, + 'foo' => 1, + 'bar' => 2, + 'quux' => 3, + 'b' => 4, +); + +['a' => 0, + 'foo' => 1, 'bar' => 2, + 'quux' => 3, 'b' => 4]; +[ + 'a' => 0, + 'foo' => 1, + 'bar' => 2, + 'quux' => 3, + 'b' => 4, +]; + +function ($a = 0, + $foo = 1, $bar = 2, + $qux = 3, $b = 4) {}; +fn($a = 0, + $foo = 1, $bar = 2, + $quux = 3, $b = 4) => null; + +PHP, + $input1, + $formatter, + ], + [ + <<<'PHP' + 0, + 'foo' => 1, 'bar' => 2, + 'quux' => 3, 'b' => 4); +array( + 'a' => 0, + 'foo' => 1, + 'bar' => 2, + 'quux' => 3, + 'b' => 4, +); + +['a' => 0, + 'foo' => 1, 'bar' => 2, + 'quux' => 3, 'b' => 4]; +[ + 'a' => 0, + 'foo' => 1, + 'bar' => 2, + 'quux' => 3, + 'b' => 4, +]; + +function ($a = 0, + $foo = 1, $bar = 2, + $qux = 3, $b = 4) {}; +fn($a = 0, + $foo = 1, $bar = 2, + $quux = 3, $b = 4) => null; + +PHP, + $input1, + $formatterB + ->enable([AlignData::class, AlignLists::class]) + ->build(), + ], + [ + <<<'PHP' + 0, + 'foo' => 1, + 'bar' => 2, + 'quux' => 3, + 'b' => 4 +); +array( + 'a' => 0, + 'foo' => 1, + 'bar' => 2, + 'quux' => 3, + 'b' => 4, +); + +[ + 'a' => 0, + 'foo' => 1, + 'bar' => 2, + 'quux' => 3, + 'b' => 4 +]; +[ + 'a' => 0, + 'foo' => 1, + 'bar' => 2, + 'quux' => 3, + 'b' => 4, +]; + +function ( + $a = 0, + $foo = 1, + $bar = 2, + $qux = 3, + $b = 4 +) {}; +fn( + $a = 0, + $foo = 1, + $bar = 2, + $quux = 3, + $b = 4 +) => null; + +PHP, + $input1, + $formatterB + ->enable([AlignData::class, StrictLists::class]) + ->build(), + ], + ]; } } diff --git a/tests/unit/Rule/AlignListsTest.php b/tests/unit/Rule/AlignListsTest.php index e63a6973..5042871b 100644 --- a/tests/unit/Rule/AlignListsTest.php +++ b/tests/unit/Rule/AlignListsTest.php @@ -148,6 +148,84 @@ public static function outputProvider(): array $f)) { // } +PHP, + ], + 'nested lists + alternative syntax' => [ + <<<'PHP' + [ + <<<'PHP' + [