Expanding the scope of the #[NoDiscard] attribute

php.internals

Kenta Usami

1 year ago
Hi internals, I'm writing to start a discussion about expanding the use of the newly accepted #[NoDiscard] attribute. I believe this attribute is a fantastic addition that will help prevent many subtle bugs, and we can make it even more effective by applying it to a broader set of core functions. My initial thoughts for strong candidates are: - All is_* prefixed functions - The password_*() functions - Most string functions: Many string functions like strlen(), substr(), trim(), and str_replace() are also clear candidates. We would naturally exclude functions with output side effects, such as the printf() family. - Mathematical functions - Pure array_*() functions: Functions that don't accept callbacks, such as array_keys(), array_values(), array_reverse(), and array_unique(), are also strong candidates. Their return value is their only result. On the other hand, applying this to all array_* functions is more debatable. For example, functions like array_map() and array_filter() can be used with callbacks that have side effects (impure functions). In such cases, the return value might be intentionally discarded. Applying #[NoDiscard] to them could lead to false positives for legitimate code. This nuance extends to other areas, like many filesystem functions which require case-by-case evaluation. A tricky example is file_get_contents(). At first glance, it seems like an obvious candidate. However, despite its name, it can be used to send external HTTP requests where the primary goal might be the side effect of the request itself (like a webhook ping), and the returned content is intentionally ignored. Adding #[NoDiscard] here could create issues for existing code. This might be a type of change best reserved for a major version like PHP 9.0 to avoid backward compatibility breaks. Finally, I would love to hear the community's thoughts on the timeline. Should we aim to add these attributes for PHP 8.5, or should we introduce potentially breaking changes like these more gradually, perhaps leading up to PHP 9.0? Thanks, Kenta
-- ヾ(〃><)ノ゙☆ Kenta Usami (宇佐美健太) github.com/zonuexe ; https://tadsan.github.io/

Tim Düsterhus

1 year ago
Hi Am 2025-07-01 16:41, schrieb Kenta Usami:
> I'm writing to start a discussion about expanding the use of the newly > accepted #[NoDiscard] attribute. > I believe this attribute is a fantastic addition that will help > prevent many subtle bugs, and we can make it even more effective by > applying it to a broader set of core functions. > > My initial thoughts for strong candidates are: > > - All is_* prefixed functions > - The password_*() functions > - Most string functions: Many string functions like strlen(), > substr(), trim(), and str_replace() are also clear candidates. > We would naturally exclude functions with output side effects, such > as the printf() family. > - Mathematical functions > - Pure array_*() functions: Functions that don't accept callbacks, > such as array_keys(), array_values(), array_reverse(), and > array_unique(), are also strong candidates. Their return value is > their only result.
As one of the RFC authors, adding `#[\NoDiscard]` to every pure function was an explicit non-goal of the RFC. Please let me refer to: https://wiki.php.net/rfc/marking_return_value_as_important#recommended_usage The goal of the attribute is to prevent hard-to-detect-but-easy-to-create bugs. Forgetting to use the return value of `str_contains()` is not particularly easy to do unintentionally.
> This nuance extends to other areas, like many filesystem functions > which require case-by-case evaluation. A tricky example is > file_get_contents(). At first glance, it seems like an obvious > candidate. However, despite its name, it can be used to send external > HTTP requests where the primary goal might be the side effect of the > request itself (like a webhook ping), and the returned content is > intentionally ignored. Adding #[NoDiscard] here could create issues
`file_get_contents()` on the other hand might be a good candidate, since even for a “ping” request you are generally interested in finding out whether or not the request succeeded. However `file_get_contents()` already emits warnings and notices in this case (and would probably throw an Exception if it was newly introduced). Best regards Tim Düsterhus

Kenta Usami

1 year ago
Hi Tim, Thank you for your reply and for the clarification. As one of the RFC authors, your insight is very helpful. I now have a much clearer understanding of the RFC's intent. Your point that #[NoDiscard] is not meant for every pure function, but rather for those where ignoring the return value leads to dangerous, hard-to-detect bugs, makes perfect sense. I agree that focusing the runtime check on high-risk functions, while leaving most pure functions to static analysis, is a reasonable approach. Based on that criterion, I understand that very few built-in functions should be marked with #[NoDiscard]. To give some context, my initial proposal was motivated by the ongoing confusion around the semantics of the #[Pure] attribute in the JetBrains stubs for static analysis. I had hoped that marking more built-in functions with #[NoDiscard] could help clarify the situation, but I now see that this is not the appropriate goal for this attribute. https://github.com/JetBrains/phpstorm-stubs/pull/1730 It seems a different approach is needed to solve that particular problem for static analyzers. Thanks again for your time and the explanation. Best regards, Kenta
-- ヾ(〃><)ノ゙☆ Kenta Usami (宇佐美健太) github.com/zonuexe ; https://tadsan.github.io/