From 7b53675a12112f552bd02721c1fa0d50aa316ef4 Mon Sep 17 00:00:00 2001 From: ErickSkrauch Date: Tue, 9 Jan 2024 16:12:36 +0100 Subject: [PATCH] Fixes #8. ErickSkrauch/align_multiline_parameters produces new line for promoted properties without any types --- CHANGELOG.md | 2 + .../AlignMultilineParametersFixer.php | 126 ++++++++++-------- .../AlignMultilineParametersFixerTest.php | 48 ++++++- 3 files changed, 123 insertions(+), 53 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 80ef8e5..431d56c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/src/FunctionNotation/AlignMultilineParametersFixer.php b/src/FunctionNotation/AlignMultilineParametersFixer.php index 0d234e3..96c1b9e 100644 --- a/src/FunctionNotation/AlignMultilineParametersFixer.php +++ b/src/FunctionNotation/AlignMultilineParametersFixer.php @@ -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 { @@ -40,19 +46,11 @@ final class AlignMultilineParametersFixer extends AbstractFixer implements Confi /** * @var list */ - 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( @@ -133,35 +131,38 @@ protected function applyFix(SplFileInfo $file, Tokens $tokens): void { $longestType = 0; $longestVariableName = 0; $hasAtLeastOneTypedArgument = false; + /** @var list $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, ' '); } @@ -169,26 +170,18 @@ protected function applyFix(SplFileInfo $file, Tokens $tokens): void { } 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); @@ -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, + ]; } } diff --git a/tests/FunctionNotation/AlignMultilineParametersFixerTest.php b/tests/FunctionNotation/AlignMultilineParametersFixerTest.php index e75004a..b36cf9d 100644 --- a/tests/FunctionNotation/AlignMultilineParametersFixerTest.php +++ b/tests/FunctionNotation/AlignMultilineParametersFixerTest.php @@ -299,7 +299,25 @@ public function test80BothTrue(string $expected, ?string $input = null): void { * @return iterable */ public function provide80TrueCases(): iterable { - yield 'constructor promotion, defaults' => [ + yield 'constructor promotion without types, defaults' => [ + ' [ 'fixer->configure([ + AlignMultilineParametersFixer::C_VARIABLES => true, + AlignMultilineParametersFixer::C_DEFAULTS => true, + ]); + $this->doTest( + '