diff --git a/config.xsd b/config.xsd index d17eed828..b5c7e4508 100644 --- a/config.xsd +++ b/config.xsd @@ -342,6 +342,7 @@ + diff --git a/docs/running_psalm/issues.md b/docs/running_psalm/issues.md index 9681651ed..1571cd9fd 100644 --- a/docs/running_psalm/issues.md +++ b/docs/running_psalm/issues.md @@ -2414,7 +2414,7 @@ function foo(callable $c) : int { ### UnusedFunctionCall -Emitted when `--find-dead-code` is turned on and Psalm finds a function call that is not used anywhere +Emitted when `--find-dead-code` is turned on and Psalm finds a function call whose return value is not used anywhere ```php $a = strlen("hello"); @@ -2422,6 +2422,27 @@ strlen("goodbye"); // unused echo $a; ``` +### UnusedMethodCall + +Emitted when `--find-dead-code` is turned on and Psalm finds a method call whose return value is not used anywhere + +```php +class A { + private string $foo; + + public function __construct(string $foo) { + $this->foo = $foo; + } + + public function getFoo() : string { + return $this->foo; + } +} + +$a = new A("hello"); +$a->getFoo(); +``` + ### UnusedParam Emitted when `--find-dead-code` is turned on and Psalm cannot find any uses of a particular parameter in a private method or function diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/MethodCallAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/MethodCallAnalyzer.php index 5c9879489..f202cdc5b 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/MethodCallAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/MethodCallAnalyzer.php @@ -69,10 +69,18 @@ class MethodCallAnalyzer extends \Psalm\Internal\Analyzer\Statements\Expression\ ) { $stmt->inferredType = null; + $was_inside_call = $context->inside_call; + + $context->inside_call = true; + if (ExpressionAnalyzer::analyze($statements_analyzer, $stmt->var, $context) === false) { return false; } + if (!$was_inside_call) { + $context->inside_call = false; + } + if (!$stmt->name instanceof PhpParser\Node\Identifier) { $was_inside_call = $context->inside_call; $context->inside_call = true; @@ -1200,7 +1208,7 @@ class MethodCallAnalyzer extends \Psalm\Internal\Analyzer\Statements\Expression\ if ($context->pure && !$method_storage->pure) { if (IssueBuffer::accepts( new ImpureMethodCall( - 'Cannot call an impure method from a pure context', + 'Cannot call an impure method ' . $method_id . ' from a pure context', new CodeLocation($source, $stmt->name) ), $statements_analyzer->getSuppressedIssues() @@ -1210,13 +1218,34 @@ class MethodCallAnalyzer extends \Psalm\Internal\Analyzer\Statements\Expression\ } elseif ($context->mutation_free && !$method_storage->mutation_free) { if (IssueBuffer::accepts( new ImpureMethodCall( - 'Cannot call an possibly-mutating method from a mutation-free context', + 'Cannot call an possibly-mutating method ' + . $method_id . ' from a mutation-free context', new CodeLocation($source, $stmt->name) ), $statements_analyzer->getSuppressedIssues() )) { // fall through } + } elseif ($method_storage->mutation_free + && $codebase->find_unused_variables + && !$context->inside_conditional + && !$context->inside_unset + ) { + if (!$context->inside_assignment && !$context->inside_call) { + if (IssueBuffer::accepts( + new \Psalm\Issue\UnusedMethodCall( + 'The call to ' . $method_id . ' is not used', + new CodeLocation($statements_analyzer, $stmt->name), + $method_id + ), + $statements_analyzer->getSuppressedIssues() + )) { + // fall through + } + } else { + /** @psalm-suppress UndefinedPropertyAssignment */ + $stmt->pure = true; + } } if ($method_storage->assertions) { diff --git a/src/Psalm/Internal/Visitor/ReflectorVisitor.php b/src/Psalm/Internal/Visitor/ReflectorVisitor.php index 500798498..3b052f9d6 100644 --- a/src/Psalm/Internal/Visitor/ReflectorVisitor.php +++ b/src/Psalm/Internal/Visitor/ReflectorVisitor.php @@ -1777,71 +1777,82 @@ class ReflectorVisitor extends PhpParser\NodeVisitorAbstract implements PhpParse if (($stmt instanceof PhpParser\Node\Stmt\Function_ || $stmt instanceof PhpParser\Node\Stmt\ClassMethod) - && strpos($stmt->name->name, 'assert') === 0 && $stmt->stmts ) { - $var_assertions = []; + if ($stmt instanceof PhpParser\Node\Stmt\ClassMethod + && $storage instanceof MethodStorage + && count($stmt->stmts) === 1 + && !count($stmt->params) + && $stmt->stmts[0] instanceof PhpParser\Node\Stmt\Return_ + && $stmt->stmts[0]->expr instanceof PhpParser\Node\Expr\PropertyFetch + && $stmt->stmts[0]->expr->var instanceof PhpParser\Node\Expr\Variable + && $stmt->stmts[0]->expr->var->name === 'this' + ) { + $storage->mutation_free = true; + } elseif (strpos($stmt->name->name, 'assert') === 0) { + $var_assertions = []; - foreach ($stmt->stmts as $function_stmt) { - if ($function_stmt instanceof PhpParser\Node\Stmt\If_) { - $final_actions = \Psalm\Internal\Analyzer\ScopeAnalyzer::getFinalControlActions( - $function_stmt->stmts, - $this->config->exit_functions, - false, - false - ); + foreach ($stmt->stmts as $function_stmt) { + if ($function_stmt instanceof PhpParser\Node\Stmt\If_) { + $final_actions = \Psalm\Internal\Analyzer\ScopeAnalyzer::getFinalControlActions( + $function_stmt->stmts, + $this->config->exit_functions, + false, + false + ); - if ($final_actions !== [\Psalm\Internal\Analyzer\ScopeAnalyzer::ACTION_END]) { - $var_assertions = []; - break; - } + if ($final_actions !== [\Psalm\Internal\Analyzer\ScopeAnalyzer::ACTION_END]) { + $var_assertions = []; + break; + } - $if_clauses = \Psalm\Type\Algebra::getFormula( - $function_stmt->cond, - $this->fq_classlike_names - ? $this->fq_classlike_names[count($this->fq_classlike_names) - 1] - : null, - $this->file_scanner, - null - ); + $if_clauses = \Psalm\Type\Algebra::getFormula( + $function_stmt->cond, + $this->fq_classlike_names + ? $this->fq_classlike_names[count($this->fq_classlike_names) - 1] + : null, + $this->file_scanner, + null + ); - $negated_formula = \Psalm\Type\Algebra::negateFormula($if_clauses); + $negated_formula = \Psalm\Type\Algebra::negateFormula($if_clauses); - $rules = \Psalm\Type\Algebra::getTruthsFromFormula($negated_formula); + $rules = \Psalm\Type\Algebra::getTruthsFromFormula($negated_formula); - if (!$rules) { - $var_assertions = []; - break; - } + if (!$rules) { + $var_assertions = []; + break; + } - foreach ($rules as $var_id => $rule) { - foreach ($rule as $rule_part) { - if (count($rule_part) > 1) { - continue 2; + foreach ($rules as $var_id => $rule) { + foreach ($rule as $rule_part) { + if (count($rule_part) > 1) { + continue 2; + } + } + + if (isset($existing_params[$var_id])) { + $param_offset = $existing_params[$var_id]; + + $var_assertions[] = new \Psalm\Storage\Assertion( + $param_offset, + $rule + ); + } elseif (strpos($var_id, '$this->') === 0) { + $var_assertions[] = new \Psalm\Storage\Assertion( + $var_id, + $rule + ); } } - - if (isset($existing_params[$var_id])) { - $param_offset = $existing_params[$var_id]; - - $var_assertions[] = new \Psalm\Storage\Assertion( - $param_offset, - $rule - ); - } elseif (strpos($var_id, '$this->') === 0) { - $var_assertions[] = new \Psalm\Storage\Assertion( - $var_id, - $rule - ); - } + } else { + $var_assertions = []; + break; } - } else { - $var_assertions = []; - break; } - } - $storage->assertions = $var_assertions; + $storage->assertions = $var_assertions; + } } if (!$this->scan_deep diff --git a/src/Psalm/Issue/UnusedMethodCall.php b/src/Psalm/Issue/UnusedMethodCall.php new file mode 100644 index 000000000..b3ae2ffc0 --- /dev/null +++ b/src/Psalm/Issue/UnusedMethodCall.php @@ -0,0 +1,6 @@ +foo; + public function bar() : void { + $this->foo = 5; } } @@ -688,6 +688,24 @@ class UnusedCodeTest extends TestCase strlen("goodbye");', 'error_message' => 'UnusedFunctionCall', ], + 'unusedMethodCall' => [ + 'foo = $foo; + } + + public function getFoo() : string { + return $this->foo; + } + } + + $a = new A("hello"); + $a->getFoo();', + 'error_message' => 'UnusedMethodCall', + ], 'propertyOverriddenDownstreamAndNotUsed' => [ '