Opened 5 weeks ago
Last modified 4 weeks ago
#62366 new defect (bug)
get_post_states() throws PHP warning
Reported by: | akramipro | Owned by: | |
---|---|---|---|
Milestone: | Awaiting Review | Priority: | normal |
Severity: | normal | Version: | 6.7 |
Component: | Posts, Post Types | Keywords: | has-patch |
Focuses: | coding-standards, php-compatibility | Cc: |
Description
the $post
sanity does not check in get_post_states()
.
this patch can fix bugs like #58932
Attachments (1)
Change History (7)
#2
follow-up:
↓ 3
@
5 weeks ago
We cannot hope that the user will enter the correct value.
We should not allow it to cause an error.
Why is it incorrect to check this?
#3
in reply to:
↑ 2
@
5 weeks ago
Replying to akramipro:
We cannot hope that the user will enter the correct value.
User? I would think the developer. When a user enters somthing it sholud be validated, bit fail to see the relvance here.
We should not allow it to cause an error.
It's not a fatal error, it's a warning to developers.
Why is it incorrect to check this?
How would developers otherwise see that they have made an error, passing the wrong type argument to a function?
If strict types was, or will later be, enforced, and a type hint is put in the function signature, this would cause the a error to be emitted while referencing the caller file and line.
If this was common, due to misunderstanding a complex documentation and guidelines, a "doing it wrong" could be emitted.
Suggestion: Trace the error and see where it comes from. If core, investigate the cases it may happen, report here, then we can perhaps make that code better. If theme/plugin inform the authors.
#4
@
5 weeks ago
User? I would think the developer. When a user enters somthing it sholud be validated, bit fail to see the relvance here.
by user i mean developers 😅
Suggestion: Trace the error and see where it comes from. If core, investigate the cases it may happen, report here, then we can perhaps make that code better. If theme/plugin inform the authors.
as i mentioned before it happens in wp_setup_nav_menu_item()
there is already a ticket for that: #58932
they suggest checking $post
before pass to get_post_states()
like you say.
but i was thinking it is better to check that inside function.
i understand what you mean and i agree with you that we can add a "doing it wrong" to let developers know they are passing bad $post
.
#6
@
4 weeks ago
The function might be updated with types in the future to make it clearer and more reliable. Right now, it looks like this:
<?php /** * Retrieves an array of post states from a post. * * @since 5.3.0 * * @param WP_Post $post The post to retrieve states for. * @return string[] Array of post state labels keyed by their state. */ function get_post_states( $post ) {}
If types are added to this function, it will become the caller's responsibility to make sure the input is correct. This is why I think we should not hide the error, as it helps us see and fix potential problems.
Since the cause for this must be that the function is called without the correct varibale type, then this does not fix anyting, just hides an error. This function rightously assumes $post as a WP_Post.
It shuld be rectified in the calling code.