Skip to content

Remove deprecated instanceof#184

Merged
szepeviktor merged 5 commits intoszepeviktor:masterfrom
IanDelMar:get-posts
Apr 23, 2023
Merged

Remove deprecated instanceof#184
szepeviktor merged 5 commits intoszepeviktor:masterfrom
IanDelMar:get-posts

Conversation

@IanDelMar
Copy link
Copy Markdown
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 <viktor@szepe.net>
@szepeviktor
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Contributor Author

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

@szepeviktor
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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 <viktor@szepe.net>

* Return early

* Fix handling of fields unions

* Add tests

---------

Co-authored-by: Viktor Szépe <viktor@szepe.net>
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 <viktor@szepe.net>

* 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 <viktor@szepe.net>

* Return early

* Fix handling of fields unions

* Add tests

---------

Co-authored-by: Viktor Szépe <viktor@szepe.net>

* 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 <viktor@szepe.net>
Co-authored-by: Der Mundschenk & Compagnie <mundschenk-at@users.noreply.github.com>
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