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

object types with optional properties don't get their inferred type changed after isset() check #5077

Open
ShadauxCat opened this issue Jan 21, 2021 · 5 comments
Labels

Comments

@ShadauxCat
Copy link

<?php
/**
 * @param object{
 *    bar?: object{
 *      baz?: string
 *   }
 * }|null $foo
 * @return void
 */
function testSparce($foo)
{
    // This should change the inferred type, no?
    if (isset($foo->bar->baz))
    {
        testConcrete($foo);
    }
}
/**
 * @param object{
 *    bar: object{
 *      baz: string
 *   }
 * } $foo
 * @return void
 */
function testConcrete($foo)
{
    echo "" . $foo->bar->baz;
}

After that isset() check, I would expect psalm to change its interpretation of the type of $foo to know that all the optional parameters are set. The call to testConcrete($foo) should work here. Instead, it flags this error:

ERROR: ArgumentTypeCoercion - 15:22 - Argument 1 of testConcrete expects object{bar:object{baz:string}}, parent type object{bar?:object{baz?:string}} provided

https://psalm.dev/r/9560e1b060

@psalm-github-bot
Copy link

I found these snippets:

https://psalm.dev/r/9560e1b060
<?php
/**
 * @param object{
 *    bar?: object{
 *      baz?: string
 *   }
 * }|null $foo
 * @return void
 */
function testSparce($foo)
{
    // This should change the inferred type, no?
	if (isset($foo->bar->baz))
    {
        testConcrete($foo);
	}
}
/**
 * @param object{
 *    bar: object{
 *      baz: string
 *   }
 * } $foo
 * @return void
 */
function testConcrete($foo)
{
	echo "" . $foo->bar->baz;
}
Psalm output (using commit c912b6c):

ERROR: ArgumentTypeCoercion - 15:22 - Argument 1 of testConcrete expects object{bar:object{baz:string}}, parent type object{bar?:object{baz?:string}} provided

@weirdan weirdan added the bug label Jan 22, 2021
@M1ke
Copy link
Contributor

M1ke commented Jan 21, 2022

I think this is the same issue I just reproduced here:

https://psalm.dev/r/c90854d4fc

@psalm-github-bot
Copy link

I found these snippets:

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

/**
 * @param array{a?: array{foo: bool, bar: bool}} $arr
 */
function foo(array $arr):void {
   $a = $arr['a'] ?? [];
    if ($a){
        bar($a);
    }
}

/**
 * @param array{foo: bool, bar: bool} $arr
 */
function bar(array $arr):void{
}
Psalm output (using commit f6369dc):

ERROR: ArgumentTypeCoercion - 9:13 - Argument 1 of bar expects array{bar: bool, foo: bool}, parent type array{bar?: bool, foo?: bool} provided

INFO: UnusedParam - 16:20 - Param $arr is never referenced in this method

@M1ke
Copy link
Contributor

M1ke commented Jan 21, 2022

I think it's because the type-checker converts our optional array into an array where each key is optional - thus even when the original key is determined to exist, the types of it's content have changed to an array of optional keys.

I presume the fix therefore is to resolve that an array with non-optional keys that is a type of an optional key in a parent array doesn't have that behaviour implied.

@orklah
Copy link
Collaborator

orklah commented Jan 21, 2022

This issue is very related to #2206

@M1ke your case is different though. Psalm is not fine enough to understand that you basically have array{foo: bool, bar: bool}|array<never, never>. It simplifies that into array{foo?: bool, bar?: bool} which means it transforms either foo AND bar are defined or we have an empty array into foo and bar can both be possibly_undefined. The second version is not enough to infer anything when we go through if($a)

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

No branches or pull requests

4 participants