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

Demo: Add conditional return type for $args array of get_posts - Can we? #134

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

IanDelMar
Copy link
Contributor

(PR for demonstration)

https://phpstan.org/r/5f05dffa-73cd-4c46-95f5-7b65702264a0

<?php declare(strict_types = 1);

class WP_Post {}

/** 
 * @param array<string, mixed> $args 
 * @phpstan-return ($args is array{'fields': 'id=>parent'}|array{'fields': 'ids'} ? array<int, int> : array<int, WP_Post>)
 */
function getPostsExample(?array $args = null): array
{
	if (isset($args['fields'])) {
		if ($args['fields'] === 'id=>parent' || $args['fields'] === 'ids') {
			return [1];
		}
	}
	return [new WP_Post()];
}

\PHPStan\dumpType(getPostsExample()); // expected: array<int, WP_Post>
\PHPStan\dumpType(getPostsExample([])); // expected: array<int, WP_Post>
\PHPStan\dumpType(getPostsExample(['some'=>'thing'])); // expected: array<int, WP_Post>
\PHPStan\dumpType(getPostsExample(['fields'=>'ofGold'])); // expected: array<int, WP_Post>
\PHPStan\dumpType(getPostsExample(['fields'=>'ids'])); // expected: array<int, int>
\PHPStan\dumpType(getPostsExample(['fields'=>'id=>parent'])); // expected: array<int, int>
\PHPStan\dumpType(getPostsExample(['some'=>'thing', 'fields'=>'ofGold'])); // expected: array<int, WP_Post>
\PHPStan\dumpType(getPostsExample(['some'=>'thing', 'fields'=>'ids'])); // expected: array<int, int>
\PHPStan\dumpType(getPostsExample(['some'=>'thing', 'fields'=>'id=>parent'])); // expected: array<int, int>
\PHPStan\dumpType(getPostsExample(['fields'=>'ofGold','some'=>'thing'])); // expected: array<int, WP_Post>
\PHPStan\dumpType(getPostsExample(['fields'=>'ids','some'=>'thing'])); // expected: array<int, int>
\PHPStan\dumpType(getPostsExample(['fields'=>'id=>parent','some'=>'thing'])); // expected: array<int, int>
\PHPStan\dumpType(getPostsExample(rand(0,1) === 0 ? ['fields'=>'id=>parent'] : ['some'=>'thing'])); // expected: array<int, int|WP_Post>

The actual value always matches the expected value in all of the dumps. Also phpunit passes all tests that I copy pasted from szepeviktor/phpstan-wordpress

@szepeviktor
Copy link
Member

szepeviktor commented Nov 9, 2023

Why do people use get_posts? Is not WP_Query much better?

Now I feel that I support/encourage people to use this get_posts 🤮

@szepeviktor
Copy link
Member

@johnbillion Please comment on this PR.

@IanDelMar
Copy link
Contributor Author

IanDelMar commented Nov 9, 2023

Now I feel that I support/encourage people to use this

There's a dynamic return type extension. That's why used it as an example. I thought we can't do dynamic return types via the function map when it comes to array keys and their values. However, it appears I was wrong.

@IanDelMar
Copy link
Contributor Author

I have to check again, but I think it does not work with this void unions in core. (szepeviktor/phpstan-wordpress#176)

@IanDelMar
Copy link
Contributor Author

Ok, there's a failed test now. I don't know why this didn't happen on my first run of phpunit. However, the test expectation is wrong.

// The assertion:
$union = $_GET['foo'] ? (string)$_GET['string'] : 'fields';
assertType('array<int, int|WP_Post>', get_posts([$union => '']));

This is a string-constantString union for the key. The value is an empty string and therefor the return type is array<int, WP_Post>
See: https://github.com/WordPress/wordpress-develop/blob/f413a31ce9ab785b21d7f9f13378f535c325673e/src/wp-includes/class-wp-query.php#L2033-L2042

@IanDelMar
Copy link
Contributor Author

IanDelMar commented Nov 9, 2023

No matter what I tried, PHPStan would not accept the conditional return type for the_title_attribute. It always returns string|void for any conditional return type; even for something like

'the_title_attribute' => ["(\$args is string ? 'array<int, int>', 'int')"],

I also tried removing the core return tag manually. Then PHPStan always returns mixed. I also tried manually restricting the type of $args to array only. No chance of PHPStan listening to @phpstan-return.

PHPStan ignoring the conditional return type means that the crt is somehow incorrect. This works for the_title_attribute:

'the_title_attribute' => ["(\$args is empty|array{'echo': true} ? void : string|void)"],

@johnbillion I'm curious to hear your thoughts on this. This would enable us to bring over several functions from szepeviktor/phpstan-wordpress and rely on PHPStan to check all the maybes and unions. Also @herndlm, what do you think?

https://phpstan.org/r/33a71a36-5d18-43f9-9dbb-25345dfff458

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.

None yet

2 participants