Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Improve in_array assertion to extract assertions for non-literal types #6233

Merged
merged 11 commits into from
Aug 12, 2021
Merged

Improve in_array assertion to extract assertions for non-literal types #6233

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
e278b4f
Add failing test cases
supersmile2009 Aug 3, 2021
097e88a
Improved in_array assertion extraction
supersmile2009 Aug 3, 2021
586e8a1
Fix tests with loose types. Fix return type in the reconciler.
supersmile2009 Aug 3, 2021
c51f0f4
Add tests with needle/haystack value of union types which intersect p…
supersmile2009 Aug 3, 2021
6dd2428
Return Empty type if there's no intersection between existing and ass…
supersmile2009 Aug 3, 2021
3d61f6c
Add more tests
supersmile2009 Aug 5, 2021
9687836
Fix intersection calculation for unions of scalar types
supersmile2009 Aug 5, 2021
65a129e
Fix type reconciliation for complex cases with union types
supersmile2009 Aug 5, 2021
d7f83c3
Fix issues detected by psalm
supersmile2009 Aug 10, 2021
568a9e0
Prevented empty assertions
weirdan Aug 12, 2021
a4b6fbc
Simplified InArrayTest a bit
weirdan Aug 12, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
61 changes: 35 additions & 26 deletions src/Psalm/Internal/Analyzer/Statements/Expression/AssertionFinder.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3429,43 +3429,52 @@ private static function getInarrayAssertions(
|| $atomic_type instanceof Type\Atomic\TKeyedArray
|| $atomic_type instanceof Type\Atomic\TList
) {
$is_sealed = false;
if ($atomic_type instanceof Type\Atomic\TList) {
$value_type = $atomic_type->type_param;
} elseif ($atomic_type instanceof Type\Atomic\TKeyedArray) {
$value_type = $atomic_type->getGenericValueType();
$is_sealed = $atomic_type->sealed;
} else {
$value_type = $atomic_type->type_params[1];
}

$array_literal_types = \array_filter(
$value_type->getAtomicTypes(),
function ($type) {
return $type instanceof Type\Atomic\TLiteralInt
|| $type instanceof Type\Atomic\TLiteralString
|| $type instanceof Type\Atomic\TLiteralFloat
|| $type instanceof Type\Atomic\TEnumCase;
$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-';
supersmile2009 marked this conversation as resolved.
Show resolved Hide resolved
}
);

if ($array_literal_types
&& count($value_type->getAtomicTypes())
) {
$literal_assertions = [];

foreach ($array_literal_types as $array_literal_type) {
$literal_assertions[] = '=' . $array_literal_type->getAssertionString();
}

if ($value_type->isFalsable()) {
$literal_assertions[] = 'false';
}

if ($value_type->isNullable()) {
$literal_assertions[] = 'null';
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();
}

$if_types[$first_var_name] = [$literal_assertions];
$assertions[] = $assertion;
}

$if_types[$first_var_name] = [$assertions];
}
}
}
Expand Down
10 changes: 9 additions & 1 deletion src/Psalm/Internal/Type/SimpleAssertionReconciler.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

use Psalm\CodeLocation;
use Psalm\Codebase;
use Psalm\Exception\TypeParseTreeException;
use Psalm\Internal\Type\Comparator\UnionTypeComparator;
use Psalm\Issue\ParadoxicalCondition;
use Psalm\Issue\RedundantCondition;
Expand Down Expand Up @@ -1513,7 +1514,14 @@ private static function reconcileInArray(
}
}

$existing_var_type->removeType('null');
try {
$new_var_type = Type::parseString($assertion);

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 $existing_var_type;
}
Expand Down
198 changes: 198 additions & 0 deletions tests/TypeReconciliation/InArrayTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,198 @@
<?php
namespace Psalm\Tests\TypeReconciliation;

use const DIRECTORY_SEPARATOR;

class InArrayTest extends \Psalm\Tests\TestCase
{
use \Psalm\Tests\Traits\ValidCodeAnalysisTestTrait;
use \Psalm\Tests\Traits\InvalidCodeAnalysisTestTrait;

/**
* @return iterable<string,array{string,assertions?:array<string,string>,error_levels?:string[]}>
*/
public function providerValidCodeParse(): iterable
{
return [
'nullTypeRemovedAfterNegatedAssertionAgainstArrayOfInt' => [
'<?php
/**
* @param int|null $x
* @return int
*/
function assertInArray($x) {
if (!in_array($x, range(0, 5), true)) {
throw new \Exception();
}

return $x;
}',
],
'nullTypeRemovedAfterAssertionAgainstArrayOfInt' => [
'<?php
/**
* @param int|null $x
* @param non-empty-list<int> $y
* @return int|null
*/
function assertInArray($x, $y) {
if (in_array($x, $y, true)) {
acceptInt($x);
}

return $x;
}
/**
* @param int $x
*/
function acceptInt($x): void {
}',
],
'typeNotChangedAfterAssertionAgainstArrayOfMixed' => [
'<?php
/**
* @param int|null $x
* @param list<mixed> $y
* @return int|null
*/
function assertInArray($x, $y) {
if (!in_array($x, $y, true)) {
throw new \Exception();
}

return $x;
}',
],
'unionTypeReconciledToUnionTypeOfHaystackValueTypes' => [
'<?php
/**
* @param int|string|bool $x
* @param non-empty-list<int|string> $y
* @return int|string|bool
*/
function assertInArray($x, $y) {
if (in_array($x, $y, true)) {
acceptIntAndStr($x);
}

return $x;
}
/**
* @param int|string $x
*/
function acceptIntAndStr($x): void {
}',
],
'unionTypesReducedToIntersectionWithinAssertion' => [
'<?php
/**
* @param int|bool $x
* @param non-empty-list<int|string> $y
* @return int
*/
function assertInArray($x, $y) {
if (in_array($x, $y, true)) {
return $x;
}

throw new Exception();
}',
],
'unionTypesReducedToIntersectionOutsideOfNegatedAssertion' => [
'<?php
/**
* @param int|bool $x
* @param non-empty-list<int|string> $y
* @return int
*/
function assertInArray($x, $y) {
if (!in_array($x, $y, true)) {
throw new Exception();
}
return $x;
}',
],
];
}

/**
* @return iterable<string,array{string,error_message:string,1?:string[],2?:bool,3?:string}>
*/
public function providerInvalidCodeParse(): iterable
{
return [
'typeNotChangedAfterNegatedAssertionAgainstUnsealedArrayOfMixed' => [
'<?php
/**
* @param int|null $x
* @param non-empty-list<mixed> $y
* @return int|null
*/
function assertInArray($x, $y) {
if (!in_array($x, $y, true)) {
acceptInt($x);
}

return $x;
}
/**
* @param int $x
*/
function acceptInt($x): void {
}',
'error_message' => 'PossiblyNullArgument - src' . DIRECTORY_SEPARATOR . 'somefile.php:9:39 - Argument 1 of acceptInt cannot be null, possibly null value provided',
],
'typeNotChangedAfterNegatedAssertionAgainstUnsealedArrayOfUnionType' => [
'<?php
/**
* @param int|null $x
* @param non-empty-list<int|null> $y
* @return int|null
*/
function assertInArray($x, $y) {
if (!in_array($x, $y, true)) {
acceptInt($x);
}

return $x;
}
/**
* @param int $x
*/
function acceptInt($x): void {
}',
'error_message' => 'PossiblyNullArgument - src' . DIRECTORY_SEPARATOR . 'somefile.php:9:39 - Argument 1 of acceptInt cannot be null, possibly null value provided',
],
'initialTypeRemainsOutsideOfAssertion' => [
'<?php
/**
* @param int|bool $x
* @param non-empty-list<int|string> $y
* @return int
*/
function assertInArray($x, $y) {
if (in_array($x, $y, true)) {
throw new Exception();
}
return $x;
}',
'error_message' => 'InvalidReturnStatement - src' . DIRECTORY_SEPARATOR . 'somefile.php:11:32 - The inferred type \'bool|int\' does not match the declared return type \'int\' for assertInArray',
],
'initialTypeRemainsWithinTheNegatedAssertion' => [
'<?php
/**
* @param int|bool $x
* @param non-empty-list<int|string> $y
* @return int
*/
function assertInArray($x, $y) {
if (!in_array($x, $y, true)) {
return $x;
}
throw new Exception();
}',
'error_message' => 'InvalidReturnStatement - src' . DIRECTORY_SEPARATOR . 'somefile.php:9:36 - The inferred type \'bool|int\' does not match the declared return type \'int\' for assertInArray',
],
];
}
}
4 changes: 2 additions & 2 deletions tests/TypeReconciliation/ValueTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -794,7 +794,7 @@ function foo(int $a): void {
'returnFromUnionLiteral' => [
'<?php
/**
* @return list<"a1"|"a2">
* @return array{"a1", "a2"}
*/
function getSupportedConsts() {
return ["a1", "a2"];
Expand All @@ -814,7 +814,7 @@ function foo(mixed $file) : string {
'returnFromUnionLiteralNegated' => [
'<?php
/**
* @return list<"a1"|"a2">
* @return array{"a1", "a2"}
*/
function getSupportedConsts() {
return ["a1", "a2"];
Expand Down