From 8c49ffbebe9dc18f2dc4ae0c4b08027bc043bbb9 Mon Sep 17 00:00:00 2001 From: AndrolGenhald Date: Mon, 14 Feb 2022 16:43:54 -0600 Subject: [PATCH 1/7] Rewrite TryAnalyzer. --- config.xsd | 1 + docs/running_psalm/issues.md | 1 + docs/running_psalm/issues/RedundantCatch.md | 23 + src/Psalm/Context.php | 30 +- .../Analyzer/Statements/Block/TryAnalyzer.php | 836 ++++++++++-------- .../Analyzer/Statements/BreakAnalyzer.php | 18 +- .../Analyzer/Statements/ContinueAnalyzer.php | 16 - .../Expression/AssignmentAnalyzer.php | 36 +- .../Expression/Call/ArgumentAnalyzer.php | 6 + .../Call/NamedFunctionCallHandler.php | 3 + .../Fetch/VariableFetchAnalyzer.php | 39 +- .../Analyzer/Statements/ReturnAnalyzer.php | 16 - .../Analyzer/Statements/StaticAnalyzer.php | 3 + .../Analyzer/Statements/ThrowAnalyzer.php | 16 - .../Analyzer/Statements/UnsetAnalyzer.php | 4 + .../Statements/UnusedAssignmentRemover.php | 62 +- .../Reflector/FunctionLikeNodeScanner.php | 2 +- src/Psalm/Internal/Scope/FinallyScope.php | 24 - src/Psalm/Internal/Scope/TryCatchScope.php | 43 + .../Type/SimpleNegatedAssertionReconciler.php | 6 + src/Psalm/Issue/RedundantCatch.php | 17 + tests/DocumentationTest.php | 1 + .../UnusedVariableManipulationTest.php | 79 +- tests/MemoizeTest.php | 243 +++++ tests/MethodCallTest.php | 169 ---- tests/Traits/ValidCodeAnalysisTestTrait.php | 16 +- tests/TryCatchTest.php | 670 +++++++++++++- .../RedundantConditionTest.php | 24 + tests/UnusedCodeTest.php | 2 +- tests/UnusedVariableTest.php | 48 +- 30 files changed, 1762 insertions(+), 692 deletions(-) create mode 100644 docs/running_psalm/issues/RedundantCatch.md delete mode 100644 src/Psalm/Internal/Scope/FinallyScope.php create mode 100644 src/Psalm/Internal/Scope/TryCatchScope.php create mode 100644 src/Psalm/Issue/RedundantCatch.php create mode 100644 tests/MemoizeTest.php diff --git a/config.xsd b/config.xsd index 3fe1e510b43..91aaea41e56 100644 --- a/config.xsd +++ b/config.xsd @@ -399,6 +399,7 @@ + diff --git a/docs/running_psalm/issues.md b/docs/running_psalm/issues.md index 1411a772fb1..fd9c9a663fb 100644 --- a/docs/running_psalm/issues.md +++ b/docs/running_psalm/issues.md @@ -212,6 +212,7 @@ - [RawObjectIteration](issues/RawObjectIteration.md) - [RedundantCast](issues/RedundantCast.md) - [RedundantCastGivenDocblockType](issues/RedundantCastGivenDocblockType.md) + - [RedundantCatch](issues/RedundantCatch.md) - [RedundantCondition](issues/RedundantCondition.md) - [RedundantConditionGivenDocblockType](issues/RedundantConditionGivenDocblockType.md) - [RedundantFunctionCall](issues/RedundantFunctionCall.md) diff --git a/docs/running_psalm/issues/RedundantCatch.md b/docs/running_psalm/issues/RedundantCatch.md new file mode 100644 index 00000000000..8f140db14da --- /dev/null +++ b/docs/running_psalm/issues/RedundantCatch.md @@ -0,0 +1,23 @@ +# RedundantCatch + +Emitted when catching an exception that was already caught. + +```php +vars_in_scope; + $this->vars_in_scope = []; + foreach ($old_vars_in_scope as $var_id => $var_type) { + $this->vars_in_scope[$var_id] = clone $var_type; + } + foreach ($this->references_in_scope as $reference_id => $referenced_id) { + $this->vars_in_scope[$reference_id] = &$this->vars_in_scope[$referenced_id]; + } + } + /** * Updates the parent context, looking at the changes within a block and then applying those changes, where * necessary, to the parent context diff --git a/src/Psalm/Internal/Analyzer/Statements/Block/TryAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Block/TryAnalyzer.php index 70c2db92b3c..a2c0e5adb02 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Block/TryAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Block/TryAnalyzer.php @@ -3,31 +3,34 @@ namespace Psalm\Internal\Analyzer\Statements\Block; use PhpParser; +use PhpParser\Node\Stmt\Catch_; use Psalm\CodeLocation; +use Psalm\Codebase; use Psalm\Context; use Psalm\Internal\Analyzer\ClassLikeAnalyzer; use Psalm\Internal\Analyzer\ClassLikeNameOptions; use Psalm\Internal\Analyzer\ScopeAnalyzer; use Psalm\Internal\Analyzer\StatementsAnalyzer; -use Psalm\Internal\Codebase\VariableUseGraph; use Psalm\Internal\DataFlow\DataFlowNode; -use Psalm\Internal\Scope\FinallyScope; +use Psalm\Internal\Scope\TryCatchScope; +use Psalm\Internal\Type\Comparator\UnionTypeComparator; use Psalm\Issue\InvalidCatch; +use Psalm\Issue\ParseError; +use Psalm\Issue\RedundantCatch; use Psalm\IssueBuffer; use Psalm\Type; use Psalm\Type\Atomic\TNamedObject; use Psalm\Type\Union; -use UnexpectedValueException; -use function array_intersect_key; -use function array_map; +use function array_column; use function array_merge; +use function array_values; +use function assert; +use function count; use function in_array; use function is_string; +use function strpos; use function strtolower; -use function version_compare; - -use const PHP_VERSION; /** * @internal @@ -66,50 +69,38 @@ public static function analyze( $old_context = clone $context; - if ($all_catches_leave && !$stmt->finally) { - $try_context = $context; - } else { - $try_context = clone $context; + $old_try_catch_scope = $context->try_catch_scope; + $context->try_catch_scope = new TryCatchScope(); + if (!$all_catches_leave || $stmt->finally) { 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($try_context->vars_in_scope); + // TODO can this be moved? + $context->branch_point = $context->branch_point ?: (int) $stmt->getAttribute('startFilePos'); } } - $assigned_var_ids = $try_context->assigned_var_ids; + $assigned_var_ids = $context->assigned_var_ids; $context->assigned_var_ids = []; - $old_referenced_var_ids = $try_context->referenced_var_ids; - - $was_inside_try = $context->inside_try; - $context->inside_try = true; if ($statements_analyzer->analyze($stmt->stmts, $context) === false) { + $context->try_catch_scope = $old_try_catch_scope; return false; } - $context->inside_try = $was_inside_try; - - if ($try_context->finally_scope) { - foreach ($context->vars_in_scope as $var_id => $type) { - $try_context->finally_scope->vars_in_scope[$var_id] = Type::combineUnionTypes( - $try_context->finally_scope->vars_in_scope[$var_id] ?? null, - $type, - $statements_analyzer->getCodebase() - ); - } + + $end_of_try_context = clone $context; + $end_of_try_context->cloneVarsInScope(); + + // Now that the `try` has been analyzed, we have to update the context under the + // assumption that an exception may have been thrown at any point within the `try`. + self::updateContextWithTryAssignments($context, $old_context, $codebase); + + if ($old_try_catch_scope !== null) { + $old_try_catch_scope->applyInnerScope($context->try_catch_scope); } + $context->try_catch_scope = $old_try_catch_scope; $context->has_returned = false; - $try_block_control_actions = ScopeAnalyzer::getControlActions( - $stmt->stmts, - $statements_analyzer->node_data, - [] - ); - /** @var array */ $newly_assigned_var_ids = $context->assigned_var_ids; @@ -118,396 +109,543 @@ public static function analyze( $newly_assigned_var_ids ); - $possibly_referenced_var_ids = array_merge( - $context->referenced_var_ids, - $old_referenced_var_ids - ); - - if ($try_context !== $context) { - foreach ($context->vars_in_scope as $var_id => $type) { - if (!isset($try_context->vars_in_scope[$var_id])) { - $try_context->vars_in_scope[$var_id] = clone $type; - - $context->vars_in_scope[$var_id]->possibly_undefined = true; - $context->vars_in_scope[$var_id]->possibly_undefined_from_try = true; - } else { - $try_context->vars_in_scope[$var_id] = Type::combineUnionTypes( - $try_context->vars_in_scope[$var_id], - $type - ); - } - } - - $try_context->vars_possibly_in_scope = $context->vars_possibly_in_scope; - $try_context->possibly_thrown_exceptions = $context->possibly_thrown_exceptions; - - $context->referenced_var_ids = array_intersect_key( - $try_context->referenced_var_ids, - $context->referenced_var_ids - ); - } - $try_leaves_loop = $context->loop_scope && $context->loop_scope->final_actions && !in_array(ScopeAnalyzer::ACTION_NONE, $context->loop_scope->final_actions, true); - if (!$all_catches_leave) { - foreach ($newly_assigned_var_ids as $assigned_var_id => $_) { - $context->removeVarFromConflictingClauses($assigned_var_id); + // Maps catch block number to list of tuple(caught exception, CodeLocation for that exception) + $caught_types = []; + $has_universal_catch = false; + // array set of leaving catches + $leaving_catches = []; + $catch_contexts = []; + $i = -1; + /** @var int<0, max> $i */ + foreach ($stmt->catches as $i => $catch) { + $catch_context = self::analyzeCatch( + $context, + $statements_analyzer, + $catch, + $has_universal_catch, + $caught_types, + $i, + ); + $catch_contexts[$i] = $catch_context; + + // recalculate in case there's a no-return clause + $catch_actions[$i] = ScopeAnalyzer::getControlActions( + $catch->stmts, + $statements_analyzer->node_data, + [], + ); + if (!in_array(ScopeAnalyzer::ACTION_NONE, $catch_actions[$i], true)) { + $leaving_catches[$i] = true; } - } else { - foreach ($newly_assigned_var_ids as $assigned_var_id => $_) { - $try_context->removeVarFromConflictingClauses($assigned_var_id); + + if ($catch_context->collect_exceptions) { + $context->mergeExceptions($catch_context); } } + $catch_block_count = $i + 1; + $all_catches_leave = count($leaving_catches) === $catch_block_count; + $vars_at_ends_of_catches = self::collectVarsFromCatchBlocks($catch_contexts); - // at this point we have two contexts – $context, in which it is assumed that everything was fine, - // and $try_context - which allows all variables to have the union of the values before and after - // the try was applied - $original_context = clone $try_context; + $try_block_control_actions = ScopeAnalyzer::getControlActions( + $stmt->stmts, + $statements_analyzer->node_data, + [], + ); + $try_leaves_scope = $try_block_control_actions === [ScopeAnalyzer::ACTION_END]; - $issues_to_suppress = [ - 'RedundantCondition', - 'RedundantConditionGivenDocblockType', - 'TypeDoesNotContainNull', - 'TypeDoesNotContainType', - ]; + $finally_has_returned = false; + if ($stmt->finally !== null) { + // The `finally` analysis is run twice, once with types set to what they'll + // actually be for the `finally` scope, and again with issues ignored with types + // set to what they'll be in the outer scope in order to update the outer scope. + + // Initial analysis with correct types. + $finally_context = clone $context; + $finally_context->cloneVarsInScope(); + self::applyCatchContextsForFinallyContext( + $codebase, + $finally_context, + $end_of_try_context, + $vars_at_ends_of_catches, + $catch_block_count, + $has_universal_catch, + ); + $statements_analyzer->analyze($stmt->finally->stmts, $finally_context); + $finally_has_returned = $finally_context->has_returned; + } - $definitely_newly_assigned_var_ids = $newly_assigned_var_ids; + if ($context->loop_scope + && !$try_leaves_loop + && !in_array(ScopeAnalyzer::ACTION_NONE, $context->loop_scope->final_actions, true) + ) { + $context->loop_scope->final_actions[] = ScopeAnalyzer::ACTION_NONE; + } - /** @var int $i */ - foreach ($stmt->catches as $i => $catch) { - $catch_context = clone $original_context; - $catch_context->has_returned = false; - - foreach ($catch_context->vars_in_scope as $var_id => $type) { - if (!isset($old_context->vars_in_scope[$var_id])) { - $type = clone $type; - $type->possibly_undefined_from_try = true; - $catch_context->vars_in_scope[$var_id] = $type; - } else { - $catch_context->vars_in_scope[$var_id] = Type::combineUnionTypes( - $type, - $old_context->vars_in_scope[$var_id] - ); - } - } + self::applyCatchContextsForOuterContext( + $codebase, + $context, + $end_of_try_context, + $vars_at_ends_of_catches, + $catch_block_count, + $leaving_catches, + $try_leaves_scope, + ); - $fq_catch_classes = []; + if ($stmt->finally !== null) { + // Second `finally` analysis with types for the outer scope + IssueBuffer::startRecording(); + $statements_analyzer->analyze($stmt->finally->stmts, $context); + IssueBuffer::clearRecordingLevel(); + IssueBuffer::stopRecording(); + } - if (!$catch->types) { - throw new UnexpectedValueException('Very bad'); + foreach ($existing_thrown_exceptions as $possibly_thrown_exception => $codelocations) { + foreach ($codelocations as $hash => $codelocation) { + $context->possibly_thrown_exceptions[$possibly_thrown_exception][$hash] = $codelocation; } + } - foreach ($catch->types as $catch_type) { - $fq_catch_class = ClassLikeAnalyzer::getFQCLNFromNameObject( - $catch_type, - $statements_analyzer->getAliases() - ); - - $fq_catch_class = $codebase->classlikes->getUnAliasedName($fq_catch_class); + $body_has_returned = !in_array(ScopeAnalyzer::ACTION_NONE, $try_block_control_actions, true); + $context->has_returned = ($body_has_returned && $all_catches_leave) || $finally_has_returned; - if ($codebase->alter_code && $fq_catch_class) { - $codebase->classlikes->handleClassLikeReferenceInMigration( - $codebase, - $statements_analyzer, - $catch_type, - $fq_catch_class, - $context->calling_method_id - ); - } + // Remove all newly memoized properties and method calls from the context + foreach ($context->vars_in_scope as $var_id => $var_type) { + if (!isset($context->vars_in_scope[$var_id])) { + continue; // Removed as descendant in previous iteration + } - if ($original_context->check_classes) { - if (ClassLikeAnalyzer::checkFullyQualifiedClassLikeName( - $statements_analyzer, - $fq_catch_class, - new CodeLocation($statements_analyzer->getSource(), $catch_type, $context->include_location), - $context->self, - $context->calling_method_id, - $statements_analyzer->getSuppressedIssues(), - new ClassLikeNameOptions(true) - ) === false) { - // fall through - } - } + if (strpos($var_id, "->") !== false) { + // Set memoized type as always defined + $var_type->possibly_undefined = $var_type->possibly_undefined_from_try = false; - if (($codebase->classExists($fq_catch_class) - && strtolower($fq_catch_class) !== 'exception' - && !($codebase->classExtends($fq_catch_class, 'Exception') - || $codebase->classImplements($fq_catch_class, 'Throwable'))) - || ($codebase->interfaceExists($fq_catch_class) - && strtolower($fq_catch_class) !== 'throwable' - && !$codebase->interfaceExtends($fq_catch_class, 'Throwable')) + if (!isset($old_context->vars_in_scope[$var_id]) + || !UnionTypeComparator::isContainedBy($codebase, $var_type, $old_context->vars_in_scope[$var_id]) + || !UnionTypeComparator::isContainedBy($codebase, $old_context->vars_in_scope[$var_id], $var_type) ) { - IssueBuffer::maybeAdd( - new InvalidCatch( - 'Class/interface ' . $fq_catch_class . ' cannot be caught', - new CodeLocation($statements_analyzer->getSource(), $stmt), - $fq_catch_class - ), - $statements_analyzer->getSuppressedIssues() - ); + // If it doesn't exist in the outer scope or the type has changed, remove it + $context->remove($var_id); } - - $fq_catch_classes[] = $fq_catch_class; } + } - if ($catch_context->collect_exceptions) { - foreach ($fq_catch_classes as $fq_catch_class) { - $fq_catch_class_lower = strtolower($fq_catch_class); - - foreach ($catch_context->possibly_thrown_exceptions as $exception_fqcln => $_) { - $exception_fqcln_lower = strtolower($exception_fqcln); - - if ($exception_fqcln_lower === $fq_catch_class_lower - || ($codebase->classExists($exception_fqcln) - && $codebase->classExtendsOrImplements($exception_fqcln, $fq_catch_class)) - || ($codebase->interfaceExists($exception_fqcln) - && $codebase->interfaceExtends($exception_fqcln, $fq_catch_class)) - ) { - unset($original_context->possibly_thrown_exceptions[$exception_fqcln]); - unset($context->possibly_thrown_exceptions[$exception_fqcln]); - unset($catch_context->possibly_thrown_exceptions[$exception_fqcln]); - } - } - } + return null; + } - $catch_context->possibly_thrown_exceptions = []; - } + /** + * @param array> $caught_types Maps catch block number to list of + * caught types and their associated + * CodeLocations. + */ + private static function analyzeCatch( + Context $before_catch_context, + StatementsAnalyzer $statements_analyzer, + Catch_ $catch, + bool &$has_universal_catch, + array &$caught_types, + int $catch_number + ): Context { + $codebase = $statements_analyzer->getCodebase(); - // discard all clauses because crazy stuff may have happened in try block - $catch_context->clauses = []; - - if ($catch->var && is_string($catch->var->name)) { - $catch_var_id = '$' . $catch->var->name; - - $catch_context->vars_in_scope[$catch_var_id] = new Union( - array_map( - function (string $fq_catch_class) use ($codebase): TNamedObject { - $catch_class_type = new TNamedObject($fq_catch_class); - - if (version_compare(PHP_VERSION, '7.0.0dev', '>=') - && strtolower($fq_catch_class) !== 'throwable' - && $codebase->interfaceExists($fq_catch_class) - && !$codebase->interfaceExtends($fq_catch_class, 'Throwable') - ) { - $catch_class_type->addIntersectionType(new TNamedObject('Throwable')); - } - - return $catch_class_type; - }, - $fq_catch_classes - ) - ); + $catch_context = clone $before_catch_context; + $catch_context->cloneVarsInScope(); + $catch_context->has_returned = false; + $catch_context->try_catch_scope = new TryCatchScope(); - // removes dependent vars from $context - $catch_context->removeDescendents( - $catch_var_id, - $catch_context->vars_in_scope[$catch_var_id], - $catch_context->vars_in_scope[$catch_var_id], - $statements_analyzer - ); + $fq_catch_classes = []; - $catch_context->vars_possibly_in_scope[$catch_var_id] = true; + assert(!empty($catch->types)); + foreach ($catch->types as $catch_type_stmt) { + $fq_catch_class = ClassLikeAnalyzer::getFQCLNFromNameObject( + $catch_type_stmt, + $statements_analyzer->getAliases() + ); - $location = new CodeLocation($statements_analyzer->getSource(), $catch->var); + $fq_catch_class = $codebase->classlikes->getUnAliasedName($fq_catch_class); - if (!$statements_analyzer->hasVariable($catch_var_id)) { - $statements_analyzer->registerVariable( - $catch_var_id, - $location, - $catch_context->branch_point - ); - } else { - $statements_analyzer->registerVariableAssignment( - $catch_var_id, - $location - ); + if ($codebase->alter_code && $fq_catch_class) { + $codebase->classlikes->handleClassLikeReferenceInMigration( + $codebase, + $statements_analyzer, + $catch_type_stmt, + $fq_catch_class, + $catch_context->calling_method_id + ); + } + + if ($catch_context->check_classes) { + if (ClassLikeAnalyzer::checkFullyQualifiedClassLikeName( + $statements_analyzer, + $fq_catch_class, + new CodeLocation( + $statements_analyzer->getSource(), + $catch_type_stmt, + $catch_context->include_location, + ), + $catch_context->self, + $catch_context->calling_method_id, + $statements_analyzer->getSuppressedIssues(), + new ClassLikeNameOptions(true) + ) === false) { + // fall through } + } - if ($statements_analyzer->data_flow_graph) { - $catch_var_node = DataFlowNode::getForAssignment($catch_var_id, $location); + $caught_atomic = new TNamedObject($fq_catch_class); + + if (($codebase->classExists($fq_catch_class) + && strtolower($fq_catch_class) !== 'exception' + && !($codebase->classExtends($fq_catch_class, 'Exception') + || $codebase->classImplements($fq_catch_class, 'Throwable'))) + || ($codebase->interfaceExists($fq_catch_class) + && strtolower($fq_catch_class) !== 'throwable' + && !$codebase->interfaceExtends($fq_catch_class, 'Throwable')) + ) { + IssueBuffer::maybeAdd( + new InvalidCatch( + 'Class/interface ' . $fq_catch_class . ' cannot be caught', + new CodeLocation($statements_analyzer->getSource(), $catch), + $fq_catch_class + ), + $statements_analyzer->getSuppressedIssues() + ); + // Since we already show an InvalidCatch, we just assume that it's a Throwable from here on out. + $caught_atomic->addIntersectionType(new TNamedObject('Throwable')); + } - $catch_context->vars_in_scope[$catch_var_id]->parent_nodes = [ - $catch_var_node->id => $catch_var_node - ]; + $has_universal_catch = $has_universal_catch || strtolower($fq_catch_class) === "throwable"; - if ($statements_analyzer->data_flow_graph instanceof VariableUseGraph) { - $statements_analyzer->data_flow_graph->addPath( - $catch_var_node, - new DataFlowNode('variable-use', 'variable use', null), - 'variable-use' + // Handle redundant catches + $caught_type = new Union([$caught_atomic]); + $caught_location = new CodeLocation($statements_analyzer->getSource(), $catch_type_stmt); + foreach ($caught_types as $already_caught_block => $caught_types_list) { + foreach ($caught_types_list as [$already_caught, $already_caught_location]) { + if (UnionTypeComparator::isContainedBy( + $codebase, + $caught_type, + $already_caught, + )) { + IssueBuffer::maybeAdd( + new RedundantCatch( + "{$caught_type->getId()} has already been caught", + $caught_location, + $caught_type->getId(), + ), + $statements_analyzer->getSuppressedIssues(), + ); + } elseif ($already_caught_block === $catch_number && UnionTypeComparator::isContainedBy( + $codebase, + $already_caught, + $caught_type, + )) { + // If this is the same catch block it's still redundant even if the wider type appears later + IssueBuffer::maybeAdd( + new RedundantCatch( + "{$already_caught->getId()} has already been caught", + $already_caught_location, + $already_caught->getId(), + ), + $statements_analyzer->getSuppressedIssues(), ); } } } + $caught_types[$catch_number][] = [$caught_type, $caught_location]; + + $fq_catch_classes[] = $fq_catch_class; + } + + if ($catch_context->collect_exceptions) { + foreach ($fq_catch_classes as $fq_catch_class) { + $fq_catch_class_lower = strtolower($fq_catch_class); - $suppressed_issues = $statements_analyzer->getSuppressedIssues(); + foreach ($catch_context->possibly_thrown_exceptions as $exception_fqcln => $_) { + $exception_fqcln_lower = strtolower($exception_fqcln); - foreach ($issues_to_suppress as $issue_to_suppress) { - if (!in_array($issue_to_suppress, $suppressed_issues, true)) { - $statements_analyzer->addSuppressedIssues([$issue_to_suppress]); + if ($exception_fqcln_lower === $fq_catch_class_lower + || ($codebase->classExists($exception_fqcln) + && $codebase->classExtendsOrImplements($exception_fqcln, $fq_catch_class)) + || ($codebase->interfaceExists($exception_fqcln) + && $codebase->interfaceExtends($exception_fqcln, $fq_catch_class)) + ) { + unset($before_catch_context->possibly_thrown_exceptions[$exception_fqcln]); + unset($catch_context->possibly_thrown_exceptions[$exception_fqcln]); + } } } - $old_catch_assigned_var_ids = $catch_context->referenced_var_ids; + $catch_context->possibly_thrown_exceptions = []; + } - $catch_context->assigned_var_ids = []; + // discard all clauses because crazy stuff may have happened in try block + $catch_context->clauses = []; - $statements_analyzer->analyze($catch->stmts, $catch_context); + if ($catch->var && is_string($catch->var->name)) { + $catch_var_id = '$' . $catch->var->name; - // recalculate in case there's a no-return clause - $catch_actions[$i] = ScopeAnalyzer::getControlActions( - $catch->stmts, - $statements_analyzer->node_data, - [] + $catch_context->vars_in_scope[$catch_var_id] = Type::combineUnionTypeArray( + array_column($caught_types[$catch_number], 0), + $codebase, ); - foreach ($issues_to_suppress as $issue_to_suppress) { - if (!in_array($issue_to_suppress, $suppressed_issues, true)) { - $statements_analyzer->removeSuppressedIssues([$issue_to_suppress]); - } - } + // removes dependent vars from $catch_context + $catch_context->removeDescendents( + $catch_var_id, + $catch_context->vars_in_scope[$catch_var_id], + $catch_context->vars_in_scope[$catch_var_id], + $statements_analyzer + ); - /** @var array */ - $new_catch_assigned_var_ids = $catch_context->assigned_var_ids; + $catch_context->vars_possibly_in_scope[$catch_var_id] = true; - $catch_context->assigned_var_ids += $old_catch_assigned_var_ids; + $location = new CodeLocation($statements_analyzer->getSource(), $catch->var); - $context->referenced_var_ids = array_intersect_key( - $catch_context->referenced_var_ids, - $context->referenced_var_ids - ); + if (!$statements_analyzer->hasVariable($catch_var_id)) { + $statements_analyzer->registerVariable( + $catch_var_id, + $location, + $catch_context->branch_point + ); + } else { + $statements_analyzer->registerVariableAssignment( + $catch_var_id, + $location + ); + } - $possibly_referenced_var_ids = array_merge( - $catch_context->referenced_var_ids, - $possibly_referenced_var_ids - ); + if ($statements_analyzer->data_flow_graph) { + $catch_var_node = DataFlowNode::getForAssignment($catch_var_id, $location); - if ($catch_context->collect_exceptions) { - $context->mergeExceptions($catch_context); + $catch_context->vars_in_scope[$catch_var_id]->parent_nodes = [ + $catch_var_node->id => $catch_var_node + ]; } + } elseif ($catch->var === null && $codebase->analysis_php_version_id < 8_00_00) { + IssueBuffer::maybeAdd( + new ParseError( + "Catch must have variable before PHP 8.0", + new CodeLocation($statements_analyzer->getSource(), $catch), + ), + $statements_analyzer->getSuppressedIssues(), + ); + } - $catch_doesnt_leave_parent_scope = $catch_actions[$i] !== [ScopeAnalyzer::ACTION_END] - && $catch_actions[$i] !== [ScopeAnalyzer::ACTION_CONTINUE] - && $catch_actions[$i] !== [ScopeAnalyzer::ACTION_BREAK]; + $statements_analyzer->analyze($catch->stmts, $catch_context); - if ($catch_doesnt_leave_parent_scope) { - $definitely_newly_assigned_var_ids = array_intersect_key( - $new_catch_assigned_var_ids, - $definitely_newly_assigned_var_ids - ); + if ($catch_context->collect_exceptions) { + $before_catch_context->mergeExceptions($catch_context); + } - foreach ($catch_context->vars_in_scope as $var_id => $type) { - if ($try_block_control_actions === [ScopeAnalyzer::ACTION_END]) { - $context->vars_in_scope[$var_id] = $type; - } elseif (isset($context->vars_in_scope[$var_id])) { - $context->vars_in_scope[$var_id] = Type::combineUnionTypes( - $context->vars_in_scope[$var_id], - $type - ); - } - } + if ($before_catch_context->try_catch_scope !== null) { + $before_catch_context->try_catch_scope->applyInnerScope($catch_context->try_catch_scope); + } - $context->vars_possibly_in_scope = array_merge( - $catch_context->vars_possibly_in_scope, - $context->vars_possibly_in_scope - ); + return $catch_context; + } + + /** + * Update a context that has a try_catch_scope to combine assignments from a `try` scope. + */ + private static function updateContextWithTryAssignments( + Context $context, + Context $outer_context, + Codebase $codebase + ): void { + assert($context->try_catch_scope !== null); + + // Reset $vars_in_scope to outer scope, in case any types have been + // reconciled (see TryCatchTest typeDoesNotContainTypeInCatch). + $context->vars_in_scope = $outer_context->vars_in_scope; + $context->references_in_scope = $outer_context->references_in_scope; + + foreach ($context->try_catch_scope->assignments_from_scope as $var_id => $assigned_types) { + // All assignments in the `try` are unioned together and combined with the original type + if (isset($outer_context->vars_in_scope[$var_id])) { + $assigned_types[] = clone $outer_context->vars_in_scope[$var_id]; + $possibly_undefined_from_try = false; } else { - if ($stmt->finally) { - $context->vars_possibly_in_scope = array_merge( - $catch_context->vars_possibly_in_scope, - $context->vars_possibly_in_scope - ); - } + // Variable didn't exist before try + $possibly_undefined_from_try = true; } - - 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; - $type->possibly_undefined = true; - $type->possibly_undefined_from_try = true; - } - } + $context->vars_in_scope[$var_id] = Type::combineUnionTypeArray($assigned_types, $codebase); + $context->vars_in_scope[$var_id]->possibly_undefined = + $possibly_undefined_from_try ?: $outer_context->vars_in_scope[$var_id]->possibly_undefined; + $context->vars_in_scope[$var_id]->possibly_undefined_from_try = + $possibly_undefined_from_try ?: $outer_context->vars_in_scope[$var_id]->possibly_undefined_from_try; + } + foreach ($context->try_catch_scope->unset_from_scope as $var_id => $_) { + if (!isset($context->vars_in_scope[$var_id])) { + // If the variable no longer exists (ie it wasn't reassigned + // after being unset), reset it to its old value. + $context->vars_in_scope[$var_id] = clone $outer_context->vars_in_scope[$var_id]; } + $context->vars_in_scope[$var_id]->possibly_undefined = true; + $context->vars_in_scope[$var_id]->possibly_undefined_from_try = true; } + } - if ($context->loop_scope - && !$try_leaves_loop - && !in_array(ScopeAnalyzer::ACTION_NONE, $context->loop_scope->final_actions, true) - ) { - $context->loop_scope->final_actions[] = ScopeAnalyzer::ACTION_NONE; + /** + * Collects variable types from `catch` blocks into a format that's easier to use. + * + * @param iterable, Context> $catch_contexts + * + * @return array, Union>> + */ + private static function collectVarsFromCatchBlocks(iterable $catch_contexts): array + { + $vars_at_ends_of_catches = []; + foreach ($catch_contexts as $catch_number => $catch_context) { + foreach ($catch_context->vars_in_scope as $var_id => $var_type) { + $vars_at_ends_of_catches[$var_id][$catch_number] = $var_type; + } } + return $vars_at_ends_of_catches; + } - $finally_has_returned = false; - if ($stmt->finally) { - if ($try_context->finally_scope) { - $finally_context = clone $context; + /** + * Applies variables from `catch` blocks to the current context in preparation for the `finally` block. + * + * @param array, Union>> $vars_at_ends_of_catches + */ + private static function applyCatchContextsForFinallyContext( + Codebase $codebase, + Context $context, + Context $end_of_try_context, + array $vars_at_ends_of_catches, + int $catch_block_count, + bool $has_universal_catch + ): void { + // Update context based on types from `catch` blocks. + foreach ($vars_at_ends_of_catches as $var_id => $catch_types) { + // Check if variable is always defined in `catch` blocks, including those that leave the scope. + $always_defined_in_catches = count($catch_types) === $catch_block_count; + if ($always_defined_in_catches) { + foreach ($catch_types as $type) { + if ($type->possibly_undefined) { + $always_defined_in_catches = false; + break; + } + } + } - $finally_context->assigned_var_ids = []; - $finally_context->possibly_assigned_var_ids = []; + // Union all catch types + $types = array_values($catch_types); - $finally_context->vars_in_scope = $try_context->finally_scope->vars_in_scope; + if ($always_defined_in_catches) { + if ($has_universal_catch) { + // Since the variable is always defined in the `catch` blocks, it's + // either the type at the end of the `try` or one of the `catch` types. + $previous_type = $end_of_try_context->vars_in_scope[$var_id] ?? null; + } else { + // Since there isn't a universal `catch`, the `finally` context can + // still have the variable with a type from any point within the `try`. + $previous_type = $context->vars_in_scope[$var_id] ?? null; + } + $possibly_undefined = $previous_type->possibly_undefined ?? true; + } else { + $previous_type = $context->vars_in_scope[$var_id] ?? null; + $possibly_undefined = true; + } + if ($previous_type !== null) { + $types[] = $previous_type; + } - $statements_analyzer->analyze($stmt->finally->stmts, $finally_context); + if (count($types) === 0) { + $context->remove($var_id); + continue; + } - $finally_has_returned = $finally_context->has_returned; + $context->vars_in_scope[$var_id] = Type::combineUnionTypeArray($types, $codebase); + $context->vars_in_scope[$var_id]->possibly_undefined = $possibly_undefined; + $context->vars_in_scope[$var_id]->possibly_undefined_from_try = false; + } + } - /** @var string $var_id */ - foreach ($finally_context->assigned_var_ids as $var_id => $_) { - if (isset($context->vars_in_scope[$var_id]) - && isset($finally_context->vars_in_scope[$var_id]) - ) { - if ($context->vars_in_scope[$var_id]->possibly_undefined - && $context->vars_in_scope[$var_id]->possibly_undefined_from_try - ) { - $context->vars_in_scope[$var_id]->possibly_undefined = false; - $context->vars_in_scope[$var_id]->possibly_undefined_from_try = false; + /** + * Applies variables from `catch` blocks to the current context. + * + * @param array, Union>> $vars_at_ends_of_catches + * @param array $leaving_catches + */ + private static function applyCatchContextsForOuterContext( + Codebase $codebase, + Context $context, + Context $end_of_try_context, + array $vars_at_ends_of_catches, + int $catch_block_count, + array $leaving_catches, + bool $try_leaves_scope + ): void { + if ($catch_block_count > 0) { + // Update context based on types from `catch` blocks. + foreach ($vars_at_ends_of_catches as $var_id => $catch_types) { + // Check if variable is defined in all non-leaving `catch` blocks + $always_defined_in_catches = count($catch_types + $leaving_catches) === $catch_block_count; + if ($always_defined_in_catches) { + foreach ($catch_types as $catch_number => $type_from_catch) { + if (isset($leaving_catches[$catch_number]) || !$type_from_catch->possibly_undefined) { + continue; } + $always_defined_in_catches = false; + break; + } + } - $context->vars_in_scope[$var_id] = Type::combineUnionTypes( - $context->vars_in_scope[$var_id], - $finally_context->vars_in_scope[$var_id], - $codebase - ); - } elseif (isset($finally_context->vars_in_scope[$var_id])) { - $context->vars_in_scope[$var_id] = clone $finally_context->vars_in_scope[$var_id]; + // Only union types from non-leaving catches + $types = []; + foreach ($catch_types as $catch_number => $type_from_catch) { + if (isset($leaving_catches[$catch_number])) { + continue; } + $types[] = $type_from_catch; } - } - } - foreach ($definitely_newly_assigned_var_ids as $var_id => $_) { - if (isset($context->vars_in_scope[$var_id])) { - $new_type = clone $context->vars_in_scope[$var_id]; + if ($always_defined_in_catches) { + if ($try_leaves_scope) { + // Discard the type from the outer context and the `try` block and only use `catch` block types + $previous_type = null; + $possibly_undefined = false; + } else { + // Since the variable is always defined in the `catch` blocks, it's + // either the type at the end of the `try` or one of the `catch` types. + $previous_type = $end_of_try_context->vars_in_scope[$var_id] ?? null; + $possibly_undefined = $previous_type->possibly_undefined ?? true; + } + } else { + $previous_type = $context->vars_in_scope[$var_id] ?? null; + $possibly_undefined = true; + } + if ($previous_type !== null) { + $types[] = $previous_type; + } - if ($new_type->possibly_undefined_from_try) { - $new_type->possibly_undefined = false; - $new_type->possibly_undefined_from_try = false; + if (count($types) === 0) { + $context->remove($var_id); + continue; } - $context->vars_in_scope[$var_id] = $new_type; + $context->vars_in_scope[$var_id] = Type::combineUnionTypeArray($types, $codebase); + $context->vars_in_scope[$var_id]->possibly_undefined = $possibly_undefined; + $context->vars_in_scope[$var_id]->possibly_undefined_from_try = false; } - } - foreach ($existing_thrown_exceptions as $possibly_thrown_exception => $codelocations) { - foreach ($codelocations as $hash => $codelocation) { - $context->possibly_thrown_exceptions[$possibly_thrown_exception][$hash] = $codelocation; + if (count($leaving_catches) < $catch_block_count) { + // Check for variables unset in every `catch` block unless all `catch` blocks leave the scope + foreach ($context->vars_in_scope as $var_id => $_) { + if (!isset($vars_at_ends_of_catches[$var_id])) { + $context->vars_in_scope[$var_id]->possibly_undefined = true; + } + } } + } else { + // No catch blocks + // Set all variables to their types at the end of the `try` block, since + // it would have to complete successfully for execution to continue. + $context->vars_in_scope = $end_of_try_context->vars_in_scope; + $context->references_in_scope = $end_of_try_context->references_in_scope; } - - $body_has_returned = !in_array(ScopeAnalyzer::ACTION_NONE, $try_block_control_actions, true); - $context->has_returned = ($body_has_returned && $all_catches_leave) || $finally_has_returned; - - return null; } } diff --git a/src/Psalm/Internal/Analyzer/Statements/BreakAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/BreakAnalyzer.php index 6110745c307..9758db1f9c0 100644 --- a/src/Psalm/Internal/Analyzer/Statements/BreakAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/BreakAnalyzer.php @@ -16,7 +16,7 @@ class BreakAnalyzer { public static function analyze( - StatementsAnalyzer $statements_analyzer, + StatementsAnalyzer $_statements_analyzer, PhpParser\Node\Stmt\Break_ $stmt, Context $context ): void { @@ -55,22 +55,6 @@ public static function analyze( } } } - - if ($context->finally_scope) { - foreach ($context->vars_in_scope as $var_id => $type) { - if (isset($context->finally_scope->vars_in_scope[$var_id])) { - $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; - $type->possibly_undefined = true; - $type->possibly_undefined_from_try = true; - } - } - } } $case_scope = $context->case_scope; diff --git a/src/Psalm/Internal/Analyzer/Statements/ContinueAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/ContinueAnalyzer.php index 60cb5527e45..9c6fe73c8e8 100644 --- a/src/Psalm/Internal/Analyzer/Statements/ContinueAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/ContinueAnalyzer.php @@ -76,22 +76,6 @@ public static function analyze( $loop_scope->possibly_redefined_loop_vars[$var] ?? null ); } - - if ($context->finally_scope) { - foreach ($context->vars_in_scope as $var_id => $type) { - if (isset($context->finally_scope->vars_in_scope[$var_id])) { - $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; - $type->possibly_undefined = true; - $type->possibly_undefined_from_try = true; - } - } - } } $context->has_returned = true; diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/AssignmentAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/AssignmentAnalyzer.php index f4bd5c222be..e1002c53d4e 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/AssignmentAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/AssignmentAnalyzer.php @@ -87,6 +87,7 @@ use function array_filter; use function array_merge; +use function assert; use function count; use function in_array; use function is_string; @@ -104,6 +105,7 @@ class AssignmentAnalyzer * @param PhpParser\Node\Expr|null $assign_value This has to be null to support list destructuring * * @return false|Union + * @psalm-suppress ComplexMethod */ public static function analyze( StatementsAnalyzer $statements_analyzer, @@ -258,6 +260,9 @@ public static function analyze( // if we're not exiting immediately, make everything mixed $context->vars_in_scope[$var_id] = $comment_type ?? Type::getMixed(); + if ($context->try_catch_scope !== null) { + $context->try_catch_scope->assignments_from_scope[$var_id][] = $context->vars_in_scope[$var_id]; + } } return false; @@ -328,14 +333,6 @@ public static function analyze( $assign_value_type->parent_nodes = [ $assignment_node->id => $assignment_node ]; - - if ($context->inside_try) { - // Copy previous assignment's parent nodes inside a try. Since an exception could be thrown at any - // point this is a workaround to ensure that use of a variable also uses all previous assignments. - if (isset($context->vars_in_scope[$extended_var_id])) { - $assign_value_type->parent_nodes += $context->vars_in_scope[$extended_var_id]->parent_nodes; - } - } } if ($extended_var_id && isset($context->vars_in_scope[$extended_var_id])) { @@ -576,6 +573,10 @@ public static function analyze( $context->inside_assignment = $was_in_assignment; + if ($context->try_catch_scope !== null) { + $context->try_catch_scope->assignments_from_scope[$var_id][] = $context->vars_in_scope[$var_id]; + } + return $context->vars_in_scope[$var_id]; } @@ -594,6 +595,10 @@ public static function analyze( $context->inside_assignment = $was_in_assignment; + if ($context->try_catch_scope !== null) { + $context->try_catch_scope->assignments_from_scope[$var_id][] = $context->vars_in_scope[$var_id]; + } + return $context->vars_in_scope[$var_id]; } @@ -649,6 +654,13 @@ public static function analyze( } } } + + if ($context->try_catch_scope !== null) { + $context->try_catch_scope->assignments_from_scope[$var_id][] = $context->vars_in_scope[$var_id]; + } + } elseif (isset($root_var_id) && $context->try_catch_scope !== null) { + assert(is_string($root_var_id)); + $context->try_catch_scope->assignments_from_scope[$root_var_id][] = $context->vars_in_scope[$root_var_id]; } $context->inside_assignment = $was_in_assignment; @@ -1146,6 +1158,9 @@ public static function assignByRefParam( $context->assigned_var_ids[$var_id] = (int) $stmt->getAttribute('startFilePos'); $context->vars_in_scope[$var_id] = $by_ref_out_type; + if ($context->try_catch_scope !== null) { + $context->try_catch_scope->assignments_from_scope[$var_id][] = $context->vars_in_scope[$var_id]; + } $stmt_type = $statements_analyzer->node_data->getType($stmt); @@ -1585,6 +1600,11 @@ private static function analyzeDestructuringAssignment( ) { $context->vars_in_scope[$list_var_id]->addType(new TNull); } + + if ($context->try_catch_scope !== null) { + $context->try_catch_scope->assignments_from_scope[$list_var_id][] = + $context->vars_in_scope[$list_var_id]; + } } } } diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/ArgumentAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/ArgumentAnalyzer.php index c410388f8d8..bc560e0779d 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/ArgumentAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/ArgumentAnalyzer.php @@ -1417,8 +1417,10 @@ private static function coerceValueAfterGatekeeperArgument( $input_type->by_ref = $by_ref; } + $new_variable = false; if ($context->inside_conditional && !isset($context->assigned_var_ids[$var_id])) { $context->assigned_var_ids[$var_id] = 0; + $new_variable = true; } if ($was_cloned) { @@ -1454,6 +1456,10 @@ private static function coerceValueAfterGatekeeperArgument( } else { $context->vars_in_scope[$var_id] = $input_type; } + + if ($new_variable && $context->try_catch_scope !== null) { + $context->try_catch_scope->assignments_from_scope[$var_id][] = $context->vars_in_scope[$var_id]; + } } } diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/NamedFunctionCallHandler.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/NamedFunctionCallHandler.php index 0116cbe7e0d..5923369a472 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/NamedFunctionCallHandler.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/NamedFunctionCallHandler.php @@ -227,6 +227,9 @@ public static function handle( $context->vars_in_scope[$var_id] = $mixed_type; $context->assigned_var_ids[$var_id] = (int) $stmt->getAttribute('startFilePos'); $context->possibly_assigned_var_ids[$var_id] = true; + if ($context->try_catch_scope !== null) { + $context->try_catch_scope->assignments_from_scope[$var_id][] = $context->vars_in_scope[$var_id]; + } } return; diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/VariableFetchAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/VariableFetchAnalyzer.php index 331c818c709..e513d42218c 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/VariableFetchAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/VariableFetchAnalyzer.php @@ -367,24 +367,27 @@ public static function analyze( self::addDataFlowToVariable($statements_analyzer, $stmt, $var_name, $stmt_type, $context); - if ($stmt_type->possibly_undefined_from_try && !$context->inside_isset) { - if ($context->is_global) { - IssueBuffer::maybeAdd( - new PossiblyUndefinedGlobalVariable( - 'Possibly undefined global variable ' . $var_name . ' defined in try block', - new CodeLocation($statements_analyzer->getSource(), $stmt), - $var_name - ), - $statements_analyzer->getSuppressedIssues() - ); - } else { - IssueBuffer::maybeAdd( - new PossiblyUndefinedVariable( - 'Possibly undefined variable ' . $var_name . ' defined in try block', - new CodeLocation($statements_analyzer->getSource(), $stmt) - ), - $statements_analyzer->getSuppressedIssues() - ); + if (!$context->inside_isset) { + $from_try_message = $stmt_type->possibly_undefined_from_try ? " defined in try block" : ""; + if ($stmt_type->possibly_undefined) { + if ($context->is_global) { + IssueBuffer::maybeAdd( + new PossiblyUndefinedGlobalVariable( + "Possibly undefined global variable {$var_name}{$from_try_message}", + new CodeLocation($statements_analyzer->getSource(), $stmt), + $var_name + ), + $statements_analyzer->getSuppressedIssues() + ); + } else { + IssueBuffer::maybeAdd( + new PossiblyUndefinedVariable( + "Possibly undefined variable {$var_name}{$from_try_message}", + new CodeLocation($statements_analyzer->getSource(), $stmt) + ), + $statements_analyzer->getSuppressedIssues() + ); + } } } diff --git a/src/Psalm/Internal/Analyzer/Statements/ReturnAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/ReturnAnalyzer.php index dc3909caf2d..8fbbafeb3f9 100644 --- a/src/Psalm/Internal/Analyzer/Statements/ReturnAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/ReturnAnalyzer.php @@ -197,22 +197,6 @@ public static function analyze( $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])) { - $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; - $type->possibly_undefined = true; - $type->possibly_undefined_from_try = true; - } - } - } - if ($source instanceof FunctionLikeAnalyzer && !($source->getSource() instanceof TraitAnalyzer) ) { diff --git a/src/Psalm/Internal/Analyzer/Statements/StaticAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/StaticAnalyzer.php index d537e8ca870..aaf24e4615e 100644 --- a/src/Psalm/Internal/Analyzer/Statements/StaticAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/StaticAnalyzer.php @@ -176,6 +176,9 @@ public static function analyze( $context->vars_in_scope[$var_id] = $comment_type ? clone $comment_type : Type::getMixed(); $context->vars_possibly_in_scope[$var_id] = true; $context->assigned_var_ids[$var_id] = (int) $stmt->getAttribute('startFilePos'); + if ($context->try_catch_scope !== null) { + $context->try_catch_scope->assignments_from_scope[$var_id][] = $context->vars_in_scope[$var_id]; + } $statements_analyzer->byref_uses[$var_id] = true; $location = new CodeLocation($statements_analyzer, $var); diff --git a/src/Psalm/Internal/Analyzer/Statements/ThrowAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/ThrowAnalyzer.php index fbe259dfeff..025f65326a7 100644 --- a/src/Psalm/Internal/Analyzer/Statements/ThrowAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/ThrowAnalyzer.php @@ -32,22 +32,6 @@ public static function analyze( } $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])) { - $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; - $type->possibly_undefined = true; - $type->possibly_undefined_from_try = true; - } - } - } - if ($context->check_classes && ($throw_type = $statements_analyzer->node_data->getType($stmt->expr)) && !$throw_type->hasMixed() diff --git a/src/Psalm/Internal/Analyzer/Statements/UnsetAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/UnsetAnalyzer.php index 2c9a1644902..de02fd9c6a9 100644 --- a/src/Psalm/Internal/Analyzer/Statements/UnsetAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/UnsetAnalyzer.php @@ -48,6 +48,10 @@ public static function analyze( if ($var_id) { $context->remove($var_id); unset($context->references_possibly_from_confusing_scope[$var_id]); + + if ($context->try_catch_scope !== null) { + $context->try_catch_scope->unset_from_scope[$var_id] = true; + } } if ($var instanceof PhpParser\Node\Expr\ArrayDimFetch && $var->dim) { diff --git a/src/Psalm/Internal/Analyzer/Statements/UnusedAssignmentRemover.php b/src/Psalm/Internal/Analyzer/Statements/UnusedAssignmentRemover.php index 3f6e50e81a7..b0dd57fddb1 100644 --- a/src/Psalm/Internal/Analyzer/Statements/UnusedAssignmentRemover.php +++ b/src/Psalm/Internal/Analyzer/Statements/UnusedAssignmentRemover.php @@ -11,7 +11,9 @@ use function array_key_exists; use function array_slice; +use function assert; use function count; +use function ctype_space; use function is_array; use function is_string; use function strlen; @@ -45,7 +47,28 @@ public function findUnusedAssignment( [$assign_stmt, $assign_exp] = $search_result; $chain_assignment = false; - if ($assign_stmt !== null && $assign_exp !== null) { + if ($assign_stmt instanceof PhpParser\Node\Stmt\Catch_) { + assert($assign_stmt->var !== null && is_string($assign_stmt->var->name)); + if ($codebase->analysis_php_version_id < 8_00_00) { + $new_file_manipulation = new FileManipulation( + $original_location->raw_file_start + 1, + $original_location->raw_file_start + 1 + strlen($assign_stmt->var->name), + "_", + ); + } else { + $file_content = $codebase->file_provider->getContents( + $original_location->file_path + ); + $end_of_exception = $original_location->raw_file_start - 1; + for (; ctype_space($file_content[$end_of_exception]); --$end_of_exception); + $new_file_manipulation = new FileManipulation( + $end_of_exception + 1, + $original_location->raw_file_start + 1 + strlen($assign_stmt->var->name), + "", + ); + } + FileManipulationBuffer::add($original_location->file_path, [$new_file_manipulation]); + } elseif ($assign_stmt !== null && $assign_exp !== null) { // Check if we have to remove assignment statement as expression (i.e. just "$var = ") // Consider chain of assignments @@ -231,9 +254,9 @@ private function checkRemovableChainAssignment(PhpParser\Node\Expr $cur_assign, /** * @param array $stmts * @return array{ - * 0: PhpParser\Node\Stmt|null, - * 1: PhpParser\Node\Expr\Assign|PhpParser\Node\Expr\AssignOp|PhpParser\Node\Expr\AssignRef|null - * } + * PhpParser\Node\Stmt|null, + * PhpParser\Node\Expr\Assign|PhpParser\Node\Expr\AssignOp|PhpParser\Node\Expr\AssignRef|null + * } */ private function findAssignStmt(array $stmts, string $var_id, CodeLocation $original_location): array { @@ -255,17 +278,32 @@ private function findAssignStmt(array $stmts, string $var_id, CodeLocation $orig $assign_exp = $target_exp; $assign_stmt = $levels_taken === 1 ? $stmt : null; } + } elseif ($stmt instanceof PhpParser\Node\Stmt\Catch_) { + $var = $stmt->var; + + if ($var instanceof PhpParser\Node\Expr\Variable + && $var->name === substr($var_id, 1) + && $var->getStartFilePos() === $original_location->raw_file_start + ) { + $assign_exp_found = true; + $assign_stmt = $stmt; + } } elseif ($stmt instanceof PhpParser\Node\Stmt\TryCatch) { $search_result = $this->findAssignStmt($stmt->stmts, $var_id, $original_location); - if ($search_result[0] && $search_result[1]) { + if ($search_result[0]) { + return $search_result; + } + + $search_result = $this->findAssignStmt($stmt->catches, $var_id, $original_location); + if ($search_result[0]) { return $search_result; } foreach ($stmt->catches as $catch_stmt) { $search_result = $this->findAssignStmt($catch_stmt->stmts, $var_id, $original_location); - if ($search_result[0] && $search_result[1]) { + if ($search_result[0]) { return $search_result; } } @@ -274,32 +312,32 @@ private function findAssignStmt(array $stmts, string $var_id, CodeLocation $orig ) { $search_result = $this->findAssignStmt($stmt->stmts, $var_id, $original_location); - if ($search_result[0] && $search_result[1]) { + if ($search_result[0]) { return $search_result; } } elseif ($stmt instanceof PhpParser\Node\Stmt\Foreach_) { $search_result = $this->findAssignStmt($stmt->stmts, $var_id, $original_location); - if ($search_result[0] && $search_result[1]) { + if ($search_result[0]) { return $search_result; } } elseif ($stmt instanceof PhpParser\Node\Stmt\For_) { $search_result = $this->findAssignStmt($stmt->stmts, $var_id, $original_location); - if ($search_result[0] && $search_result[1]) { + if ($search_result[0]) { return $search_result; } } elseif ($stmt instanceof PhpParser\Node\Stmt\If_) { $search_result = $this->findAssignStmt($stmt->stmts, $var_id, $original_location); - if ($search_result[0] && $search_result[1]) { + if ($search_result[0]) { return $search_result; } foreach ($stmt->elseifs as $elseif_stmt) { $search_result = $this->findAssignStmt($elseif_stmt->stmts, $var_id, $original_location); - if ($search_result[0] && $search_result[1]) { + if ($search_result[0]) { return $search_result; } } @@ -307,7 +345,7 @@ private function findAssignStmt(array $stmts, string $var_id, CodeLocation $orig if ($stmt->else) { $search_result = $this->findAssignStmt($stmt->else->stmts, $var_id, $original_location); - if ($search_result[0] && $search_result[1]) { + if ($search_result[0]) { return $search_result; } } diff --git a/src/Psalm/Internal/PhpVisitor/Reflector/FunctionLikeNodeScanner.php b/src/Psalm/Internal/PhpVisitor/Reflector/FunctionLikeNodeScanner.php index fe5e1db2c35..f842f9e42fd 100644 --- a/src/Psalm/Internal/PhpVisitor/Reflector/FunctionLikeNodeScanner.php +++ b/src/Psalm/Internal/PhpVisitor/Reflector/FunctionLikeNodeScanner.php @@ -1048,7 +1048,7 @@ private function createStorageForFunctionLike( $docblock_info = null; try { $docblock_info = FunctionLikeDocblockParser::parse($doc_comment); - } catch (IncorrectDocblockException|DocblockParseException $e) { + } catch (DocblockParseException $e) { } if ($docblock_info) { if ($docblock_info->since_php_major_version && !$this->aliases->namespace) { diff --git a/src/Psalm/Internal/Scope/FinallyScope.php b/src/Psalm/Internal/Scope/FinallyScope.php deleted file mode 100644 index 0343f193002..00000000000 --- a/src/Psalm/Internal/Scope/FinallyScope.php +++ /dev/null @@ -1,24 +0,0 @@ - - */ - public $vars_in_scope = []; - - /** - * @param array $vars_in_scope - */ - public function __construct(array $vars_in_scope) - { - $this->vars_in_scope = $vars_in_scope; - } -} diff --git a/src/Psalm/Internal/Scope/TryCatchScope.php b/src/Psalm/Internal/Scope/TryCatchScope.php new file mode 100644 index 00000000000..55a2960060d --- /dev/null +++ b/src/Psalm/Internal/Scope/TryCatchScope.php @@ -0,0 +1,43 @@ +> + */ + public $assignments_from_scope = []; + + /** + * @var array + */ + public $unset_from_scope = []; + + /** + * Add assignments and unsets from an inner scope to the outer scope. + */ + public function applyInnerScope(self $other): void + { + foreach ($other->assignments_from_scope as $var_id => $assigned_types) { + $outer_types = []; + foreach ($assigned_types as $assigned_type) { + $outer_type = clone $assigned_type; + $outer_type->possibly_undefined = $outer_type->possibly_undefined_from_try = true; + $outer_types[] = $outer_type; + } + $this->assignments_from_scope[$var_id] = [ + ...($this->assignments_from_scope[$var_id] ?? []), + ...$outer_types, + ]; + } + $this->unset_from_scope += $other->unset_from_scope; + } +} diff --git a/src/Psalm/Internal/Type/SimpleNegatedAssertionReconciler.php b/src/Psalm/Internal/Type/SimpleNegatedAssertionReconciler.php index 68b45b1f9b9..29725d7b65c 100644 --- a/src/Psalm/Internal/Type/SimpleNegatedAssertionReconciler.php +++ b/src/Psalm/Internal/Type/SimpleNegatedAssertionReconciler.php @@ -595,6 +595,12 @@ private static function reconcileNull( $existing_var_type->removeType('null'); } + if ($existing_var_type->possibly_undefined) { + $did_remove_type = true; + $existing_var_type->possibly_undefined = false; + $existing_var_type->possibly_undefined_from_try = false; + } + foreach ($existing_var_type->getAtomicTypes() as $type) { if ($type instanceof TTemplateParam) { $type->as = self::reconcileNull( diff --git a/src/Psalm/Issue/RedundantCatch.php b/src/Psalm/Issue/RedundantCatch.php new file mode 100644 index 00000000000..72e4ad9f6df --- /dev/null +++ b/src/Psalm/Issue/RedundantCatch.php @@ -0,0 +1,17 @@ +dupe_key = $dupe_key; + } +} diff --git a/tests/DocumentationTest.php b/tests/DocumentationTest.php index 23484bc70cc..23f271133e1 100644 --- a/tests/DocumentationTest.php +++ b/tests/DocumentationTest.php @@ -65,6 +65,7 @@ class DocumentationTest extends TestCase * annotations that we don’t want documented */ private const INTENTIONALLY_UNDOCUMENTED_ANNOTATIONS = [ + '@psalm-check-type', // Used internally for testing try-catch-finally, not sure if we want to support it '@psalm-self-out', // Not documented as it's a legacy alias of @psalm-this-out '@psalm-variadic', ]; diff --git a/tests/FileManipulation/UnusedVariableManipulationTest.php b/tests/FileManipulation/UnusedVariableManipulationTest.php index 1e73f75fe25..33ad31e2526 100644 --- a/tests/FileManipulation/UnusedVariableManipulationTest.php +++ b/tests/FileManipulation/UnusedVariableManipulationTest.php @@ -37,14 +37,14 @@ public function foo() : void { try { $c = false; $d = null; - } catch (Exception $e) {} + } catch (Exception $_) {} } }', 'output' => ' '7.1', @@ -631,7 +631,7 @@ public function __toString() { function foo(S $a) { try { $b = (string) $a; - } catch(Exception $e){ + } catch(Exception $_){ // this class is not stringable } }', @@ -651,7 +651,7 @@ public function __toString() { function foo(S $a) { try { (string) $a; - } catch(Exception $e){ + } catch(Exception $_){ // this class is not stringable } }', @@ -678,6 +678,77 @@ function foo(S $a) { 'issues_to_fix' => ['UnusedVariable'], 'safe_types' => true, ], + 'removeUnusedException7.4' => [ + 'input' => ' ' '7.4', + 'issues_to_fix' => ['UnusedVariable'], + 'safe_types' => true, + ], + 'removeUnusedException8.0' => [ + 'input' => ' ' '8.0', + 'issues_to_fix' => ['UnusedVariable'], + 'safe_types' => true, + ], + 'nonsensicalReferenceExceptionNotRemoved' => [ + 'input' => ' ' '7.4', + 'issues_to_fix' => ['UnusedVariable'], + 'safe_types' => true, + ], + 'removeUnusedExceptionInIf' => [ + 'input' => ' ' '8.0', + 'issues_to_fix' => ['UnusedVariable'], + 'safe_types' => true, + ], ]; } } diff --git a/tests/MemoizeTest.php b/tests/MemoizeTest.php new file mode 100644 index 00000000000..3dce31f6bd9 --- /dev/null +++ b/tests/MemoizeTest.php @@ -0,0 +1,243 @@ +testConfig->memoize_method_calls = true; + } + + /** + * @return iterable,ignored_issues?:list}> + */ + public function providerValidCodeParse(): iterable + { + yield 'methodCallMemoize' => [ + 'code' => 'getFoo()) { + if ($a->getFoo()->getBar()) { + $a->getFoo()->getBar()->bat(); + } + } + ', + ]; + yield 'propertyMethodCallMemoize' => [ + 'code' => 'bar = $bar; + } + + public function getBar(): ?string { + return $this->bar; + } + } + + function doSomething(Foo $foo): string { + if ($foo->getBar() !== null){ + return $foo->getBar(); + } + + return "hello"; + } + ', + ]; + yield 'propertyMethodCallMutationFreeMemoize' => [ + 'code' => 'bar = $bar; + } + + /** + * @psalm-mutation-free + */ + public function getBar(): ?string { + return $this->bar; + } + } + + function doSomething(Foo $foo): string { + if ($foo->getBar() !== null){ + return $foo->getBar(); + } + + return "hello"; + } + ', + ]; + yield 'unchainedMethodCallMemoize' => [ + 'code' => 'int = 1; + } + + final public function getInt(): ?int { + return $this->int; + } + } + + function printInt(int $int): void { + echo $int; + } + + $obj = new SomeClass(); + + if ($obj->getInt()) { + printInt($obj->getInt()); + } + ', + ]; + yield 'unchainedMutationFreeMethodCallMemoize' => [ + 'code' => 'int = 1; + } + + /** + * @psalm-mutation-free + */ + public function getInt(): ?int { + return $this->int; + } + } + + function printInt(int $int): void { + echo $int; + } + + $obj = new SomeClass(); + + if ($obj->getInt()) { + printInt($obj->getInt()); + } + ', + ]; + yield 'memoizedPropertyInCatchIsNotPossiblyUndefinedAfter' => [ + 'code' => 'int = 1; + } + } + + $obj = new SomeClass(); + try { + } catch (Exception $_) { + echo $obj->int; + } + $foo = $obj->int; + ', + 'assertions' => [ + '$foo===' => 'int', + ], + ]; + yield 'memoizedMethodInCatchIsNotPossiblyUndefinedAfter' => [ + 'code' => 'int = 1; + } + + /** + * @psalm-mutation-free + */ + public function getInt(): int { + return $this->int; + } + } + + $obj = new SomeClass(); + try { + } catch (Exception $_) { + $obj->getInt(); + } + $foo = $obj->getInt(); + ', + 'assertions' => [ + '$foo===' => 'int', + ], + ]; + yield 'tryCatchDoesntRemovePreviouslyMemoizedProperties' => [ + 'code' => 'int = 1; + } + } + + $obj = new SomeClass(); + assert($obj->int !== null); + try { + } catch (Exception $_) { + } + ', + 'assertions' => [ + '$obj->int===' => 'int', + ], + ]; + yield 'tryCatchDoesntRemovePreviouslyMemoizedPropertiesWhenUsedInTryAndCatch' => [ + 'code' => 'obj = new stdClass(); + } + } + + $obj = new SomeClass(); + assert($obj->obj !== null); + try { + takesObj($obj->obj); + } catch (Exception $_) { + takesObj($obj->obj); + } + takesObj($obj->obj); + + function takesObj(object $obj): void {} + ', + ]; + } +} diff --git a/tests/MethodCallTest.php b/tests/MethodCallTest.php index 0102eaf050a..c329e9a203e 100644 --- a/tests/MethodCallTest.php +++ b/tests/MethodCallTest.php @@ -24,175 +24,6 @@ public function testExtendDocblockParamType(): void $this->analyzeFile('somefile.php', new Context()); } - public function testMethodCallMemoize(): void - { - $this->project_analyzer->getConfig()->memoize_method_calls = true; - - $this->addFile( - 'somefile.php', - 'getFoo()) { - if ($a->getFoo()->getBar()) { - $a->getFoo()->getBar()->bat(); - } - }' - ); - - $this->analyzeFile('somefile.php', new Context()); - } - - public function testPropertyMethodCallMemoize(): void - { - $this->project_analyzer->getConfig()->memoize_method_calls = true; - - $this->addFile( - 'somefile.php', - 'bar = $bar; - } - - public function getBar(): ?string { - return $this->bar; - } - } - - function doSomething(Foo $foo): string { - if ($foo->getBar() !== null){ - return $foo->getBar(); - } - - return "hello"; - }' - ); - - $this->analyzeFile('somefile.php', new Context()); - } - - public function testPropertyMethodCallMutationFreeMemoize(): void - { - $this->project_analyzer->getConfig()->memoize_method_calls = true; - - $this->addFile( - 'somefile.php', - 'bar = $bar; - } - - /** - * @psalm-mutation-free - */ - public function getBar(): ?string { - return $this->bar; - } - } - - function doSomething(Foo $foo): string { - if ($foo->getBar() !== null){ - return $foo->getBar(); - } - - return "hello"; - }' - ); - - $this->analyzeFile('somefile.php', new Context()); - } - - public function testUnchainedMethodCallMemoize(): void - { - $this->project_analyzer->getConfig()->memoize_method_calls = true; - - $this->addFile( - 'somefile.php', - 'int = 1; - } - - final public function getInt(): ?int { - return $this->int; - } - } - - function printInt(int $int): void { - echo $int; - } - - $obj = new SomeClass(); - - if ($obj->getInt()) { - printInt($obj->getInt()); - }' - ); - - $this->analyzeFile('somefile.php', new Context()); - } - - public function testUnchainedMutationFreeMethodCallMemoize(): void - { - $this->project_analyzer->getConfig()->memoize_method_calls = true; - - $this->addFile( - 'somefile.php', - 'int = 1; - } - - /** - * @psalm-mutation-free - */ - public function getInt(): ?int { - return $this->int; - } - } - - function printInt(int $int): void { - echo $int; - } - - $obj = new SomeClass(); - - if ($obj->getInt()) { - printInt($obj->getInt()); - }' - ); - - $this->analyzeFile('somefile.php', new Context()); - } - /** * @return iterable,ignored_issues?:list}> */ diff --git a/tests/Traits/ValidCodeAnalysisTestTrait.php b/tests/Traits/ValidCodeAnalysisTestTrait.php index 5d803a550d6..60eca2a4d6d 100644 --- a/tests/Traits/ValidCodeAnalysisTestTrait.php +++ b/tests/Traits/ValidCodeAnalysisTestTrait.php @@ -70,21 +70,29 @@ public function testValidCode( $this->analyzeFile($file_path, $context); $actual_vars = []; - foreach ($assertions as $var => $_) { + foreach ($assertions as $expr => $_) { $exact = false; + $var = $expr; if ($var && strpos($var, '===') === strlen($var) - 3) { $var = substr($var, 0, -3); $exact = true; } + if ($var && strpos($var, '?') === strlen($var) - 1) { + $var = substr($var, 0, -1); + } if (isset($context->vars_in_scope[$var])) { $value = $context->vars_in_scope[$var]->getId($exact); + + $actual_expr = $var; + if ($context->vars_in_scope[$var]->possibly_undefined) { + $actual_expr .= "?"; + } if ($exact) { - $actual_vars[$var . '==='] = $value; - } else { - $actual_vars[$var] = $value; + $actual_expr .= '==='; } + $actual_vars[$actual_expr] = $value; } } diff --git a/tests/TryCatchTest.php b/tests/TryCatchTest.php index b8fa7a47e79..861075c48fd 100644 --- a/tests/TryCatchTest.php +++ b/tests/TryCatchTest.php @@ -44,8 +44,7 @@ class CustomException extends Exception implements CustomThrowable {} 'code' => ' [ @@ -142,7 +141,7 @@ function test(): string { try { $var = test(); - } catch (Exception $e) { + } catch (Throwable $e) { return; } @@ -160,7 +159,7 @@ function test(): string { try { $var = test(); - } catch (Exception $e) { + } catch (Throwable $e) { $var = "bad"; } @@ -258,7 +257,7 @@ function example() : void { try { $str = "a"; - } catch (Exception $e) { + } catch (Throwable $e) { example(); } ord($str);', @@ -369,13 +368,6 @@ class_alias( } }' ], - 'suppressUndefinedVarInFinally' => [ - 'code' => 'end = null; - }', - ], 'returnsInTry' => [ 'code' => ' [ + 'code' => ' [ + '$one?' => 'B', + '$two?' => 'C', + '$three?' => 'D', + '$four' => 'C|D', + ], + ], + 'unionAssignmentsFromTryAndMultipleCatch' => [ + 'code' => ' [ + '$one?' => 'B', + '$two?' => 'C', + '$three?' => 'D', + '$four?' => 'E', + '$five?' => 'A|B|C', + '$six' => 'A|B|C|D|E', + ], + ], + 'unionAssignmentsWithMultipleNestedTry' => [ + 'code' => ' [ + '$one?===' => '6|7', + '$two?===' => '10|11|12|13', + '$three?===' => '14', + '$four?===' => '9|14', + '$five===' => '3|6|7|9|14', + ], + ], + 'finallyOverridesTryAndCatch' => [ + 'code' => ' [ + '$one?' => 'B', + '$two?' => 'C', + '$three?' => 'D', + '$four' => 'E', + '$five' => 'E', + ], + ], + 'unsetInTry' => [ + 'code' => ' [ + '$one?===' => '1', + '$two?===' => '2', + ], + ], + 'unsetThenReassignedInTry' => [ + 'code' => ' [ + '$one?===' => '1|3', + '$two?===' => '2', + ], + ], + 'unsetThenReassignedInFinally' => [ + 'code' => ' [ + '$one===' => '3', + '$two?===' => '2', + ], + ], + 'unsetInCatch' => [ + 'code' => ' [ + '$foo?===' => '1|2', + ], + ], + 'unsetThenReassignedInCatch' => [ + 'code' => ' [ + '$foo===' => '2|3', + ], + ], + 'unsetThenMaybeReassignedInCatch' => [ + 'code' => ' [ + '$foo?===' => '1|2|3', + ], + ], + 'unsetInNestedCatch' => [ + 'code' => ' [ + '$foo?===' => '1|2', + ], + ], + 'unsetInReturningCatch' => [ + 'code' => ' [ + '$foo===' => '1|2', + ], + ], + 'possiblyUndefinedBeforeTryIsStillPossiblyUndefined' => [ + 'code' => ' [ + '$maybeUnset?===' => '1', + ], + ], + 'catchVarShadowsExistingVar' => [ + 'code' => ' [ + '$one?===' => '2', + '$two?' => 'Exception', + '$three===' => '2|Exception', + ], + ], + 'tryInCatchResolvesTypeCorrectly' => [ + 'code' => ' [ + '$var===' => '1|2|3', + ], + ], + 'tryInFinallyResolvesTypeCorrectly' => [ + 'code' => ' [ + '$var===' => '1|2|3', + ], + ], + 'noCatchResolvesTypeCorrectly' => [ + 'code' => ' [ + '$foo===' => '3', + '$bar===' => '3', + ], + ], + 'universalCatchNarrowsTypeInFinally' => [ + 'code' => ' [ + '$var===' => '3|5|7', + ] + ], + 'nonUniversalCatchDoesntNarrowTypeInFinally' => [ + 'code' => ' [ + '$var===' => '3|5|7', + ] + ], + 'createByRefInIfInTry' => [ + 'code' => ' [ + '$test?' => 'int', + ], + ], + 'extractInTry' => [ + 'code' => ' [ + '$foo' => 'int|mixed', + '$bar' => 'mixed|string', + ], + ], + 'returnInSomeCatches' => [ + 'code' => ' [ + '$foo===' => '1|2', + ], + ], + 'tryVariableMightBeDefinedWhenTryLeavesScope' => [ + 'code' => ' [ + '$foo?===' => '1', + '$bar===' => '2', + ], + ], + 'variableOverriddenByAllCatchesWhenTryLeavesScope' => [ + 'code' => ' [ + '$foo===' => '3|4', + ], + ], + 'variableOverriddenBySomeCatchesWhenTryLeavesScope' => [ + 'code' => ' [ + '$foo===' => '1|2|3|4', + ], + ], + 'variableOverriddenByCatchesThenOverriddenByFinallyWhenTryLeavesScope' => [ + 'code' => ' [ + '$one===' => '3|4', + '$two===' => '5', + ], + ], + 'variableModifiedInTryWithoutReassignment' => [ + 'code' => ' [ + '$one===' => '3', + '$two===' => '3', + ], + ], + 'variableSetInNonLeavingCatchesButPossiblyChangedInFinally' => [ + 'code' => ' [ + '$one===' => '2|3|4', + '$foo===' => '2|3|5', + ], + ], + 'SKIPPED-finallyRunsEvenIfExceptionIsThrownInCatch' => [ + 'code' => ' [ + '$foo===' => '2|4|5|13', + ], + ], + 'SKIPPED-finallyRunsEvenIfExceptionIsThrownInCatchCorrectlyDetectsPossiblyUnset' => [ + 'code' => ' [ + '$foo===' => '1|2', + ], + ], ]; } @@ -633,6 +1216,69 @@ function doTry() : void { }', 'error_message' => 'RedundantCondition' ], + 'assignmentInTryAndCatchDoesntGuaranteeAssignment' => [ + 'code' => ' 'PossiblyUndefinedGlobalVariable', + ], + 'redundantCatch' => [ + 'code' => ' 'RedundantCatch', + ], + 'redundantCatchSameStatement' => [ + 'code' => ' 'RedundantCatch', + ], + 'redundantCatchSameStatementChildFirst' => [ + 'code' => ' 'RedundantCatch', + ], + 'missingCatchVariablePhp7' => [ + 'code' => ' 'ParseError', + 'ignored_issues' => [], + 'php_version' => '7.4', + ], + 'unsetInFinally' => [ + 'code' => ' 'UndefinedGlobalVariable', + ], ]; } } diff --git a/tests/TypeReconciliation/RedundantConditionTest.php b/tests/TypeReconciliation/RedundantConditionTest.php index 1038e354c87..8a616d6318b 100644 --- a/tests/TypeReconciliation/RedundantConditionTest.php +++ b/tests/TypeReconciliation/RedundantConditionTest.php @@ -1540,6 +1540,30 @@ function f(array $p) : void { }', 'error_message' => 'RedundantCondition' ], + 'redundantConditionInTry' => [ + 'code' => ' 0) {} + } + ', + 'error_message' => 'RedundantCondition', + ], + 'impossibleConditionInTry' => [ + 'code' => ' 'TypeDoesNotContainType', + ], ]; } } diff --git a/tests/UnusedCodeTest.php b/tests/UnusedCodeTest.php index 9484452be63..55028b29409 100644 --- a/tests/UnusedCodeTest.php +++ b/tests/UnusedCodeTest.php @@ -371,7 +371,7 @@ public function bar() : void { foreach ([1, 2, 3] as $_) { try { $c->foo(); - } catch (Exception $e) {} + } catch (Exception $_) {} } } } diff --git a/tests/UnusedVariableTest.php b/tests/UnusedVariableTest.php index a6767d8e200..1e77f0fcd1f 100644 --- a/tests/UnusedVariableTest.php +++ b/tests/UnusedVariableTest.php @@ -444,7 +444,7 @@ function dangerous(): string { function callDangerous(): void { try { $s = dangerous(); - } catch (Exception $e) { + } catch (Throwable $e) { echo $e->getMessage(); $s = "hello"; } @@ -470,7 +470,7 @@ function callDangerous(): void { } else { try { $t = dangerous(); - } catch (Exception $e) { + } catch (Throwable $e) { echo $e->getMessage(); $t = "hello"; } @@ -500,7 +500,7 @@ function callDangerous(): void { } catch (E1 $e) { echo $e->getMessage(); $s = false; - } catch (Exception $e) { + } catch (Throwable $_) { return; } @@ -592,7 +592,7 @@ function main() : void { if (!$s) { echo "Failed to get string\n"; } - } catch (Exception $e) { + } catch (Throwable $_) { $s = "fallback"; } printf("s is %s\n", $s); @@ -1008,7 +1008,7 @@ function foo() : void { $foo = rand(0, 1); if ($foo) {} - } catch (Exception $e) {} + } catch (Exception $_) {} if ($foo) {}', ], @@ -1040,7 +1040,7 @@ function getStream() { try { $stream = getStream(); \file_put_contents("./foobar", $stream); - } catch (\Exception $e) { + } catch (\Exception $_) { throw new \Exception("Something went wrong"); } finally { if ($stream) { @@ -1158,7 +1158,7 @@ function foo(A $a) : void { try { // do nothing - } catch (\Exception $exception) { + } catch (\Exception $_) { $path = "hello"; } @@ -1211,7 +1211,7 @@ function b(bool $a): void { try { $b = maybeThrows(); echo $b; - } catch (\Exception $e) {} + } catch (\Exception $_) {} echo $b; }' @@ -1228,7 +1228,7 @@ function b(): void { try { $b = maybeThrows(); echo $b; - } catch (\Exception $e) {} + } catch (\Exception $_) {} echo $b; }' @@ -1241,7 +1241,7 @@ function foo(): void { while (!$done) { try { $done = true; - } catch (\Exception $e) { + } catch (\Exception $_) { } } }', @@ -1264,7 +1264,7 @@ function foo() : ?string { try { $a = "hello"; echo $a; - } catch (Exception $e) { + } catch (Exception $_) { return $a; } @@ -1284,7 +1284,7 @@ function foo() : void { try { // do something dangerous $a = 5; - } catch (Exception $e) { + } catch (Exception $_) { $a = 4; throw new Exception("bad"); } finally { @@ -1306,7 +1306,7 @@ function foo() : void { try { foo(); $a = "hello"; - } catch (\Exception $e) { + } catch (\Exception $_) { echo $a; } @@ -2601,6 +2601,16 @@ public function setBarRef(int $ref): void } ', ], + 'allowUnusedCatchCheckedLater' => [ + 'code' => 'getMessage(); $t = "hello"; } @@ -3607,7 +3617,7 @@ function takesArray($a) : void { ', 'error_message' => 'UnusedVariable', ], - 'SKIPPED-warnAboutVariableUsedInNestedTryNotUsedInOuterTry' => [ + 'warnAboutVariableUsedInNestedTryNotUsedInOuterTry' => [ 'code' => ' 'UnusedVariable', ], + 'warnAboutUnusedCatch' => [ + 'code' => ' 'UnusedVariable', + ], 'referenceReassignmentUnusedVariable' => [ 'code' => ' Date: Wed, 16 Feb 2022 18:26:00 -0600 Subject: [PATCH 2/7] Rename unused exception variables. --- examples/plugins/FunctionCasingChecker.php | 4 +-- src/Psalm/Codebase.php | 12 +++---- src/Psalm/Config.php | 4 +-- .../Internal/Analyzer/AlgebraAnalyzer.php | 2 +- src/Psalm/Internal/Analyzer/ClassAnalyzer.php | 10 +++--- .../Internal/Analyzer/ClassLikeAnalyzer.php | 2 +- .../Internal/Analyzer/CommentAnalyzer.php | 4 +-- src/Psalm/Internal/Analyzer/FileAnalyzer.php | 2 +- .../Analyzer/FunctionLikeAnalyzer.php | 2 +- .../Internal/Analyzer/InterfaceAnalyzer.php | 2 +- .../Block/IfElse/ElseIfAnalyzer.php | 6 ++-- .../Statements/Block/IfElseAnalyzer.php | 6 ++-- .../Statements/Block/LoopAnalyzer.php | 2 +- .../Statements/Block/SwitchCaseAnalyzer.php | 4 +-- .../InstancePropertyAssignmentAnalyzer.php | 2 +- .../Expression/BinaryOp/ConcatAnalyzer.php | 2 +- .../Expression/BinaryOp/OrAnalyzer.php | 6 ++-- .../Expression/BinaryOpAnalyzer.php | 4 +-- .../Expression/Call/ArgumentsAnalyzer.php | 4 +-- .../Call/ArrayFunctionArgumentsAnalyzer.php | 2 +- .../Expression/Call/FunctionCallAnalyzer.php | 2 +- .../Call/FunctionCallReturnTypeFetcher.php | 2 +- .../ExistingAtomicMethodCallAnalyzer.php | 2 +- .../StaticMethod/AtomicStaticCallAnalyzer.php | 2 +- .../Expression/ClassConstAnalyzer.php | 4 +-- .../Fetch/AtomicPropertyFetchAnalyzer.php | 2 +- .../Statements/Expression/IncludeAnalyzer.php | 2 +- .../Expression/SimpleTypeInferer.php | 2 +- .../Statements/Expression/TernaryAnalyzer.php | 6 ++-- .../Internal/Analyzer/StatementsAnalyzer.php | 6 ++-- src/Psalm/Internal/Cli/Psalm.php | 2 +- src/Psalm/Internal/Codebase/ClassLikes.php | 16 +++++----- src/Psalm/Internal/Codebase/Functions.php | 4 +-- src/Psalm/Internal/Codebase/Methods.php | 4 +-- src/Psalm/Internal/Codebase/Populator.php | 32 +++++++++---------- src/Psalm/Internal/Codebase/Reflection.php | 4 +-- src/Psalm/Internal/Codebase/Scanner.php | 2 +- .../LanguageServer/LanguageServer.php | 4 +-- .../LanguageServer/Server/TextDocument.php | 8 ++--- .../Reflector/ClassLikeDocblockParser.php | 4 +-- .../Reflector/ClassLikeNodeScanner.php | 8 ++--- .../Reflector/ExpressionResolver.php | 2 +- .../Reflector/FunctionLikeNodeScanner.php | 4 +-- .../Internal/PhpVisitor/ReflectorVisitor.php | 2 +- src/Psalm/Internal/PhpVisitor/TraitFinder.php | 2 +- .../PluginManager/Command/DisableCommand.php | 2 +- .../PluginManager/Command/EnableCommand.php | 2 +- .../PluginManager/PluginListFactory.php | 2 +- .../Provider/FileReferenceProvider.php | 2 +- .../Internal/Provider/ParserCacheProvider.php | 2 +- .../ArrayFilterReturnTypeProvider.php | 2 +- .../Internal/Provider/StatementsProvider.php | 4 +-- .../Comparator/CallableTypeComparator.php | 6 ++-- .../Type/TemplateInferredTypeReplacer.php | 2 +- .../Type/TemplateStandinTypeReplacer.php | 6 ++-- src/Psalm/Internal/Type/TypeCombiner.php | 2 +- src/Psalm/Internal/Type/TypeExpander.php | 4 +-- .../Internal/TypeVisitor/TypeChecker.php | 2 +- src/Psalm/IssueBuffer.php | 2 +- .../Config/Plugin/Hook/SqlStringProvider.php | 2 +- 60 files changed, 126 insertions(+), 126 deletions(-) diff --git a/examples/plugins/FunctionCasingChecker.php b/examples/plugins/FunctionCasingChecker.php index e00d8d2227b..be322b2ef7f 100644 --- a/examples/plugins/FunctionCasingChecker.php +++ b/examples/plugins/FunctionCasingChecker.php @@ -58,7 +58,7 @@ public static function afterMethodCallAnalysis(AfterMethodCallAnalysisEvent $eve // fall through } } - } catch (Exception $e) { + } catch (Exception $_) { // can throw if storage is missing } } @@ -98,7 +98,7 @@ public static function afterFunctionCallAnalysis(AfterFunctionCallAnalysisEvent // fall through } } - } catch (Exception $e) { + } catch (Exception $_) { // can throw if storage is missing } } diff --git a/src/Psalm/Codebase.php b/src/Psalm/Codebase.php index 9bab2d6ed40..9a763de1fce 100644 --- a/src/Psalm/Codebase.php +++ b/src/Psalm/Codebase.php @@ -946,7 +946,7 @@ public function invalidateInformationForFile(string $file_path): void try { $file_storage = $this->file_storage_provider->get($file_path); - } catch (InvalidArgumentException $e) { + } catch (InvalidArgumentException $_) { return; } @@ -1086,7 +1086,7 @@ public function getSymbolInformation(string $file_path, string $symbol): ?array 'type' => 'abstract ? 'abstract ' : '') . 'class ' . $storage->name, 'description' => $storage->description, ]; - } catch (InvalidArgumentException $e) { + } catch (InvalidArgumentException $_) { } if (strpos($symbol, '\\')) { @@ -1198,7 +1198,7 @@ public function getSymbolLocation(string $file_path, string $symbol): ?CodeLocat error_log($e->getMessage()); return null; - } catch (InvalidArgumentException $e) { + } catch (InvalidArgumentException $_) { return null; } } @@ -1348,7 +1348,7 @@ public function getSignatureInformation( $params = $function_storage->params; $signature_label = $function_storage->cased_name; $signature_documentation = $function_storage->description; - } catch (Exception $exception) { + } catch (Exception $_) { if (InternalCallMapHandler::inCallMap($function_symbol)) { $callables = InternalCallMapHandler::getCallablesFromCallMap($function_symbol); @@ -1608,7 +1608,7 @@ public function getCompletionItemsForPartialSymbol( foreach ($file_storage->classlikes_in_file as $fq_class_name => $_) { try { $class_storage = $this->classlike_storage_provider->get($fq_class_name); - } catch (Exception $e) { + } catch (Exception $_) { continue; } @@ -1683,7 +1683,7 @@ public function getCompletionItemsForPartialSymbol( try { $class_storage = $this->classlike_storage_provider->get($fq_class_name); $description = $class_storage->description; - } catch (Exception $e) { + } catch (Exception $_) { $description = null; } diff --git a/src/Psalm/Config.php b/src/Psalm/Config.php index 054b24936b9..99e83cf2e6b 100644 --- a/src/Psalm/Config.php +++ b/src/Psalm/Config.php @@ -1643,7 +1643,7 @@ public function reportIssueInFile(string $issue_type, string $file_path): bool try { $file_storage = $codebase->file_storage_provider->get($file_path); $dependent_files += $file_storage->required_by_file_paths; - } catch (InvalidArgumentException $e) { + } catch (InvalidArgumentException $_) { // do nothing } } @@ -2406,7 +2406,7 @@ public function getPHPVersionFromComposerJson(): ?string if (file_exists($composer_json_path)) { try { $composer_json = json_decode(file_get_contents($composer_json_path), true, 512, JSON_THROW_ON_ERROR); - } catch (JsonException $e) { + } catch (JsonException $_) { $composer_json = null; } diff --git a/src/Psalm/Internal/Analyzer/AlgebraAnalyzer.php b/src/Psalm/Internal/Analyzer/AlgebraAnalyzer.php index 813513decb4..a9c4c22aec7 100644 --- a/src/Psalm/Internal/Analyzer/AlgebraAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/AlgebraAnalyzer.php @@ -43,7 +43,7 @@ public static function checkForParadox( ): void { try { $negated_formula2 = Algebra::negateFormula($formula_2); - } catch (ComplicatedExpressionException $e) { + } catch (ComplicatedExpressionException $_) { return; } diff --git a/src/Psalm/Internal/Analyzer/ClassAnalyzer.php b/src/Psalm/Internal/Analyzer/ClassAnalyzer.php index 2b4d7c005d4..c79b38d912c 100644 --- a/src/Psalm/Internal/Analyzer/ClassAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/ClassAnalyzer.php @@ -562,7 +562,7 @@ public function analyze( try { $trait_file_analyzer = $project_analyzer->getFileAnalyzerForClassLike($fq_trait_name); - } catch (Exception $e) { + } catch (Exception $_) { continue; } @@ -910,7 +910,7 @@ public static function addContextProperties( try { $docBlock = DocComment::parsePreservingLength($docComment); $suppressed = $docBlock->tags['psalm-suppress'] ?? []; - } catch (DocblockParseException $e) { + } catch (DocblockParseException $_) { // do nothing to keep original behavior } } @@ -2036,7 +2036,7 @@ private function checkImplementedInterfaces( try { $interface_storage = $classlike_storage_provider->get($fq_interface_name); - } catch (InvalidArgumentException $e) { + } catch (InvalidArgumentException $_) { return false; } @@ -2070,7 +2070,7 @@ private function checkImplementedInterfaces( foreach ($storage->class_implements as $fq_interface_name_lc => $fq_interface_name) { try { $interface_storage = $classlike_storage_provider->get($fq_interface_name_lc); - } catch (InvalidArgumentException $e) { + } catch (InvalidArgumentException $_) { return false; } @@ -2391,7 +2391,7 @@ private function checkParentClass( $code_location, $storage->template_type_extends_count[$parent_fq_class_name] ?? 0 ); - } catch (InvalidArgumentException $e) { + } catch (InvalidArgumentException $_) { // do nothing } } diff --git a/src/Psalm/Internal/Analyzer/ClassLikeAnalyzer.php b/src/Psalm/Internal/Analyzer/ClassLikeAnalyzer.php index e964cce71db..b5cf3b3e989 100644 --- a/src/Psalm/Internal/Analyzer/ClassLikeAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/ClassLikeAnalyzer.php @@ -818,7 +818,7 @@ public static function getClassesForFile(Codebase $codebase, string $file_path): { try { return $codebase->file_storage_provider->get($file_path)->classlikes_in_file; - } catch (InvalidArgumentException $e) { + } catch (InvalidArgumentException $_) { return []; } } diff --git a/src/Psalm/Internal/Analyzer/CommentAnalyzer.php b/src/Psalm/Internal/Analyzer/CommentAnalyzer.php index 7466e5557ce..03511aa2cdf 100644 --- a/src/Psalm/Internal/Analyzer/CommentAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/CommentAnalyzer.php @@ -131,7 +131,7 @@ public static function arrayToDocblocks( $template_type_map, $type_aliases ); - } catch (TypeParseTreeException $e) { + } catch (TypeParseTreeException $_) { throw new DocblockParseException($line_parts[0] . ' is not a valid type'); } @@ -161,7 +161,7 @@ public static function arrayToDocblocks( $template_type_map ?: [], $type_aliases ?: [] ); - } catch (TypeParseTreeException $e) { + } catch (TypeParseTreeException $_) { throw new DocblockParseException( $line_parts[0] . ' is not a valid type' . diff --git a/src/Psalm/Internal/Analyzer/FileAnalyzer.php b/src/Psalm/Internal/Analyzer/FileAnalyzer.php index 61a220f1090..ddf67d2f57b 100644 --- a/src/Psalm/Internal/Analyzer/FileAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/FileAnalyzer.php @@ -173,7 +173,7 @@ public function analyze( try { $stmts = $codebase->getStatementsForFile($this->file_path); - } catch (PhpParser\Error $e) { + } catch (PhpParser\Error $_) { return; } diff --git a/src/Psalm/Internal/Analyzer/FunctionLikeAnalyzer.php b/src/Psalm/Internal/Analyzer/FunctionLikeAnalyzer.php index 0a24b353ba6..371a5ed0e8b 100644 --- a/src/Psalm/Internal/Analyzer/FunctionLikeAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/FunctionLikeAnalyzer.php @@ -1597,7 +1597,7 @@ public function getFunctionLikeStorage(?StatementsAnalyzer $statements_analyzer try { return $codebase_methods->getStorage($method_id); - } catch (UnexpectedValueException $e) { + } catch (UnexpectedValueException $_) { $declaring_method_id = $codebase_methods->getDeclaringMethodId($method_id); if ($declaring_method_id === null) { diff --git a/src/Psalm/Internal/Analyzer/InterfaceAnalyzer.php b/src/Psalm/Internal/Analyzer/InterfaceAnalyzer.php index 40108a17c3c..0665b1fdf05 100644 --- a/src/Psalm/Internal/Analyzer/InterfaceAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/InterfaceAnalyzer.php @@ -70,7 +70,7 @@ public function analyze(): void try { $extended_interface_storage = $codebase->classlike_storage_provider->get($extended_interface_name); - } catch (InvalidArgumentException $e) { + } catch (InvalidArgumentException $_) { continue; } diff --git a/src/Psalm/Internal/Analyzer/Statements/Block/IfElse/ElseIfAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Block/IfElse/ElseIfAnalyzer.php index 9cda6b597b5..23bad1a59f1 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Block/IfElse/ElseIfAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Block/IfElse/ElseIfAnalyzer.php @@ -71,7 +71,7 @@ public static function analyze( $cond_referenced_var_ids = $if_conditional_scope->cond_referenced_var_ids; $assigned_in_conditional_var_ids = $if_conditional_scope->assigned_in_conditional_var_ids; $entry_clauses = $if_conditional_scope->entry_clauses; - } catch (ScopeAnalysisException $e) { + } catch (ScopeAnalysisException $_) { return false; } @@ -194,7 +194,7 @@ function (Clause $c) use ($assigned_in_conditional_var_ids, $elseif_cond_id): Cl $negated_elseif_types = Algebra::getTruthsFromFormula( Algebra::negateFormula($elseif_clauses) ); - } catch (ComplicatedExpressionException $e) { + } catch (ComplicatedExpressionException $_) { $reconcilable_elseif_types = []; $negated_elseif_types = []; } @@ -432,7 +432,7 @@ function (Clause $c) use ($assigned_in_conditional_var_ids, $elseif_cond_id): Cl Algebra::negateFormula($elseif_clauses) ) ); - } catch (ComplicatedExpressionException $e) { + } catch (ComplicatedExpressionException $_) { $if_scope->negated_clauses = []; } diff --git a/src/Psalm/Internal/Analyzer/Statements/Block/IfElseAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Block/IfElseAnalyzer.php index ac208372ffb..541202c116b 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Block/IfElseAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Block/IfElseAnalyzer.php @@ -112,7 +112,7 @@ public static function analyze( // this is the context for stuff that happens after the `if` block $post_if_context = $if_conditional_scope->post_if_context; $assigned_in_conditional_var_ids = $if_conditional_scope->assigned_in_conditional_var_ids; - } catch (ScopeAnalysisException $e) { + } catch (ScopeAnalysisException $_) { return false; } @@ -204,7 +204,7 @@ function (Clause $c) use ($mixed_var_ids, $cond_object_id): Clause { try { $if_scope->negated_clauses = Algebra::negateFormula($if_clauses); - } catch (ComplicatedExpressionException $e) { + } catch (ComplicatedExpressionException $_) { try { $if_scope->negated_clauses = FormulaGenerator::getFormula( $cond_object_id, @@ -215,7 +215,7 @@ function (Clause $c) use ($mixed_var_ids, $cond_object_id): Clause { $codebase, false ); - } catch (ComplicatedExpressionException $e) { + } catch (ComplicatedExpressionException $_) { $if_scope->negated_clauses = []; } } diff --git a/src/Psalm/Internal/Analyzer/Statements/Block/LoopAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Block/LoopAnalyzer.php index e0eb239cf78..2963776e203 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Block/LoopAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Block/LoopAnalyzer.php @@ -465,7 +465,7 @@ public static function analyze( try { $negated_pre_condition_clauses = Algebra::negateFormula(array_merge(...$pre_condition_clauses)); - } catch (ComplicatedExpressionException $e) { + } catch (ComplicatedExpressionException $_) { $negated_pre_condition_clauses = []; } diff --git a/src/Psalm/Internal/Analyzer/Statements/Block/SwitchCaseAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Block/SwitchCaseAnalyzer.php index 49177b5b414..9ac0c5081cf 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Block/SwitchCaseAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Block/SwitchCaseAnalyzer.php @@ -439,7 +439,7 @@ public static function analyze( if ($case_clauses && $case_equality_expr) { try { $negated_case_clauses = Algebra::negateFormula($case_clauses); - } catch (ComplicatedExpressionException $e) { + } catch (ComplicatedExpressionException $_) { $case_equality_expr_id = spl_object_id($case_equality_expr); try { @@ -453,7 +453,7 @@ public static function analyze( false, false ); - } catch (ComplicatedExpressionException $e) { + } catch (ComplicatedExpressionException $_) { $negated_case_clauses = []; } } diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Assignment/InstancePropertyAssignmentAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Assignment/InstancePropertyAssignmentAnalyzer.php index 767121217a9..57317c34192 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Assignment/InstancePropertyAssignmentAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Assignment/InstancePropertyAssignmentAnalyzer.php @@ -121,7 +121,7 @@ public static function analyze( $statements_analyzer, $context ); - } catch (UnexpectedValueException $e) { + } catch (UnexpectedValueException $_) { // do nothing } diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOp/ConcatAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOp/ConcatAnalyzer.php index 80ba8e84fe7..d70bfc8d1cf 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOp/ConcatAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOp/ConcatAnalyzer.php @@ -384,7 +384,7 @@ private static function analyzeOperand( )) { try { $storage = $codebase->methods->getStorage($to_string_method_id); - } catch (UnexpectedValueException $e) { + } catch (UnexpectedValueException $_) { continue; } diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOp/OrAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOp/OrAnalyzer.php index 78fa57971a6..63bc3a2bd59 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOp/OrAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOp/OrAnalyzer.php @@ -90,7 +90,7 @@ public static function analyze( if ($stmt->left instanceof PhpParser\Node\Expr\BinaryOp\BooleanOr) { $post_leaving_if_context = clone $context; } - } catch (ScopeAnalysisException $e) { + } catch (ScopeAnalysisException $_) { return false; } } else { @@ -151,7 +151,7 @@ public static function analyze( try { $negated_left_clauses = Algebra::negateFormula($left_clauses); - } catch (ComplicatedExpressionException $e) { + } catch (ComplicatedExpressionException $_) { try { $negated_left_clauses = FormulaGenerator::getFormula( $left_cond_id, @@ -162,7 +162,7 @@ public static function analyze( $codebase, false ); - } catch (ComplicatedExpressionException $e) { + } catch (ComplicatedExpressionException $_) { return false; } } diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOpAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOpAnalyzer.php index 5a4108c0127..02f590e576a 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOpAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOpAnalyzer.php @@ -468,7 +468,7 @@ private static function checkForImpureEqualityComparison( '__tostring' ) ); - } catch (UnexpectedValueException $e) { + } catch (UnexpectedValueException $_) { continue; } @@ -502,7 +502,7 @@ private static function checkForImpureEqualityComparison( '__tostring' ) ); - } catch (UnexpectedValueException $e) { + } catch (UnexpectedValueException $_) { continue; } diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/ArgumentsAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/ArgumentsAnalyzer.php index 0b6bdeb631a..71878a72f43 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/ArgumentsAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/ArgumentsAnalyzer.php @@ -398,7 +398,7 @@ private static function getHighOrderFuncStorage( return $codebase->methods->getStorage($method_id); } - } catch (UnexpectedValueException $e) { + } catch (UnexpectedValueException $_) { return null; } @@ -605,7 +605,7 @@ private static function handleClosureArg( $statements_analyzer->getFilePath(), $closure_id ); - } catch (UnexpectedValueException $e) { + } catch (UnexpectedValueException $_) { return; } diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/ArrayFunctionArgumentsAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/ArrayFunctionArgumentsAnalyzer.php index 27e262be596..eab174004d6 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/ArrayFunctionArgumentsAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/ArrayFunctionArgumentsAnalyzer.php @@ -657,7 +657,7 @@ private static function checkClosureType( try { $method_storage = $codebase->methods->getStorage($function_id_part); - } catch (UnexpectedValueException $e) { + } catch (UnexpectedValueException $_) { // the method may not exist, but we're suppressing that issue continue; } diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/FunctionCallAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/FunctionCallAnalyzer.php index 6914c45fa0d..020947185fa 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/FunctionCallAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/FunctionCallAnalyzer.php @@ -562,7 +562,7 @@ private static function handleNamedFunction( $function_call_info->defined_constants = $function_storage->defined_constants; $function_call_info->global_variables = $function_storage->global_variables; } - } catch (UnexpectedValueException $e) { + } catch (UnexpectedValueException $_) { $function_call_info->function_params = [ new FunctionLikeParameter('args', false, null, null, null, false, false, true) ]; diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/FunctionCallReturnTypeFetcher.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/FunctionCallReturnTypeFetcher.php index afe36e9b8e1..fd047ddfdf5 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/FunctionCallReturnTypeFetcher.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/FunctionCallReturnTypeFetcher.php @@ -233,7 +233,7 @@ public static function fetch( ); } } - } catch (InvalidArgumentException $e) { + } catch (InvalidArgumentException $_) { // this can happen when the function was defined in the Config startup script $stmt_type = Type::getMixed(); } diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/Method/ExistingAtomicMethodCallAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/Method/ExistingAtomicMethodCallAnalyzer.php index 2020a8490e8..3498d6f29f1 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/Method/ExistingAtomicMethodCallAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/Method/ExistingAtomicMethodCallAnalyzer.php @@ -198,7 +198,7 @@ public static function analyze( try { $method_storage = $codebase->methods->getStorage($declaring_method_id ?? $method_id); - } catch (UnexpectedValueException $e) { + } catch (UnexpectedValueException $_) { $method_storage = null; } diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/StaticMethod/AtomicStaticCallAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/StaticMethod/AtomicStaticCallAnalyzer.php index 19a5c0019ae..c0d1d74c4f1 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/StaticMethod/AtomicStaticCallAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/StaticMethod/AtomicStaticCallAnalyzer.php @@ -939,7 +939,7 @@ private static function checkPseudoMethod( new CodeLocation($statements_analyzer, $stmt), $context ); - } catch (Exception $e) { + } catch (Exception $_) { // do nothing } } diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/ClassConstAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/ClassConstAnalyzer.php index 3797f6dd642..3866f2039fd 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/ClassConstAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/ClassConstAnalyzer.php @@ -270,7 +270,7 @@ public static function analyzeFetch( ); } catch (InvalidArgumentException $_) { return true; - } catch (CircularReferenceException $e) { + } catch (CircularReferenceException $_) { IssueBuffer::maybeAdd( new CircularReference( 'Constant ' . $const_id . ' contains a circular reference', @@ -559,7 +559,7 @@ public static function analyzeFetch( ); } catch (InvalidArgumentException $_) { return true; - } catch (CircularReferenceException $e) { + } catch (CircularReferenceException $_) { IssueBuffer::maybeAdd( new CircularReference( 'Constant ' . $const_id . ' contains a circular reference', diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/AtomicPropertyFetchAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/AtomicPropertyFetchAnalyzer.php index 4126263bb93..9ccb2f2424c 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/AtomicPropertyFetchAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/AtomicPropertyFetchAnalyzer.php @@ -255,7 +255,7 @@ public static function analyze( try { $new_class_storage = $codebase->classlike_storage_provider->get($mixin->value); - } catch (InvalidArgumentException $e) { + } catch (InvalidArgumentException $_) { $new_class_storage = null; } diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/IncludeAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/IncludeAnalyzer.php index 12e98ae5d9d..9dbd2fe7e1b 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/IncludeAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/IncludeAnalyzer.php @@ -204,7 +204,7 @@ public static function analyze( $context, $global_context ); - } catch (UnpreparedAnalysisException $e) { + } catch (UnpreparedAnalysisException $_) { if ($config->skip_checks_on_unresolvable_includes) { $context->check_classes = false; $context->check_variables = false; diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/SimpleTypeInferer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/SimpleTypeInferer.php index f4c09eb6b21..ec08e73e4b0 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/SimpleTypeInferer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/SimpleTypeInferer.php @@ -296,7 +296,7 @@ public static function infer( } return null; - } catch (InvalidArgumentException | CircularReferenceException $e) { + } catch (InvalidArgumentException | CircularReferenceException $_) { return null; } } diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/TernaryAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/TernaryAnalyzer.php index f81f80fbc23..7f999024b53 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/TernaryAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/TernaryAnalyzer.php @@ -64,7 +64,7 @@ public static function analyze( $cond_referenced_var_ids = $if_conditional_scope->cond_referenced_var_ids; $assigned_in_conditional_var_ids = $if_conditional_scope->assigned_in_conditional_var_ids; - } catch (ScopeAnalysisException $e) { + } catch (ScopeAnalysisException $_) { return false; } @@ -155,7 +155,7 @@ function (Clause $c) use ($mixed_var_ids, $cond_object_id): Clause { try { $if_scope->negated_clauses = Algebra::negateFormula($if_clauses); - } catch (ComplicatedExpressionException $e) { + } catch (ComplicatedExpressionException $_) { try { $if_scope->negated_clauses = FormulaGenerator::getFormula( $cond_object_id, @@ -166,7 +166,7 @@ function (Clause $c) use ($mixed_var_ids, $cond_object_id): Clause { $codebase, false ); - } catch (ComplicatedExpressionException $e) { + } catch (ComplicatedExpressionException $_) { $if_scope->negated_clauses = []; } } diff --git a/src/Psalm/Internal/Analyzer/StatementsAnalyzer.php b/src/Psalm/Internal/Analyzer/StatementsAnalyzer.php index df6d4283a6f..5abdbf7e10a 100644 --- a/src/Psalm/Internal/Analyzer/StatementsAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/StatementsAnalyzer.php @@ -286,7 +286,7 @@ private function hoistFunctions(array $stmts, Context $context): void try { $function_analyzer = new FunctionAnalyzer($stmt, $this->source); $this->function_analyzers[$fq_function_name] = $function_analyzer; - } catch (UnexpectedValueException $e) { + } catch (UnexpectedValueException $_) { // do nothing } } @@ -606,7 +606,7 @@ private static function analyzeStatement( ); $class_analyzer->analyze(null, $global_context); - } catch (InvalidArgumentException $e) { + } catch (InvalidArgumentException $_) { // disregard this exception, we'll likely see it elsewhere in the form // of an issue } @@ -1115,7 +1115,7 @@ public function getUncaughtThrows(Context $context): array $is_expected = true; break; } - } catch (InvalidArgumentException $e) { + } catch (InvalidArgumentException $_) { $is_expected = true; break; } diff --git a/src/Psalm/Internal/Cli/Psalm.php b/src/Psalm/Internal/Cli/Psalm.php index 2063355c711..64b323f3325 100644 --- a/src/Psalm/Internal/Cli/Psalm.php +++ b/src/Psalm/Internal/Cli/Psalm.php @@ -654,7 +654,7 @@ private static function generateBaseline( new FileProvider, $options['set-baseline'] ); - } catch (ConfigException $e) { + } catch (ConfigException $_) { $issue_baseline = []; } diff --git a/src/Psalm/Internal/Codebase/ClassLikes.php b/src/Psalm/Internal/Codebase/ClassLikes.php index 1d394e2fa37..738cfc6c98a 100644 --- a/src/Psalm/Internal/Codebase/ClassLikes.php +++ b/src/Psalm/Internal/Codebase/ClassLikes.php @@ -867,7 +867,7 @@ public function consolidateAnalyzedData(Methods $methods, ?Progress $progress, b foreach ($this->existing_classlikes_lc as $fq_class_name_lc => $_) { try { $classlike_storage = $this->classlike_storage_provider->get($fq_class_name_lc); - } catch (InvalidArgumentException $e) { + } catch (InvalidArgumentException $_) { continue; } @@ -971,7 +971,7 @@ public function moveMethods(Methods $methods, ?Progress $progress = null): void $source_method_storage = $methods->getStorage( new MethodIdentifier(...$source_parts) ); - } catch (InvalidArgumentException $e) { + } catch (InvalidArgumentException $_) { continue; } @@ -979,7 +979,7 @@ public function moveMethods(Methods $methods, ?Progress $progress = null): void try { $classlike_storage = $this->classlike_storage_provider->get($destination_fq_class_name); - } catch (InvalidArgumentException $e) { + } catch (InvalidArgumentException $_) { continue; } @@ -1048,7 +1048,7 @@ public function moveProperties(Properties $properties, ?Progress $progress = nul foreach ($codebase->properties_to_move as $source => $destination) { try { $source_property_storage = $properties->getStorage($source); - } catch (InvalidArgumentException $e) { + } catch (InvalidArgumentException $_) { continue; } @@ -1704,7 +1704,7 @@ private function checkMethodReferences(ClassLikeStorage $classlike_storage, Meth try { $declaring_classlike_storage = $this->classlike_storage_provider->get($declaring_fq_classlike_name); - } catch (InvalidArgumentException $e) { + } catch (InvalidArgumentException $_) { continue; } @@ -1790,7 +1790,7 @@ private function checkMethodReferences(ClassLikeStorage $classlike_storage, Meth foreach ($classlike_storage->class_implements as $fq_interface_name_lc => $_) { try { $interface_storage = $this->classlike_storage_provider->get($fq_interface_name_lc); - } catch (InvalidArgumentException $e) { + } catch (InvalidArgumentException $_) { continue; } @@ -1985,7 +1985,7 @@ private function findPossibleMethodParamTypes(ClassLikeStorage $classlike_storag try { $declaring_classlike_storage = $this->classlike_storage_provider->get($declaring_fq_classlike_name); - } catch (InvalidArgumentException $e) { + } catch (InvalidArgumentException $_) { continue; } @@ -2324,7 +2324,7 @@ public function getStorageFor(string $fq_class_name): ?ClassLikeStorage try { return $this->classlike_storage_provider->get($fq_class_name); - } catch (InvalidArgumentException $e) { + } catch (InvalidArgumentException $_) { return null; } } diff --git a/src/Psalm/Internal/Codebase/Functions.php b/src/Psalm/Internal/Codebase/Functions.php index 323e5fa5580..27a364e5c72 100644 --- a/src/Psalm/Internal/Codebase/Functions.php +++ b/src/Psalm/Internal/Codebase/Functions.php @@ -323,7 +323,7 @@ public function getMatchingFunctionNames( if (strpos($alias_name, $stub) === 0) { try { $match_function_patterns[] = $function_name; - } catch (Exception $e) { + } catch (Exception $_) { } } } @@ -547,7 +547,7 @@ public function isCallMapFunctionPure( try { return $codebase->methods->getStorage($count_method_id)->mutation_free; - } catch (Exception $e) { + } catch (Exception $_) { // do nothing } } diff --git a/src/Psalm/Internal/Codebase/Methods.php b/src/Psalm/Internal/Codebase/Methods.php index c32c69a3683..87795c49cc2 100644 --- a/src/Psalm/Internal/Codebase/Methods.php +++ b/src/Psalm/Internal/Codebase/Methods.php @@ -142,7 +142,7 @@ public function methodExists( try { $class_storage = $this->classlike_storage_provider->get($fq_class_name); - } catch (InvalidArgumentException $e) { + } catch (InvalidArgumentException $_) { return false; } @@ -1239,7 +1239,7 @@ public function hasStorage(MethodIdentifier $method_id): bool { try { $class_storage = $this->classlike_storage_provider->get($method_id->fq_class_name); - } catch (InvalidArgumentException $e) { + } catch (InvalidArgumentException $_) { return false; } diff --git a/src/Psalm/Internal/Codebase/Populator.php b/src/Psalm/Internal/Codebase/Populator.php index 7abd7713f2c..468a33018aa 100644 --- a/src/Psalm/Internal/Codebase/Populator.php +++ b/src/Psalm/Internal/Codebase/Populator.php @@ -102,7 +102,7 @@ public function populateCodebase(): void foreach ($class_storage->dependent_classlikes as $dependent_classlike_lc => $_) { try { $dependee_storage = $this->classlike_storage_provider->get($dependent_classlike_lc); - } catch (InvalidArgumentException $exception) { + } catch (InvalidArgumentException $_) { continue; } @@ -296,7 +296,7 @@ private function populateOverriddenMethods( ) ); $implemented_interface_storage = $storage_provider->get($implemented_interface); - } catch (InvalidArgumentException $e) { + } catch (InvalidArgumentException $_) { continue; } @@ -447,7 +447,7 @@ private function populateDataFromTrait( ) ); $trait_storage = $storage_provider->get($used_trait_lc); - } catch (InvalidArgumentException $e) { + } catch (InvalidArgumentException $_) { return; } @@ -549,7 +549,7 @@ private function populateDataFromParentClass( try { $parent_storage = $storage_provider->get($parent_storage_class); - } catch (InvalidArgumentException $e) { + } catch (InvalidArgumentException $_) { $this->progress->debug('Populator could not find dependency (' . __LINE__ . ")\n"); $storage->invalid_dependencies[$parent_storage_class] = true; @@ -732,7 +732,7 @@ private function populateInterfaceData( ) ); $new_parent_interface_storage = $storage_provider->get($new_parent); - } catch (InvalidArgumentException $e) { + } catch (InvalidArgumentException $_) { continue; } @@ -753,7 +753,7 @@ private function populateInterfaceDataFromParentInterface( ) ); $parent_interface_storage = $storage_provider->get($parent_interface_lc); - } catch (InvalidArgumentException $e) { + } catch (InvalidArgumentException $_) { $this->progress->debug('Populator could not find dependency (' . __LINE__ . ")\n"); $storage->invalid_dependencies[$parent_interface_lc] = true; @@ -786,7 +786,7 @@ private function populateDataFromImplementedInterface( ) ); $implemented_interface_storage = $storage_provider->get($implemented_interface_lc); - } catch (InvalidArgumentException $e) { + } catch (InvalidArgumentException $_) { $this->progress->debug('Populator could not find dependency (' . __LINE__ . ")\n"); $storage->invalid_dependencies[$implemented_interface_lc] = true; @@ -828,7 +828,7 @@ private function populateFileStorage(FileStorage $storage, array $dependent_file foreach ($storage->required_file_paths as $included_file_path => $_) { try { $included_file_storage = $this->file_storage_provider->get($included_file_path); - } catch (InvalidArgumentException $e) { + } catch (InvalidArgumentException $_) { continue; } @@ -840,7 +840,7 @@ private function populateFileStorage(FileStorage $storage, array $dependent_file foreach ($all_required_file_paths as $included_file_path => $_) { try { $included_file_storage = $this->file_storage_provider->get($included_file_path); - } catch (InvalidArgumentException $e) { + } catch (InvalidArgumentException $_) { continue; } @@ -858,7 +858,7 @@ private function populateFileStorage(FileStorage $storage, array $dependent_file foreach ($storage->referenced_classlikes as $fq_class_name) { try { $classlike_storage = $this->classlike_storage_provider->get($fq_class_name); - } catch (InvalidArgumentException $e) { + } catch (InvalidArgumentException $_) { continue; } @@ -868,14 +868,14 @@ private function populateFileStorage(FileStorage $storage, array $dependent_file try { $included_file_storage = $this->file_storage_provider->get($classlike_storage->location->file_path); - } catch (InvalidArgumentException $e) { + } catch (InvalidArgumentException $_) { continue; } foreach ($classlike_storage->used_traits as $used_trait) { try { $trait_storage = $this->classlike_storage_provider->get($used_trait); - } catch (InvalidArgumentException $e) { + } catch (InvalidArgumentException $_) { continue; } @@ -887,7 +887,7 @@ private function populateFileStorage(FileStorage $storage, array $dependent_file $included_trait_file_storage = $this->file_storage_provider->get( $trait_storage->location->file_path ); - } catch (InvalidArgumentException $e) { + } catch (InvalidArgumentException $_) { continue; } @@ -908,7 +908,7 @@ private function populateFileStorage(FileStorage $storage, array $dependent_file foreach ($all_required_file_paths as $required_file_path) { try { $required_file_storage = $this->file_storage_provider->get($required_file_path); - } catch (InvalidArgumentException $e) { + } catch (InvalidArgumentException $_) { continue; } @@ -918,7 +918,7 @@ private function populateFileStorage(FileStorage $storage, array $dependent_file foreach ($storage->required_classes as $required_classlike) { try { $classlike_storage = $this->classlike_storage_provider->get($required_classlike); - } catch (InvalidArgumentException $e) { + } catch (InvalidArgumentException $_) { continue; } @@ -928,7 +928,7 @@ private function populateFileStorage(FileStorage $storage, array $dependent_file try { $required_file_storage = $this->file_storage_provider->get($classlike_storage->location->file_path); - } catch (InvalidArgumentException $e) { + } catch (InvalidArgumentException $_) { continue; } diff --git a/src/Psalm/Internal/Codebase/Reflection.php b/src/Psalm/Internal/Codebase/Reflection.php index 45a12766cec..b083082fc4d 100644 --- a/src/Psalm/Internal/Codebase/Reflection.php +++ b/src/Psalm/Internal/Codebase/Reflection.php @@ -75,7 +75,7 @@ public function registerClass(ReflectionClass $reflected_class): void $this->storage_provider->get($class_name_lower); return; - } catch (Exception $e) { + } catch (Exception $_) { // this is fine } @@ -412,7 +412,7 @@ public function registerFunction(string $function_id): ?bool } $storage->cased_name = $reflection_function->getName(); - } catch (ReflectionException $e) { + } catch (ReflectionException $_) { return false; } diff --git a/src/Psalm/Internal/Codebase/Scanner.php b/src/Psalm/Internal/Codebase/Scanner.php index e510bf63827..fce983c3de6 100644 --- a/src/Psalm/Internal/Codebase/Scanner.php +++ b/src/Psalm/Internal/Codebase/Scanner.php @@ -709,7 +709,7 @@ function () use ($fq_class_name): ?ReflectionClass { /** @psalm-suppress ArgumentTypeCoercion */ return new ReflectionClass($fq_class_name); - } catch (Throwable $e) { + } catch (Throwable $_) { // do not cache any results here (as case-sensitive filenames can screw things up) return null; diff --git a/src/Psalm/Internal/LanguageServer/LanguageServer.php b/src/Psalm/Internal/LanguageServer/LanguageServer.php index b78e41d3a30..ca5b47b03fe 100644 --- a/src/Psalm/Internal/LanguageServer/LanguageServer.php +++ b/src/Psalm/Internal/LanguageServer/LanguageServer.php @@ -483,7 +483,7 @@ public function verboseLog(string $message, int $type = 4): void '[Psalm ' .PSALM_VERSION. ' - PHP Language Server] ' . $message, $type ); - } catch (Throwable $err) { + } catch (Throwable $_) { // do nothing } } @@ -507,7 +507,7 @@ private function clientStatus(string $status, ?string $additional_info = null): 3, 'telemetry/event' ); - } catch (Throwable $err) { + } catch (Throwable $_) { // do nothing } new Success(null); diff --git a/src/Psalm/Internal/LanguageServer/Server/TextDocument.php b/src/Psalm/Internal/LanguageServer/Server/TextDocument.php index 8b5b92013bf..3942857f969 100644 --- a/src/Psalm/Internal/LanguageServer/Server/TextDocument.php +++ b/src/Psalm/Internal/LanguageServer/Server/TextDocument.php @@ -177,7 +177,7 @@ public function definition(TextDocumentIdentifier $textDocument, Position $posit try { $reference_location = $this->codebase->getReferenceAtPosition($file_path, $position); - } catch (UnanalyzedFileException $e) { + } catch (UnanalyzedFileException $_) { $this->codebase->file_provider->openFile($file_path); $this->server->queueFileAnalysis($file_path, $textDocument->uri); @@ -221,7 +221,7 @@ public function hover(TextDocumentIdentifier $textDocument, Position $position): try { $reference_location = $this->codebase->getReferenceAtPosition($file_path, $position); - } catch (UnanalyzedFileException $e) { + } catch (UnanalyzedFileException $_) { $this->codebase->file_provider->openFile($file_path); $this->server->queueFileAnalysis($file_path, $textDocument->uri); @@ -277,7 +277,7 @@ public function completion(TextDocumentIdentifier $textDocument, Position $posit try { $completion_data = $this->codebase->getCompletionDataAtPosition($file_path, $position); - } catch (UnanalyzedFileException $e) { + } catch (UnanalyzedFileException $_) { $this->codebase->file_provider->openFile($file_path); $this->server->queueFileAnalysis($file_path, $textDocument->uri); @@ -328,7 +328,7 @@ public function signatureHelp(TextDocumentIdentifier $textDocument, Position $po try { $argument_location = $this->codebase->getFunctionArgumentAtPosition($file_path, $position); - } catch (UnanalyzedFileException $e) { + } catch (UnanalyzedFileException $_) { $this->codebase->file_provider->openFile($file_path); $this->server->queueFileAnalysis($file_path, $textDocument->uri); diff --git a/src/Psalm/Internal/PhpVisitor/Reflector/ClassLikeDocblockParser.php b/src/Psalm/Internal/PhpVisitor/Reflector/ClassLikeDocblockParser.php index cc913806c19..06cce399f00 100644 --- a/src/Psalm/Internal/PhpVisitor/Reflector/ClassLikeDocblockParser.php +++ b/src/Psalm/Internal/PhpVisitor/Reflector/ClassLikeDocblockParser.php @@ -354,7 +354,7 @@ public static function parse( ); $method_tree = $parse_tree_creator->create(); - } catch (TypeParseTreeException $e) { + } catch (TypeParseTreeException $_) { throw new DocblockParseException($method_entry . ' is not a valid method'); } @@ -425,7 +425,7 @@ public static function parse( $codebase->analysis_php_version_id, $has_errors ); - } catch (Exception $e) { + } catch (Exception $_) { throw new DocblockParseException('Badly-formatted @method string ' . $method_entry); } diff --git a/src/Psalm/Internal/PhpVisitor/Reflector/ClassLikeNodeScanner.php b/src/Psalm/Internal/PhpVisitor/Reflector/ClassLikeNodeScanner.php index ae25c5cadeb..6877c2506b8 100644 --- a/src/Psalm/Internal/PhpVisitor/Reflector/ClassLikeNodeScanner.php +++ b/src/Psalm/Internal/PhpVisitor/Reflector/ClassLikeNodeScanner.php @@ -233,7 +233,7 @@ public function start(PhpParser\Node\Stmt\ClassLike $node): ?bool foreach ($storage->dependent_classlikes as $dependent_name_lc => $_) { try { $dependent_storage = $this->codebase->classlike_storage_provider->get($dependent_name_lc); - } catch (InvalidArgumentException $exception) { + } catch (InvalidArgumentException $_) { continue; } $dependent_storage->populated = false; @@ -510,7 +510,7 @@ public function start(PhpParser\Node\Stmt\ClassLike $node): ?bool ); $storage->yield = $yield_type; - } catch (TypeParseTreeException $e) { + } catch (TypeParseTreeException $_) { // do nothing } } @@ -809,7 +809,7 @@ function (InlineTypeAlias $t): ?ClassTypeAlias { return new ClassTypeAlias( array_values($union->getAtomicTypes()) ); - } catch (Exception $e) { + } catch (Exception $_) { return null; } }, @@ -1885,7 +1885,7 @@ private static function getTypeAliasesFromCommentLines( $type_alias_tokens + $type_aliases, $self_fqcln ); - } catch (TypeParseTreeException $e) { + } catch (TypeParseTreeException $_) { throw new DocblockParseException($type_string . ' is not a valid type'); } diff --git a/src/Psalm/Internal/PhpVisitor/Reflector/ExpressionResolver.php b/src/Psalm/Internal/PhpVisitor/Reflector/ExpressionResolver.php index 65a78a746c9..b27b435fb8a 100644 --- a/src/Psalm/Internal/PhpVisitor/Reflector/ExpressionResolver.php +++ b/src/Psalm/Internal/PhpVisitor/Reflector/ExpressionResolver.php @@ -352,7 +352,7 @@ public static function enterConditional( }); try { return (bool) $evaluator->evaluateSilently($expr); - } catch (ConstExprEvaluationException $e) { + } catch (ConstExprEvaluationException $_) { return null; } } diff --git a/src/Psalm/Internal/PhpVisitor/Reflector/FunctionLikeNodeScanner.php b/src/Psalm/Internal/PhpVisitor/Reflector/FunctionLikeNodeScanner.php index f842f9e42fd..e33c128c004 100644 --- a/src/Psalm/Internal/PhpVisitor/Reflector/FunctionLikeNodeScanner.php +++ b/src/Psalm/Internal/PhpVisitor/Reflector/FunctionLikeNodeScanner.php @@ -326,7 +326,7 @@ public function start(PhpParser\Node\FunctionLike $stmt, bool $fake_method = fal try { $negated_formula = Algebra::negateFormula($if_clauses); - } catch (ComplicatedExpressionException $e) { + } catch (ComplicatedExpressionException $_) { $var_assertions = []; break; } @@ -1048,7 +1048,7 @@ private function createStorageForFunctionLike( $docblock_info = null; try { $docblock_info = FunctionLikeDocblockParser::parse($doc_comment); - } catch (DocblockParseException $e) { + } catch (DocblockParseException $_) { } if ($docblock_info) { if ($docblock_info->since_php_major_version && !$this->aliases->namespace) { diff --git a/src/Psalm/Internal/PhpVisitor/ReflectorVisitor.php b/src/Psalm/Internal/PhpVisitor/ReflectorVisitor.php index 85fa7351967..76a3b951b63 100644 --- a/src/Psalm/Internal/PhpVisitor/ReflectorVisitor.php +++ b/src/Psalm/Internal/PhpVisitor/ReflectorVisitor.php @@ -371,7 +371,7 @@ public function enterNode(PhpParser\Node $node): ?int $template_types, $this->type_aliases ); - } catch (DocblockParseException $e) { + } catch (DocblockParseException $_) { // do nothing } diff --git a/src/Psalm/Internal/PhpVisitor/TraitFinder.php b/src/Psalm/Internal/PhpVisitor/TraitFinder.php index 55d177e351d..81db1cfc47c 100644 --- a/src/Psalm/Internal/PhpVisitor/TraitFinder.php +++ b/src/Psalm/Internal/PhpVisitor/TraitFinder.php @@ -71,7 +71,7 @@ public function getNode(): ?PhpParser\Node\Stmt\Trait_ try { $reflection_trait = new ReflectionClass($this->fq_trait_name); - } catch (Throwable $t) { + } catch (Throwable $_) { return null; } diff --git a/src/Psalm/Internal/PluginManager/Command/DisableCommand.php b/src/Psalm/Internal/PluginManager/Command/DisableCommand.php index 6bd53f5f3c1..8e2ddeb7052 100644 --- a/src/Psalm/Internal/PluginManager/Command/DisableCommand.php +++ b/src/Psalm/Internal/PluginManager/Command/DisableCommand.php @@ -64,7 +64,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int try { $plugin_class = $plugin_list->resolvePluginClass($plugin_name); - } catch (InvalidArgumentException $e) { + } catch (InvalidArgumentException $_) { $io->error('Unknown plugin class ' . $plugin_name); return 2; diff --git a/src/Psalm/Internal/PluginManager/Command/EnableCommand.php b/src/Psalm/Internal/PluginManager/Command/EnableCommand.php index e57ee4d5d19..0f0aeae7228 100644 --- a/src/Psalm/Internal/PluginManager/Command/EnableCommand.php +++ b/src/Psalm/Internal/PluginManager/Command/EnableCommand.php @@ -64,7 +64,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int try { $plugin_class = $plugin_list->resolvePluginClass($plugin_name); - } catch (InvalidArgumentException $e) { + } catch (InvalidArgumentException $_) { $io->error('Unknown plugin class ' . $plugin_name); return 2; diff --git a/src/Psalm/Internal/PluginManager/PluginListFactory.php b/src/Psalm/Internal/PluginManager/PluginListFactory.php index 6ae81a32b8d..ab6139290c3 100644 --- a/src/Psalm/Internal/PluginManager/PluginListFactory.php +++ b/src/Psalm/Internal/PluginManager/PluginListFactory.php @@ -34,7 +34,7 @@ public function __invoke(string $current_dir, ?string $config_file_path = null): { try { $config_file = new ConfigFile($current_dir, $config_file_path); - } catch (RuntimeException $exception) { + } catch (RuntimeException $_) { $config_file = null; } $composer_lock = new ComposerLock($this->findLockFiles()); diff --git a/src/Psalm/Internal/Provider/FileReferenceProvider.php b/src/Psalm/Internal/Provider/FileReferenceProvider.php index ece9a2ac06e..2fa276c6642 100644 --- a/src/Psalm/Internal/Provider/FileReferenceProvider.php +++ b/src/Psalm/Internal/Provider/FileReferenceProvider.php @@ -398,7 +398,7 @@ private function calculateFilesReferencingFile(Codebase $codebase, string $file) try { $referenced_files[] = $codebase->scanner->getClassLikeFilePath($fq_class_name_lc); - } catch (UnexpectedValueException $e) { + } catch (UnexpectedValueException $_) { if (isset(self::$classlike_files[$fq_class_name_lc])) { $referenced_files[] = self::$classlike_files[$fq_class_name_lc]; } diff --git a/src/Psalm/Internal/Provider/ParserCacheProvider.php b/src/Psalm/Internal/Provider/ParserCacheProvider.php index 1918279554e..e3d9fb82f88 100644 --- a/src/Psalm/Internal/Provider/ParserCacheProvider.php +++ b/src/Psalm/Internal/Provider/ParserCacheProvider.php @@ -361,7 +361,7 @@ private function createCacheDirectory(string $parser_cache_directory): void if (!is_dir($parser_cache_directory)) { try { mkdir($parser_cache_directory, 0777, true); - } catch (RuntimeException $e) { + } catch (RuntimeException $_) { // Race condition (#4483) if (!is_dir($parser_cache_directory)) { trigger_error('Could not create parser cache directory: ' . $parser_cache_directory, E_USER_ERROR); diff --git a/src/Psalm/Internal/Provider/ReturnTypeProvider/ArrayFilterReturnTypeProvider.php b/src/Psalm/Internal/Provider/ReturnTypeProvider/ArrayFilterReturnTypeProvider.php index 4f14e5d3009..a4511ebf5fc 100644 --- a/src/Psalm/Internal/Provider/ReturnTypeProvider/ArrayFilterReturnTypeProvider.php +++ b/src/Psalm/Internal/Provider/ReturnTypeProvider/ArrayFilterReturnTypeProvider.php @@ -279,7 +279,7 @@ static function ($keyed_type) use ($statements_source, $context) { $statements_source, $codebase ); - } catch (ComplicatedExpressionException $e) { + } catch (ComplicatedExpressionException $_) { $filter_clauses = []; } diff --git a/src/Psalm/Internal/Provider/StatementsProvider.php b/src/Psalm/Internal/Provider/StatementsProvider.php index d6d5d4abccc..2caad7ef1dc 100644 --- a/src/Psalm/Internal/Provider/StatementsProvider.php +++ b/src/Psalm/Internal/Provider/StatementsProvider.php @@ -443,7 +443,7 @@ public static function parseStatements( try { /** @var list */ $stmts = self::$parser->parse($file_contents, $error_handler) ?: []; - } catch (Throwable $t) { + } catch (Throwable $_) { $stmts = []; // hope this got caught below @@ -453,7 +453,7 @@ public static function parseStatements( try { /** @var list */ $stmts = self::$parser->parse($file_contents, $error_handler) ?: []; - } catch (Throwable $t) { + } catch (Throwable $_) { $stmts = []; // hope this got caught below diff --git a/src/Psalm/Internal/Type/Comparator/CallableTypeComparator.php b/src/Psalm/Internal/Type/Comparator/CallableTypeComparator.php index e89269c3e0d..35317a188c4 100644 --- a/src/Psalm/Internal/Type/Comparator/CallableTypeComparator.php +++ b/src/Psalm/Internal/Type/Comparator/CallableTypeComparator.php @@ -211,7 +211,7 @@ public static function isNotExplicitlyCallableTypeCallable( } $codebase->methods->getStorage($method_id); - } catch (Exception $e) { + } catch (Exception $_) { return false; } } @@ -305,7 +305,7 @@ public static function getCallableFromAtomic( $return_type, $function_storage->pure ); - } catch (UnexpectedValueException $e) { + } catch (UnexpectedValueException $_) { if (InternalCallMapHandler::inCallMap($input_type_part->value)) { $args = []; @@ -370,7 +370,7 @@ public static function getCallableFromAtomic( $converted_return_type, $method_storage->pure ); - } catch (UnexpectedValueException $e) { + } catch (UnexpectedValueException $_) { // do nothing } } diff --git a/src/Psalm/Internal/Type/TemplateInferredTypeReplacer.php b/src/Psalm/Internal/Type/TemplateInferredTypeReplacer.php index 925b79b6bfd..a4d0e35b84d 100644 --- a/src/Psalm/Internal/Type/TemplateInferredTypeReplacer.php +++ b/src/Psalm/Internal/Type/TemplateInferredTypeReplacer.php @@ -129,7 +129,7 @@ public static function replace( } } } - } catch (InvalidArgumentException $e) { + } catch (InvalidArgumentException $_) { } } } diff --git a/src/Psalm/Internal/Type/TemplateStandinTypeReplacer.php b/src/Psalm/Internal/Type/TemplateStandinTypeReplacer.php index 926d85a0b5c..abe5219d704 100644 --- a/src/Psalm/Internal/Type/TemplateStandinTypeReplacer.php +++ b/src/Psalm/Internal/Type/TemplateStandinTypeReplacer.php @@ -454,7 +454,7 @@ private static function findMatchingAtomicTypesForTemplate( $matching_atomic_types[$atomic_input_type->getId()] = $atomic_input_type; continue; } - } catch (InvalidArgumentException $e) { + } catch (InvalidArgumentException $_) { // do nothing } } @@ -516,7 +516,7 @@ private static function findMatchingAtomicTypesForTemplate( $matching_atomic_types[$atomic_input_type->getId()] = $atomic_input_type; continue; } - } catch (InvalidArgumentException $e) { + } catch (InvalidArgumentException $_) { // do nothing } } @@ -1146,7 +1146,7 @@ public static function getMappedGenericTypeParams( $input_class_storage = $codebase->classlike_storage_provider->get($input_type_part->value); $container_class_storage = $codebase->classlike_storage_provider->get($container_type_part->value); $container_type_params_covariant = $container_class_storage->template_covariants; - } catch (Throwable $e) { + } catch (Throwable $_) { $input_class_storage = null; } diff --git a/src/Psalm/Internal/Type/TypeCombiner.php b/src/Psalm/Internal/Type/TypeCombiner.php index c28f119ef45..24d9fdbb978 100644 --- a/src/Psalm/Internal/Type/TypeCombiner.php +++ b/src/Psalm/Internal/Type/TypeCombiner.php @@ -1287,7 +1287,7 @@ private static function getClassLikes(Codebase $codebase, string $fq_classlike_n { try { $class_storage = $codebase->classlike_storage_provider->get($fq_classlike_name); - } catch (InvalidArgumentException $e) { + } catch (InvalidArgumentException $_) { return []; } diff --git a/src/Psalm/Internal/Type/TypeExpander.php b/src/Psalm/Internal/Type/TypeExpander.php index 7b6ddea2dd9..660e68bbda0 100644 --- a/src/Psalm/Internal/Type/TypeExpander.php +++ b/src/Psalm/Internal/Type/TypeExpander.php @@ -278,7 +278,7 @@ public static function expandAtomic( $matching_constant, ReflectionProperty::IS_PRIVATE ); - } catch (CircularReferenceException $e) { + } catch (CircularReferenceException $_) { $class_constant = null; } @@ -912,7 +912,7 @@ private static function expandKeyOfValueOfArray( $type_param->const_name, ReflectionProperty::IS_PRIVATE ); - } catch (CircularReferenceException $e) { + } catch (CircularReferenceException $_) { return [$return_type]; } diff --git a/src/Psalm/Internal/TypeVisitor/TypeChecker.php b/src/Psalm/Internal/TypeVisitor/TypeChecker.php index af0ad30c33c..82b68e389e8 100644 --- a/src/Psalm/Internal/TypeVisitor/TypeChecker.php +++ b/src/Psalm/Internal/TypeVisitor/TypeChecker.php @@ -211,7 +211,7 @@ private function checkGenericParams(TGenericObject $atomic): void try { $class_storage = $codebase->classlike_storage_provider->get(strtolower($atomic->value)); - } catch (InvalidArgumentException $e) { + } catch (InvalidArgumentException $_) { return; } diff --git a/src/Psalm/IssueBuffer.php b/src/Psalm/IssueBuffer.php index a89f053792b..760f0b114ac 100644 --- a/src/Psalm/IssueBuffer.php +++ b/src/Psalm/IssueBuffer.php @@ -648,7 +648,7 @@ function (IssueData $d1, IssueData $d2): int { try { $source_control_info = (new GitInfoCollector())->collect(); - } catch (RuntimeException $e) { + } catch (RuntimeException $_) { // do nothing } diff --git a/tests/Config/Plugin/Hook/SqlStringProvider.php b/tests/Config/Plugin/Hook/SqlStringProvider.php index 8b233a9a251..37ddff00cd9 100644 --- a/tests/Config/Plugin/Hook/SqlStringProvider.php +++ b/tests/Config/Plugin/Hook/SqlStringProvider.php @@ -23,7 +23,7 @@ public static function getTypeFromValue(StringInterpreterEvent $event): ?TLitera if (!$parser->errors) { return new TSqlSelectString($value); } - } catch (Throwable $e) { + } catch (Throwable $_) { // fall through } } From 2330ace884fce5782c233cb8ef65d588bb8d285f Mon Sep 17 00:00:00 2001 From: AndrolGenhald Date: Thu, 17 Feb 2022 17:44:39 -0600 Subject: [PATCH 3/7] Add upgrading note. --- UPGRADING.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/UPGRADING.md b/UPGRADING.md index b44f1d37cbc..e9bc6d89b39 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -836,3 +836,5 @@ - :information_source: migration possible using `Psalm\PluginFileExtensionsSocket` - [BC] Method `\Psalm\Plugin\EventHandler\Event\AfterFunctionLikeAnalysisEvent::getClasslikeStorage()` was removed, use correct `\Psalm\Plugin\EventHandler\Event\AfterFunctionLikeAnalysisEvent::getFunctionlikeStorage()` instead + - [BC] Property `Psalm\Context::$inside_try` was removed + - [BC] Property `Psalm\Context::$finally_scope` was removed From 0831069bf89ac740cf3139af914a6369ebe01f92 Mon Sep 17 00:00:00 2001 From: AndrolGenhald Date: Sun, 20 Feb 2022 11:31:44 -0600 Subject: [PATCH 4/7] Re-add incorrectly removed test. --- tests/TryCatchTest.php | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/TryCatchTest.php b/tests/TryCatchTest.php index 861075c48fd..bba89cb602d 100644 --- a/tests/TryCatchTest.php +++ b/tests/TryCatchTest.php @@ -368,6 +368,14 @@ class_alias( } }' ], + 'suppressUndefinedVarInFinally' => [ + 'code' => 'end = null; + } + ', + ], 'returnsInTry' => [ 'code' => ' Date: Sun, 20 Feb 2022 15:52:08 -0600 Subject: [PATCH 5/7] Update vars_possibly_in_scope in TryAnalyzer. --- .../Analyzer/Statements/Block/TryAnalyzer.php | 3 +++ tests/TryCatchTest.php | 16 ++++++++++++++++ 2 files changed, 19 insertions(+) diff --git a/src/Psalm/Internal/Analyzer/Statements/Block/TryAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Block/TryAnalyzer.php index a2c0e5adb02..32e84343d1c 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Block/TryAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Block/TryAnalyzer.php @@ -561,6 +561,7 @@ private static function applyCatchContextsForFinallyContext( $context->vars_in_scope[$var_id] = Type::combineUnionTypeArray($types, $codebase); $context->vars_in_scope[$var_id]->possibly_undefined = $possibly_undefined; $context->vars_in_scope[$var_id]->possibly_undefined_from_try = false; + $context->vars_possibly_in_scope[$var_id] = true; } } @@ -630,6 +631,7 @@ private static function applyCatchContextsForOuterContext( $context->vars_in_scope[$var_id] = Type::combineUnionTypeArray($types, $codebase); $context->vars_in_scope[$var_id]->possibly_undefined = $possibly_undefined; $context->vars_in_scope[$var_id]->possibly_undefined_from_try = false; + $context->vars_possibly_in_scope[$var_id] = true; } if (count($leaving_catches) < $catch_block_count) { @@ -645,6 +647,7 @@ private static function applyCatchContextsForOuterContext( // Set all variables to their types at the end of the `try` block, since // it would have to complete successfully for execution to continue. $context->vars_in_scope = $end_of_try_context->vars_in_scope; + $context->vars_possibly_in_scope = $end_of_try_context->vars_possibly_in_scope; $context->references_in_scope = $end_of_try_context->references_in_scope; } } diff --git a/tests/TryCatchTest.php b/tests/TryCatchTest.php index bba89cb602d..ac027890693 100644 --- a/tests/TryCatchTest.php +++ b/tests/TryCatchTest.php @@ -1082,6 +1082,22 @@ function maybeThrow(): void {} '$foo===' => '1|2', ], ], + 'tryInsideIfPossiblyUndefined' => [ + 'code' => ' Date: Sun, 20 Feb 2022 16:55:23 -0600 Subject: [PATCH 6/7] Fix issues from review. --- .../Analyzer/Statements/Block/TryAnalyzer.php | 1 - .../Analyzer/Statements/BreakAnalyzer.php | 2 - .../Internal/Analyzer/StatementsAnalyzer.php | 2 +- stubs/phpparser.phpstub | 62 ++++++++++++++----- tests/DocumentationTest.php | 1 - tests/TryCatchTest.php | 8 +-- tests/UnusedVariableTest.php | 10 +-- 7 files changed, 57 insertions(+), 29 deletions(-) diff --git a/src/Psalm/Internal/Analyzer/Statements/Block/TryAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Block/TryAnalyzer.php index 32e84343d1c..1219051493b 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Block/TryAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Block/TryAnalyzer.php @@ -257,7 +257,6 @@ private static function analyzeCatch( $fq_catch_classes = []; - assert(!empty($catch->types)); foreach ($catch->types as $catch_type_stmt) { $fq_catch_class = ClassLikeAnalyzer::getFQCLNFromNameObject( $catch_type_stmt, diff --git a/src/Psalm/Internal/Analyzer/Statements/BreakAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/BreakAnalyzer.php index 9758db1f9c0..e25b0d990a0 100644 --- a/src/Psalm/Internal/Analyzer/Statements/BreakAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/BreakAnalyzer.php @@ -5,7 +5,6 @@ use PhpParser; use Psalm\Context; use Psalm\Internal\Analyzer\ScopeAnalyzer; -use Psalm\Internal\Analyzer\StatementsAnalyzer; use Psalm\Type; use function end; @@ -16,7 +15,6 @@ class BreakAnalyzer { public static function analyze( - StatementsAnalyzer $_statements_analyzer, PhpParser\Node\Stmt\Break_ $stmt, Context $context ): void { diff --git a/src/Psalm/Internal/Analyzer/StatementsAnalyzer.php b/src/Psalm/Internal/Analyzer/StatementsAnalyzer.php index 5abdbf7e10a..4eeb5fb3bd0 100644 --- a/src/Psalm/Internal/Analyzer/StatementsAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/StatementsAnalyzer.php @@ -567,7 +567,7 @@ private static function analyzeStatement( } elseif ($stmt instanceof PhpParser\Node\Stmt\Switch_) { SwitchAnalyzer::analyze($statements_analyzer, $stmt, $context); } elseif ($stmt instanceof PhpParser\Node\Stmt\Break_) { - BreakAnalyzer::analyze($statements_analyzer, $stmt, $context); + BreakAnalyzer::analyze($stmt, $context); } elseif ($stmt instanceof PhpParser\Node\Stmt\Continue_) { ContinueAnalyzer::analyze($statements_analyzer, $stmt, $context); } elseif ($stmt instanceof PhpParser\Node\Stmt\Static_) { diff --git a/stubs/phpparser.phpstub b/stubs/phpparser.phpstub index 834f8f62539..44bc02274ee 100644 --- a/stubs/phpparser.phpstub +++ b/stubs/phpparser.phpstub @@ -1,22 +1,54 @@ + */ + abstract public function getRawArgs(): array; -abstract class CallLike extends Expr { - /** - * @return list - */ - abstract public function getRawArgs(): array; + public function isFirstClassCallable(): bool {} - public function isFirstClassCallable(): bool {} + /** + * @psalm-pure + * @return list + */ + public function getArgs(): array{} + } +} + +namespace PhpParser\Node\Stmt { + use PhpParser\Node; + use PhpParser\Node\Expr; + + class Catch_ extends Node\Stmt + { + /** @var non-empty-list Types of exceptions to catch */ + public $types; + /** @var Expr\Variable|null Variable for exception */ + public $var; + /** @var list Statements */ + public $stmts; + + /** + * Constructs a catch node. + * + * @param non-empty-list $types Types of exceptions to catch + * @param Expr\Variable|null $var Variable for exception + * @param list $stmts Statements + * @param array $attributes Additional attributes + */ + public function __construct( + array $types, Expr\Variable $var = null, array $stmts = [], array $attributes = [] + ) {} - /** - * @psalm-pure - * @return list - */ - public function getArgs(): array{} + /** @return list */ + public function getSubNodeNames() : array {} + } } diff --git a/tests/DocumentationTest.php b/tests/DocumentationTest.php index 23f271133e1..23484bc70cc 100644 --- a/tests/DocumentationTest.php +++ b/tests/DocumentationTest.php @@ -65,7 +65,6 @@ class DocumentationTest extends TestCase * annotations that we don’t want documented */ private const INTENTIONALLY_UNDOCUMENTED_ANNOTATIONS = [ - '@psalm-check-type', // Used internally for testing try-catch-finally, not sure if we want to support it '@psalm-self-out', // Not documented as it's a legacy alias of @psalm-this-out '@psalm-variadic', ]; diff --git a/tests/TryCatchTest.php b/tests/TryCatchTest.php index ac027890693..a16a036303b 100644 --- a/tests/TryCatchTest.php +++ b/tests/TryCatchTest.php @@ -44,7 +44,7 @@ class CustomException extends Exception implements CustomThrowable {} 'code' => ' [ @@ -141,7 +141,7 @@ function test(): string { try { $var = test(); - } catch (Throwable $e) { + } catch (Exception $e) { return; } @@ -159,7 +159,7 @@ function test(): string { try { $var = test(); - } catch (Throwable $e) { + } catch (Exception $e) { $var = "bad"; } @@ -257,7 +257,7 @@ function example() : void { try { $str = "a"; - } catch (Throwable $e) { + } catch (Exception $e) { example(); } ord($str);', diff --git a/tests/UnusedVariableTest.php b/tests/UnusedVariableTest.php index 1e77f0fcd1f..d0b09e303f8 100644 --- a/tests/UnusedVariableTest.php +++ b/tests/UnusedVariableTest.php @@ -444,7 +444,7 @@ function dangerous(): string { function callDangerous(): void { try { $s = dangerous(); - } catch (Throwable $e) { + } catch (Exception $e) { echo $e->getMessage(); $s = "hello"; } @@ -470,7 +470,7 @@ function callDangerous(): void { } else { try { $t = dangerous(); - } catch (Throwable $e) { + } catch (Exception $e) { echo $e->getMessage(); $t = "hello"; } @@ -500,7 +500,7 @@ function callDangerous(): void { } catch (E1 $e) { echo $e->getMessage(); $s = false; - } catch (Throwable $_) { + } catch (Exception $_) { return; } @@ -592,7 +592,7 @@ function main() : void { if (!$s) { echo "Failed to get string\n"; } - } catch (Throwable $_) { + } catch (Exception $_) { $s = "fallback"; } printf("s is %s\n", $s); @@ -2889,7 +2889,7 @@ function callDangerous(): void { } else { try { $t = dangerous(); - } catch (Throwable $e) { + } catch (Exception $e) { echo $e->getMessage(); $t = "hello"; } From 36d8cfa0ef7ed576f16c6193963319d16ef8b620 Mon Sep 17 00:00:00 2001 From: AndrolGenhald Date: Sun, 20 Feb 2022 18:00:35 -0600 Subject: [PATCH 7/7] Fix TODO and add skipped test. --- .../Analyzer/Statements/Block/TryAnalyzer.php | 18 ++------------- .../UndefinedVariableManipulationTest.php | 22 +++++++++++++++++++ 2 files changed, 24 insertions(+), 16 deletions(-) diff --git a/src/Psalm/Internal/Analyzer/Statements/Block/TryAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Block/TryAnalyzer.php index 1219051493b..0971d22dda4 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Block/TryAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Block/TryAnalyzer.php @@ -46,18 +46,11 @@ public static function analyze( Context $context ): ?bool { $catch_actions = []; - $all_catches_leave = true; $codebase = $statements_analyzer->getCodebase(); - /** @var int $i */ - foreach ($stmt->catches as $i => $catch) { - $catch_actions[$i] = ScopeAnalyzer::getControlActions( - $catch->stmts, - $statements_analyzer->node_data, - [] - ); - $all_catches_leave = $all_catches_leave && !in_array(ScopeAnalyzer::ACTION_NONE, $catch_actions[$i], true); + if ($codebase->alter_code) { + $context->branch_point = (int) $stmt->getAttribute('startFilePos'); } $existing_thrown_exceptions = $context->possibly_thrown_exceptions; @@ -72,13 +65,6 @@ public static function analyze( $old_try_catch_scope = $context->try_catch_scope; $context->try_catch_scope = new TryCatchScope(); - if (!$all_catches_leave || $stmt->finally) { - if ($codebase->alter_code) { - // TODO can this be moved? - $context->branch_point = $context->branch_point ?: (int) $stmt->getAttribute('startFilePos'); - } - } - $assigned_var_ids = $context->assigned_var_ids; $context->assigned_var_ids = []; diff --git a/tests/FileManipulation/UndefinedVariableManipulationTest.php b/tests/FileManipulation/UndefinedVariableManipulationTest.php index 7801fc74efb..b0ebbe388c1 100644 --- a/tests/FileManipulation/UndefinedVariableManipulationTest.php +++ b/tests/FileManipulation/UndefinedVariableManipulationTest.php @@ -101,6 +101,28 @@ public function providerValidCodeParse(): array 'issues_to_fix' => ['PossiblyUndefinedGlobalVariable'], 'safe_types' => true, ], + 'SKIPPED-possiblyUndefinedVariableInTry' => [ + 'input' => ' ' '7.3', + 'issues_to_fix' => ['PossiblyUndefinedGlobalVariable'], + 'safe_types' => true, + ], 'unsetPossiblyUndefinedVariable' => [ 'input' => '