mirror of
https://github.com/danog/psalm.git
synced 2024-11-27 04:45:20 +01:00
Fix #2177 - mark variables in try block as potentially undefined
This commit is contained in:
parent
62a3598ed3
commit
94d4b876ba
@ -1217,8 +1217,6 @@ class Codebase
|
||||
*/
|
||||
public function getSignatureInformation(string $function_symbol) : ?\LanguageServerProtocol\SignatureInformation
|
||||
{
|
||||
$params = null;
|
||||
|
||||
if (strpos($function_symbol, '::') !== false) {
|
||||
$declaring_method_id = $this->methods->getDeclaringMethodId($function_symbol);
|
||||
|
||||
|
@ -78,6 +78,8 @@ class TryAnalyzer
|
||||
$old_unreferenced_vars = $try_context->unreferenced_vars;
|
||||
$newly_unreferenced_vars = [];
|
||||
|
||||
$old_context_vars = $context->vars_in_scope;
|
||||
|
||||
if ($statements_analyzer->analyze($stmt->stmts, $context) === false) {
|
||||
return false;
|
||||
}
|
||||
@ -159,10 +161,18 @@ class TryAnalyzer
|
||||
'TypeDoesNotContainType',
|
||||
];
|
||||
|
||||
$definitely_newly_assigned_var_ids = $newly_assigned_var_ids;
|
||||
|
||||
/** @var int $i */
|
||||
foreach ($stmt->catches as $i => $catch) {
|
||||
$catch_context = clone $original_context;
|
||||
|
||||
foreach ($catch_context->vars_in_scope as $var_id => $type) {
|
||||
if (!isset($old_context_vars[$var_id])) {
|
||||
$type->possibly_undefined_from_try = true;
|
||||
}
|
||||
}
|
||||
|
||||
$fq_catch_classes = [];
|
||||
|
||||
$catch_var_name = $catch->var->name;
|
||||
@ -312,6 +322,10 @@ class TryAnalyzer
|
||||
}
|
||||
}
|
||||
|
||||
$old_catch_assigned_var_ids = $catch_context->referenced_var_ids;
|
||||
|
||||
$catch_context->assigned_var_ids = [];
|
||||
|
||||
$statements_analyzer->analyze($catch->stmts, $catch_context);
|
||||
|
||||
// recalculate in case there's a no-return clause
|
||||
@ -327,6 +341,11 @@ class TryAnalyzer
|
||||
}
|
||||
}
|
||||
|
||||
/** @var array<string, bool> */
|
||||
$new_catch_assigned_var_ids = $catch_context->assigned_var_ids;
|
||||
|
||||
$catch_context->assigned_var_ids += $old_catch_assigned_var_ids;
|
||||
|
||||
$context->referenced_var_ids = array_intersect_key(
|
||||
$catch_context->referenced_var_ids,
|
||||
$context->referenced_var_ids
|
||||
@ -369,7 +388,16 @@ class TryAnalyzer
|
||||
}
|
||||
|
||||
if ($catch_actions[$i] !== [ScopeAnalyzer::ACTION_END]) {
|
||||
$definitely_newly_assigned_var_ids = array_intersect_key(
|
||||
$new_catch_assigned_var_ids,
|
||||
$definitely_newly_assigned_var_ids
|
||||
);
|
||||
|
||||
foreach ($catch_context->vars_in_scope as $var_id => $type) {
|
||||
if (!isset($old_context_vars[$var_id])) {
|
||||
$type->possibly_undefined_from_try = false;
|
||||
}
|
||||
|
||||
if ($stmt_control_actions === [ScopeAnalyzer::ACTION_END]) {
|
||||
$context->vars_in_scope[$var_id] = $type;
|
||||
} elseif (isset($context->vars_in_scope[$var_id])
|
||||
@ -386,13 +414,21 @@ class TryAnalyzer
|
||||
$catch_context->vars_possibly_in_scope,
|
||||
$context->vars_possibly_in_scope
|
||||
);
|
||||
} elseif ($stmt->finally) {
|
||||
} else {
|
||||
if ($stmt->finally) {
|
||||
$context->vars_possibly_in_scope = array_merge(
|
||||
$catch_context->vars_possibly_in_scope,
|
||||
$context->vars_possibly_in_scope
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
foreach ($definitely_newly_assigned_var_ids as $var_id => $_) {
|
||||
if (isset($context->vars_in_scope[$var_id])) {
|
||||
$context->vars_in_scope[$var_id]->possibly_undefined_from_try = false;
|
||||
}
|
||||
}
|
||||
|
||||
if ($context->loop_scope
|
||||
&& !$try_leaves_loop
|
||||
|
@ -272,6 +272,30 @@ class VariableFetchAnalyzer
|
||||
} else {
|
||||
$stmt->inferredType = clone $context->vars_in_scope[$var_name];
|
||||
|
||||
if ($stmt->inferredType->possibly_undefined_from_try && !$context->inside_isset) {
|
||||
if ($context->is_global) {
|
||||
if (IssueBuffer::accepts(
|
||||
new PossiblyUndefinedGlobalVariable(
|
||||
'Possibly undefined global variable ' . $var_name . ' defined in try block',
|
||||
new CodeLocation($statements_analyzer->getSource(), $stmt)
|
||||
),
|
||||
$statements_analyzer->getSuppressedIssues()
|
||||
)) {
|
||||
// fall through
|
||||
}
|
||||
} else {
|
||||
if (IssueBuffer::accepts(
|
||||
new PossiblyUndefinedVariable(
|
||||
'Possibly undefined variable ' . $var_name . ' defined in try block',
|
||||
new CodeLocation($statements_analyzer->getSource(), $stmt)
|
||||
),
|
||||
$statements_analyzer->getSuppressedIssues()
|
||||
)) {
|
||||
// fall through
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if ($codebase->store_node_types
|
||||
&& !$context->collect_initializations
|
||||
&& !$context->collect_mutations
|
||||
|
@ -60,8 +60,9 @@ class DisableCommand extends Command
|
||||
|
||||
$plugin_list = ($this->plugin_list_factory)($current_dir, $config_file_path);
|
||||
|
||||
try {
|
||||
$plugin_name = $i->getArgument('pluginName');
|
||||
|
||||
try {
|
||||
assert(is_string($plugin_name));
|
||||
|
||||
$plugin_class = $plugin_list->resolvePluginClass($plugin_name);
|
||||
|
@ -60,8 +60,9 @@ class EnableCommand extends Command
|
||||
|
||||
$plugin_list = ($this->plugin_list_factory)($current_dir, $config_file_path);
|
||||
|
||||
try {
|
||||
$plugin_name = $i->getArgument('pluginName');
|
||||
|
||||
try {
|
||||
assert(is_string($plugin_name));
|
||||
|
||||
$plugin_class = $plugin_list->resolvePluginClass($plugin_name);
|
||||
|
@ -110,14 +110,15 @@ class TryCatchTest extends TestCase
|
||||
'MixedMethodCall' => \Psalm\Config::REPORT_INFO,
|
||||
],
|
||||
],
|
||||
'issetAfterTryCatch' => [
|
||||
'issetAfterTryCatchWithoutAssignmentInCatch' => [
|
||||
'<?php
|
||||
function test(): string {
|
||||
throw new Exception("bad");
|
||||
}
|
||||
|
||||
try {
|
||||
$a = "foo";
|
||||
|
||||
try {
|
||||
$var = test();
|
||||
} catch (Exception $e) {
|
||||
echo "bad";
|
||||
@ -127,23 +128,45 @@ class TryCatchTest extends TestCase
|
||||
|
||||
echo $a;',
|
||||
],
|
||||
'issetAfterTryCatchWithCombinedType' => [
|
||||
'issetAfterTryCatchWithAssignmentInCatch' => [
|
||||
'<?php
|
||||
function test(): string {
|
||||
throw new Exception("bad");
|
||||
}
|
||||
|
||||
try {
|
||||
$a = "foo";
|
||||
|
||||
try {
|
||||
$var = test();
|
||||
} catch (Exception $e) {
|
||||
$var = "bad";
|
||||
}
|
||||
|
||||
if (isset($var)) {}
|
||||
|
||||
echo $var;
|
||||
echo $a;',
|
||||
],
|
||||
'issetAfterTryCatchWithIfInCatch' => [
|
||||
'<?php
|
||||
function test(): string {
|
||||
throw new Exception("bad");
|
||||
}
|
||||
|
||||
function foo() : void {
|
||||
$a = null;
|
||||
|
||||
$params = null;
|
||||
|
||||
try {
|
||||
$a = test();
|
||||
|
||||
$params = $a;
|
||||
} catch (\Exception $exception) {
|
||||
$params = "hello";
|
||||
}
|
||||
|
||||
echo $params;
|
||||
}',
|
||||
],
|
||||
'noRedundantConditionsInFinally' => [
|
||||
'<?php
|
||||
function doThings(): void {}
|
||||
@ -318,6 +341,32 @@ class TryCatchTest extends TestCase
|
||||
}',
|
||||
'error_message' => 'InvalidReturnType',
|
||||
],
|
||||
'preventPossiblyUndefinedVarInTry' => [
|
||||
'<?php
|
||||
class Foo {
|
||||
public static function possiblyThrows(): bool {
|
||||
$result = (bool)rand(0, 1);
|
||||
|
||||
if (!$result) {
|
||||
throw new \Exception("BOOM");
|
||||
}
|
||||
|
||||
return true;
|
||||
}
|
||||
}
|
||||
|
||||
try {
|
||||
$result = Foo::possiblyThrows();
|
||||
$a = "ACME";
|
||||
|
||||
if ($result) {
|
||||
echo $a;
|
||||
}
|
||||
} catch (\Exception $e) {
|
||||
echo $a;
|
||||
}',
|
||||
'error_message' => 'PossiblyUndefinedGlobalVariable',
|
||||
],
|
||||
];
|
||||
}
|
||||
}
|
||||
|
Loading…
Reference in New Issue
Block a user