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

Broken assert for array and list - DocblockTypeContradiction #9037

Closed
kkmuffme opened this issue Jan 1, 2023 · 9 comments · Fixed by #9853
Closed

Broken assert for array and list - DocblockTypeContradiction #9037

kkmuffme opened this issue Jan 1, 2023 · 9 comments · Fixed by #9853

Comments

@kkmuffme
Copy link
Contributor

kkmuffme commented Jan 1, 2023

https://psalm.dev/r/ea9ecf75b9

As we see in line 20, psalm correctly infers that $foo can be type list when checking with array_values

When using a custom assert function, it correctly asserts it, but reports a false positive DocblockTypeContradiction on the if condition in line 23

What actually happens here is, that it assumes that @psalm-assert type array<string, string> does not contain list<string> which is correct - however why does it report this as a DocblockTypeContradiction in the place where the function is called?

How can I assert if true => list<string>, if false => array<string, string>?

@psalm-github-bot
Copy link

I found these snippets:

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

/**
 *
 * @param array<string, string>|list<string> $arg
 * @return bool
 *
 * @psalm-assert array<string, string> $arg
 * @psalm-assert-if-true =list<string> $arg
 */
function is_array_or_list( $arg ) {}

/**
 * @var array<string, string>|list<string> $foo
 */
$bar = $foo;
/** @psalm-trace $bar */;

if ( $foo === array_values( $foo ) ) {
   /** @psalm-trace $foo */;
}

if ( is_array_or_list( $foo ) === true ) {
    /** @psalm-trace $foo */;
} else {
    /** @psalm-trace $foo */;
}
Psalm output (using commit ef1264b):

ERROR: InvalidReturnType - 6:12 - No return statements were found for method is_array_or_list but return type 'bool' was expected

INFO: Trace - 17:25 - $bar: array<int<0, max>|string, string>

INFO: Trace - 20:28 - $foo: list<string>

ERROR: DocblockTypeContradiction - 23:6 - Cannot resolve types for $foo - docblock-defined type array<string, string> does not contain list<string>

INFO: Trace - 24:29 - $foo: list<string>

INFO: Trace - 26:29 - $foo: array<string, string>

@orklah
Copy link
Collaborator

orklah commented Jan 1, 2023

Current situation seems correct @psalm-assert means that if the function doesn't throw, you have the type array<string, string> but you also say that if the function returns true, you have a list<string>. Those two are contradiction and assertions are considered docblocks so there's a fail.

In theory, this should work: https://psalm.dev/r/06c99cf8ac but it doesn't

@psalm-github-bot
Copy link

I found these snippets:

https://psalm.dev/r/06c99cf8ac
<?php

/**
 *
 * @param array<string, string>|list<string> $arg
 * @return bool
 *
 * @psalm-assert-if-false =array<string, string> $arg
 * @psalm-assert-if-true =list<string> $arg
 */
function is_array_or_list( $arg ) {}

/**
 * @var array<string, string>|list<string> $foo
 */
$bar = $foo;
/** @psalm-trace $bar */;

if ( $foo === array_values( $foo ) ) {
   /** @psalm-trace $foo */;
}

if ( is_array_or_list( $foo ) === true ) {
    /** @psalm-trace $foo */;
} else {
    /** @psalm-trace $foo */;
}
Psalm output (using commit e81823e):

ERROR: InvalidReturnType - 6:12 - No return statements were found for method is_array_or_list but return type 'bool' was expected

INFO: Trace - 17:25 - $bar: array<int<0, max>|string, string>

INFO: Trace - 20:28 - $foo: list<string>

INFO: Trace - 24:29 - $foo: array<int<0, max>|string, string>

INFO: Trace - 26:29 - $foo: array<int<0, max>|string, string>

@kkmuffme
Copy link
Contributor Author

kkmuffme commented Jan 1, 2023

Current situation seems correct @psalm-assert means that if the function doesn't throw, you have the type array<string, string>

Which is fine, the issue is that it should report this for the assertion, not at every place this function is called?

Afaik this was working fine in v4, this is a new issue in v5

In theory, this should work: https://psalm.dev/r/06c99cf8ac but it doesn't

Yes @psalm-assert-if-false and @psalm-assert-if-true should work when used together.

@psalm-github-bot
Copy link

I found these snippets:

https://psalm.dev/r/06c99cf8ac
<?php

/**
 *
 * @param array<string, string>|list<string> $arg
 * @return bool
 *
 * @psalm-assert-if-false =array<string, string> $arg
 * @psalm-assert-if-true =list<string> $arg
 */
function is_array_or_list( $arg ) {}

/**
 * @var array<string, string>|list<string> $foo
 */
$bar = $foo;
/** @psalm-trace $bar */;

if ( $foo === array_values( $foo ) ) {
   /** @psalm-trace $foo */;
}

if ( is_array_or_list( $foo ) === true ) {
    /** @psalm-trace $foo */;
} else {
    /** @psalm-trace $foo */;
}
Psalm output (using commit e81823e):

ERROR: InvalidReturnType - 6:12 - No return statements were found for method is_array_or_list but return type 'bool' was expected

INFO: Trace - 17:25 - $bar: array<int<0, max>|string, string>

INFO: Trace - 20:28 - $foo: list<string>

INFO: Trace - 24:29 - $foo: array<int<0, max>|string, string>

INFO: Trace - 26:29 - $foo: array<int<0, max>|string, string>

@orklah
Copy link
Collaborator

orklah commented Jan 1, 2023

As far as I know, contradiction in assertions were always reported where the contradiction is processed, on the method call.

@kkmuffme
Copy link
Contributor Author

kkmuffme commented Jan 1, 2023

I didn't have this issue in v4 though (this same code passed in v4 without any errors, as I am just revisiting a file that was last modified in September and passed without errors back then)

@kkmuffme
Copy link
Contributor Author

kkmuffme commented Jan 5, 2023

Works fine for any types e.g. int|list https://psalm.dev/r/4733a4c121 except for array|list since they are combined into 1 type - which is where the use of @psalm-assert-if-false combined with @psalm-assert-if-true would come in handy

@psalm-github-bot
Copy link

I found these snippets:

https://psalm.dev/r/4733a4c121
<?php

/**
 *
 * @param int|list<string> $arg
 * @return bool
 *
 * @psalm-assert-if-true list<string> $arg
 */
function is_array_or_int( $arg ) {}

/**
 * @var int|list<string> $foo
 */
$bar = $foo;

if ( is_array_or_int( $foo ) ) {
   /** @psalm-trace $foo */;
} else {
    /** @psalm-trace $foo */;
}
Psalm output (using commit 7d6a48a):

ERROR: InvalidReturnType - 6:12 - No return statements were found for method is_array_or_int but return type 'bool' was expected

INFO: Trace - 18:28 - $foo: list<string>

INFO: Trace - 20:29 - $foo: int

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