Skip to content

Commit

Permalink
Improve ternary expression formatting
Browse files Browse the repository at this point in the history
- In `AlignTernaryOperators`, when operators are aligned with an
  expression that breaks over multiple lines, align with the first token
  on the previous line (like in `AlignChains` and `AlignArrowFunctions`)
- To better reflect operator precedence and associativity in chained
  ternary expressions--whether unambiguous (e.g. `1 ? 2 ? 3 : 4 : 5`) or
  deprecated (e.g. `1 ? 2 : 3 ? 4 : 5`)--stop iterating over previous
  siblings in `TokenUtil::getTernaryContext()` when a ternary expression
  with expr2 is found
- Make equivalent changes to `TokenUtil::getTernaryEndExpression()`
- Add and adopt `TokenUtil::getTernaryExpression()`
- Add `TokenUtil::getPrecedenceOf()`
- Add `Token::hasNewlineAfterPrevCode()`
- Add `TokenCollection::isEmpty()`
- Add alignment callbacks to token data and use them to re-align ternary
  operators after hanging indentation is collapsed
- Fix issue where ternary expressions are incorrectly aligned across
  logical operator boundaries
- Add tests
  • Loading branch information
lkrms committed Dec 6, 2024
1 parent 40e7fa3 commit 86212b8
Show file tree
Hide file tree
Showing 10 changed files with 739 additions and 55 deletions.
4 changes: 2 additions & 2 deletions docs/Rules.md

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

8 changes: 7 additions & 1 deletion src/Catalog/TokenData.php
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,14 @@ interface TokenData
*/
public const PROPERTY_HOOKS = 9;

/**
* A list of closures that align other tokens with the token when its output
* column changes
*/
public const ALIGNMENT_CALLBACKS = 10;

/**
* The type applied to an open bracket by the HangingIndentation rule
*/
public const HANGING_INDENT_PARENT_TYPE = 10;
public const HANGING_INDENT_PARENT_TYPE = 11;
}
9 changes: 9 additions & 0 deletions src/Internal/TokenCollection.php
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,15 @@ public static function collect(Token $from, Token $to): self
return $instance;
}

/**
* @phpstan-assert-if-false !null $this->first()
* @phpstan-assert-if-false !null $this->last()
*/
public function isEmpty(): bool
{
return !$this->Items;
}

/**
* @phpstan-assert-if-true !null $this->first()
* @phpstan-assert-if-true !null $this->last()
Expand Down
32 changes: 25 additions & 7 deletions src/Rule/AlignTernaryOperators.php
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,12 @@ public static function needsSortedTokens(): bool
* Apply the rule to the given tokens
*
* If a ternary or null coalescing operator has a leading newline, a
* callback is registered to align it with its expression.
* callback is registered to align it with its expression, or with the first
* token on the previous line if its expression breaks over multiple lines.
*
* @prettyphp-callback Ternary and null coalescing operators with leading
* newlines are aligned with their expressions.
* newlines are aligned with their expressions, or with the first token on
* the previous line if their expressions break over multiple lines.
*
* This is achieved by:
*
Expand Down Expand Up @@ -85,7 +87,17 @@ public function processTokens(array $tokens): void
continue;
}

$alignWith = TokenUtil::getOperatorExpression($prevTernary ?? $token);
$expr = TokenUtil::getTernaryExpression($prevTernary ?? $token);
/** @var Token */
$prev = ($prevTernary ?? $token)->PrevCode;
/** @var Token */
$alignWith = $expr->collect($prev)
->reverse()
->find(fn(Token $t) =>
$t === $expr || (
$t->Flags & TokenFlag::CODE
&& $t->hasNewlineBefore()
));

$this->setAlignedWith($token, $alignWith);

Expand All @@ -96,13 +108,19 @@ public function processTokens(array $tokens): void
static::class,
$token,
static function () use ($token, $alignWith, $until, $tabSize) {
$delta = $token->getColumnDelta($alignWith, true) + $tabSize;
while ($adjacent = $until->adjacentBeforeNewline()) {
$until = TokenUtil::getOperatorEndExpression($adjacent);
}
foreach ($token->collect($until) as $token) {
$token->LinePadding += $delta;
}
$tokens = $token->collect($until);
($callback = static function () use ($token, $alignWith, $tabSize, $tokens) {
$delta = $token->getColumnDelta($alignWith, true) + $tabSize;
if ($delta) {
foreach ($tokens as $token) {
$token->LinePadding += $delta;
}
}
})();
$alignWith->Data[TokenData::ALIGNMENT_CALLBACKS][] = $callback;
}
);
}
Expand Down
99 changes: 96 additions & 3 deletions src/Rule/HangingIndentation.php
Original file line number Diff line number Diff line change
Expand Up @@ -203,8 +203,11 @@ public function processTokens(array $tokens): void
$context[] = TokenUtil::getTernaryContext($trigger)
?? TokenUtil::getTernary1($trigger)
?? $trigger;
$until = TokenUtil::getTernaryEndExpression($trigger);
$apply = $trigger;
$until = TokenUtil::getTernaryEndExpression($trigger);
while ($adjacent = $until->adjacentBeforeNewline()) {
$until = TokenUtil::getOperatorEndExpression($adjacent);
}
} elseif ($this->Idx->Chain[$token->id]) {
$context[] = $token->Data[TokenData::CHAIN_OPENED_BY];
} elseif ($token->Heredoc && $token->Heredoc === $prev) {
Expand Down Expand Up @@ -262,7 +265,7 @@ public function processTokens(array $tokens): void
// $baz->qux()) ||
// $baz->quux())
// ```
$until = $until ?? $apply->pragmaticEndOfExpression(true, true, true);
$until = $until ?? $this->getLastIndentable($apply);
$indent = 0;
$hanging = [];
$parents =
Expand Down Expand Up @@ -306,7 +309,7 @@ public function processTokens(array $tokens): void
if ($adjacent = $until->adjacentBeforeNewline()) {
foreach ($adjacent->collect($adjacent->endOfLine()) as $t) {
if ($t->AlignedWith) {
$until = $adjacent->pragmaticEndOfExpression();
$until = $this->getLastIndentable($adjacent);
break;
}
}
Expand Down Expand Up @@ -355,12 +358,93 @@ public function processTokens(array $tokens): void
}
}

private function getLastIndentable(Token $token): Token
{
// Check if the token is part of a declaration with a body that hasn't
// been reached yet, not including anonymous functions or classes like
// the following, which can be moved around in their entirety:
//
// ```
// $foo = new
// #[Attribute]
// class implements
// Bar,
// Baz
// {
// // ...
// };
// ```
//
// But anonymous classes like this cannot:
//
// ```
// $foo = new class implements
// Bar,
// Baz
// {
// // ...
// };
// ```
$parts = $token->skipToStartOfDeclaration()
->declarationParts();
if (
!$parts->isEmpty()
&& (
$parts->hasOneFrom($this->Idx->DeclarationTopLevel)
|| ($token->Statement && $token->Statement->isProperty())
)
&& ($last = $parts->last())->index >= $token->index
&& $last->skipPrevSiblingFrom($this->Idx->Ampersand)->id !== T_FUNCTION
&& !(
($first = $parts->first())->id === T_NEW
&& $first->NextCode === $token
&& $first->nextSiblingOf(T_CLASS)->hasNewlineAfterPrevCode()
) && ($body = $last->nextSiblingOf(T_OPEN_BRACE, true))->id !== T_NULL
) {
/** @var Token */
return $body->PrevCode;
}

// If the token is between `?` and `:` in a ternary expression, return
// the last token before `:`
$t = $token;
$ternaryColon = null;
while (($t = $t->prevSiblingFrom($this->Idx->OperatorTernary, true))->id !== T_NULL) {
if ($t->Flags & TokenFlag::TERNARY_OPERATOR) {
if ($t->id === T_QUESTION) {
/** @var Token */
return $t->Data[TokenData::OTHER_TERNARY_OPERATOR]->PrevCode;
}
$ternaryColon = $t;
break;
}
}

if (
$ternaryColon
&& TokenUtil::getOperatorEndExpression($ternaryColon)->index >= $token->index
) {
// If the token is between `:` and `?` in chained ternary
// expressions, return the last token before `?`
$t = $token;
while (($t = $t->nextSiblingOf(T_QUESTION, true))->id !== T_NULL) {
if ($t->Flags & TokenFlag::TERNARY_OPERATOR) {
/** @var Token */
return $t->PrevCode;
}
}
}

return $token->EndStatement ?? $token;
}

/**
* @param array<int,int> $hanging
*/
private function maybeCollapseOverhanging(Token $token, Token $until, array $hanging): void
{
$tokens = $token->collect($until);
$collapsed = [];
foreach ($hanging as $index => $count) {
for ($i = 0; $i < $count; $i++) {
// The purpose of overhanging indents is to visually separate
Expand Down Expand Up @@ -442,10 +526,19 @@ private function maybeCollapseOverhanging(Token $token, Token $until, array $han
if ($t->HangingIndentParentLevels[$index] ?? 0) {
$t->HangingIndent--;
$t->HangingIndentParentLevels[$index]--;
$collapsed[$t->index] = $t;
}
}
}
}

foreach ($collapsed as $t) {
if ($callbacks = $t->Data[TokenData::ALIGNMENT_CALLBACKS] ?? null) {
foreach ($callbacks as $callback) {
$callback();
}
}
}
}

private function indent(Token $token): int
Expand Down
11 changes: 10 additions & 1 deletion src/Token.php
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ final class Token extends GenericToken implements HasTokenNames, JsonSerializabl

/**
* @var array<TokenData::*,mixed>
* @phpstan-var array{string,TokenCollection,int,self,self,self,self,TokenCollection,int,TokenCollection,int}
* @phpstan-var array{string,TokenCollection,int,self,self,self,self,TokenCollection,int,TokenCollection,Closure[],int}
*/
public array $Data;

Expand Down Expand Up @@ -1116,6 +1116,15 @@ public function removeWhitespace(int $whitespace): void
}
}

/**
* Check if, between the previous code token and the token, there's a
* newline between tokens
*/
public function hasNewlineAfterPrevCode(): bool
{
return $this->PrevCode && $this->PrevCode->hasNewlineBeforeNextCode();
}

/**
* Check if, between the token and the next code token, there's a newline
* between tokens
Expand Down
8 changes: 8 additions & 0 deletions src/TokenIndex.php
Original file line number Diff line number Diff line change
Expand Up @@ -582,6 +582,14 @@ class TokenIndex implements HasTokenIndex, Immutable
+ self::OPERATOR_COMPARISON
+ self::TOKEN_INDEX;

/**
* Ternary operators
*
* @var array<int,bool>
*/
public array $OperatorTernary = self::OPERATOR_TERNARY
+ self::TOKEN_INDEX;

/**
* T_AT, T_DEC, T_DOLLAR, T_INC, T_LOGICAL_NOT, T_NOT
*
Expand Down
Loading

0 comments on commit 86212b8

Please sign in to comment.