Skip to content

Remove foreach.nonIterable baseline and fix its issue #13079

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.

Sign up for GitHub

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

Closed
dpanta94 wants to merge 1 commit into from
Closed

Remove foreach.nonIterable baseline and fix its issue #13079

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion phpstan.neon.dist
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ includes:
- tests/phpstan/baselines/empty.property.neon
- tests/phpstan/baselines/empty.variable.neon
- tests/phpstan/baselines/encapsedStringPart.nonString.neon
- tests/phpstan/baselines/foreach.nonIterable.neon
- tests/phpstan/baselines/function.alreadyNarrowedType.neon
- tests/phpstan/baselines/function.impossibleType.neon
- tests/phpstan/baselines/function.resultUnused.neon
Expand Down
2 changes: 1 addition & 1 deletion src/wp-includes/class-wp-post-type.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -820,7 +820,7 @@ public function remove_rewrite_rules() {
}

// Remove registered custom meta capabilities.
foreach ( $this->cap as $cap ) {
foreach ( (array) $this->cap as $cap ) {

@dmsnell dmsnell Aug 16, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if $this->cap is a stdClass as the class property docblock claims, then will the (array) cast not create a clone of the data, iterate over the clone, delete properties from the clone, and then leave the original $this->cap untouched?

if so, wouldn’t that create a logical defect and nullify this code?

@dpanta94 dpanta94 Aug 17, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your comment would be valid i believe if we were actually unsetting properties of the actual $cap property. We don't though. So the only downside i see with this code is that its actually using a bit of extra memory because of the cloning.

@dmsnell dmsnell Aug 24, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gotcha. thanks for pointing that out.

because I want to be careful here and make sure we are careful about changing code just because a linter flags a warning, I am checking this again.

in explicitly-casing to (array) we will still accept runtime corruption for things not castable into an array, but the PHPStan warning is silenced.

additionally, it seems like the change from iterating over an object’s properties to the array cast changes what’s iterated. whereas the existing foreach only iterates over public properties, the array cast is more equivalent to get_mangled_object_vars() and returns private and protected properties.

php > var_dump( $b, (array) $b, get_object_vars( $b ) );
object(Supervisor)#2 (4) {
  ["name":"Employee":private]=>
  string(9) "sensitive"
  ["job"]=>
  string(8) "recorder"
  ["tenure":protected]=>
  int(0)
  ["name":"Supervisor":private]=>
  string(9) "overruled"
}
array(4) {
  ["Employeename"]=>
  string(9) "sensitive"
  ["job"]=>
  string(8) "recorder"
  ["*tenure"]=>
  int(0)
  ["Supervisorname"]=>
  string(9) "overruled"
}
array(1) {
  ["job"]=>
  string(8) "recorder"
}

php > foreach ( $b as $cap ) { var_dump( $cap ); }
string(8) "recorder"

the cast to (array) silences the warning, but doesn’t really address the issue PHPStan raised, which is that we’re attempting to iterate over something that may not be iterable.

we could always throw in a check for is_iterable() around the foreach, which would address the reported warning, but I am also curious why PHPStan isn’t inferring that already?

  • $this->cap is typed as stdClass (which is iterable)
  • it’s assigned from get_post_type_capabilities() which is typed to return an object

perhaps the problem is in PHPStan, or I’m still missing something.

@westonruter westonruter Aug 30, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Casting to an array also doesn't address errors at higher PHPStan error levels. There remains an error on the next line at level >5:

Possibly invalid array key type mixed.

This is getting at the underlying problem, that $cap is not an array-key at all, but rather a stdClass which can never be an array key.

@westonruter westonruter Aug 30, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code here was introduced in r37890 (bded47a).

The code used to live on unregister_post_type() here it looked like this:

wordpress-develop/src/wp-includes/post.php

Lines 1258 to 1261 in 78eb45a

// Remove registered custom meta capabilities.
foreach ( $post_type_args->cap as $cap ) {
unset( $post_type_meta_caps[ $cap ] );
}

The cap was populated by get_post_type_capabilities():

wordpress-develop/src/wp-includes/post.php

Lines 1114 to 1115 in 78eb45a

$args->cap = get_post_type_capabilities( $args );
unset( $args->capabilities );

Even then this was an object:

wordpress-develop/src/wp-includes/post.php

Lines 1347 to 1349 in 78eb45a

* @return object object with all the capabilities as member variables.
*/
function get_post_type_capabilities( $args ) {

@westonruter westonruter Aug 30, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm working up an alternative. This is a long-standing bug that PHPStan has caught which looks like it warrants a separate Trac ticket.

@westonruter westonruter Aug 31, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See Core-66008

unset( $post_type_meta_caps[ $cap ] );
}
}
Expand Down
25 changes: 0 additions & 25 deletions tests/phpstan/baselines/foreach.nonIterable.neon
View file
Open in desktop

This file was deleted.

Loading