Skip to content
Permalink
Browse files

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
  • Loading branch information...
2e3s authored and muglug committed Jul 22, 2019
1 parent d13d37b commit f15cc7dd5bb7be7f0deb97c587dadb83c82ea24a
@@ -315,7 +315,6 @@ public function __construct(
);
$this->methods = new Internal\Codebase\Methods(
$config,
$providers->classlike_storage_provider,
$providers->file_reference_provider,
$this->classlikes
@@ -1232,7 +1232,7 @@ function (FunctionLikeParameter $param) : PhpParser\Node\Arg {
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 @@
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 @@ private function checkPropertyReferences(ClassLikeStorage $classlike_storage)
$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 @@ private function checkPropertyReferences(ClassLikeStorage $classlike_storage)
);
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 B extends A {
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',
],
];
}
}

0 comments on commit f15cc7d

Please sign in to comment.
You can’t perform that action at this time.