Skip to content
New issue

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

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

Already on GitHub? # to your account

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

Closed
wants to merge 11 commits into 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

@johnbillion
Copy link
Contributor

If this allows us to remove some dynamic return type extensions from phpstan-wordpress then I'm in favour. I'm taking a look now.

@IanDelMar
Copy link
Contributor Author

Replaced by #195

@IanDelMar IanDelMar closed this Aug 23, 2024
@IanDelMar IanDelMar deleted the cond-types-demo branch August 23, 2024 20:32
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants