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.

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

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

Merged
merged 11 commits into from Aug 12, 2021

Conversation

supersmile2009
Copy link
Contributor

Fixes #6220

Thanks @orklah for the tip on where to check.

Extract non-literal type assertion from in_array

Extract false, true and null assertion

Fix assertion against arrays with union-typed values

Fix assertion against non-sealed arrays
@supersmile2009
Copy link
Contributor Author

The last commit fixes 2 old tests, which used to pass before because AssertionFinder::getInarrayAssertions() wasn't treating lists of literals correctly. Since list<"a"|"b"> doesn't guarantee that either of those values is actually present in the array, the assertions extracted by getInarrayAssertions() were not safe. That's what the large comment block added to AssertionFinder::getInarrayAssertions() is about.

By looking at those tests I believe that original intention was to check against a known and fixed list of constants, which is array{"a", "b"}. In fact those tests would fail even without this patch if restrict_return_types was enabled.

So I wouldn't call it a regression, it's a bug fix rather.

@orklah
Copy link
Collaborator

orklah commented Aug 3, 2021

Indeed, list<"a"|"b"> doesn't guarantee the presence of one or the other, and the fact that non-empty-list<"a"|"b"> wasn't used, it didn't even guaranteed the array contained anything at all.

@orklah
Copy link
Collaborator

orklah commented Aug 3, 2021

Seems awesome! Thanks :)

@supersmile2009
Copy link
Contributor Author

  1. The change to Psalm\Type::intersectUnionTypes().
    It turned out that it couldn't intersect some scalar type unions properly. (int|string)&(int|bool) returned null (like no intersection at all, not null type) and not int as expected. Now it's fixed.
  2. in-array- assertion now uses slightly different syntax. Type key is simply concatenated with the prefix. The first approach that I took with separate assertion for each type couldn't work. You need to have all the types in a single context to do the intersection. Otherwise if you keep returning Empty type after separate assertion reconciliation, you get wrong errors in some cases. With the new implementation Reconcilers can intersect the types and produce correct errors, like if condition is redundant or if assertion makes no sense, because types are completely different.

@orklah
Copy link
Collaborator

orklah commented Aug 9, 2021

@supersmile2009 seems like something broke in Psalm. Can you check it?

@supersmile2009
Copy link
Contributor Author

@orklah done. Could you trigger the CI again?

@weirdan weirdan merged commit 7ff2a66 into vimeo:master Aug 12, 2021
@weirdan
Copy link
Collaborator

weirdan commented Aug 12, 2021

Thanks!

@orklah
Copy link
Collaborator

orklah commented Aug 16, 2021

This triggered a new false positive in my work codebase: https://psalm.dev/r/aaa65f492c

@psalm-github-bot
Copy link

I found these snippets:

https://psalm.dev/r/aaa65f492c
<?php

class A{
 	public static array $array1 = ['a', 'b'];
 	public static array $array2 = ['c', 'd'];
}

$element = 'element2';
if(in_array($element, A::$array1, true)) {
    $_a = 1;
} elseif(in_array($element, A::$array2, true)) {
    $_a = 2;
}
Psalm output (using commit 2e7763c):

ERROR: ParadoxicalCondition - 11:10 - Condition ($element is in-array-mixed) contradicts a previously-established condition ($element is not in-array-mixed)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

in_array assertion with dynamic haystack doesn't reconcile needle type to array's value type
3 participants