Skip to content

Commit

Permalink
Fixes #8. ErickSkrauch/align_multiline_parameters produces new line f…
Browse files Browse the repository at this point in the history 
…or promoted properties without any types
  • Loading branch information
@erickskrauch
erickskrauch committed Jan 9, 2024
1 parent ec69923 commit 7b53675
Show file tree
Hide file tree
Showing 3 changed files with 123 additions and 53 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)
and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).

## [Unreleased]
### Fixed
- Bug #8: `ErickSkrauch/align_multiline_parameters` produces new line for promoted properties without any types.

## [1.2.2] - 2024-01-07
### Added
Expand Down
126 changes: 74 additions & 52 deletions src/FunctionNotation/AlignMultilineParametersFixer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,12 @@
* variables: bool|null,
* defaults: bool|null,
* } $configuration
*
* @phpstan-type DeclarationAnalysis array{
* typeLength: non-negative-int,
* nameLength: positive-int,
* nameIndex: int,
* }
*/
final class AlignMultilineParametersFixer extends AbstractFixer implements ConfigurableFixerInterface, WhitespacesAwareFixerInterface {

Expand All @@ -40,19 +46,11 @@ final class AlignMultilineParametersFixer extends AbstractFixer implements Confi
/**
* @var list<int>
*/
private array $parameterModifiers;

public function __construct() {
parent::__construct();
$this->parameterModifiers = [
CT::T_CONSTRUCTOR_PROPERTY_PROMOTION_PUBLIC,
CT::T_CONSTRUCTOR_PROPERTY_PROMOTION_PROTECTED,
CT::T_CONSTRUCTOR_PROPERTY_PROMOTION_PRIVATE,
];
if (defined('T_READONLY')) {
$this->parameterModifiers[] = T_READONLY;
}
}
private const PROMOTIONAL_TOKENS = [
CT::T_CONSTRUCTOR_PROPERTY_PROMOTION_PUBLIC,
CT::T_CONSTRUCTOR_PROPERTY_PROMOTION_PROTECTED,
CT::T_CONSTRUCTOR_PROPERTY_PROMOTION_PRIVATE,
];

public function getDefinition(): FixerDefinitionInterface {
return new FixerDefinition(
Expand Down Expand Up @@ -133,62 +131,57 @@ protected function applyFix(SplFileInfo $file, Tokens $tokens): void {
$longestType = 0;
$longestVariableName = 0;
$hasAtLeastOneTypedArgument = false;
/** @var list<DeclarationAnalysis> $analysedArguments */
$analysedArguments = [];
foreach ($arguments as $argument) {
$typeAnalysis = $argument->getTypeAnalysis();
if ($typeAnalysis !== null) {
$declarationAnalysis = $this->getDeclarationAnalysis($tokens, $argument->getNameIndex(), $typeAnalysis);
if ($declarationAnalysis['typeLength'] > 0) {
$hasAtLeastOneTypedArgument = true;
$typeLength = $this->getFullTypeLength($tokens, $typeAnalysis);
if ($typeLength > $longestType) {
$longestType = $typeLength;
}
}

$variableNameLength = mb_strlen($argument->getName());
if ($variableNameLength > $longestVariableName) {
$longestVariableName = $variableNameLength;
if ($declarationAnalysis['typeLength'] > $longestType) {
$longestType = $declarationAnalysis['typeLength'];
}

if ($declarationAnalysis['nameLength'] > $longestVariableName) {
$longestVariableName = $declarationAnalysis['nameLength'];
}

$analysedArguments[] = $declarationAnalysis;
}

$argsIndent = WhitespacesAnalyzer::detectIndent($tokens, $i) . $this->whitespacesConfig->getIndent();
// Since we perform insertion of new tokens in this loop, if we go sequentially,
// at each new iteration the token indices will shift due to the addition of new whitespaces.
// If we go from the end, this problem will not occur.
foreach (array_reverse($arguments) as $argument) {
foreach (array_reverse($analysedArguments) as $argument) {
if ($this->configuration[self::C_DEFAULTS] !== null) {
// Can't use $argument->hasDefault() because it's null when it's default for a type (e.g. 0 for int)
$equalToken = $tokens[$tokens->getNextMeaningfulToken($argument->getNameIndex())];
$equalToken = $tokens[$tokens->getNextMeaningfulToken($argument['nameIndex'])];
if ($equalToken->getContent() === '=') {
$nameLen = mb_strlen($argument->getName());
$whitespaceIndex = $argument->getNameIndex() + 1;
$whitespaceIndex = $argument['nameIndex'] + 1;
if ($this->configuration[self::C_DEFAULTS] === true) {
$tokens->ensureWhitespaceAtIndex($whitespaceIndex, 0, str_repeat(' ', $longestVariableName - $nameLen + 1));
$tokens->ensureWhitespaceAtIndex($whitespaceIndex, 0, str_repeat(' ', $longestVariableName - $argument['nameLength'] + 1));
} else {
$tokens->ensureWhitespaceAtIndex($whitespaceIndex, 0, ' ');
}
}
}

if ($this->configuration[self::C_VARIABLES] !== null) {
$whitespaceIndex = $argument->getNameIndex() - 1;
$whitespaceIndex = $argument['nameIndex'] - 1;
if ($this->configuration[self::C_VARIABLES] === true) {
$typeLen = 0;
$typeAnalysis = $argument->getTypeAnalysis();
if ($typeAnalysis !== null) {
$typeLen = $this->getFullTypeLength($tokens, $typeAnalysis);
}

$appendix = str_repeat(' ', $longestType - $typeLen + (int)$hasAtLeastOneTypedArgument);
if ($argument->hasTypeAnalysis()) {
$appendix = str_repeat(' ', $longestType - $argument['typeLength'] + (int)$hasAtLeastOneTypedArgument);
if ($argument['typeLength'] > 0) {
$whitespaceToken = $appendix;
} else {
$whitespaceToken = $this->whitespacesConfig->getLineEnding() . $argsIndent . $appendix;
}
} elseif ($argument['typeLength'] > 0) {
$whitespaceToken = ' ';
} else {
if ($argument->hasTypeAnalysis()) {
$whitespaceToken = ' ';
} else {
$whitespaceToken = $this->whitespacesConfig->getLineEnding() . $argsIndent;
}
$whitespaceToken = $this->whitespacesConfig->getLineEnding() . $argsIndent;
}

$tokens->ensureWhitespaceAtIndex($whitespaceIndex, 1, $whitespaceToken);
Expand All @@ -197,25 +190,54 @@ protected function applyFix(SplFileInfo $file, Tokens $tokens): void {
}
}

private function getFullTypeLength(Tokens $tokens, TypeAnalysis $typeAnalysis): int {
/**
* @phpstan-return DeclarationAnalysis
*/
private function getDeclarationAnalysis(Tokens $tokens, int $nameIndex, ?TypeAnalysis $typeAnalysis): array {
$searchIndex = $nameIndex;
$includeNextWhitespace = false;
$typeLength = 0;
for ($i = $typeAnalysis->getStartIndex(); $i <= $typeAnalysis->getEndIndex(); $i++) {
$typeLength += mb_strlen($tokens[$i]->getContent());
if ($typeAnalysis !== null) {
$searchIndex = $typeAnalysis->getStartIndex();
$includeNextWhitespace = true;
for ($i = $typeAnalysis->getStartIndex(); $i <= $typeAnalysis->getEndIndex(); $i++) {
$typeLength += mb_strlen($tokens[$i]->getContent());
}
}

$possiblyReadonlyToken = $tokens[$typeAnalysis->getStartIndex() - 2];
if ($possiblyReadonlyToken->isGivenKind($this->parameterModifiers)) {
$whitespaceToken = $tokens[$typeAnalysis->getStartIndex() - 1];
$typeLength += strlen($possiblyReadonlyToken->getContent() . $whitespaceToken->getContent());
$readonlyTokenIndex = $tokens->getPrevMeaningfulToken($searchIndex);
$readonlyToken = $tokens[$readonlyTokenIndex];
if (defined('T_READONLY') && $readonlyToken->isGivenKind(T_READONLY)) {
// The readonly can't be assigned on a promoted property without a type,
// so there is always will be a space between readonly and the next token
$whitespaceToken = $tokens[$searchIndex - 1];
$typeLength += strlen($readonlyToken->getContent() . $whitespaceToken->getContent());
$searchIndex = $readonlyTokenIndex;
$includeNextWhitespace = true;
}

$possiblyPromotionToken = $tokens[$typeAnalysis->getStartIndex() - 4];
if ($possiblyPromotionToken->isGivenKind($this->parameterModifiers)) {
$whitespaceToken = $tokens[$typeAnalysis->getStartIndex() - 3];
$typeLength += strlen($possiblyPromotionToken->getContent() . $whitespaceToken->getContent());
$promotionTokenIndex = $tokens->getPrevMeaningfulToken($searchIndex);
$promotionToken = $tokens[$promotionTokenIndex];
if ($promotionToken->isGivenKind(self::PROMOTIONAL_TOKENS)) {
$promotionalStr = $promotionToken->getContent();
if ($includeNextWhitespace) {
$whitespaceToken = $tokens[$promotionTokenIndex + 1];
if ($whitespaceToken->isWhitespace()) {
$promotionalStr .= $whitespaceToken->getContent();
}
}

$typeLength += strlen($promotionalStr);
}

return $typeLength;
/** @var positive-int $nameLength force type for PHPStan to avoid type error on return statement */
$nameLength = mb_strlen($tokens[$nameIndex]->getContent());

return [
'typeLength' => $typeLength,
'nameLength' => $nameLength,
'nameIndex' => $nameIndex,
];
}

}
48 changes: 47 additions & 1 deletion tests/FunctionNotation/AlignMultilineParametersFixerTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,25 @@ public function test80BothTrue(string $expected, ?string $input = null): void {
* @return iterable<array{0: string, 1?: string}>
*/
public function provide80TrueCases(): iterable {
yield 'constructor promotion, defaults' => [
yield 'constructor promotion without types, defaults' => [
'<?php
class Test {
public function __construct(
public $string = "string",
protected $bool = true
) {}
}
',
'<?php
class Test {
public function __construct(
public $string = "string",
protected $bool = true
) {}
}
',
];
yield 'constructor promotion with types, defaults' => [
'<?php
class Test {
public function __construct(
Expand Down Expand Up @@ -344,6 +362,34 @@ public function provideFalse80Cases(): iterable {
}
}

/**
* @requires PHP 8.0
*/
public function testNoWhitespaceAroundPromotion(): void {
$this->fixer->configure([
AlignMultilineParametersFixer::C_VARIABLES => true,
AlignMultilineParametersFixer::C_DEFAULTS => true,
]);
$this->doTest(
'<?php
class Test {
public function __construct(
public $string = "string",
protected $bool = true
) {}
}
',
'<?php
class Test {
public function __construct(
public$string = "string",
protected$bool = true
) {}
}
',
);
}

/**
* @dataProvider provide81TrueCases
* @requires PHP 8.1
Expand Down

0 comments on commit 7b53675

Please sign in to comment.