diff --git a/CHANGELOG.md b/CHANGELOG.md index cd5a864..90ace83 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ### Fixed - Bug #6: `ErickSkrauch/align_multiline_parameters` not working correctly with unions and intersections. +- `ErickSkrauch/align_multiline_parameters` inserted a space between the type and the param name in the wrong position when there were no whitespace between them. ## [1.2.1] - 2023-11-16 ### Fixed diff --git a/src/FunctionNotation/AlignMultilineParametersFixer.php b/src/FunctionNotation/AlignMultilineParametersFixer.php index 5ac0c4b..688e065 100644 --- a/src/FunctionNotation/AlignMultilineParametersFixer.php +++ b/src/FunctionNotation/AlignMultilineParametersFixer.php @@ -12,6 +12,7 @@ use PhpCsFixer\FixerDefinition\CodeSample; use PhpCsFixer\FixerDefinition\FixerDefinition; use PhpCsFixer\FixerDefinition\FixerDefinitionInterface; +use PhpCsFixer\Tokenizer\Analyzer\Analysis\TypeAnalysis; use PhpCsFixer\Tokenizer\Analyzer\FunctionsAnalyzer; use PhpCsFixer\Tokenizer\Analyzer\WhitespacesAnalyzer; use PhpCsFixer\Tokenizer\CT; @@ -127,7 +128,7 @@ protected function applyFix(SplFileInfo $file, Tokens $tokens): void { $typeAnalysis = $argument->getTypeAnalysis(); if ($typeAnalysis) { $hasAtLeastOneTypedArgument = true; - $typeLength = $this->getFullTypeLength($tokens, $typeAnalysis->getStartIndex()); + $typeLength = $this->getFullTypeLength($tokens, $typeAnalysis); if ($typeLength > $longestType) { $longestType = $typeLength; } @@ -140,13 +141,31 @@ protected function applyFix(SplFileInfo $file, Tokens $tokens): void { } $argsIndent = WhitespacesAnalyzer::detectIndent($tokens, $i) . $this->whitespacesConfig->getIndent(); - foreach ($arguments as $argument) { + // 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) { + 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())]; + if ($equalToken->getContent() === '=') { + $nameLen = mb_strlen($argument->getName()); + $whitespaceIndex = $argument->getNameIndex() + 1; + if ($this->configuration[self::C_DEFAULTS] === true) { + $tokens->ensureWhitespaceAtIndex($whitespaceIndex, 0, str_repeat(' ', $longestVariableName - $nameLen + 1)); + } else { + $tokens->ensureWhitespaceAtIndex($whitespaceIndex, 0, ' '); + } + } + } + if ($this->configuration[self::C_VARIABLES] !== null) { $whitespaceIndex = $argument->getNameIndex() - 1; if ($this->configuration[self::C_VARIABLES] === true) { $typeLen = 0; - if ($argument->getTypeAnalysis() !== null) { - $typeLen = $this->getFullTypeLength($tokens, $argument->getTypeAnalysis()->getStartIndex()); + $typeAnalysis = $argument->getTypeAnalysis(); + if ($typeAnalysis !== null) { + $typeLen = $this->getFullTypeLength($tokens, $typeAnalysis); } $appendix = str_repeat(' ', $longestType - $typeLen + (int)$hasAtLeastOneTypedArgument); @@ -163,22 +182,7 @@ protected function applyFix(SplFileInfo $file, Tokens $tokens): void { } } - $tokens->ensureWhitespaceAtIndex($whitespaceIndex, 0, $whitespaceToken); - } - - 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) - /** @var \PhpCsFixer\Tokenizer\Token $equalToken */ - $equalToken = $tokens[$tokens->getNextMeaningfulToken($argument->getNameIndex())]; - if ($equalToken->getContent() === '=') { - $nameLen = mb_strlen($argument->getName()); - $whitespaceIndex = $argument->getNameIndex() + 1; - if ($this->configuration[self::C_DEFAULTS] === true) { - $tokens->ensureWhitespaceAtIndex($whitespaceIndex, 0, str_repeat(' ', $longestVariableName - $nameLen + 1)); - } else { - $tokens->ensureWhitespaceAtIndex($whitespaceIndex, 0, ' '); - } - } + $tokens->ensureWhitespaceAtIndex($whitespaceIndex, 1, $whitespaceToken); } } } @@ -187,25 +191,22 @@ protected function applyFix(SplFileInfo $file, Tokens $tokens): void { /** * TODO: The declaration might be split across multiple lines. * In such case we need to find the longest line and return it as the full type length - * - * @param int $typeIndex points to the beginning of the type */ - private function getFullTypeLength(Tokens $tokens, int $typeIndex): int { + private function getFullTypeLength(Tokens $tokens, TypeAnalysis $typeAnalysis): int { $typeLength = 0; - $varNameTokenIndex = $tokens->getNextTokenOfKind($typeIndex, [[T_VARIABLE]]); - for ($i = $typeIndex; $i < $varNameTokenIndex - 1; $i++) { // -1 to avoid whitespace between param name and type + for ($i = $typeAnalysis->getStartIndex(); $i <= $typeAnalysis->getEndIndex(); $i++) { $typeLength += mb_strlen($tokens[$i]->getContent()); } - $possiblyReadonlyToken = $tokens[$typeIndex - 2]; + $possiblyReadonlyToken = $tokens[$typeAnalysis->getStartIndex() - 2]; if ($possiblyReadonlyToken->isGivenKind($this->parameterModifiers)) { - $whitespaceToken = $tokens[$typeIndex - 1]; + $whitespaceToken = $tokens[$typeAnalysis->getStartIndex() - 1]; $typeLength += strlen($possiblyReadonlyToken->getContent() . $whitespaceToken->getContent()); } - $possiblyPromotionToken = $tokens[$typeIndex - 4]; + $possiblyPromotionToken = $tokens[$typeAnalysis->getStartIndex() - 4]; if ($possiblyPromotionToken->isGivenKind($this->parameterModifiers)) { - $whitespaceToken = $tokens[$typeIndex - 3]; + $whitespaceToken = $tokens[$typeAnalysis->getStartIndex() - 3]; $typeLength += strlen($possiblyPromotionToken->getContent() . $whitespaceToken->getContent()); } diff --git a/tests/FunctionNotation/AlignMultilineParametersFixerTest.php b/tests/FunctionNotation/AlignMultilineParametersFixerTest.php index e670515..69b3c05 100644 --- a/tests/FunctionNotation/AlignMultilineParametersFixerTest.php +++ b/tests/FunctionNotation/AlignMultilineParametersFixerTest.php @@ -253,6 +253,27 @@ public function provideNullCases(): iterable { } } + public function testNoWhitespace(): void { + $this->fixer->configure([ + AlignMultilineParametersFixer::C_VARIABLES => true, + AlignMultilineParametersFixer::C_DEFAULTS => true, + ]); + $this->doTest( + '