Skip to content

Commit 12185ab

Browse files
committed
Better error message for only readable/only writable properties in UnusedPrivatePropertyRule
1 parent dc576b9 commit 12185ab

File tree

3 files changed

+61
-9
lines changed

3 files changed

+61
-9
lines changed

src/Rules/DeadCode/UnusedPrivatePropertyRule.php

Lines changed: 25 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
use function array_map;
1717
use function count;
1818
use function is_string;
19+
use function lcfirst;
1920
use function sprintf;
2021
use function str_contains;
2122

@@ -111,6 +112,8 @@ public function processNode(Node $node, Scope $scope): array
111112
'read' => $read,
112113
'written' => $written,
113114
'node' => $property,
115+
'onlyReadable' => $property->isReadable() && !$property->isWritable(),
116+
'onlyWritable' => $property->isWritable() && !$property->isReadable(),
114117
];
115118
}
116119

@@ -222,18 +225,32 @@ public function processNode(Node $node, Scope $scope): array
222225
->identifier('property.unused')
223226
->build();
224227
} else {
225-
$errors[] = RuleErrorBuilder::message(sprintf('%s is never read, only written.', $propertyName))
228+
if ($data['onlyReadable']) {
229+
$errors[] = RuleErrorBuilder::message(sprintf('Readable %s is never read.', lcfirst($propertyName)))
230+
->line($propertyNode->getStartLine())
231+
->identifier('property.neverRead')
232+
->build();
233+
} else {
234+
$errors[] = RuleErrorBuilder::message(sprintf('%s is never read, only written.', $propertyName))
235+
->line($propertyNode->getStartLine())
236+
->identifier('property.onlyWritten')
237+
->tip($tip)
238+
->build();
239+
}
240+
}
241+
} elseif (!$data['written'] && (!array_key_exists($name, $uninitializedProperties) || !$this->checkUninitializedProperties)) {
242+
if ($data['onlyWritable']) {
243+
$errors[] = RuleErrorBuilder::message(sprintf('Writable %s is never written.', lcfirst($propertyName)))
226244
->line($propertyNode->getStartLine())
227-
->identifier('property.onlyWritten')
245+
->identifier('property.neverWritten')
246+
->build();
247+
} else {
248+
$errors[] = RuleErrorBuilder::message(sprintf('%s is never written, only read.', $propertyName))
249+
->line($propertyNode->getStartLine())
250+
->identifier('property.onlyRead')
228251
->tip($tip)
229252
->build();
230253
}
231-
} elseif (!$data['written'] && (!array_key_exists($name, $uninitializedProperties) || !$this->checkUninitializedProperties)) {
232-
$errors[] = RuleErrorBuilder::message(sprintf('%s is never written, only read.', $propertyName))
233-
->line($propertyNode->getStartLine())
234-
->identifier('property.onlyRead')
235-
->tip($tip)
236-
->build();
237254
}
238255
}
239256

tests/PHPStan/Rules/DeadCode/UnusedPrivatePropertyRuleTest.php

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -408,7 +408,16 @@ public function testBug12702(): void
408408
$this->alwaysWrittenTags = [];
409409
$this->alwaysReadTags = [];
410410

411-
$this->analyse([__DIR__ . '/data/bug-12702.php'], []);
411+
$this->analyse([__DIR__ . '/data/bug-12702.php'], [
412+
[
413+
'Readable property Bug12702\Foo2::$i is never read.',
414+
43,
415+
],
416+
[
417+
'Writable property Bug12702\Bar2::$i is never written.',
418+
54,
419+
],
420+
]);
412421
}
413422

414423
}

tests/PHPStan/Rules/DeadCode/data/bug-12702.php

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,3 +33,29 @@ public function x(): void {
3333
$this->i = 'foo';
3434
}
3535
}
36+
37+
class Foo2
38+
{
39+
/**
40+
* @var string[]
41+
*/
42+
public array $x = [];
43+
private ?string $i { get => $this->x[$this->k] ?? null; }
44+
private int $k = 0;
45+
46+
}
47+
48+
class Bar2
49+
{
50+
/**
51+
* @var string[]
52+
*/
53+
public array $x = [];
54+
private ?string $i {
55+
set {
56+
$this->x[$this->k] = $value;
57+
}
58+
}
59+
private int $k = 0;
60+
61+
}

0 commit comments

Comments
 (0)