danog/psalm
1
0
Fork 0
mirror of https://github.com/danog/psalm.git synced 2025-01-21 21:31:13 +01:00
Code Issues Projects Releases Wiki Activity

Detect more issues inside finally block

Browse Source
This commit is contained in:
Brown 2020-09-21 15:16:19 -04:00
parent fe94ae0603
commit 3e0f449163
9 changed files with 147 additions and 19 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
src/Psalm/Context.php
View File

@ -298,6 +298,11 @@ class Context
*/
public $case_scope = null;
/**
* @var Internal\Scope\FinallyScope|null
*/
public $finally_scope = null;
/**
* @var Context|null
*/
@ -792,7 +797,7 @@ class Context
}
}
}
public function isSuppressingExceptions(StatementsAnalyzer $statements_analyzer): bool
{
if (!$this->collect_exceptions) {

4
src/Psalm/Internal/Analyzer/ScopeAnalyzer.php
View File

@ -162,7 +162,7 @@ class ScopeAnalyzer
return array_merge($control_actions, [self::ACTION_LEAVE_SWITCH]);
}
return array_unique(array_merge($control_actions, [self::ACTION_CONTINUE]));
return \array_values(array_unique(array_merge($control_actions, [self::ACTION_CONTINUE])));
}
if ($stmt instanceof PhpParser\Node\Stmt\Break_) {
@ -173,7 +173,7 @@ class ScopeAnalyzer
return [self::ACTION_LEAVE_SWITCH];
}
return array_unique(array_merge($control_actions, [self::ACTION_BREAK]));
return \array_values(array_unique(array_merge($control_actions, [self::ACTION_BREAK])));
}
if ($stmt instanceof PhpParser\Node\Stmt\If_) {

49
src/Psalm/Internal/Analyzer/Statements/Block/TryAnalyzer.php
View File

@ -12,6 +12,7 @@ use Psalm\IssueBuffer;
use Psalm\Type;
use Psalm\Type\Atomic\TNamedObject;
use Psalm\Type\Union;
use Psalm\Internal\Scope\FinallyScope;
use function in_array;
use function array_merge;
use function array_intersect_key;
@ -67,6 +68,10 @@ class TryAnalyzer
if ($codebase->alter_code) {
$try_context->branch_point = $try_context->branch_point ?: (int) $stmt->getAttribute('startFilePos');
}
if ($stmt->finally) {
$try_context->finally_scope = new FinallyScope();
}
}
$assigned_var_ids = $try_context->assigned_var_ids;
@ -432,22 +437,18 @@ class TryAnalyzer
}
}
if ($stmt->finally) {
$suppressed_issues = $statements_analyzer->getSuppressedIssues();
foreach ($issues_to_suppress as $issue_to_suppress) {
if (!in_array($issue_to_suppress, $suppressed_issues, true)) {
$statements_analyzer->addSuppressedIssues([$issue_to_suppress]);
}
}
$catch_context->has_returned = false;
$statements_analyzer->analyze($stmt->finally->stmts, $catch_context);
foreach ($issues_to_suppress as $issue_to_suppress) {
if (!in_array($issue_to_suppress, $suppressed_issues, true)) {
$statements_analyzer->removeSuppressedIssues([$issue_to_suppress]);
if ($try_context->finally_scope) {
foreach ($catch_context->vars_in_scope as $var_id => $type) {
if (isset($try_context->finally_scope->vars_in_scope[$var_id])) {
if ($try_context->finally_scope->vars_in_scope[$var_id] !== $type) {
$try_context->finally_scope->vars_in_scope[$var_id] = Type::combineUnionTypes(
$try_context->finally_scope->vars_in_scope[$var_id],
$type,
$statements_analyzer->getCodebase()
);
}
} else {
$try_context->finally_scope->vars_in_scope[$var_id] = $type;
}
}
}
@ -498,6 +499,22 @@ class TryAnalyzer
}
if ($stmt->finally) {
if ($try_context->finally_scope) {
$finally_context = clone $context;
foreach ($try_context->finally_scope->vars_in_scope as $var_id => $type) {
if (isset($finally_context->vars_in_scope[$var_id])) {
$finally_context->vars_in_scope[$var_id] = Type::combineUnionTypes(
$finally_context->vars_in_scope[$var_id],
$type,
$codebase
);
}
}
$statements_analyzer->analyze($stmt->finally->stmts, $finally_context);
}
$suppressed_issues = $statements_analyzer->getSuppressedIssues();
foreach ($issues_to_suppress as $issue_to_suppress) {

16
src/Psalm/Internal/Analyzer/Statements/BreakAnalyzer.php
View File

@ -80,6 +80,22 @@ class BreakAnalyzer
$loop_scope->referenced_var_ids += $context->referenced_var_ids;
}
if ($context->finally_scope) {
foreach ($context->vars_in_scope as $var_id => $type) {
if (isset($context->finally_scope->vars_in_scope[$var_id])) {
if ($context->finally_scope->vars_in_scope[$var_id] !== $type) {
$context->finally_scope->vars_in_scope[$var_id] = Type::combineUnionTypes(
$context->finally_scope->vars_in_scope[$var_id],
$type,
$statements_analyzer->getCodebase()
);
}
} else {
$context->finally_scope->vars_in_scope[$var_id] = $type;
}
}
}
}
$case_scope = $context->case_scope;

16
src/Psalm/Internal/Analyzer/Statements/ContinueAnalyzer.php
View File

@ -99,6 +99,22 @@ class ContinueAnalyzer
$loop_scope->referenced_var_ids += $context->referenced_var_ids;
}
if ($context->finally_scope) {
foreach ($context->vars_in_scope as $var_id => $type) {
if (isset($context->finally_scope->vars_in_scope[$var_id])) {
if ($context->finally_scope->vars_in_scope[$var_id] !== $type) {
$context->finally_scope->vars_in_scope[$var_id] = Type::combineUnionTypes(
$context->finally_scope->vars_in_scope[$var_id],
$type,
$statements_analyzer->getCodebase()
);
}
} else {
$context->finally_scope->vars_in_scope[$var_id] = $type;
}
}
}
}
$case_scope = $context->case_scope;

16
src/Psalm/Internal/Analyzer/Statements/ReturnAnalyzer.php
View File

@ -169,6 +169,22 @@ class ReturnAnalyzer
$statements_analyzer->node_data->setType($stmt, $stmt_type);
if ($context->finally_scope) {
foreach ($context->vars_in_scope as $var_id => $type) {
if (isset($context->finally_scope->vars_in_scope[$var_id])) {
if ($context->finally_scope->vars_in_scope[$var_id] !== $type) {
$context->finally_scope->vars_in_scope[$var_id] = Type::combineUnionTypes(
$context->finally_scope->vars_in_scope[$var_id],
$type,
$statements_analyzer->getCodebase()
);
}
} else {
$context->finally_scope->vars_in_scope[$var_id] = $type;
}
}
}
if ($source instanceof FunctionLikeAnalyzer
&& !($source->getSource() instanceof TraitAnalyzer)
) {

17
src/Psalm/Internal/Analyzer/Statements/ThrowAnalyzer.php
View File

@ -8,6 +8,7 @@ use Psalm\CodeLocation;
use Psalm\Context;
use Psalm\Issue\InvalidThrow;
use Psalm\IssueBuffer;
use Psalm\Type;
use Psalm\Type\Atomic\TNamedObject;
use Psalm\Type\Union;
@ -30,6 +31,22 @@ class ThrowAnalyzer
}
$context->inside_throw = false;
if ($context->finally_scope) {
foreach ($context->vars_in_scope as $var_id => $type) {
if (isset($context->finally_scope->vars_in_scope[$var_id])) {
if ($context->finally_scope->vars_in_scope[$var_id] !== $type) {
$context->finally_scope->vars_in_scope[$var_id] = Type::combineUnionTypes(
$context->finally_scope->vars_in_scope[$var_id],
$type,
$statements_analyzer->getCodebase()
);
}
} else {
$context->finally_scope->vars_in_scope[$var_id] = $type;
}
}
}
if ($context->check_classes
&& ($throw_type = $statements_analyzer->node_data->getType($stmt->expr))
&& !$throw_type->hasMixed()

17
src/Psalm/Internal/Scope/FinallyScope.php Normal file
View File

@ -0,0 +1,17 @@
<?php
namespace Psalm\Internal\Scope;
use Psalm\CodeLocation;
use Psalm\Context;
use Psalm\Type;
/**
* @internal
*/
class FinallyScope
{
/**
* @var array<string, Type\Union>
*/
public $vars_in_scope = [];
}

24
tests/TryCatchTest.php
View File

@ -469,6 +469,30 @@ class TryCatchTest extends TestCase
}',
'error_message' => 'NullableReturnStatement'
],
'isAlwaysDefinedInFinally' => [
'<?php
function maybeThrows() : void {
if (rand(0, 1)) {
throw new UnexpectedValueException();
}
}
function doTry() : void {
$exception = new \Exception();
try {
maybeThrows();
return;
} catch (Exception $exception) {
throw $exception;
} finally {
if ($exception) {
echo "here";
}
}
}',
'error_message' => 'RedundantCondition'
],
];
}
}