From 86173d29c9a6a6af7f8f7d82ef757631de89b51b Mon Sep 17 00:00:00 2001 From: Evan Shaw Date: Mon, 26 Feb 2024 20:48:42 +1300 Subject: [PATCH 1/3] When inside isset, array fetch can return null This prevents false positive for various types of issues inside empty, such as RedundantConditionGivenDocblockType and TypeDoesNotContainType. --- .../Expression/Fetch/ArrayFetchAnalyzer.php | 8 ++++++-- tests/ArrayAssignmentTest.php | 2 ++ tests/ArrayFunctionCallTest.php | 1 + tests/TypeReconciliation/EmptyTest.php | 13 +++++++++++++ tests/TypeReconciliation/RedundantConditionTest.php | 2 ++ 5 files changed, 24 insertions(+), 2 deletions(-) diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/ArrayFetchAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/ArrayFetchAnalyzer.php index 7f8121133..34ed8f510 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/ArrayFetchAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/ArrayFetchAnalyzer.php @@ -171,7 +171,7 @@ final class ArrayFetchAnalyzer $codebase = $statements_analyzer->getCodebase(); - if ($keyed_array_var_id + if ($keyed_array_var_id !== null && $context->hasVariable($keyed_array_var_id) && !$context->vars_in_scope[$keyed_array_var_id]->possibly_undefined && $stmt_var_type @@ -250,6 +250,10 @@ final class ArrayFetchAnalyzer } } + if ($context->inside_isset && !$stmt_type->hasMixed()) { + $stmt_type = Type::combineUnionTypes($stmt_type, Type::getNull()); + } + $statements_analyzer->node_data->setType($stmt, $stmt_type); if ($context->inside_isset @@ -304,7 +308,7 @@ final class ArrayFetchAnalyzer } } - if ($keyed_array_var_id + if ($keyed_array_var_id !== null && $context->hasVariable($keyed_array_var_id) && (!($stmt_type = $statements_analyzer->node_data->getType($stmt)) || $stmt_type->isVanillaMixed()) ) { diff --git a/tests/ArrayAssignmentTest.php b/tests/ArrayAssignmentTest.php index fa65da822..488550453 100644 --- a/tests/ArrayAssignmentTest.php +++ b/tests/ArrayAssignmentTest.php @@ -1234,6 +1234,8 @@ class ArrayAssignmentTest extends TestCase foreach ($arr[0] as $k => $v) {} } }', + 'assertions' => [], + 'ignored_issues' => ['RiskyTruthyFalsyComparison'], ], 'nonEmptyAssignmentToListElement' => [ 'code' => ' [ '$line===' => 'array{0: int, ...}', ], + 'ignored_issues' => ['RiskyTruthyFalsyComparison'], ], 'arrayUnshiftOnEmptyArrayMeansNonEmptyList' => [ 'code' => ' [], + 'ignored_issues' => ['RiskyTruthyFalsyComparison'], ], 'multipleEmptiesInConditionWithMixedOffset' => [ 'code' => ' [], + 'ignored_issues' => ['RiskyTruthyFalsyComparison'], ], 'doubleEmptyCheckTwoArrays' => [ 'code' => ' [], + 'ignored_issues' => ['RiskyTruthyFalsyComparison'], ], 'doubleEmptyCheckOnTKeyedArrayVariableOffsets' => [ 'code' => ' [], + 'ignored_issues' => ['RiskyTruthyFalsyComparison'], ], 'checkArrayEmptyUnknownRoot' => [ 'code' => ' 'true', ], ], + 'emptyArrayFetch' => [ + 'code' => ' $a */ + if (empty($a["a"])) {}', + ], ]; } diff --git a/tests/TypeReconciliation/RedundantConditionTest.php b/tests/TypeReconciliation/RedundantConditionTest.php index 74f06e44e..e54aa2bd3 100644 --- a/tests/TypeReconciliation/RedundantConditionTest.php +++ b/tests/TypeReconciliation/RedundantConditionTest.php @@ -575,6 +575,8 @@ class RedundantConditionTest extends TestCase if (empty($a["foo"])) {} } }', + 'assertions' => [], + 'ignored_issues' => ['RiskyTruthyFalsyComparison'], ], 'suppressRedundantConditionAfterAssertNonEmpty' => [ 'code' => ' Date: Wed, 28 Feb 2024 21:46:58 +1300 Subject: [PATCH 2/3] Update baseline --- psalm-baseline.xml | 59 +++++++++++++++++++++++----------------------- 1 file changed, 29 insertions(+), 30 deletions(-) diff --git a/psalm-baseline.xml b/psalm-baseline.xml index 8d205b058..987aab124 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -1,10 +1,5 @@ - - - - - - + tags['variablesfrom'][0]]]> @@ -75,7 +70,6 @@ - function_id]]> @@ -716,6 +710,7 @@ template_extended_params]]> template_types]]> + overridden_method_ids[$method_name])]]> @@ -865,8 +860,6 @@ self]]> mixin_declaring_fqcln]]> - parent_class]]> - parent_class]]> calling_method_id]]> calling_method_id]]> self]]> @@ -945,8 +938,6 @@ - - @@ -1104,7 +1095,6 @@ - @@ -1115,9 +1105,6 @@ error_baseline]]> - - - threads]]> @@ -1128,7 +1115,6 @@ - @@ -1140,7 +1126,6 @@ - @@ -1201,11 +1186,7 @@ - - - - - value]]> + overridden_method_ids[$method_name])]]> @@ -1441,7 +1422,6 @@ - children[0]]]> children[1]]]> @@ -1475,6 +1455,9 @@ line_number]]> type_end]]> type_start]]> + + + @@ -1498,12 +1481,12 @@ - - - 0]]> + + + @@ -1636,6 +1619,13 @@ cache->getFileMapCache()]]> + + + + + + + @@ -1730,6 +1720,7 @@ + @@ -1884,6 +1875,7 @@ template_extended_params]]> + offset_param_name])]]> @@ -1897,6 +1889,7 @@ template_extended_params[$container_class])]]> template_extended_params[$base_type->as_type->value])]]> template_extended_params[$base_type->value])]]> + lower_bounds[$atomic_type->offset_param_name])]]> @@ -1929,9 +1922,6 @@ strings]]> strings]]> strings]]> - value_types['string'] instanceof TNonFalsyString - ? $type->value - : $type->value !== '']]> @@ -2278,7 +2268,6 @@ - @@ -2348,6 +2337,11 @@ + + + + + @@ -2357,4 +2351,9 @@ + + + + + From 4b707d1233610f5b81a87cef575c3ce8d4e80aac Mon Sep 17 00:00:00 2001 From: Evan Shaw Date: Mon, 4 Mar 2024 21:39:18 +1300 Subject: [PATCH 3/3] Additional array fetch test case --- tests/TypeReconciliation/EmptyTest.php | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/TypeReconciliation/EmptyTest.php b/tests/TypeReconciliation/EmptyTest.php index d144faa39..bce863e0d 100644 --- a/tests/TypeReconciliation/EmptyTest.php +++ b/tests/TypeReconciliation/EmptyTest.php @@ -771,6 +771,13 @@ class EmptyTest extends TestCase }', 'error_message' => 'RedundantConditionGivenDocblockType', ], + 'redundantEmptyArrayFetch' => [ + 'code' => ' $a */; + assert(isset($a["a"])); + if (empty($a["a"])) {}', + 'error_message' => 'DocblockTypeContradiction', + ], ]; } }