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

Remove deprecated instanceof #184

Merged
merged 5 commits into from
Apr 23, 2023
Merged

Conversation

IanDelMar
Copy link
Contributor

@IanDelMar IanDelMar commented Apr 22, 2023

What I was wondering when updating this extension. I can have a non constant array that has a key 'fields'.

// Non constant array with fields first
$array = [
    'fields' => 'ids',
    $_GET['foo'] => $_GET['bar'],
];
assertType('array<int, int|WP_Post>', get_posts($array));

// Non constant array with fields last
$array = [
    $_GET['foo'] => $_GET['bar'],
    'fields' => 'ids',
];
assertType('array<int, int>', get_posts($array));

get_post($array) with the fields first case returns array<int, int|WP_Post> because $_GET['foo'] may be 'fields' and overwrite 'ids'. get_post($array) with the fields last case returns array<int, int> as 'fields' is not affected by $_GET['foo'] and hence will be 'ids'. With limited PHPStan knowledge, I was not able to find a way to check the order of $_GET['foo'] and 'fields' or more specifically to check whether there is a non constant key after the fields key.

Is there a simple way to check the order of the keys with PHPStan?

Part of #149

Co-authored-by: Viktor Szépe <[email protected]>
@szepeviktor
Copy link
Owner

szepeviktor commented Apr 22, 2023

We could open a ticket at @phpstan and talk to @ondrejmirtes about a function changing its return type based on its parameter 'fields' => 'ids' which is 😢 an array.
This is so anti-PHPStan, so PHP 4, I am so ashamed.

Let's support only usage where there is only 1 fields (string constant) element (anywhere) in the array.

@IanDelMar
Copy link
Contributor Author

I'd like to add more tests. Please wait with merging.

@szepeviktor szepeviktor marked this pull request as draft April 22, 2023 20:05
@IanDelMar
Copy link
Contributor Author

We could open a ticket at @phpstan and talk to @ondrejmirtes about a function changing its return type based on its parameter 'fields' => 'ids' which is 😢 an array.

I also thought we need something like that.

@IanDelMar
Copy link
Contributor Author

I can't think of any other test cases to add. Do you?

@szepeviktor
Copy link
Owner

We already have much more tests than possible problems.

Thank you.

@IanDelMar IanDelMar marked this pull request as ready for review April 22, 2023 23:05
@herndlm
Copy link
Contributor

herndlm commented Apr 23, 2023

How exactly is the type looking in your more dynamic example? It's still a constant array though, right? (= array shape and not just e.g. array<.., ..>) Is the key just string then? Maybe it helps if you do checks for the key of the whole array? string|'fields' would be generalised to string and then you can handle this case differently maybe? Something like if the key is a supertype of string -> handle different. Or I misunderstood the problem :)

@szepeviktor
Copy link
Owner

We are digging too deep.
Let's fix future problems in the next PR!

@szepeviktor szepeviktor merged commit 2189394 into szepeviktor:master Apr 23, 2023
@IanDelMar IanDelMar deleted the get-posts branch April 23, 2023 11:04
IanDelMar added a commit to IanDelMar/phpstan-wordpress that referenced this pull request Jul 27, 2023
* Remove deprecated `instanceof`

* Remove space

Co-authored-by: Viktor Szépe <[email protected]>

* Return early

* Fix handling of fields unions

* Add tests

---------

Co-authored-by: Viktor Szépe <[email protected]>
szepeviktor added a commit that referenced this pull request Jul 27, 2023
* Remove has_filter extension

* Remove current_time extension

* Remove mysql2date extension

* Remove get_object_taxonomies extension

* Remove get_taxonomies extension

* Adapt get_post extension to new stub file

* Adapt get_comment extension to new stub file

* Fix CS

* Update composer.json

* Remove WP_Theme::get()

* Remove get_permalink extension

* Update .travis.yml

* Remove term_exists extension

* Update wp_error parameter extension

* Update GetPostDynamicFunctionReturnTypeExtension.php

* Fully remove wp_error parameter extension

* Merge get_comment extension into get_post extension

* Remove type specifiying extension and rule for `is_wp_error()`

* Update README.md

Co-authored-by: Viktor Szépe <[email protected]>

* Remove echo parameter extension

* Remove _get_list_table extension

* Revert "Remove _get_list_table extension"

This reverts commit 0191253.

* Update get_post.php

* Fix earlyTerminatingMethodCalls syntax (#173)

* Remove deprecated instanceof (#183)

* Remove deprecated `instanceof` (#184)

* Remove deprecated `instanceof`

* Remove space

Co-authored-by: Viktor Szépe <[email protected]>

* Return early

* Fix handling of fields unions

* Add tests

---------

Co-authored-by: Viktor Szépe <[email protected]>

* Remove deprecated `instanceof` (#185)

* Fix CS (#186)

* Fix CS

* Fix PHP 7.2 compat.

* Fix _get_list_table extension (#190)

* Revert "Update composer.json"

This reverts commit d15b7e1.

---------

Co-authored-by: Viktor Szépe <[email protected]>
Co-authored-by: Der Mundschenk & Compagnie <[email protected]>
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.

3 participants