Add return types for query strings to echo key extension

[IanDelMar]

This PR addresses EchoKeyDynamicFunctionReturnTypeExtension. It

• adds return types for query string arguments,
• adds tests for $args['echo']=0, unknown values and function calls with query string,
• improves the logic of the helper methods.

[johnbillion]

I don't think echo=false works as you would expect. I'm pretty sure it results in string 'false' which evaluates to true. Needs testing.

[IanDelMar]

@johnbillion True!

[IanDelMar]

3e1c9d1

1. fixes the return types for query strings (Add return types for query strings to echo key extension #146 (comment)),
2. removes instanceof (see Add dynamic return type extension for wp_tag_cloud() #147 (comment)),
3. reduces complexity by not checking whether a function accepts an integer as value for echo as WP uses loose comparison to determine whether to echo or not (if ($args['echo]) {)

@szepeviktor 3. requires PHPStan 1.9.4. I can either revert using the new methods or bump the minimum supported version in composer.json.

[szepeviktor]

bump the minimum supported version in composer.json.

OK!

phpstan-wordpress/composer.json

Line 16 in 927c0f1

"phpstan/phpstan": "^1.8.7",

[szepeviktor]

@johnbillion Please give the start signal.

[herndlm]

I only skimmed through it but saw first element access of array and string arrays. Jfyi in most cases the generic approach is to handle all of them and union the results then 😊

[IanDelMar]

I only skimmed through it but saw first element access of array and string arrays. Jfyi in most cases the generic approach is to handle all of them and union the results then 😊

I thought about going that route. But don't we know that there is only one array element? Generically looping through non existent elements (all but one) and then generically building a union of all the non existent elements seems pretty verbose to me. I think that in most WP cases there is no need to handle more than one element. However, if there is a clear advantage to using this general approach, I am happy to do so.

[herndlm]

Don't worry, I haven't thought it through. I'm just used to that approach from phpstan internals. Here it might not add much benefits. And it could still be improved any time anyways

[IanDelMar]

Would you please point me to some use cases of the generic approach? Maybe there is some benefit to it I can't imagine if I don't see it in action?! Of course, doesn't have to be right now.

[herndlm]

you can then support things like https://github.com/phpstan/phpstan-src/blob/3103141be78f902ff7caa207d41cc7e80d528e19/tests/PHPStan/Analyser/data/array-slice.php#L44 but this a very artificial example I guess. not sure about real world use cases, e.g. everything involving if conditions along the way like

$in = $someExpression ? ['foo'] : ['bar'];
$out = array_pop($in);
// $out is 'foo'|'bar' now

but I guess this is also quite artificial :)

[IanDelMar]

It is clearer to me now. Thank you for taking the time!