Skip to content

Commit 7b53675

Browse files
erickskraucherickskrauch
committed
Fixes #8. ErickSkrauch/align_multiline_parameters produces new line for promoted properties without any types
1 parent ec69923 commit 7b53675

File tree

3 files changed

+123
-53
lines changed

3 files changed

+123
-53
lines changed

‎CHANGELOG.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)
55
and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).
66

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

911
## [1.2.2] - 2024-01-07
1012
### Added

‎src/FunctionNotation/AlignMultilineParametersFixer.php‎

Lines changed: 74 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,12 @@
2525
* variables: bool|null,
2626
* defaults: bool|null,
2727
* } $configuration
28+
*
29+
* @phpstan-type DeclarationAnalysis array{
30+
* typeLength: non-negative-int,
31+
* nameLength: positive-int,
32+
* nameIndex: int,
33+
* }
2834
*/
2935
final class AlignMultilineParametersFixer extends AbstractFixer implements ConfigurableFixerInterface, WhitespacesAwareFixerInterface {
3036

@@ -40,19 +46,11 @@ final class AlignMultilineParametersFixer extends AbstractFixer implements Confi
4046
/**
4147
* @var list<int>
4248
*/
43-
private array $parameterModifiers;
44-
45-
public function __construct() {
46-
parent::__construct();
47-
$this->parameterModifiers = [
48-
CT::T_CONSTRUCTOR_PROPERTY_PROMOTION_PUBLIC,
49-
CT::T_CONSTRUCTOR_PROPERTY_PROMOTION_PROTECTED,
50-
CT::T_CONSTRUCTOR_PROPERTY_PROMOTION_PRIVATE,
51-
];
52-
if (defined('T_READONLY')) {
53-
$this->parameterModifiers[] = T_READONLY;
54-
}
55-
}
49+
private const PROMOTIONAL_TOKENS = [
50+
CT::T_CONSTRUCTOR_PROPERTY_PROMOTION_PUBLIC,
51+
CT::T_CONSTRUCTOR_PROPERTY_PROMOTION_PROTECTED,
52+
CT::T_CONSTRUCTOR_PROPERTY_PROMOTION_PRIVATE,
53+
];
5654

5755
public function getDefinition(): FixerDefinitionInterface {
5856
return new FixerDefinition(
@@ -133,62 +131,57 @@ protected function applyFix(SplFileInfo $file, Tokens $tokens): void {
133131
$longestType = 0;
134132
$longestVariableName = 0;
135133
$hasAtLeastOneTypedArgument = false;
134+
/** @var list<DeclarationAnalysis> $analysedArguments */
135+
$analysedArguments = [];
136136
foreach ($arguments as $argument) {
137137
$typeAnalysis = $argument->getTypeAnalysis();
138-
if ($typeAnalysis !== null) {
138+
$declarationAnalysis = $this->getDeclarationAnalysis($tokens, $argument->getNameIndex(), $typeAnalysis);
139+
if ($declarationAnalysis['typeLength'] > 0) {
139140
$hasAtLeastOneTypedArgument = true;
140-
$typeLength = $this->getFullTypeLength($tokens, $typeAnalysis);
141-
if ($typeLength > $longestType) {
142-
$longestType = $typeLength;
143-
}
144141
}
145142

146-
$variableNameLength = mb_strlen($argument->getName());
147-
if ($variableNameLength > $longestVariableName) {
148-
$longestVariableName = $variableNameLength;
143+
if ($declarationAnalysis['typeLength'] > $longestType) {
144+
$longestType = $declarationAnalysis['typeLength'];
145+
}
146+
147+
if ($declarationAnalysis['nameLength'] > $longestVariableName) {
148+
$longestVariableName = $declarationAnalysis['nameLength'];
149149
}
150+
151+
$analysedArguments[] = $declarationAnalysis;
150152
}
151153

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

171172
if ($this->configuration[self::C_VARIABLES] !== null) {
172-
$whitespaceIndex = $argument->getNameIndex() - 1;
173+
$whitespaceIndex = $argument['nameIndex'] - 1;
173174
if ($this->configuration[self::C_VARIABLES] === true) {
174-
$typeLen = 0;
175-
$typeAnalysis = $argument->getTypeAnalysis();
176-
if ($typeAnalysis !== null) {
177-
$typeLen = $this->getFullTypeLength($tokens, $typeAnalysis);
178-
}
179-
180-
$appendix = str_repeat(' ', $longestType - $typeLen + (int)$hasAtLeastOneTypedArgument);
181-
if ($argument->hasTypeAnalysis()) {
175+
$appendix = str_repeat(' ', $longestType - $argument['typeLength'] + (int)$hasAtLeastOneTypedArgument);
176+
if ($argument['typeLength'] > 0) {
182177
$whitespaceToken = $appendix;
183178
} else {
184179
$whitespaceToken = $this->whitespacesConfig->getLineEnding() . $argsIndent . $appendix;
185180
}
181+
} elseif ($argument['typeLength'] > 0) {
182+
$whitespaceToken = ' ';
186183
} else {
187-
if ($argument->hasTypeAnalysis()) {
188-
$whitespaceToken = ' ';
189-
} else {
190-
$whitespaceToken = $this->whitespacesConfig->getLineEnding() . $argsIndent;
191-
}
184+
$whitespaceToken = $this->whitespacesConfig->getLineEnding() . $argsIndent;
192185
}
193186

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

200-
private function getFullTypeLength(Tokens $tokens, TypeAnalysis $typeAnalysis): int {
193+
/**
194+
* @phpstan-return DeclarationAnalysis
195+
*/
196+
private function getDeclarationAnalysis(Tokens $tokens, int $nameIndex, ?TypeAnalysis $typeAnalysis): array {
197+
$searchIndex = $nameIndex;
198+
$includeNextWhitespace = false;
201199
$typeLength = 0;
202-
for ($i = $typeAnalysis->getStartIndex(); $i <= $typeAnalysis->getEndIndex(); $i++) {
203-
$typeLength += mb_strlen($tokens[$i]->getContent());
200+
if ($typeAnalysis !== null) {
201+
$searchIndex = $typeAnalysis->getStartIndex();
202+
$includeNextWhitespace = true;
203+
for ($i = $typeAnalysis->getStartIndex(); $i <= $typeAnalysis->getEndIndex(); $i++) {
204+
$typeLength += mb_strlen($tokens[$i]->getContent());
205+
}
204206
}
205207

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

212-
$possiblyPromotionToken = $tokens[$typeAnalysis->getStartIndex() - 4];
213-
if ($possiblyPromotionToken->isGivenKind($this->parameterModifiers)) {
214-
$whitespaceToken = $tokens[$typeAnalysis->getStartIndex() - 3];
215-
$typeLength += strlen($possiblyPromotionToken->getContent() . $whitespaceToken->getContent());
219+
$promotionTokenIndex = $tokens->getPrevMeaningfulToken($searchIndex);
220+
$promotionToken = $tokens[$promotionTokenIndex];
221+
if ($promotionToken->isGivenKind(self::PROMOTIONAL_TOKENS)) {
222+
$promotionalStr = $promotionToken->getContent();
223+
if ($includeNextWhitespace) {
224+
$whitespaceToken = $tokens[$promotionTokenIndex + 1];
225+
if ($whitespaceToken->isWhitespace()) {
226+
$promotionalStr .= $whitespaceToken->getContent();
227+
}
228+
}
229+
230+
$typeLength += strlen($promotionalStr);
216231
}
217232

218-
return $typeLength;
233+
/** @var positive-int $nameLength force type for PHPStan to avoid type error on return statement */
234+
$nameLength = mb_strlen($tokens[$nameIndex]->getContent());
235+
236+
return [
237+
'typeLength' => $typeLength,
238+
'nameLength' => $nameLength,
239+
'nameIndex' => $nameIndex,
240+
];
219241
}
220242

221243
}

‎tests/FunctionNotation/AlignMultilineParametersFixerTest.php‎

Lines changed: 47 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -299,7 +299,25 @@ public function test80BothTrue(string $expected, ?string $input = null): void {
299299
* @return iterable<array{0: string, 1?: string}>
300300
*/
301301
public function provide80TrueCases(): iterable {
302-
yield 'constructor promotion, defaults' => [
302+
yield 'constructor promotion without types, defaults' => [
303+
'<?php
304+
class Test {
305+
public function __construct(
306+
public $string = "string",
307+
protected $bool = true
308+
) {}
309+
}
310+
',
311+
'<?php
312+
class Test {
313+
public function __construct(
314+
public $string = "string",
315+
protected $bool = true
316+
) {}
317+
}
318+
',
319+
];
320+
yield 'constructor promotion with types, defaults' => [
303321
'<?php
304322
class Test {
305323
public function __construct(
@@ -344,6 +362,34 @@ public function provideFalse80Cases(): iterable {
344362
}
345363
}
346364

365+
/**
366+
* @requires PHP 8.0
367+
*/
368+
public function testNoWhitespaceAroundPromotion(): void {
369+
$this->fixer->configure([
370+
AlignMultilineParametersFixer::C_VARIABLES => true,
371+
AlignMultilineParametersFixer::C_DEFAULTS => true,
372+
]);
373+
$this->doTest(
374+
'<?php
375+
class Test {
376+
public function __construct(
377+
public $string = "string",
378+
protected $bool = true
379+
) {}
380+
}
381+
',
382+
'<?php
383+
class Test {
384+
public function __construct(
385+
public$string = "string",
386+
protected$bool = true
387+
) {}
388+
}
389+
',
390+
);
391+
}
392+
347393
/**
348394
* @dataProvider provide81TrueCases
349395
* @requires PHP 8.1

0 commit comments

Comments
 (0)