1
0
mirror of https://github.com/danog/psalm.git synced 2024-11-26 20:34:47 +01:00

Fix #4414 - allow multiple @psalm-assert-if-true on same var

This commit is contained in:
Matt Brown 2020-10-25 10:49:39 -04:00 committed by Daniil Gentili
parent 7703a18bdd
commit 5aee8e77d5
Signed by: danog
GPG Key ID: 8C1BE3B34B230CA7
7 changed files with 169 additions and 97 deletions

View File

@ -37,7 +37,7 @@ class AssertionFinder
/**
* Gets all the type assertions in a conditional
*
* @return array<string, non-empty-list<non-empty-list<string>>>
* @return list<non-empty-array<string, non-empty-list<non-empty-list<string>>>>
*/
public static function scrapeAssertions(
PhpParser\Node\Expr $conditional,
@ -123,7 +123,7 @@ class AssertionFinder
}
}
return $if_types;
return $if_types ? [$if_types] : [];
}
if ($conditional instanceof PhpParser\Node\Expr\Assign) {
@ -147,13 +147,13 @@ class AssertionFinder
if ($var_name) {
if ($candidate_if_types) {
$if_types[$var_name] = [['>' . \json_encode($candidate_if_types)]];
$if_types[$var_name] = [['>' . \json_encode($candidate_if_types[0])]];
} else {
$if_types[$var_name] = [['!falsy']];
}
}
return $if_types;
return $if_types ? [$if_types] : [];
}
$var_name = ExpressionIdentifier::getArrayVarId(
@ -168,7 +168,7 @@ class AssertionFinder
if (!$conditional instanceof PhpParser\Node\Expr\MethodCall
&& !$conditional instanceof PhpParser\Node\Expr\StaticCall
) {
return $if_types;
return [$if_types];
}
}
@ -199,15 +199,21 @@ class AssertionFinder
return [];
}
$expr_assertions = $expr_assertions[0];
if (count($expr_assertions) !== 1) {
return [];
}
$if_types = \Psalm\Type\Algebra::negateTypes($expr_assertions);
return $if_types;
return $if_types ? [$if_types] : [];
}
if ($conditional instanceof PhpParser\Node\Expr\BinaryOp\Identical ||
$conditional instanceof PhpParser\Node\Expr\BinaryOp\Equal
) {
$if_types = self::scrapeEqualityAssertions(
$and_types = self::scrapeEqualityAssertions(
$conditional,
$this_class_name,
$source,
@ -217,13 +223,13 @@ class AssertionFinder
$inside_conditional
);
return $if_types;
return $and_types;
}
if ($conditional instanceof PhpParser\Node\Expr\BinaryOp\NotIdentical ||
$conditional instanceof PhpParser\Node\Expr\BinaryOp\NotEqual
) {
$if_types = self::scrapeInequalityAssertions(
$and_types = self::scrapeInequalityAssertions(
$conditional,
$this_class_name,
$source,
@ -233,7 +239,7 @@ class AssertionFinder
$inside_conditional
);
return $if_types;
return $and_types;
}
if ($conditional instanceof PhpParser\Node\Expr\BinaryOp\Greater
@ -272,7 +278,7 @@ class AssertionFinder
}
}
return $if_types;
return $if_types ? [$if_types] : [];
}
if ($count_inequality_position) {
@ -297,7 +303,7 @@ class AssertionFinder
}
}
return $if_types;
return $if_types ? [$if_types] : [];
}
if ($positive_number_position) {
@ -353,7 +359,7 @@ class AssertionFinder
$if_types[$var_name] = [[($min_comparison === 1 ? '' : '=') . 'positive-numeric']];
}
return $if_types;
return $if_types ? [$if_types] : [];
}
return [];
@ -391,7 +397,7 @@ class AssertionFinder
}
}
return $if_types;
return $if_types ? [$if_types] : [];
}
if ($count_inequality_position) {
@ -416,7 +422,7 @@ class AssertionFinder
}
}
return $if_types;
return $if_types ? [$if_types] : [];
}
if ($typed_value_position) {
@ -452,16 +458,16 @@ class AssertionFinder
$if_types[$var_name] = [['=isset']];
}
return $if_types;
return $if_types ? [$if_types] : [];
}
return [];
}
if ($conditional instanceof PhpParser\Node\Expr\FuncCall) {
$if_types = self::processFunctionCall($conditional, $this_class_name, $source, $codebase, false);
$and_types = self::processFunctionCall($conditional, $this_class_name, $source, $codebase, false);
return $if_types;
return $and_types;
}
if ($conditional instanceof PhpParser\Node\Expr\MethodCall
@ -473,7 +479,7 @@ class AssertionFinder
return $custom_assertions;
}
return $if_types;
return $if_types ? [$if_types] : [];
}
if ($conditional instanceof PhpParser\Node\Expr\Empty_) {
@ -496,7 +502,7 @@ class AssertionFinder
}
}
return $if_types;
return $if_types ? [$if_types] : [];
}
if ($conditional instanceof PhpParser\Node\Expr\Isset_) {
@ -529,7 +535,7 @@ class AssertionFinder
}
}
return $if_types;
return $if_types ? [$if_types] : [];
}
if ($conditional instanceof PhpParser\Node\Expr\BinaryOp\Coalesce) {
@ -557,7 +563,7 @@ class AssertionFinder
/**
* @param PhpParser\Node\Expr\BinaryOp\Identical|PhpParser\Node\Expr\BinaryOp\Equal $conditional
*
* @return array<string, non-empty-list<non-empty-list<string>>>
* @return list<non-empty-array<string, non-empty-list<non-empty-list<string>>>>
*/
private static function scrapeEqualityAssertions(
PhpParser\Node\Expr\BinaryOp $conditional,
@ -648,7 +654,7 @@ class AssertionFinder
}
}
return $if_types;
return $if_types ? [$if_types] : [];
}
if ($true_position) {
@ -681,6 +687,8 @@ class AssertionFinder
} else {
$if_types[$var_name] = [['!falsy']];
}
$if_types = [$if_types];
} else {
$base_assertions = null;
@ -822,12 +830,18 @@ class AssertionFinder
$notif_types = $base_assertions;
if (count($notif_types) === 1) {
$notif_types = $notif_types[0];
if (count($notif_types) === 1) {
$if_types = \Psalm\Type\Algebra::negateTypes($notif_types);
}
}
}
$if_types = $if_types ? [$if_types] : [];
}
if ($codebase
&& $source instanceof StatementsAnalyzer
&& ($var_type = $source->node_data->getType($base_conditional))
@ -931,7 +945,7 @@ class AssertionFinder
}
}
return $if_types;
return $if_types ? [$if_types] : [];
}
if ($gettype_position) {
@ -970,7 +984,7 @@ class AssertionFinder
}
}
return $if_types;
return $if_types ? [$if_types] : [];
}
if ($get_debug_type_position) {
@ -1016,7 +1030,7 @@ class AssertionFinder
}
}
return $if_types;
return $if_types ? [$if_types] : [];
}
if ($count_equality_position) {
@ -1043,7 +1057,7 @@ class AssertionFinder
}
}
return $if_types;
return $if_types ? [$if_types] : [];
}
if (!$source instanceof StatementsAnalyzer) {
@ -1098,7 +1112,7 @@ class AssertionFinder
true
) === false
) {
return $if_types;
return [];
}
}
@ -1117,7 +1131,7 @@ class AssertionFinder
}
}
return $if_types;
return $if_types ? [$if_types] : [];
}
$typed_value_position = self::hasTypedValueComparison($conditional, $source);
@ -1209,7 +1223,7 @@ class AssertionFinder
}
}
return $if_types;
return $if_types ? [$if_types] : [];
}
$var_type = $source->node_data->getType($conditional->left);
@ -1240,7 +1254,7 @@ class AssertionFinder
/**
* @param PhpParser\Node\Expr\BinaryOp\NotIdentical|PhpParser\Node\Expr\BinaryOp\NotEqual $conditional
*
* @return array<string, non-empty-list<non-empty-list<string>>>
* @return list<non-empty-array<string, non-empty-list<non-empty-list<string>>>>
*/
private static function scrapeInequalityAssertions(
PhpParser\Node\Expr\BinaryOp $conditional,
@ -1332,7 +1346,7 @@ class AssertionFinder
}
}
return $if_types;
return $if_types ? [$if_types] : [];
}
if ($false_position) {
@ -1381,10 +1395,14 @@ class AssertionFinder
$notif_types = $base_assertions;
if (count($notif_types) === 1) {
$notif_types = $notif_types[0];
if (count($notif_types) === 1) {
$if_types = \Psalm\Type\Algebra::negateTypes($notif_types);
}
}
}
if ($codebase
&& $source instanceof StatementsAnalyzer
@ -1429,7 +1447,7 @@ class AssertionFinder
}
}
return $if_types;
return $if_types ? [$if_types] : [];
}
if ($true_position) {
@ -1487,12 +1505,18 @@ class AssertionFinder
$notif_types = $base_assertions;
if (count($notif_types) === 1) {
$notif_types = $notif_types[0];
if (count($notif_types) === 1) {
$if_types = \Psalm\Type\Algebra::negateTypes($notif_types);
}
}
}
$if_types = $if_types ? [$if_types] : [];
}
if ($codebase
&& $source instanceof StatementsAnalyzer
&& ($var_type = $source->node_data->getType($base_conditional))
@ -1563,7 +1587,7 @@ class AssertionFinder
}
}
return $if_types;
return $if_types ? [$if_types] : [];
}
if ($empty_array_position !== null) {
@ -1632,7 +1656,7 @@ class AssertionFinder
}
}
return $if_types;
return $if_types ? [$if_types] : [];
}
if ($gettype_position) {
@ -1681,7 +1705,7 @@ class AssertionFinder
}
}
return $if_types;
return $if_types ? [$if_types] : [];
}
if ($get_debug_type_position) {
@ -1727,7 +1751,7 @@ class AssertionFinder
}
}
return $if_types;
return $if_types ? [$if_types] : [];
}
if (!$source instanceof StatementsAnalyzer) {
@ -1784,7 +1808,7 @@ class AssertionFinder
}
}
return $if_types;
return $if_types ? [$if_types] : [];
}
if ($var_type
@ -1805,7 +1829,7 @@ class AssertionFinder
}
}
return $if_types;
return $if_types ? [$if_types] : [];
}
if ($typed_value_position) {
@ -1893,14 +1917,14 @@ class AssertionFinder
}
}
return $if_types;
return $if_types ? [$if_types] : [];
}
return [];
}
/**
* @return array<string, non-empty-list<non-empty-list<string>>>
* @return list<non-empty-array<string, non-empty-list<non-empty-list<string>>>>
*/
public static function processFunctionCall(
PhpParser\Node\Expr\FuncCall $expr,
@ -1948,7 +1972,7 @@ class AssertionFinder
if ($third_arg instanceof PhpParser\Node\Expr\ConstFetch) {
if (!in_array(strtolower($third_arg->name->parts[0]), ['true', 'false'])) {
return $if_types;
return [];
}
$third_arg_value = strtolower($third_arg->name->parts[0]);
@ -2321,10 +2345,10 @@ class AssertionFinder
$if_types[$first_var_name] = [[$prefix . 'non-empty-countable']];
}
} else {
$if_types = self::processCustomAssertion($expr, $this_class_name, $source, $negate);
return self::processCustomAssertion($expr, $this_class_name, $source, $negate);
}
return $if_types;
return $if_types ? [$if_types] : [];
}
private static function processIrreconcilableFunctionCall(
@ -2401,7 +2425,7 @@ class AssertionFinder
/**
* @param PhpParser\Node\Expr\FuncCall|PhpParser\Node\Expr\MethodCall|PhpParser\Node\Expr\StaticCall $expr
*
* @return array<string, non-empty-list<non-empty-list<string>>>
* @return list<non-empty-array<string, non-empty-list<non-empty-list<string>>>>
*/
protected static function processCustomAssertion(
PhpParser\Node\Expr $expr,
@ -2430,10 +2454,12 @@ class AssertionFinder
)
: null;
$if_types = [];
$anded_types = [];
if ($if_true_assertions) {
foreach ($if_true_assertions as $assertion) {
$if_types = [];
$assertion = clone $assertion;
foreach ($assertion->rule as $i => $and_rules) {
@ -2493,6 +2519,10 @@ class AssertionFinder
$if_types[$assertion->var_id] = [[$prefix . $assertion->rule[0][0]]];
}
}
if ($if_types) {
$anded_types[] = $if_types;
}
}
}
@ -2500,6 +2530,8 @@ class AssertionFinder
$negated_prefix = !$negate ? '!' : '';
foreach ($if_false_assertions as $assertion) {
$if_types = [];
$assertion = clone $assertion;
foreach ($assertion->rule as $i => $and_rules) {
@ -2559,10 +2591,14 @@ class AssertionFinder
$if_types[$assertion->var_id] = [[$negated_prefix . $assertion->rule[0][0]]];
}
}
if ($if_types) {
$anded_types[] = $if_types;
}
}
}
return $if_types;
return $anded_types;
}
/**

View File

@ -1878,9 +1878,9 @@ class FunctionCallAnalyzer extends CallAnalyzer
$stmt_assertions = $statements_analyzer->node_data->getAssertions($stmt);
if ($stmt_assertions !== null) {
$assertions = $stmt_assertions;
$anded_assertions = $stmt_assertions;
} else {
$assertions = AssertionFinder::processFunctionCall(
$anded_assertions = AssertionFinder::processFunctionCall(
$stmt,
$context->self,
$statements_analyzer,
@ -1891,6 +1891,7 @@ class FunctionCallAnalyzer extends CallAnalyzer
$changed_vars = [];
foreach ($anded_assertions as $assertions) {
$referenced_var_ids = array_map(
function (array $_) : bool {
return true;
@ -1898,7 +1899,6 @@ class FunctionCallAnalyzer extends CallAnalyzer
$assertions
);
if ($assertions) {
Reconciler::reconcileKeyedTypes(
$assertions,
$assertions,

View File

@ -11,7 +11,12 @@ class NodeDataProvider implements \Psalm\NodeTypeProvider
/** @var SplObjectStorage<PhpParser\Node, Union> */
private $node_types;
/** @var SplObjectStorage<PhpParser\Node, array<string, non-empty-list<non-empty-list<string>>>|null> */
/**
* @var SplObjectStorage<
* PhpParser\Node,
* list<non-empty-array<string, non-empty-list<non-empty-list<string>>>>|null
* >
*/
private $node_assertions;
/** @var SplObjectStorage<PhpParser\Node, array<int, \Psalm\Storage\Assertion>> */
@ -52,7 +57,7 @@ class NodeDataProvider implements \Psalm\NodeTypeProvider
}
/**
* @param array<string, non-empty-list<non-empty-list<string>>>|null $assertions
* @param list<non-empty-array<string, non-empty-list<non-empty-list<string>>>>|null $assertions
*/
public function setAssertions(PhpParser\Node\Expr $node, ?array $assertions) : void
{
@ -64,7 +69,7 @@ class NodeDataProvider implements \Psalm\NodeTypeProvider
}
/**
* @return array<string, non-empty-list<non-empty-list<string>>>|null
* @return list<non-empty-array<string, non-empty-list<non-empty-list<string>>>>|null
*/
public function getAssertions(PhpParser\Node\Expr $node) : ?array
{

View File

@ -240,13 +240,15 @@ class ArrayFilterReturnTypeProvider implements \Psalm\Plugin\Hook\FunctionReturn
) {
$codebase = $statements_source->getCodebase();
$assertions = AssertionFinder::scrapeAssertions(
$anded_assertions = AssertionFinder::scrapeAssertions(
$stmt->expr,
null,
$statements_source,
$codebase
);
$assertions = $anded_assertions[0] ?? [];
if (isset($assertions['$' . $first_param->var->name])) {
$changed_var_ids = [];

View File

@ -281,12 +281,14 @@ class ArrayMapReturnTypeProvider implements \Psalm\Plugin\Hook\FunctionReturnTyp
$codebase = $statements_analyzer->getCodebase();
if ($assertions !== null) {
$assertions = AssertionFinder::scrapeAssertions(
$anded_assertions = AssertionFinder::scrapeAssertions(
$fake_call,
null,
$statements_analyzer,
$codebase
);
$assertions = $anded_assertions[0] ?? [];
}
$context->inside_call = $was_inside_call;

View File

@ -178,14 +178,14 @@ class Algebra
if ($conditional->expr instanceof PhpParser\Node\Expr\Isset_
&& count($conditional->expr->vars) > 1
) {
$assertions = null;
$anded_assertions = null;
if ($cache && $source instanceof \Psalm\Internal\Analyzer\StatementsAnalyzer) {
$assertions = $source->node_data->getAssertions($conditional->expr);
$anded_assertions = $source->node_data->getAssertions($conditional->expr);
}
if ($assertions === null) {
$assertions = AssertionFinder::scrapeAssertions(
if ($anded_assertions === null) {
$anded_assertions = AssertionFinder::scrapeAssertions(
$conditional->expr,
$this_class_name,
$source,
@ -195,12 +195,13 @@ class Algebra
);
if ($cache && $source instanceof \Psalm\Internal\Analyzer\StatementsAnalyzer) {
$source->node_data->setAssertions($conditional->expr, $assertions);
$source->node_data->setAssertions($conditional->expr, $anded_assertions);
}
}
$clauses = [];
foreach ($anded_assertions as $assertions) {
foreach ($assertions as $var => $anded_types) {
$redefined = false;
@ -226,6 +227,7 @@ class Algebra
);
}
}
}
return self::negateFormula($clauses);
}
@ -356,14 +358,14 @@ class Algebra
);
}
$assertions = null;
$anded_assertions = null;
if ($cache && $source instanceof \Psalm\Internal\Analyzer\StatementsAnalyzer) {
$assertions = $source->node_data->getAssertions($conditional);
$anded_assertions = $source->node_data->getAssertions($conditional);
}
if ($assertions === null) {
$assertions = AssertionFinder::scrapeAssertions(
if ($anded_assertions === null) {
$anded_assertions = AssertionFinder::scrapeAssertions(
$conditional,
$this_class_name,
$source,
@ -373,13 +375,13 @@ class Algebra
);
if ($cache && $source instanceof \Psalm\Internal\Analyzer\StatementsAnalyzer) {
$source->node_data->setAssertions($conditional, $assertions);
$source->node_data->setAssertions($conditional, $anded_assertions);
}
}
if ($assertions) {
$clauses = [];
foreach ($anded_assertions as $assertions) {
foreach ($assertions as $var => $anded_types) {
$redefined = false;
@ -405,7 +407,9 @@ class Algebra
);
}
}
}
if ($clauses) {
return $clauses;
}

View File

@ -1266,6 +1266,29 @@ class AssertAnnotationTest extends TestCase
}
}'
],
'multipleAssertIfTrueOnSameVariable' => [
'<?php
class A {}
function foo(string|null|A $a) : A {
if (isComputed($a)) {
return $a;
}
throw new Exception("bad");
}
/**
* @psalm-assert-if-true !null $value
* @psalm-assert-if-true !string $value
*/
function isComputed(mixed $value): bool {
return $value !== null && !is_string($value);
}',
[],
[],
'8.0'
],
];
}