Skip to content

Commit

Permalink
Fix type reconciliation for complex cases with union types
Browse files Browse the repository at this point in the history
  • Loading branch information
@supersmile2009
supersmile2009 committed Aug 5, 2021
1 parent 9687836 commit 02324a1
Show file tree
Hide file tree
Showing 3 changed files with 85 additions and 64 deletions.
54 changes: 24 additions & 30 deletions src/Psalm/Internal/Analyzer/Statements/Expression/AssertionFinder.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
use Psalm\Type;

use function count;
use function implode;
use function in_array;
use function is_int;
use function strpos;
Expand Down Expand Up @@ -3441,40 +3442,33 @@ private static function getInarrayAssertions(

$assertions = [];

foreach ($value_type->getAtomicTypes() as $atomic_value_type) {
$assertion = '';
// If it's not a sealed (fixed, known) array, we can't simply return value types as assertions.
// E. g. in_array($x, ['a', 'b']) is the same as $x === 'a' || $x === 'b',
// which can also be negated correctly.
// However, in_array($x, $y), where y is list<'a'|'b'> doesn't work the same way.
// With positive assertion it has similar meaning: $x is 'a'|'b'.
// But without knowing exact contents of $y, it's not an equality assertion
// threfore it cannot be safely negated.
// If we simply return =string(a) and =string(b) assertions, they will have the same semantics
// as in the first example. So when negated, we will end up with $x !== 'a' && $x !== 'b'.
// That won't work for unknown (not sealed) haystack.
// 'in-array-' prefix is added to distinguish such assertions.
if (!$is_sealed) {
$assertion .= 'in-array-';
if (!$is_sealed) {
if ($value_type->getId() !== '') {
$assertions = ['in-array-' . $value_type->getId()];
}
if ($atomic_value_type instanceof Type\Atomic\TLiteralInt
|| $atomic_value_type instanceof Type\Atomic\TLiteralString
|| $atomic_value_type instanceof Type\Atomic\TLiteralFloat
|| $atomic_value_type instanceof Type\Atomic\TEnumCase
) {
$assertion .= '=' . $atomic_value_type->getAssertionString();
} elseif ($atomic_value_type instanceof Type\Atomic\TFalse
|| $atomic_value_type instanceof Type\Atomic\TTrue
|| $atomic_value_type instanceof Type\Atomic\TNull
) {
$assertion .= $atomic_value_type->getAssertionString();
} else {
$assertion .= $atomic_value_type->getAssertionString();
} else {
$assertions = [];
foreach ($value_type->getAtomicTypes() as $atomic_value_type) {
$assertion = '';
if ($atomic_value_type instanceof Type\Atomic\TLiteralInt
|| $atomic_value_type instanceof Type\Atomic\TLiteralString
|| $atomic_value_type instanceof Type\Atomic\TLiteralFloat
|| $atomic_value_type instanceof Type\Atomic\TEnumCase
) {
$assertion .= '='.$atomic_value_type->getAssertionString();
} elseif ($atomic_value_type instanceof Type\Atomic\TFalse
|| $atomic_value_type instanceof Type\Atomic\TTrue
|| $atomic_value_type instanceof Type\Atomic\TNull
) {
$assertion .= $atomic_value_type->getAssertionString();
}
$assertions[] = $assertion;
}
$assertions[] = $assertion;
}

$if_types[$first_var_name] = [$assertions];
if ($assertions !== []) {
$if_types[$first_var_name] = [$assertions];
}
}
}
}
Expand Down
26 changes: 26 additions & 0 deletions src/Psalm/Internal/Type/NegatedAssertionReconciler.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,32 @@ public static function reconcile(
} elseif ($assertion === 'array-key-exists') {
return Type::getEmpty();
} elseif (substr($assertion, 0, 9) === 'in-array-') {
$assertion = substr($assertion, 9);
$new_var_type = Type::parseString($assertion);

$intersection = Type::intersectUnionTypes(
$new_var_type,
$existing_var_type,
$statements_analyzer->getCodebase()
);

if ($intersection === null) {
if ($key && $code_location) {
self::triggerIssueForImpossible(
$existing_var_type,
$existing_var_type->getId(),
$key,
'!' . $assertion,
true,
$negated,
$code_location,
$suppressed_issues
);
}

$failed_reconciliation = 2;
}

return $existing_var_type;
} elseif (substr($assertion, 0, 14) === 'has-array-key-') {
return $existing_var_type;
Expand Down
69 changes: 35 additions & 34 deletions src/Psalm/Internal/Type/SimpleAssertionReconciler.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,12 @@ public static function reconcile(
return self::reconcileInArray(
$codebase,
$existing_var_type,
substr($assertion, 9)
substr($assertion, 9),
$key,
$negated,
$code_location,
$suppressed_issues,
$failed_reconciliation
);
}

Expand Down Expand Up @@ -1482,48 +1487,44 @@ private static function reconcileIterable(
private static function reconcileInArray(
Codebase $codebase,
Union $existing_var_type,
string $assertion
string $assertion,
?string $key,
bool $negated,
?CodeLocation $code_location,
array $suppressed_issues,
int &$failed_reconciliation
) : Union {
if (strpos($assertion, '::')) {
[$fq_classlike_name, $const_name] = explode('::', $assertion);
try {
$new_var_type = Type::parseString($assertion);
} catch (TypeParseTreeException $e) {
// Not all assertions can be parsed as type, it's fine.
// One particular case is variable array key (e. g. $arr[$key]), which end up as in-array-$arr assertion

$class_constant_type = $codebase->classlikes->getClassConstantType(
$fq_classlike_name,
$const_name,
\ReflectionProperty::IS_PRIVATE
);
return $existing_var_type;
}

if ($class_constant_type) {
foreach ($class_constant_type->getAtomicTypes() as $const_type_atomic) {
if ($const_type_atomic instanceof Type\Atomic\TKeyedArray
|| $const_type_atomic instanceof Type\Atomic\TArray
) {
if ($const_type_atomic instanceof Type\Atomic\TKeyedArray) {
$const_type_atomic = $const_type_atomic->getGenericArrayType();
}
$intersection = Type::intersectUnionTypes($new_var_type, $existing_var_type, $codebase);

if (UnionTypeComparator::isContainedBy(
$codebase,
$const_type_atomic->type_params[0],
$existing_var_type
)) {
return clone $const_type_atomic->type_params[0];
}
}
}
if ($intersection === null) {
if ($key && $code_location) {
self::triggerIssueForImpossible(
$existing_var_type,
$existing_var_type->getId(),
$key,
'!' . $assertion,
true,
$negated,
$code_location,
$suppressed_issues
);
}
}

try {
$new_var_type = Type::parseString($assertion);
$failed_reconciliation = 2;

return Type::intersectUnionTypes($new_var_type, $existing_var_type, $codebase) ?? Type::getEmpty();
} catch (TypeParseTreeException $e) {
// Not all assertions can be parsed as type, it's fine.
// One particular case is variable array key (E. g. $arr[$key])
return Type::getMixed();
}

return $existing_var_type;
return $intersection;
}

private static function reconcileHasArrayKey(
Expand Down

0 comments on commit 02324a1

Please sign in to comment.