Skip to content

Commit

Permalink
Improve list formatting
Browse files Browse the repository at this point in the history
- Pass more lists to list rules like `AlignLists` and `StrictLists`:
  - `static` and `global` variable lists
  - `insteadof` lists in trait adaptations
  - import lists (e.g. `use Foo\Bar, Foo\Baz`)
  - import list groups (e.g. `use Foo\{Bar, Baz}`)
  - property and constant declaration lists
  - trait insertion lists
- Add `$lastChild` parameter to `ListRule::processList()`
- Add `TokenCollection::tokenHasNewlineBeforeNextCode()`
  • Loading branch information
lkrms committed Jan 3, 2025
1 parent 247242c commit fec88ee
Show file tree
Hide file tree
Showing 23 changed files with 706 additions and 53 deletions.
14 changes: 7 additions & 7 deletions docs/Rules.md

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 6 additions & 6 deletions src/Contract/ListRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,17 @@ interface ListRule extends Rule
/**
* Apply the rule to a token and the list of items associated with it
*
* If `$parent` is a `T_OPEN_PARENTHESIS`, `T_OPEN_BRACKET` or `T_ATTRIBUTE`
* token, `$items` has at least one item.
* If `$parent` is a `T_OPEN_PARENTHESIS`, `T_OPEN_BRACKET`, `T_OPEN_BRACE`
* or `T_ATTRIBUTE` token, `$items` has at least one item.
*
* Otherwise, `$parent` is a `T_EXTENDS` or `T_IMPLEMENTS` token, and
* Otherwise, `$parent` is a `T_EXTENDS`, `T_IMPLEMENTS`, `T_CONST`,
* `T_USE`, `T_INSTEADOF`, `T_STATIC`, `T_GLOBAL` or modifier token, and
* `$items` has at least two items.
*
* Each token in `$items` is the first code token after `$parent` or a
* delimiter.
*
* This method is not called for empty lists or for classes that extend or
* implement fewer than two interfaces.
* This method is not called for empty lists.
*/
public function processList(Token $parent, TokenCollection $items): void;
public function processList(Token $parent, TokenCollection $items, Token $lastChild): void;
}
123 changes: 114 additions & 9 deletions src/Formatter.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

namespace Lkrms\PrettyPHP;

use Lkrms\PrettyPHP\Catalog\DeclarationType;
use Lkrms\PrettyPHP\Catalog\DeclarationType as Type;
use Lkrms\PrettyPHP\Catalog\FormatterFlag;
use Lkrms\PrettyPHP\Catalog\HeredocIndent;
use Lkrms\PrettyPHP\Catalog\ImportSortOrder;
Expand Down Expand Up @@ -711,7 +711,7 @@ private function apply(): self
private function getAllDeclarationTypes(): array
{
/** @var int[] */
$types = Reflect::getConstants(DeclarationType::class);
$types = Reflect::getConstants(Type::class);

Check warning on line 714 in src/Formatter.php

View check run for this annotation

Codecov / codecov/patch

src/Formatter.php#L714

Added line #L714 was not covered by tests
return array_fill_keys($types, true);
}

Expand Down Expand Up @@ -949,14 +949,18 @@ public function format(
}

Profile::startTimer(__METHOD__ . '#find-lists');
$parents = $this->sortTokensByType($this->TokensById, [
$lists = [];
$parents = $this->getTokensByType($this->TokensById, [
\T_OPEN_PARENTHESIS => true,
\T_OPEN_BRACKET => true,
\T_OPEN_BRACE => true,
\T_ATTRIBUTE => true,
\T_EXTENDS => true,
\T_IMPLEMENTS => true,
\T_INSTEADOF => true,
\T_STATIC => true,
\T_GLOBAL => true,
]);
$lists = [];
foreach ($parents as $i => $parent) {
if ($parent->CloseBracket === $parent->NextCode) {
continue;
Expand All @@ -967,16 +971,33 @@ public function format(
&& $parent->PrevCode->id === \T_FOR
? \T_SEMICOLON
: \T_COMMA;
$last = null;
$items = null;
$minCount = 1;

/** @var Token */
$endStatement = $parent->EndStatement;

switch ($parent->id) {
case \T_STATIC:
if (
$parent->Statement !== $parent
|| $parent->Flags & TokenFlag::NAMED_DECLARATION
) {
continue 2;
}
// No break
case \T_INSTEADOF:
case \T_GLOBAL:
/** @var Token */
$last = $endStatement->PrevCode;
// No break
case \T_EXTENDS:
case \T_IMPLEMENTS:
/** @var Token */
$first = $parent->NextCode;
/** @var Token */
$last = $parent->nextSiblingFrom($idx->OpenBraceOrImplements)->PrevCode;
$last ??= $parent->nextSiblingFrom($idx->OpenBraceOrImplements)->PrevCode;
/** @var Token $last */
$items = $first->withNextSiblings($last)
->filter(
fn(Token $t, ?Token $next, ?Token $prev) =>
Expand Down Expand Up @@ -1008,8 +1029,24 @@ public function format(
continue 2;
}
break;

case \T_OPEN_BRACE:
/** @var Token */
$statement = $parent->Statement;
if (!(
$statement->Flags & TokenFlag::NAMED_DECLARATION
&& ($statement->Data[TokenData::NAMED_DECLARATION_TYPE] & (
Type::_USE
| Type::_TRAIT
)) === Type::_USE
)) {
continue 2;
}
break;
}

$last ??= ($parent->CloseBracket ?? $endStatement)->PrevCode;
/** @var Token $last */
$items ??= $parent->children()
->filter(
fn(Token $t, ?Token $next, ?Token $prev) =>
Expand All @@ -1034,8 +1071,76 @@ public function format(
$token->Flags |= TokenFlag::LIST_ITEM;
$token->Data[TokenData::LIST_PARENT] = $parent;
}
$lists[$i] = $items;
$lists[$i] = [$parent, $items, $last];
}

$parents = $this->getTokensByType($this->DeclarationsByType, [
Type::_CONST => true,
Type::_USE => true,
Type::PROPERTY => true,
Type::USE_CONST => true,
Type::USE_FUNCTION => true,
Type::USE_TRAIT => true,
]);
foreach ($parents as $i => $parent) {
$type = $parent->Data[TokenData::NAMED_DECLARATION_TYPE];
$first = null;
$last = null;

/** @var Token */
$endStatement = $parent->EndStatement;

switch ($type) {
case Type::_USE:
case Type::USE_CONST:
case Type::USE_FUNCTION:
case Type::USE_TRAIT:
/** @var Token */
$first = $parent->NextCode;
$first = $first->skipNextSiblingFrom($idx->ConstOrFunction);
if ($type === Type::USE_TRAIT) {
$last = $parent->nextSiblingFrom($idx->OpenBraceOrSemicolon)
->PrevCode;
}
break;

case Type::PROPERTY:
$first = $parent->nextSiblingOf(\T_VARIABLE);
/** @var Token */
$last = $parent->nextSiblingFrom($idx->OpenBraceOrSemicolon)
->PrevCode;
break;
}

$first ??= $parent->nextSiblingOf(\T_EQUAL)->PrevCode;
/** @var Token $first */
$parent = $first->PrevCode;
/** @var Token $parent */
$last ??= $endStatement->PrevCode;
/** @var Token $last */
$items = $first->withNextSiblings($last)
->filter(
fn(Token $t, ?Token $next, ?Token $prev) =>
!$prev
|| !$t->PrevCode
|| $t->PrevCode->id === \T_COMMA
);

$count = $items->count();
if ($count < 2) {
continue;
}
$parent->Flags |= TokenFlag::LIST_PARENT;
$parent->Data[TokenData::LIST_DELIMITER] = \T_COMMA;
$parent->Data[TokenData::LIST_ITEMS] = $items;
$parent->Data[TokenData::LIST_ITEM_COUNT] = $count;
foreach ($items as $token) {
$token->Flags |= TokenFlag::LIST_ITEM;
$token->Data[TokenData::LIST_PARENT] = $parent;
}
$lists[$i] = [$parent, $items, $last];
}
ksort($lists, \SORT_NUMERIC);
Profile::stopTimer(__METHOD__ . '#find-lists');

$logProgress = $this->LogProgress
Expand All @@ -1057,9 +1162,9 @@ public function format(
}

if ($method === ListRule::PROCESS_LIST) {
foreach ($lists as $i => $list) {
foreach ($lists as [$parent, $items, $last]) {
/** @var ListRule $rule */
$rule->processList($parents[$i], clone $list);
$rule->processList($parent, clone $items, $last);
}
Profile::stopTimer($_rule, 'rule');
$logProgress($_rule, ListRule::PROCESS_LIST);
Expand Down
17 changes: 17 additions & 0 deletions src/Internal/TokenCollection.php
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,23 @@ public function tokenHasNewlineAfter(bool $closedBy = false): bool
return false;
}

/**
* Check if, between a token in the collection and its next code token,
* there's a newline between tokens
*/
public function tokenHasNewlineBeforeNextCode(bool $closedBy = false): bool
{
foreach ($this->Items as $token) {
if ($closedBy && $token->CloseBracket) {
$token = $token->CloseBracket;

Check warning on line 196 in src/Internal/TokenCollection.php

View check run for this annotation

Codecov / codecov/patch

src/Internal/TokenCollection.php#L196

Added line #L196 was not covered by tests
}
if ($token->hasNewlineBeforeNextCode()) {
return true;
}
}
return false;
}

/**
* Check if any tokens in the collection are separated by one or more line
* breaks
Expand Down
6 changes: 2 additions & 4 deletions src/Rule/AlignLists.php
Original file line number Diff line number Diff line change
Expand Up @@ -48,17 +48,15 @@ public function reset(): void
* after their open brackets, or with the first item in the list if they
* have no enclosing brackets.
*/
public function processList(Token $parent, TokenCollection $items): void
public function processList(Token $parent, TokenCollection $items, Token $lastChild): void
{
/** @var Token */
$first = $items->first();
if ($last = $parent->CloseBracket) {
/** @var Token */
$until = $last->PrevCode;
} else {
/** @var Token */
$last = $parent->nextSiblingFrom($this->Idx->OpenBraceOrImplements)
->PrevCode;
$last = $lastChild;
$until = $last;
}

Expand Down
9 changes: 0 additions & 9 deletions src/Rule/DeclarationSpacing.php
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,6 @@ public function boot(): void
/**
* Apply the rule to the given declarations
*
* Newlines are added between comma-delimited constant or property
* declarations.
*
* One-line declarations with a collapsed or collapsible DocBlock, or no
* DocBlock at all, are considered "collapsible". Declarations that break
* over multiple lines or have a DocBlock that cannot be collapsed to one
Expand Down Expand Up @@ -131,12 +128,6 @@ public function processDeclarations(array $declarations): void

$decl = new Declaration($token, $type, $modifiers);

if ($type === Type::_CONST || $type === Type::PROPERTY) {
foreach ($token->withNextSiblings($decl->End)->getAnyOf(\T_COMMA) as $comma) {
$comma->Whitespace |= Space::LINE_AFTER;
}
}

// Group consecutive declarations by parent
$parentIndex = $token->Parent
? $token->Parent->index
Expand Down
Loading

0 comments on commit fec88ee

Please sign in to comment.