danog/psalm
1
0
Fork 0
mirror of https://github.com/danog/psalm.git synced 2024-11-26 20:34:47 +01:00
Code Issues Projects Releases Wiki Activity

Improve unused variable detection slightly

Browse Source
This commit is contained in:
Matthew Brown 2018-01-25 01:04:26 -05:00
parent 3b55ea5a00
commit ea28ee709d
20 changed files with 340 additions and 99 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
src/Psalm/Checker/ClassChecker.php
View File

@ -380,7 +380,6 @@ class ClassChecker extends ClassLikeChecker
foreach ($storage->appearing_property_ids as $property_name => $appearing_property_id) {
$property_class_name = self::getDeclaringClassForProperty($project_checker, $appearing_property_id);
$property_class_storage = $classlike_storage_provider->get((string)$property_class_name);
$property_class_name = self::getDeclaringClassForProperty($project_checker, $appearing_property_id);
$property = $property_class_storage->properties[$property_name];
@ -510,7 +509,6 @@ class ClassChecker extends ClassLikeChecker
foreach ($storage->appearing_property_ids as $property_name => $appearing_property_id) {
$property_class_name = self::getDeclaringClassForProperty($project_checker, $appearing_property_id);
$property_class_storage = $classlike_storage_provider->get((string)$property_class_name);
$property_class_name = self::getDeclaringClassForProperty($project_checker, $appearing_property_id);
$property = $property_class_storage->properties[$property_name];

1
src/Psalm/Checker/FunctionChecker.php
View File

@ -636,7 +636,6 @@ class FunctionChecker extends FunctionLikeChecker
&& $stmt instanceof PhpParser\Node\Stmt\Return_
&& $stmt->expr
) {
$first_param_name = $first_param->name;
$assertions = AssertionFinder::getAssertions($stmt->expr, null, $statements_checker);
if (isset($assertions['$' . $first_param->name])) {

154
src/Psalm/Checker/FunctionLikeChecker.php
View File

@ -32,7 +32,6 @@ use Psalm\Issue\OverriddenMethodAccess;
use Psalm\Issue\ReservedWord;
use Psalm\Issue\UntypedParam;
use Psalm\Issue\UnusedParam;
use Psalm\Issue\UnusedVariable;
use Psalm\IssueBuffer;
use Psalm\StatementsSource;
use Psalm\Storage\ClassLikeStorage;
@ -162,17 +161,22 @@ abstract class FunctionLikeChecker extends SourceChecker implements StatementsSo
$context->vars_possibly_in_scope['$this'] = true;
}
$declaring_method_id = MethodChecker::getDeclaringMethodId($project_checker, $method_id);
if (!is_string($declaring_method_id)) {
throw new \UnexpectedValueException('The declaring method of ' . $method_id . ' should not be null');
}
$fq_class_name = (string)$context->self;
$class_storage = $classlike_storage_provider->get($fq_class_name);
$storage = $codebase->getMethodStorage($declaring_method_id);
try {
$storage = $codebase->getMethodStorage($real_method_id);
} catch (\UnexpectedValueException $e) {
if (!$class_storage->parent_classes) {
throw $e;
}
$declaring_method_id = (string) MethodChecker::getDeclaringMethodId($project_checker, $method_id);
// happens for fake constructors
$storage = $codebase->getMethodStorage($declaring_method_id);
}
$cased_method_id = $fq_class_name . '::' . $storage->cased_name;
@ -285,6 +289,10 @@ abstract class FunctionLikeChecker extends SourceChecker implements StatementsSo
$context->vars_in_scope['$' . $function_param->name] = $param_type;
$context->vars_possibly_in_scope['$' . $function_param->name] = true;
if ($context->collect_references && $function_param->location) {
$context->unreferenced_vars['$' . $function_param->name] = $function_param->location;
}
if (!$function_param->type_location || !$function_param->location) {
continue;
}
@ -404,8 +412,9 @@ abstract class FunctionLikeChecker extends SourceChecker implements StatementsSo
}
}
if ($function_param->by_ref && !$param_type->isMixed()) {
$context->byref_constraints['$' . $function_param->name] = new \Psalm\ReferenceConstraint($param_type);
if ($function_param->by_ref) {
$context->byref_constraints['$' . $function_param->name]
= new \Psalm\ReferenceConstraint(!$param_type->isMixed() ? $param_type : null);
}
if ($function_param->by_ref) {
@ -555,71 +564,57 @@ abstract class FunctionLikeChecker extends SourceChecker implements StatementsSo
}
}
if ($context->collect_references &&
!$this->getFileChecker()->project_checker->find_references_to &&
$context->check_variables
if ($context->collect_references
&& !$project_checker->find_references_to
&& $context->check_variables
) {
foreach ($context->vars_possibly_in_scope as $var_name => $_) {
if (strpos($var_name, '->') === false &&
$var_name !== '$this' &&
strpos($var_name, '::$') === false &&
strpos($var_name, '[') === false &&
$var_name !== '$_'
foreach ($context->unreferenced_vars as $var_name => $original_location) {
if (!array_key_exists(substr($var_name, 1), $storage->param_types)) {
continue;
}
if (!($storage instanceof MethodStorage)
|| $storage->visibility === ClassLikeChecker::VISIBILITY_PRIVATE
) {
$original_location = $statements_checker->getFirstAppearance($var_name);
if (IssueBuffer::accepts(
new UnusedParam(
'Param ' . $var_name . ' is never referenced in this method',
$original_location
),
$this->getSuppressedIssues()
)) {
// fall through
}
} else {
$fq_class_name = (string)$context->self;
if (!isset($context->referenced_var_ids[$var_name]) && $original_location) {
if (!array_key_exists(substr($var_name, 1), $storage->param_types)) {
if (IssueBuffer::accepts(
new UnusedVariable(
'Variable ' . $var_name . ' is never referenced',
$original_location
),
$this->getSuppressedIssues()
)) {
// fall through
}
} elseif (!$storage instanceof MethodStorage
|| $storage->visibility === ClassLikeChecker::VISIBILITY_PRIVATE
$class_storage = $codebase->classlike_storage_provider->get($fq_class_name);
if (!$class_storage || $storage->abstract) {
continue;
}
$method_name_lc = strtolower($storage->cased_name);
$parent_method_id = end($class_storage->overridden_method_ids[$method_name_lc]);
$position = array_search(substr($var_name, 1), array_keys($storage->param_types), true);
if ($position === false) {
throw new \UnexpectedValueException('$position should not be false here');
}
if ($parent_method_id) {
$parent_method_storage = $codebase->getMethodStorage($parent_method_id);
// if the parent method has a param at that position and isn't abstract
if (!$parent_method_storage->abstract
&& isset($parent_method_storage->params[$position])
) {
if (IssueBuffer::accepts(
new UnusedParam(
'Param ' . $var_name . ' is never referenced in this method',
$original_location
),
$this->getSuppressedIssues()
)) {
// fall through
}
} else {
if (!$class_storage || $storage->abstract) {
continue;
}
/** @var ClassMethod $this->function */
$method_name_lc = strtolower((string)$this->function->name);
$parent_method_id = end($class_storage->overridden_method_ids[$method_name_lc]);
$position = array_search(substr($var_name, 1), array_keys($storage->param_types), true);
if ($position === false) {
throw new \UnexpectedValueException('$position should not be false here');
}
if ($parent_method_id) {
$parent_method_storage = $codebase->getMethodStorage($parent_method_id);
// if the parent method has a param at that position and isn't abstract
if (!$parent_method_storage->abstract
&& isset($parent_method_storage->params[$position])
) {
continue;
}
}
$storage->unused_params[$position] = $original_location;
continue;
}
}
$storage->unused_params[$position] = $original_location;
}
}
@ -629,7 +624,7 @@ abstract class FunctionLikeChecker extends SourceChecker implements StatementsSo
$storage->used_params[$i] = true;
/** @var ClassMethod $this->function */
$method_name_lc = strtolower((string)$this->function->name);
$method_name_lc = strtolower($storage->cased_name);
if (!isset($class_storage->overridden_method_ids[$method_name_lc])) {
continue;
@ -1066,22 +1061,23 @@ abstract class FunctionLikeChecker extends SourceChecker implements StatementsSo
*/
public function getFunctionLikeStorage(StatementsChecker $statements_checker)
{
$function_id = $this->getMethodId();
$project_checker = $this->getFileChecker()->project_checker;
$codebase = $project_checker->codebase;
if (strpos($function_id, '::')) {
$declaring_method_id = MethodChecker::getDeclaringMethodId($project_checker, $function_id);
if ($this->function instanceof ClassMethod) {
$method_id = (string) $this->getMethodId();
if (!$declaring_method_id) {
throw new \UnexpectedValueException('The declaring method of ' . $function_id . ' should not be null');
try {
return $codebase->getMethodStorage($method_id);
} catch (\UnexpectedValueException $e) {
$declaring_method_id = (string) MethodChecker::getDeclaringMethodId($project_checker, $method_id);
// happens for fake constructors
return $codebase->getMethodStorage($declaring_method_id);
}
return $codebase->getMethodStorage($declaring_method_id);
}
return $codebase->getFunctionStorage($statements_checker, $function_id);
return $codebase->getFunctionStorage($statements_checker, (string) $this->getMethodId());
}
/**

7
src/Psalm/Checker/Statements/Block/DoChecker.php
View File

@ -72,6 +72,13 @@ class DoChecker
$do_context->referenced_var_ids
);
if ($context->collect_references) {
$context->unreferenced_vars = array_intersect_key(
$do_context->unreferenced_vars,
$context->unreferenced_vars
);
}
return ExpressionChecker::analyze($statements_checker, $stmt->cond, $context);
}
}

7
src/Psalm/Checker/Statements/Block/ForChecker.php
View File

@ -98,6 +98,13 @@ class ForChecker
$context->referenced_var_ids
);
if ($context->collect_references) {
$context->unreferenced_vars = array_intersect_key(
$for_context->unreferenced_vars,
$context->unreferenced_vars
);
}
return null;
}
}

13
src/Psalm/Checker/Statements/Block/ForeachChecker.php
View File

@ -272,6 +272,15 @@ class ForeachChecker
}
}
if ($context->collect_references
&& $stmt->byRef
&& $stmt->valueVar instanceof PhpParser\Node\Expr\Variable
&& is_string($stmt->valueVar->name)
) {
$foreach_context->byref_constraints['$' . $stmt->valueVar->name]
= new \Psalm\ReferenceConstraint($value_type);
}
AssignmentChecker::analyze(
$statements_checker,
$stmt->valueVar,
@ -335,6 +344,10 @@ class ForeachChecker
$context->referenced_var_ids
);
if ($context->collect_references) {
$context->unreferenced_vars = $foreach_context->unreferenced_vars;
}
return null;
}
}

85
src/Psalm/Checker/Statements/Block/IfChecker.php
View File

@ -283,6 +283,22 @@ class IfChecker
return false;
}
$newly_unreferenced_vars = [];
$old_unreferenced_vars = $context->unreferenced_vars;
if ($context->collect_references) {
foreach ($context->unreferenced_vars as $var_id => $_) {
if (!isset($if_context->unreferenced_vars[$var_id])) {
unset($context->unreferenced_vars[$var_id]);
}
}
$newly_unreferenced_vars = array_diff_key(
$if_context->unreferenced_vars,
$old_unreferenced_vars
);
}
// check the elseifs
foreach ($stmt->elseifs as $elseif) {
$elseif_context = clone $original_context;
@ -302,6 +318,22 @@ class IfChecker
) === false) {
return false;
}
if ($context->collect_references) {
foreach ($context->unreferenced_vars as $var_id => $_) {
if (!isset($elseif_context->unreferenced_vars[$var_id])) {
unset($context->unreferenced_vars[$var_id]);
}
}
$newly_unreferenced_vars = array_merge(
$newly_unreferenced_vars,
array_diff_key(
$elseif_context->unreferenced_vars,
$old_unreferenced_vars
)
);
}
}
// check the else
@ -323,6 +355,22 @@ class IfChecker
) === false) {
return false;
}
if ($context->collect_references) {
foreach ($context->unreferenced_vars as $var_id => $_) {
if (!isset($else_context->unreferenced_vars[$var_id])) {
unset($context->unreferenced_vars[$var_id]);
}
}
$newly_unreferenced_vars = array_merge(
$newly_unreferenced_vars,
array_diff_key(
$else_context->unreferenced_vars,
$old_unreferenced_vars
)
);
}
} else {
$if_scope->final_actions[] = ScopeChecker::ACTION_NONE;
}
@ -385,6 +433,13 @@ class IfChecker
}
}
if ($context->collect_references) {
$context->unreferenced_vars = array_merge(
$context->unreferenced_vars,
$newly_unreferenced_vars
);
}
return null;
}
@ -441,12 +496,14 @@ class IfChecker
if ($if_context->byref_constraints !== null) {
foreach ($if_context->byref_constraints as $var_id => $byref_constraint) {
if ($outer_context->byref_constraints !== null &&
isset($outer_context->byref_constraints[$var_id]) &&
!TypeChecker::isContainedBy(
if ($outer_context->byref_constraints !== null
&& isset($outer_context->byref_constraints[$var_id])
&& $byref_constraint->type
&& ($outer_constraint_type = $outer_context->byref_constraints[$var_id]->type)
&& !TypeChecker::isContainedBy(
$project_checker,
$byref_constraint->type,
$outer_context->byref_constraints[$var_id]->type
$outer_constraint_type
)
) {
if (IssueBuffer::accepts(
@ -829,12 +886,14 @@ class IfChecker
if ($elseif_context->byref_constraints !== null) {
foreach ($elseif_context->byref_constraints as $var_id => $byref_constraint) {
if ($outer_context->byref_constraints !== null &&
isset($outer_context->byref_constraints[$var_id]) &&
!TypeChecker::isContainedBy(
if ($outer_context->byref_constraints !== null
&& isset($outer_context->byref_constraints[$var_id])
&& ($outer_constraint_type = $outer_context->byref_constraints[$var_id]->type)
&& $byref_constraint->type
&& !TypeChecker::isContainedBy(
$project_checker,
$byref_constraint->type,
$outer_context->byref_constraints[$var_id]->type
$outer_constraint_type
)
) {
if (IssueBuffer::accepts(
@ -1087,12 +1146,14 @@ class IfChecker
$project_checker = $statements_checker->getFileChecker()->project_checker;
foreach ($else_context->byref_constraints as $var_id => $byref_constraint) {
if ($outer_context->byref_constraints !== null &&
isset($outer_context->byref_constraints[$var_id]) &&
!TypeChecker::isContainedBy(
if ($outer_context->byref_constraints !== null
&& isset($outer_context->byref_constraints[$var_id])
&& ($outer_constraint_type = $outer_context->byref_constraints[$var_id]->type)
&& $byref_constraint->type
&& !TypeChecker::isContainedBy(
$project_checker,
$byref_constraint->type,
$outer_context->byref_constraints[$var_id]->type
$outer_constraint_type
)
) {
if (IssueBuffer::accepts(

7
src/Psalm/Checker/Statements/Block/LoopChecker.php
View File

@ -365,6 +365,13 @@ class LoopChecker
$inner_context->referenced_var_ids,
$loop_scope->loop_context->referenced_var_ids
);
if ($inner_context->collect_references) {
$loop_scope->loop_context->unreferenced_vars = array_diff_key(
$inner_context->unreferenced_vars,
$inner_context->referenced_var_ids
);
}
}
/**

7
src/Psalm/Checker/Statements/Block/SwitchChecker.php
View File

@ -198,6 +198,13 @@ class SwitchChecker
$case_context->referenced_var_ids
);
if ($context->collect_references) {
$context->unreferenced_vars = array_intersect_key(
$case_context->unreferenced_vars,
$context->unreferenced_vars
);
}
if ($case_exit_type !== 'return_throw') {
$vars = array_diff_key(
$case_context->vars_possibly_in_scope,

19
src/Psalm/Checker/Statements/Block/TryChecker.php
View File

@ -75,6 +75,18 @@ class TryChecker
}
$try_context->vars_possibly_in_scope = $context->vars_possibly_in_scope;
$context->referenced_var_ids = array_merge(
$try_context->referenced_var_ids,
$context->referenced_var_ids
);
if ($context->collect_references) {
$context->unreferenced_vars = array_intersect_key(
$try_context->unreferenced_vars,
$context->unreferenced_vars
);
}
}
$try_leaves_loop = $loop_scope
@ -200,6 +212,13 @@ class TryChecker
$context->referenced_var_ids
);
if ($context->collect_references) {
$context->unreferenced_vars = array_intersect_key(
$catch_context->unreferenced_vars,
$context->unreferenced_vars
);
}
if ($catch_actions[$i] !== [ScopeChecker::ACTION_END]) {
foreach ($catch_context->vars_in_scope as $var_id => $type) {
if ($catch->var !== $var_id &&

7
src/Psalm/Checker/Statements/Block/WhileChecker.php
View File

@ -83,6 +83,13 @@ class WhileChecker
$while_context->referenced_var_ids
);
if ($context->collect_references) {
$context->unreferenced_vars = array_intersect_key(
$while_context->unreferenced_vars,
$context->unreferenced_vars
);
}
return null;
}
}

2
src/Psalm/Checker/Statements/Expression/ArrayChecker.php
View File

@ -120,7 +120,7 @@ class ArrayChecker
) {
$array_key = $item->key ? $item->key->value : $int_offset;
$property_types[$item->key ? $item->key->value : $int_offset] = $item_value_type;
$property_types[$array_key] = $item_value_type;
} else {
$can_create_objectlike = false;
}

11
src/Psalm/Checker/Statements/Expression/AssignmentChecker.php
View File

@ -165,11 +165,14 @@ class AssignmentChecker
)) {
// fall through
}
} elseif ($var_id && isset($context->byref_constraints[$var_id])) {
} elseif ($var_id
&& isset($context->byref_constraints[$var_id])
&& ($outer_constraint_type = $context->byref_constraints[$var_id]->type)
) {
if (!TypeChecker::isContainedBy(
$statements_checker->getFileChecker()->project_checker,
$assign_value_type,
$context->byref_constraints[$var_id]->type,
$outer_constraint_type,
$assign_value_type->ignore_nullable_issues,
$assign_value_type->ignore_falsable_issues
)
@ -214,6 +217,10 @@ class AssignmentChecker
$context->vars_possibly_in_scope[$var_id] = true;
$context->assigned_var_ids[$var_id] = true;
if ($context->collect_references && !isset($context->byref_constraints[$var_id])) {
$context->unreferenced_vars[$var_id] = new CodeLocation($statements_checker, $assign_var);
}
if (!$statements_checker->hasVariable($var_id)) {
$statements_checker->registerVariable(
$var_id,

28
src/Psalm/Checker/Statements/Expression/BinaryOpChecker.php
View File

@ -105,6 +105,13 @@ class BinaryOpChecker
$context->referenced_var_ids
);
if ($context->collect_references) {
$context->unreferenced_vars = array_intersect_key(
$op_context->unreferenced_vars,
$context->unreferenced_vars
);
}
foreach ($op_context->vars_in_scope as $var_id => $type) {
if (isset($context->vars_in_scope[$var_id])) {
$context->vars_in_scope[$var_id] = Type::combineUnionTypes($context->vars_in_scope[$var_id], $type);
@ -230,6 +237,13 @@ class BinaryOpChecker
$context->referenced_var_ids
);
if ($context->collect_references) {
$context->unreferenced_vars = array_intersect_key(
$op_context->unreferenced_vars,
$context->unreferenced_vars
);
}
$context->vars_possibly_in_scope = array_merge(
$op_context->vars_possibly_in_scope,
$context->vars_possibly_in_scope
@ -296,6 +310,13 @@ class BinaryOpChecker
$t_if_context->referenced_var_ids
);
if ($context->collect_references) {
$context->unreferenced_vars = array_intersect_key(
$t_if_context->unreferenced_vars,
$context->unreferenced_vars
);
}
$t_else_context = clone $context;
if ($negated_if_types) {
@ -325,6 +346,13 @@ class BinaryOpChecker
$t_else_context->referenced_var_ids
);
if ($context->collect_references) {
$context->unreferenced_vars = array_intersect_key(
$t_else_context->unreferenced_vars,
$context->unreferenced_vars
);
}
$lhs_type = null;
if (isset($stmt->left->inferredType)) {

10
src/Psalm/Checker/Statements/Expression/TernaryChecker.php
View File

@ -87,6 +87,11 @@ class TernaryChecker
$context->referenced_var_ids,
$t_if_context->referenced_var_ids
);
$context->unreferenced_vars = array_intersect_key(
$context->unreferenced_vars,
$t_if_context->unreferenced_vars
);
}
if ($negated_if_types) {
@ -122,6 +127,11 @@ class TernaryChecker
$t_else_context->referenced_var_ids
);
$context->unreferenced_vars = array_intersect_key(
$context->unreferenced_vars,
$t_else_context->unreferenced_vars
);
$lhs_type = null;
if ($stmt->if) {

38
src/Psalm/Checker/StatementsChecker.php
View File

@ -26,6 +26,7 @@ use Psalm\Issue\InvalidDocblock;
use Psalm\Issue\InvalidGlobal;
use Psalm\Issue\UnevaluatedCode;
use Psalm\Issue\UnrecognizedStatement;
use Psalm\Issue\UnusedVariable;
use Psalm\IssueBuffer;
use Psalm\Scope\LoopScope;
use Psalm\StatementsSource;
@ -516,6 +517,14 @@ class StatementsChecker extends SourceChecker implements StatementsSource
}
}
if ($root_scope
&& $context->collect_references
&& !$project_checker->find_references_to
&& $context->check_variables
) {
$this->checkUnreferencedVars($context);
}
if ($project_checker->alter_code && $root_scope && $this->vars_to_initialize) {
$file_contents = $project_checker->codebase->getFileContents($this->getFilePath());
@ -531,6 +540,35 @@ class StatementsChecker extends SourceChecker implements StatementsSource
return null;
}
/**
* @param Context $context
*
* @return void
*/
private function checkUnreferencedVars(Context $context)
{
$source = $this->getSource();
$function_storage = $source instanceof FunctionLikeChecker ? $source->getFunctionLikeStorage($this) : null;
foreach ($context->unreferenced_vars as $var_name => $original_location) {
if ($var_name === '$_') {
continue;
}
if (!$function_storage || !array_key_exists(substr($var_name, 1), $function_storage->param_types)) {
if (IssueBuffer::accepts(
new UnusedVariable(
'Variable ' . $var_name . ' is never referenced',
$original_location
),
$this->getSuppressedIssues()
)) {
// fall through
}
}
}
}
/**
* @param PhpParser\Node\Stmt\Static_ $stmt
* @param Context $context

11
src/Psalm/Context.php
View File

@ -138,6 +138,13 @@ class Context
*/
public $referenced_var_ids = [];
/**
* A list of variables that have never been referenced
*
* @var array<string, CodeLocation>
*/
public $unreferenced_vars = [];
/**
* A list of variables that have been passed by reference (where we know their type)
*
@ -629,6 +636,10 @@ class Context
if ($stripped_var[0] === '$' && $stripped_var !== '$this') {
$this->referenced_var_ids[$var_name] = true;
if ($this->collect_references) {
unset($this->unreferenced_vars[$var_name]);
}
}
return isset($this->vars_in_scope[$var_name]);

4
src/Psalm/ReferenceConstraint.php
View File

@ -3,13 +3,13 @@ namespace Psalm;
class ReferenceConstraint
{
/** @var Type\Union */
/** @var Type\Union|null */
public $type;
/**
* @param Type\Union $type
*/
public function __construct(Type\Union $type)
public function __construct(Type\Union $type = null)
{
$this->type = $type;
}

12
src/Psalm/Stubs/CoreGenericFunctions.php
View File

@ -49,6 +49,18 @@ function array_slice(array $arr, int $offset, int $length = null, bool $preserve
*/
function array_intersect(array $arr, array $arr2, array $arr3 = null, array $arr4 = null) {}
/**
* @template TKey
* @template TValue
*
* @param array<TKey, TValue> $arr
* @param array $arr2
* @param array|null $arr3
* @param array|null $arr4
* @return array<TKey, TValue>
*/
function array_intersect_key(array $arr, array $arr2, array $arr3 = null, array $arr4 = null) {}
/**
* @template TKey
* @template TValue

14
tests/UnusedCodeTest.php
View File

@ -247,6 +247,20 @@ class UnusedCodeTest extends TestCase
}',
'error_message' => 'UnusedVariable',
],
'unusuedVariableInBranchOfIf' => [
'<?php
function foo(): void {
if (rand(0, 1)) {
$a = "foo";
echo $a;
}
if (rand(0, 1)) {
$a = "foo";
}
}',
'error_message' => 'UnusedVariable',
],
'unusedClass' => [
'<?php
class A { }',