Skip to content

Commit 784bb29

Browse files
committed
RequireConstructorPropertyPromotionSniff: Fixed false positive
1 parent 8e049b5 commit 784bb29

File tree

4 files changed

+75
-5
lines changed

4 files changed

+75
-5
lines changed

SlevomatCodingStandard/Sniffs/Classes/RequireConstructorPropertyPromotionSniff.php

Lines changed: 35 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -132,8 +132,12 @@ public function process(File $phpcsFile, $functionPointer): void
132132
continue;
133133
}
134134

135-
$assignment = $this->getAssignment($phpcsFile, $functionPointer, $parameterName);
136-
if ($assignment === null) {
135+
$assignmentPointer = $this->getAssignment($phpcsFile, $functionPointer, $parameterName);
136+
if ($assignmentPointer === null) {
137+
continue;
138+
}
139+
140+
if ($this->isParameterModifiedBeforeAssigment($phpcsFile, $functionPointer, $parameterName, $assignmentPointer)) {
137141
continue;
138142
}
139143

@@ -174,8 +178,8 @@ public function process(File $phpcsFile, $functionPointer): void
174178
$parameterEqualPointer = TokenHelper::findNextEffective($phpcsFile, $parameterPointer + 1);
175179
$parameterHasDefaultValue = $tokens[$parameterEqualPointer]['code'] === T_EQUAL;
176180

177-
$pointerBeforeAssignment = TokenHelper::findFirstTokenOnLine($phpcsFile, $assignment - 1);
178-
$pointerAfterAssignment = TokenHelper::findLastTokenOnLine($phpcsFile, $assignment);
181+
$pointerBeforeAssignment = TokenHelper::findFirstTokenOnLine($phpcsFile, $assignmentPointer - 1);
182+
$pointerAfterAssignment = TokenHelper::findLastTokenOnLine($phpcsFile, $assignmentPointer);
179183

180184
$phpcsFile->fixer->beginChangeset();
181185

@@ -298,4 +302,31 @@ private function isPropertyDocCommentUseful(File $phpcsFile, int $propertyPointe
298302
return false;
299303
}
300304

305+
private function isParameterModifiedBeforeAssigment(
306+
File $phpcsFile,
307+
int $functionPointer,
308+
string $parameterName,
309+
int $assigmentPointer
310+
): bool
311+
{
312+
$tokens = $phpcsFile->getTokens();
313+
314+
for ($i = $assigmentPointer - 1; $i > $tokens[$functionPointer]['scope_opener']; $i--) {
315+
if ($tokens[$i]['code'] !== T_VARIABLE) {
316+
continue;
317+
}
318+
319+
if ($tokens[$i]['content'] !== $parameterName) {
320+
continue;
321+
}
322+
323+
$nextPointer = TokenHelper::findNextEffective($phpcsFile, $i + 1);
324+
if (in_array($tokens[$nextPointer]['code'], Tokens::$assignmentTokens, true)) {
325+
return true;
326+
}
327+
}
328+
329+
return false;
330+
}
331+
301332
}

tests/Sniffs/Classes/RequireConstructorPropertyPromotionSniffTest.php

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ public function testErrors(): void
2121
'enable' => true,
2222
]);
2323

24-
self::assertSame(3, $report->getErrorCount());
24+
self::assertSame(4, $report->getErrorCount());
2525

2626
self::assertSniffError(
2727
$report,
@@ -41,6 +41,12 @@ public function testErrors(): void
4141
RequireConstructorPropertyPromotionSniff::CODE_REQUIRED_CONSTRUCTOR_PROPERTY_PROMOTION,
4242
'Required promotion of property $c.'
4343
);
44+
self::assertSniffError(
45+
$report,
46+
29,
47+
RequireConstructorPropertyPromotionSniff::CODE_REQUIRED_CONSTRUCTOR_PROPERTY_PROMOTION,
48+
'Required promotion of property $from.'
49+
);
4450

4551
self::assertAllFixedInFile($report);
4652
}

tests/Sniffs/Classes/data/requireConstructorPropertyPromotionErrors.fixed.php

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,3 +8,18 @@ public function __construct(public string $a, protected int|null $b = 0, private
88
}
99

1010
}
11+
12+
class DontKnow
13+
{
14+
15+
private bool $active;
16+
17+
public function __construct(bool $active = false, private ?DateTimeImmutable $from = null)
18+
{
19+
if ($from !== null) {
20+
$active = true;
21+
}
22+
$this->active = $active;
23+
}
24+
25+
}

tests/Sniffs/Classes/data/requireConstructorPropertyPromotionErrors.php

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,3 +20,21 @@ public function __construct(string $a, int|null $b = 0, ?bool $c)
2020
}
2121

2222
}
23+
24+
class DontKnow
25+
{
26+
27+
private bool $active;
28+
29+
private ?DateTimeImmutable $from = null;
30+
31+
public function __construct(bool $active = false, ?DateTimeImmutable $from = null)
32+
{
33+
if ($from !== null) {
34+
$active = true;
35+
}
36+
$this->active = $active;
37+
$this->from = $from;
38+
}
39+
40+
}

0 commit comments

Comments
 (0)