1
0
mirror of https://github.com/danog/psalm.git synced 2024-11-30 04:39:00 +01:00

Annotate method getters more accurately

This commit is contained in:
Matthew Brown 2019-08-30 16:40:32 -04:00
parent b7b4baff8f
commit 6d07663d70
6 changed files with 142 additions and 56 deletions

View File

@ -342,6 +342,7 @@
<xs:element name="UnevaluatedCode" type="IssueHandlerType" minOccurs="0" /> <xs:element name="UnevaluatedCode" type="IssueHandlerType" minOccurs="0" />
<xs:element name="UnusedClosureParam" type="IssueHandlerType" minOccurs="0" /> <xs:element name="UnusedClosureParam" type="IssueHandlerType" minOccurs="0" />
<xs:element name="UnusedFunctionCall" type="FunctionIssueHandlerType" minOccurs="0" /> <xs:element name="UnusedFunctionCall" type="FunctionIssueHandlerType" minOccurs="0" />
<xs:element name="UnusedMethodCall" type="MethodIssueHandlerType" 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="PropertyIssueHandlerType" minOccurs="0" /> <xs:element name="UnusedProperty" type="PropertyIssueHandlerType" minOccurs="0" />

View File

@ -2414,7 +2414,7 @@ function foo(callable $c) : int {
### UnusedFunctionCall ### 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 ```php
$a = strlen("hello"); $a = strlen("hello");
@ -2422,6 +2422,27 @@ strlen("goodbye"); // unused
echo $a; 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 ### 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 Emitted when `--find-dead-code` is turned on and Psalm cannot find any uses of a particular parameter in a private method or function

View File

@ -69,10 +69,18 @@ class MethodCallAnalyzer extends \Psalm\Internal\Analyzer\Statements\Expression\
) { ) {
$stmt->inferredType = null; $stmt->inferredType = null;
$was_inside_call = $context->inside_call;
$context->inside_call = true;
if (ExpressionAnalyzer::analyze($statements_analyzer, $stmt->var, $context) === false) { if (ExpressionAnalyzer::analyze($statements_analyzer, $stmt->var, $context) === false) {
return false; return false;
} }
if (!$was_inside_call) {
$context->inside_call = false;
}
if (!$stmt->name instanceof PhpParser\Node\Identifier) { if (!$stmt->name instanceof PhpParser\Node\Identifier) {
$was_inside_call = $context->inside_call; $was_inside_call = $context->inside_call;
$context->inside_call = true; $context->inside_call = true;
@ -1200,7 +1208,7 @@ class MethodCallAnalyzer extends \Psalm\Internal\Analyzer\Statements\Expression\
if ($context->pure && !$method_storage->pure) { if ($context->pure && !$method_storage->pure) {
if (IssueBuffer::accepts( if (IssueBuffer::accepts(
new ImpureMethodCall( 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) new CodeLocation($source, $stmt->name)
), ),
$statements_analyzer->getSuppressedIssues() $statements_analyzer->getSuppressedIssues()
@ -1210,13 +1218,34 @@ class MethodCallAnalyzer extends \Psalm\Internal\Analyzer\Statements\Expression\
} elseif ($context->mutation_free && !$method_storage->mutation_free) { } elseif ($context->mutation_free && !$method_storage->mutation_free) {
if (IssueBuffer::accepts( if (IssueBuffer::accepts(
new ImpureMethodCall( 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) new CodeLocation($source, $stmt->name)
), ),
$statements_analyzer->getSuppressedIssues() $statements_analyzer->getSuppressedIssues()
)) { )) {
// fall through // 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) { if ($method_storage->assertions) {

View File

@ -1777,71 +1777,82 @@ class ReflectorVisitor extends PhpParser\NodeVisitorAbstract implements PhpParse
if (($stmt instanceof PhpParser\Node\Stmt\Function_ if (($stmt instanceof PhpParser\Node\Stmt\Function_
|| $stmt instanceof PhpParser\Node\Stmt\ClassMethod) || $stmt instanceof PhpParser\Node\Stmt\ClassMethod)
&& strpos($stmt->name->name, 'assert') === 0
&& $stmt->stmts && $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) { foreach ($stmt->stmts as $function_stmt) {
if ($function_stmt instanceof PhpParser\Node\Stmt\If_) { if ($function_stmt instanceof PhpParser\Node\Stmt\If_) {
$final_actions = \Psalm\Internal\Analyzer\ScopeAnalyzer::getFinalControlActions( $final_actions = \Psalm\Internal\Analyzer\ScopeAnalyzer::getFinalControlActions(
$function_stmt->stmts, $function_stmt->stmts,
$this->config->exit_functions, $this->config->exit_functions,
false, false,
false false
); );
if ($final_actions !== [\Psalm\Internal\Analyzer\ScopeAnalyzer::ACTION_END]) { if ($final_actions !== [\Psalm\Internal\Analyzer\ScopeAnalyzer::ACTION_END]) {
$var_assertions = []; $var_assertions = [];
break; break;
} }
$if_clauses = \Psalm\Type\Algebra::getFormula( $if_clauses = \Psalm\Type\Algebra::getFormula(
$function_stmt->cond, $function_stmt->cond,
$this->fq_classlike_names $this->fq_classlike_names
? $this->fq_classlike_names[count($this->fq_classlike_names) - 1] ? $this->fq_classlike_names[count($this->fq_classlike_names) - 1]
: null, : null,
$this->file_scanner, $this->file_scanner,
null 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) { if (!$rules) {
$var_assertions = []; $var_assertions = [];
break; break;
} }
foreach ($rules as $var_id => $rule) { foreach ($rules as $var_id => $rule) {
foreach ($rule as $rule_part) { foreach ($rule as $rule_part) {
if (count($rule_part) > 1) { if (count($rule_part) > 1) {
continue 2; 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
);
} }
} }
} else {
if (isset($existing_params[$var_id])) { $var_assertions = [];
$param_offset = $existing_params[$var_id]; break;
$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;
} }
}
$storage->assertions = $var_assertions; $storage->assertions = $var_assertions;
}
} }
if (!$this->scan_deep if (!$this->scan_deep

View File

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

View File

@ -385,8 +385,8 @@ class UnusedCodeTest extends TestCase
class C { class C {
/** @var int */ /** @var int */
protected $foo = 1; protected $foo = 1;
public function bar() : int { public function bar() : void {
return $this->foo; $this->foo = 5;
} }
} }
@ -688,6 +688,24 @@ class UnusedCodeTest extends TestCase
strlen("goodbye");', strlen("goodbye");',
'error_message' => 'UnusedFunctionCall', 'error_message' => 'UnusedFunctionCall',
], ],
'unusedMethodCall' => [
'<?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();',
'error_message' => 'UnusedMethodCall',
],
'propertyOverriddenDownstreamAndNotUsed' => [ 'propertyOverriddenDownstreamAndNotUsed' => [
'<?php '<?php
class A { class A {