mirror of
https://github.com/danog/psalm.git
synced 2024-11-26 20:34:47 +01:00
Mark private properties unused when referenced only in constructor (#1962)
* Mark private properties unused when referenced only in constructor If a private property is used only in constructor then most likely it's a dead code since there is no need to have the class property. But such static properties can be accessed between the calls. * Ignore the private property issue on alter * Fix the related dead code psalm * Add a missing condition into the test
This commit is contained in:
parent
d13d37b75d
commit
f15cc7dd5b
@ -315,7 +315,6 @@ class Codebase
|
||||
);
|
||||
|
||||
$this->methods = new Internal\Codebase\Methods(
|
||||
$config,
|
||||
$providers->classlike_storage_provider,
|
||||
$providers->file_reference_provider,
|
||||
$this->classlikes
|
||||
|
@ -1232,7 +1232,7 @@ class ClassAnalyzer extends ClassLikeAnalyzer
|
||||
if (IssueBuffer::accepts(
|
||||
new PropertyNotSetInConstructor(
|
||||
'Property ' . $property_id . ' is not defined in constructor of ' .
|
||||
$this->fq_class_name . ' or in any methods called in the constructor',
|
||||
$this->fq_class_name . ' and in any methods called in the constructor',
|
||||
$error_location,
|
||||
$property_id
|
||||
),
|
||||
|
@ -3,6 +3,7 @@ namespace Psalm\Internal\Codebase;
|
||||
|
||||
use function array_merge;
|
||||
use function array_pop;
|
||||
use function count;
|
||||
use function end;
|
||||
use function explode;
|
||||
use function get_declared_classes;
|
||||
@ -1688,11 +1689,26 @@ class ClassLikes
|
||||
$codebase = $project_analyzer->getCodebase();
|
||||
|
||||
foreach ($classlike_storage->properties as $property_name => $property_storage) {
|
||||
$referenced_property_name = strtolower($classlike_storage->name) . '::$' . $property_name;
|
||||
$property_referenced = $this->file_reference_provider->isClassPropertyReferenced(
|
||||
strtolower($classlike_storage->name) . '::$' . $property_name
|
||||
$referenced_property_name
|
||||
);
|
||||
|
||||
if (!$property_referenced
|
||||
$property_constructor_referenced = false;
|
||||
if ($property_referenced && $property_storage->visibility === ClassLikeAnalyzer::VISIBILITY_PRIVATE) {
|
||||
$all_method_references = $this->file_reference_provider->getAllMethodReferencesToClassMembers();
|
||||
|
||||
if (isset($all_method_references[$referenced_property_name])
|
||||
&& count($all_method_references[$referenced_property_name]) === 1) {
|
||||
$constructor_name = strtolower($classlike_storage->name) . '::__construct';
|
||||
$property_references = $all_method_references[$referenced_property_name];
|
||||
|
||||
$property_constructor_referenced = isset($property_references[$constructor_name])
|
||||
&& !$property_storage->is_static;
|
||||
}
|
||||
}
|
||||
|
||||
if ((!$property_referenced || $property_constructor_referenced)
|
||||
&& (substr($property_name, 0, 2) !== '__' || $property_name === '__construct')
|
||||
&& $property_storage->location
|
||||
) {
|
||||
@ -1765,7 +1781,8 @@ class ClassLikes
|
||||
);
|
||||
|
||||
if ($codebase->alter_code) {
|
||||
if ($property_storage->stmt_location
|
||||
if (!$property_constructor_referenced
|
||||
&& $property_storage->stmt_location
|
||||
&& isset($project_analyzer->getIssuesToFix()['UnusedProperty'])
|
||||
&& !$has_variable_calls
|
||||
&& !IssueBuffer::isSuppressed($issue, $classlike_storage->suppressed_issues)
|
||||
|
@ -36,11 +36,6 @@ class Methods
|
||||
*/
|
||||
private $classlike_storage_provider;
|
||||
|
||||
/**
|
||||
* @var \Psalm\Config
|
||||
*/
|
||||
private $config;
|
||||
|
||||
/**
|
||||
* @var bool
|
||||
*/
|
||||
@ -72,13 +67,11 @@ class Methods
|
||||
* @param ClassLikeStorageProvider $storage_provider
|
||||
*/
|
||||
public function __construct(
|
||||
\Psalm\Config $config,
|
||||
ClassLikeStorageProvider $storage_provider,
|
||||
FileReferenceProvider $file_reference_provider,
|
||||
ClassLikes $classlikes
|
||||
) {
|
||||
$this->classlike_storage_provider = $storage_provider;
|
||||
$this->config = $config;
|
||||
$this->file_reference_provider = $file_reference_provider;
|
||||
$this->classlikes = $classlikes;
|
||||
$this->return_type_provider = new MethodReturnTypeProvider();
|
||||
|
@ -580,6 +580,32 @@ class UnusedCodeTest extends TestCase
|
||||
new B();',
|
||||
'error_message' => 'PossiblyUnusedProperty',
|
||||
],
|
||||
'propertyUsedOnlyInConstructor' => [
|
||||
'<?php
|
||||
class A {
|
||||
/** @var int */
|
||||
private $used;
|
||||
|
||||
/** @var int */
|
||||
private $unused;
|
||||
|
||||
/** @var int */
|
||||
private static $staticUnused;
|
||||
|
||||
public function __construct() {
|
||||
$this->used = 4;
|
||||
$this->unused = 4;
|
||||
self::$staticUnused = 4;
|
||||
}
|
||||
|
||||
public function handle(): void
|
||||
{
|
||||
$this->used++;
|
||||
}
|
||||
}
|
||||
(new A())->handle();',
|
||||
'error_message' => 'UnusedProperty',
|
||||
],
|
||||
];
|
||||
}
|
||||
}
|
||||
|
Loading…
Reference in New Issue
Block a user