Skip to content

Conversation

@gziolo
Copy link
Member

@gziolo gziolo commented Aug 22, 2025

I audited ignoreErrors added to PHPStan (#6 (comment)). I decided to remove it and fix the PHPDoc to make PHPStan happy.

@gziolo gziolo self-assigned this Aug 22, 2025
@gziolo gziolo added [Type] Bug Something isn't working [Tool] Issues related to development tooling, such as linting, testing, or CI labels Aug 22, 2025
@gziolo gziolo mentioned this pull request Aug 22, 2025
9 tasks
Copy link
Contributor

@justlevine justlevine left a comment

Choose a reason for hiding this comment

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

@gziolo if you don't mind the extra work stripping them out before merge into WP Core then go ahead and merge 🤷

(my 2c: We're not using nor relying on the real array shape, so this is just suppressing the external phpstan-wordpress error in the file by marking the generic they introduce as mixed so they don't get type-checked. tl;dr all this does is hide the error suppression inside the file implementation, instead of centrally declaring that we're not using the (3rd-party) generics for that file. ).

@gziolo
Copy link
Member Author

gziolo commented Aug 22, 2025

if you don't mind the extra work stripping them out before merge into WP Core then go ahead and merge 🤷

I want to make sure all files are linted for now. I will figure out next week what to do if CI on wordpress-develop is largely unhappy. As an alternative, we might consider silencing only the errors related to generics.

@gziolo gziolo merged commit bff06e1 into trunk Aug 22, 2025
15 checks passed
@gziolo gziolo deleted the fix/phpstan-ignore-errors branch August 22, 2025 12:10
@justlevine
Copy link
Contributor

justlevine commented Aug 22, 2025

As an alternative, we might consider silencing only the errors related to generics.

FTR that was the pre-merge behavior... only generics from WP_REST_Request in those wp-rest/* files were being ignored.

  • # WP_REST_Request is not actually a generic class in WordPress core.
    # PHPStan's WordPress stubs appear to define it as generic for type checking,
    # but WordPress itself doesn't use generics. This seems to be an incompatibility
    # between static analysis tools and WordPress's actual implementation.
    -
    message: '#has parameter \$request with generic class WP_REST_Request but does not specify its types#'
    paths:
    - includes/rest-api/**/*.php

@gziolo
Copy link
Member Author

gziolo commented Aug 22, 2025

True. I will revisit next week and revert if that makes more sense 👍🏻

@gziolo
Copy link
Member Author

gziolo commented Aug 26, 2025

I tested in WordPress/wordpress-develop#9410 all the latest changes from trunk, and we can safely use more complex types in PHPDoc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Tool] Issues related to development tooling, such as linting, testing, or CI [Type] Bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants