1
0
mirror of https://github.com/danog/psalm.git synced 2024-12-02 09:37:59 +01:00

Find unused properties with dead code checks

Fixes #424
This commit is contained in:
Matthew Brown 2018-01-10 23:29:18 -05:00
parent d93906243d
commit fb9f20f4b8
25 changed files with 190 additions and 177 deletions

View File

@ -203,6 +203,7 @@
<xs:element name="PossiblyUndefinedVariable" type="IssueHandlerType" minOccurs="0" /> <xs:element name="PossiblyUndefinedVariable" type="IssueHandlerType" minOccurs="0" />
<xs:element name="PossiblyUnusedMethod" type="IssueHandlerType" minOccurs="0" /> <xs:element name="PossiblyUnusedMethod" type="IssueHandlerType" minOccurs="0" />
<xs:element name="PossiblyUnusedParam" type="IssueHandlerType" minOccurs="0" /> <xs:element name="PossiblyUnusedParam" type="IssueHandlerType" minOccurs="0" />
<xs:element name="PossiblyUnusedProperty" type="IssueHandlerType" minOccurs="0" />
<xs:element name="PropertyNotSetInConstructor" type="IssueHandlerType" minOccurs="0" /> <xs:element name="PropertyNotSetInConstructor" type="IssueHandlerType" minOccurs="0" />
<xs:element name="RawObjectIteration" type="IssueHandlerType" minOccurs="0" /> <xs:element name="RawObjectIteration" type="IssueHandlerType" minOccurs="0" />
<xs:element name="RedundantCondition" type="IssueHandlerType" minOccurs="0" /> <xs:element name="RedundantCondition" type="IssueHandlerType" minOccurs="0" />
@ -233,6 +234,7 @@
<xs:element name="UnevaluatedCode" type="IssueHandlerType" minOccurs="0" /> <xs:element name="UnevaluatedCode" type="IssueHandlerType" minOccurs="0" />
<xs:element name="UnusedVariable" type="IssueHandlerType" minOccurs="0" /> <xs:element name="UnusedVariable" type="IssueHandlerType" minOccurs="0" />
<xs:element name="UnusedParam" type="IssueHandlerType" minOccurs="0" /> <xs:element name="UnusedParam" type="IssueHandlerType" minOccurs="0" />
<xs:element name="UnusedProperty" type="IssueHandlerType" minOccurs="0" />
<xs:element name="UnusedClass" type="IssueHandlerType" minOccurs="0" /> <xs:element name="UnusedClass" type="IssueHandlerType" minOccurs="0" />
<xs:element name="UnusedMethod" type="IssueHandlerType" minOccurs="0" /> <xs:element name="UnusedMethod" type="IssueHandlerType" minOccurs="0" />
</xs:choice> </xs:choice>

View File

@ -45,6 +45,18 @@
</errorLevel> </errorLevel>
</UnusedClass> </UnusedClass>
<UnusedProperty>
<errorLevel type="info">
<file name="src/Psalm/FileManipulation/FunctionDocblockManipulator.php" />
</errorLevel>
</UnusedProperty>
<PossiblyUnusedProperty>
<errorLevel type="info">
<file name="src/Psalm/Storage/FunctionLikeStorage.php" />
</errorLevel>
</PossiblyUnusedProperty>
<PossiblyUnusedMethod> <PossiblyUnusedMethod>
<errorLevel type="suppress"> <errorLevel type="suppress">
<directory name="tests" /> <directory name="tests" />

View File

@ -88,11 +88,6 @@ abstract class ClassLikeChecker extends SourceChecker implements StatementsSourc
*/ */
protected $fq_class_name; protected $fq_class_name;
/**
* @var bool
*/
protected $has_custom_get = false;
/** /**
* The parent class * The parent class
* *
@ -100,16 +95,6 @@ abstract class ClassLikeChecker extends SourceChecker implements StatementsSourc
*/ */
protected $parent_fq_class_name; protected $parent_fq_class_name;
/**
* @var array<string, MethodChecker>
*/
protected $method_checkers = [];
/**
* @var array<string, MethodChecker>
*/
protected $property_types = [];
/** /**
* @var array<string, array<string, string>>|null * @var array<string, array<string, string>>|null
*/ */
@ -1552,8 +1537,11 @@ abstract class ClassLikeChecker extends SourceChecker implements StatementsSourc
* *
* @return bool * @return bool
*/ */
public static function propertyExists(ProjectChecker $project_checker, $property_id) public static function propertyExists(
{ ProjectChecker $project_checker,
$property_id,
CodeLocation $code_location = null
) {
// remove trailing backslash if it exists // remove trailing backslash if it exists
$property_id = preg_replace('/^\\\\/', '', $property_id); $property_id = preg_replace('/^\\\\/', '', $property_id);
@ -1562,6 +1550,20 @@ abstract class ClassLikeChecker extends SourceChecker implements StatementsSourc
$class_storage = $project_checker->classlike_storage_provider->get($fq_class_name); $class_storage = $project_checker->classlike_storage_provider->get($fq_class_name);
if (isset($class_storage->declaring_property_ids[$property_name])) { if (isset($class_storage->declaring_property_ids[$property_name])) {
if ($project_checker->collect_references && $code_location) {
$declaring_property_id = $class_storage->declaring_property_ids[$property_name];
list($declaring_property_class, $declaring_property_name) = explode('::$', $declaring_property_id);
$declaring_class_storage = $project_checker->classlike_storage_provider->get($declaring_property_class);
$declaring_property_storage = $declaring_class_storage->properties[$declaring_property_name];
if ($declaring_property_storage->referencing_locations === null) {
$declaring_property_storage->referencing_locations = [];
}
$declaring_property_storage->referencing_locations[$code_location->file_path][] = $code_location;
}
return true; return true;
} }

View File

@ -48,16 +48,6 @@ class FileChecker extends SourceChecker implements StatementsSource
*/ */
protected $namespace_aliased_classes_flipped = []; protected $namespace_aliased_classes_flipped = [];
/**
* @var array<int, \PhpParser\Node\Stmt>
*/
protected $preloaded_statements = [];
/**
* @var bool
*/
public static $show_notices = true;
/** /**
* @var array<string, ClassLikeChecker> * @var array<string, ClassLikeChecker>
*/ */
@ -73,16 +63,6 @@ class FileChecker extends SourceChecker implements StatementsSource
*/ */
protected $function_checkers = []; protected $function_checkers = [];
/**
* @var array<int, NamespaceChecker>
*/
protected $namespace_checkers = [];
/**
* @var array<string, bool>
*/
private $included_file_paths = [];
/** /**
* @var ?Context * @var ?Context
*/ */

View File

@ -59,11 +59,6 @@ abstract class FunctionLikeChecker extends SourceChecker implements StatementsSo
*/ */
protected $is_static = false; protected $is_static = false;
/**
* @var StatementsChecker|null
*/
protected $statements_checker;
/** /**
* @var StatementsSource * @var StatementsSource
*/ */
@ -259,11 +254,6 @@ abstract class FunctionLikeChecker extends SourceChecker implements StatementsSo
$statements_checker = new StatementsChecker($this); $statements_checker = new StatementsChecker($this);
// this increases memory, so only do it if running under this flag
if ($project_checker->infer_types_from_usage) {
$this->statements_checker = $statements_checker;
}
$template_types = $storage->template_types; $template_types = $storage->template_types;
if ($class_storage && $class_storage->template_types) { if ($class_storage && $class_storage->template_types) {
@ -439,7 +429,23 @@ abstract class FunctionLikeChecker extends SourceChecker implements StatementsSo
[], [],
false false
); );
} elseif (!$this->function instanceof Closure) { } else {
$fleshed_out_signature_type = ExpressionChecker::fleshOutType(
$project_checker,
$storage->signature_return_type,
$context->self,
$cased_method_id
);
$fleshed_out_signature_type->check(
$this,
$storage->signature_return_type_location ?: $storage->return_type_location,
$storage->suppressed_issues,
[],
false
);
if (!$this->function instanceof Closure) {
$fleshed_out_return_type = ExpressionChecker::fleshOutType( $fleshed_out_return_type = ExpressionChecker::fleshOutType(
$project_checker, $project_checker,
$storage->return_type, $storage->return_type,
@ -478,14 +484,7 @@ abstract class FunctionLikeChecker extends SourceChecker implements StatementsSo
)) { )) {
return false; return false;
} }
}
$storage->signature_return_type->check(
$this,
$storage->return_type_location,
$storage->suppressed_issues,
[],
false
);
} }
} }
} }

View File

@ -177,7 +177,6 @@ class MethodChecker extends FunctionLikeChecker
$declaring_class = $method->getDeclaringClass(); $declaring_class = $method->getDeclaringClass();
$storage->is_static = $method->isStatic(); $storage->is_static = $method->isStatic();
$storage->namespace = $declaring_class->getNamespaceName();
$class_storage->declaring_method_ids[$method_name] = $class_storage->declaring_method_ids[$method_name] =
$declaring_class->name . '::' . strtolower((string)$method->getName()); $declaring_class->name . '::' . strtolower((string)$method->getName());

View File

@ -26,21 +26,6 @@ class NamespaceChecker extends SourceChecker implements StatementsSource
*/ */
private $namespace_name; private $namespace_name;
/**
* @var array<int, FunctionChecker>
*/
public $function_checkers = [];
/**
* @var array<int, ClassChecker>
*/
public $class_checkers = [];
/**
* @var array<int, ClassChecker>
*/
public $interface_checkers = [];
/** /**
* A lookup table for public namespace constants * A lookup table for public namespace constants
* *

View File

@ -9,8 +9,10 @@ use Psalm\FileManipulation\FunctionDocblockManipulator;
use Psalm\Issue\CircularReference; use Psalm\Issue\CircularReference;
use Psalm\Issue\PossiblyUnusedMethod; use Psalm\Issue\PossiblyUnusedMethod;
use Psalm\Issue\PossiblyUnusedParam; use Psalm\Issue\PossiblyUnusedParam;
use Psalm\Issue\PossiblyUnusedProperty;
use Psalm\Issue\UnusedClass; use Psalm\Issue\UnusedClass;
use Psalm\Issue\UnusedMethod; use Psalm\Issue\UnusedMethod;
use Psalm\Issue\UnusedProperty;
use Psalm\IssueBuffer; use Psalm\IssueBuffer;
use Psalm\Provider\ClassLikeStorageProvider; use Psalm\Provider\ClassLikeStorageProvider;
use Psalm\Provider\FileProvider; use Psalm\Provider\FileProvider;
@ -181,11 +183,6 @@ class ProjectChecker
*/ */
private $scanned_files = []; private $scanned_files = [];
/**
* @var array<string, bool>
*/
private $visited_classes = [];
/** /**
* @var array<string, FileChecker> * @var array<string, FileChecker>
*/ */
@ -1274,6 +1271,37 @@ class ProjectChecker
} }
} }
} }
foreach ($classlike_storage->properties as $property_name => $property_storage) {
if (($property_storage->referencing_locations === null
|| count($property_storage->referencing_locations) === 0)
&& (substr($property_name, 0, 2) !== '__' || $property_name === '__construct')
&& $property_storage->location
) {
$property_id = $classlike_storage->name . '::$' . $property_name;
if ($property_storage->visibility === ClassLikeChecker::VISIBILITY_PUBLIC) {
if (IssueBuffer::accepts(
new PossiblyUnusedProperty(
'Cannot find public calls to property ' . $property_id,
$property_storage->location
),
$classlike_storage->suppressed_issues
)) {
// fall through
}
} elseif (!isset($classlike_storage->declaring_method_ids['__get'])) {
if (IssueBuffer::accepts(
new UnusedProperty(
'Property ' . $property_id . ' is never used',
$property_storage->location
)
)) {
// fall through
}
}
}
}
} }
/** /**

View File

@ -89,7 +89,6 @@ class IfChecker
$context->inside_conditional = false; $context->inside_conditional = false;
$if_scope = new IfScope(); $if_scope = new IfScope();
$if_scope->has_elseifs = count($stmt->elseifs) > 0;
$if_context = clone $context; $if_context = clone $context;

View File

@ -80,6 +80,8 @@ class FetchChecker
return false; return false;
} }
$project_checker = $statements_checker->getFileChecker()->project_checker;
$stmt_var_id = ExpressionChecker::getVarId( $stmt_var_id = ExpressionChecker::getVarId(
$stmt->var, $stmt->var,
$statements_checker->getFQCLN(), $statements_checker->getFQCLN(),
@ -98,6 +100,25 @@ class FetchChecker
// we don't need to check anything // we don't need to check anything
$stmt->inferredType = $context->vars_in_scope[$var_id]; $stmt->inferredType = $context->vars_in_scope[$var_id];
if ($context->collect_references
&& isset($stmt->var->inferredType)
&& $stmt->var->inferredType->hasObjectType()
&& is_string($stmt->name)
) {
// log the appearance
foreach ($stmt->var->inferredType->getTypes() as $lhs_type_part) {
if ($lhs_type_part instanceof TNamedObject) {
$property_id = $lhs_type_part->value . '::$' . $stmt->name;
ClassLikeChecker::propertyExists(
$project_checker,
$property_id,
new CodeLocation($statements_checker->getSource(), $stmt)
);
}
}
}
return null; return null;
} }
@ -208,8 +229,6 @@ class FetchChecker
continue; continue;
} }
$project_checker = $statements_checker->getFileChecker()->project_checker;
if (!ClassChecker::classExists($project_checker, $lhs_type_part->value)) { if (!ClassChecker::classExists($project_checker, $lhs_type_part->value)) {
if (InterfaceChecker::interfaceExists($project_checker, $lhs_type_part->value)) { if (InterfaceChecker::interfaceExists($project_checker, $lhs_type_part->value)) {
if (IssueBuffer::accepts( if (IssueBuffer::accepts(
@ -260,7 +279,12 @@ class FetchChecker
} }
} }
if (!ClassLikeChecker::propertyExists($project_checker, $property_id)) { if (!ClassLikeChecker::propertyExists(
$project_checker,
$property_id,
$context->collect_references ? new CodeLocation($statements_checker->getSource(), $stmt) : null
)
) {
if ($context->inside_isset) { if ($context->inside_isset) {
return; return;
} }
@ -699,16 +723,30 @@ class FetchChecker
$statements_checker $statements_checker
); );
$property_id = $fq_class_name . '::$' . $stmt->name;
if ($var_id && $context->hasVariable($var_id)) { if ($var_id && $context->hasVariable($var_id)) {
// we don't need to check anything // we don't need to check anything
$stmt->inferredType = $context->vars_in_scope[$var_id]; $stmt->inferredType = $context->vars_in_scope[$var_id];
if ($context->collect_references) {
// log the appearance
ClassLikeChecker::propertyExists(
$project_checker,
$property_id,
new CodeLocation($statements_checker->getSource(), $stmt)
);
}
return null; return null;
} }
$property_id = $fq_class_name . '::$' . $stmt->name; if (!ClassLikeChecker::propertyExists(
$project_checker,
if (!ClassLikeChecker::propertyExists($project_checker, $property_id)) { $property_id,
$context->collect_references ? new CodeLocation($statements_checker->getSource(), $stmt) : null
)
) {
if (IssueBuffer::accepts( if (IssueBuffer::accepts(
new UndefinedPropertyFetch( new UndefinedPropertyFetch(
'Static property ' . $property_id . ' is not defined', 'Static property ' . $property_id . ' is not defined',

View File

@ -110,13 +110,6 @@ class Config
*/ */
protected $base_dir; protected $base_dir;
/**
* The path to this config file
*
* @var string
*/
public $file_path;
/** /**
* @var array<int, string> * @var array<int, string>
*/ */
@ -186,9 +179,6 @@ class Config
*/ */
private $plugins = []; private $plugins = [];
/** @var array<string, bool> */
private $predefined_classlikes = [];
/** @var array<string, mixed> */ /** @var array<string, mixed> */
private $predefined_constants; private $predefined_constants;
@ -238,8 +228,6 @@ class Config
{ {
$config = new static(); $config = new static();
$config->file_path = $file_path;
$config->base_dir = $base_dir; $config->base_dir = $base_dir;
$schema_path = dirname(dirname(__DIR__)) . '/config.xsd'; $schema_path = dirname(dirname(__DIR__)) . '/config.xsd';

View File

@ -21,11 +21,6 @@ class FileFilter
*/ */
protected $files_lowercase = []; protected $files_lowercase = [];
/**
* @var array<string>
*/
protected $patterns = [];
/** /**
* @var bool * @var bool
*/ */

View File

@ -0,0 +1,6 @@
<?php
namespace Psalm\Issue;
class PossiblyUnusedProperty extends CodeIssue
{
}

View File

@ -0,0 +1,6 @@
<?php
namespace Psalm\Issue;
class UnusedProperty extends CodeIssue
{
}

View File

@ -46,11 +46,6 @@ class IfScope
*/ */
public $negatable_if_types = null; public $negatable_if_types = null;
/**
* @var bool
*/
public $has_elseifs;
/** /**
* @var array<int, Clause> * @var array<int, Clause>
*/ */

View File

@ -32,11 +32,6 @@ class ClassLikeStorage
*/ */
public $populated = false; public $populated = false;
/**
* @var bool
*/
public $reflected = false;
/** /**
* @var bool * @var bool
*/ */
@ -62,11 +57,6 @@ class ClassLikeStorage
*/ */
public $name; public $name;
/**
* @var array<string, string>
*/
public $aliased_classes;
/** /**
* Is this class user-defined * Is this class user-defined
* *

View File

@ -22,11 +22,6 @@ class FunctionLikeStorage
*/ */
public $param_types = []; public $param_types = [];
/**
* @var string
*/
public $namespace;
/** /**
* @var Type\Union|null * @var Type\Union|null
*/ */

View File

@ -10,16 +10,6 @@ class MethodStorage extends FunctionLikeStorage
*/ */
public $is_static; public $is_static;
/**
* @var bool
*/
public $reflected;
/**
* @var bool
*/
public $registered;
/** /**
* @var int * @var int
*/ */

View File

@ -45,4 +45,9 @@ class PropertyStorage
* @var bool * @var bool
*/ */
public $deprecated = false; public $deprecated = false;
/**
* @var array<string, array<int, CodeLocation>>|null
*/
public $referencing_locations;
} }

View File

@ -6,11 +6,6 @@ use Psalm\Type\Union;
class Fn extends TNamedObject class Fn extends TNamedObject
{ {
/**
* @var string
*/
public $value = 'Closure';
/** /**
* @var array<int, FunctionLikeParameter> * @var array<int, FunctionLikeParameter>
*/ */
@ -30,6 +25,7 @@ class Fn extends TNamedObject
*/ */
public function __construct($value, array $params, Union $return_type) public function __construct($value, array $params, Union $return_type)
{ {
$this->value = 'Closure';
$this->params = $params; $this->params = $params;
$this->return_type = $return_type; $this->return_type = $return_type;
} }

View File

@ -41,11 +41,6 @@ class DependencyFinderVisitor extends PhpParser\NodeVisitorAbstract implements P
/** @var Aliases */ /** @var Aliases */
protected $file_aliases; protected $file_aliases;
/**
* @var bool
*/
protected $in_classlike = false;
/** /**
* @var string[] * @var string[]
*/ */

View File

@ -6,9 +6,6 @@ use Psalm\Config;
class ConfigTest extends TestCase class ConfigTest extends TestCase
{ {
/** @var \PhpParser\Parser */
protected static $parser;
/** @var TestConfig */ /** @var TestConfig */
protected static $config; protected static $config;

View File

@ -7,9 +7,6 @@ use Psalm\Checker\ProjectChecker;
class TestCase extends BaseTestCase class TestCase extends BaseTestCase
{ {
/** @var \PhpParser\Parser */
protected static $parser;
/** @var TestConfig */ /** @var TestConfig */
protected static $config; protected static $config;

View File

@ -958,6 +958,15 @@ class TypeTest extends TestCase
public function providerFileCheckerInvalidCodeParse() public function providerFileCheckerInvalidCodeParse()
{ {
return [ return [
'possiblyUndefinedVariable' => [
'<?php
if (rand(0, 1)) {
$a = 5;
}
echo $a;',
'error_message' => 'PossiblyUndefinedGlobalVariable',
],
'nullableMethodCall' => [ 'nullableMethodCall' => [
'<?php '<?php
class A { class A {

View File

@ -156,11 +156,16 @@ class UnusedCodeTest extends TestCase
public function modifyFoo(string $value) : void { public function modifyFoo(string $value) : void {
$this->value = $value; $this->value = $value;
} }
public function getFoo() : string {
return $this->value;
}
} }
$m = new A(); $m = new A();
$m->foo("value"); $m->foo("value");
$m->modifyFoo("value2");', $m->modifyFoo("value2");
echo $m->getFoo();',
], ],
'usedTraitMethod' => [ 'usedTraitMethod' => [
'<?php '<?php