Expanding the scope of the #[NoDiscard] attribute

11 months ago by Kenta Usami — view source — reply

unread

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/

11 months ago by tim@bastelstu.be — view source — reply

unread

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

11 months ago by Kenta Usami — view source — reply

unread

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/