====== PHP RFC: Marking return values as important (#[\NoDiscard]) ======
* Version: 1.0
* Date: 2025-01-27
* Author: Tim Düsterhus (tim@tideways-gmbh.com), Volker Dusch (volker@tideways-gmbh.com)
* Status: Implemented
* Target Version: PHP 8.5
* Implementation: https://github.com/php/php-src/commit/8779e2a6031b78bb0e3cac980410eb038b9932c4 and https://github.com/php/php-src/commit/5544be7018fe64584945c49fffcda20402bece73
* First Published at: http://wiki.php.net/rfc/marking_return_value_as_important
===== Introduction =====
PHP’s error handling mechanism of choice in newer APIs is the use of Exceptions, where forgetting to handle the failure case results in a very obvious error, when the functionality fails. However not all functionality maps to a binary result of either succeeding or failing, instead also supporting a “partial success” state.
This kind of “partial success” can only be reliably communicated by means of a return value, where forgetting to check the return value might result in silent failure. Depending on the nature of the API, the failure case might be rare or non-deterministic, making it hard to notice the issue on a cursory glance:
$items
* @return array
*/
#[\NoDiscard("as processing might fail for individual items")]
function bulk_process(array $items): array {
$results = [];
foreach ($items as $key => $item) {
if (\random_int(0, 9999) < 9999) {
// Pretend to do something useful with $item,
// which will succeed in 99.99% of cases.
echo "Processing {$item}", PHP_EOL;
$error = null;
} else {
$error = new \Exception("Failed to process {$item}.");
}
$results[$key] = $error;
}
return $results;
}
===== Proposal =====
This RFC proposes a new attribute #[\NoDiscard] that allows to indicate that the return value of a function or method is “important” and that not doing anything with the return value is likely to be a bug. In the interest of readability, the term “function” shall mean “function or method” in the rest of the RFC text.
#[Attribute(Attribute::TARGET_METHOD|Attribute::TARGET_FUNCTION)]
final class NoDiscard
{
public readonly ?string $message;
public function __construct(?string $message = null) {}
}
When calling a function with the #[\NoDiscard] attribute, the engine shall validate whether or not the return value of the function is used and emit a E_WARNING (for native functions) or E_USER_WARNING (for userland functions) if the return value is not used.
A E_(USER_)WARNING was chosen over an exception to avoid BC concerns, as throwing would potentially break existing code that "works as implemented" and because this is only a warning in the literal sense of the word, given that the function executes successfully.
The $message property of the #[\NoDiscard] attribute shall be part of the emitted warning, similar to the functionality of the [[rfc:deprecated_attribute|#[\Deprecated] attribute]].
“Using the return value” means that the returned value is part of any other expressions, it does not require the expression to be useful. As an example, assigning the return value to a dummy variable will be considered using the return value, as will casting the return value and not using the result of the cast.
==== (void) cast to suppress the warning ====
It should however be noted that OPcache might optimize away this kind of useless operations. As this would then produce a warning this RFC also proposes a new (void) cast that may be used to indicate that not using the value of an expression (or more specifically of a function call) is intentional, which will be excluded from being optimized away by OPcache. As the result of a (void) cast is not defined, the (void) cast is a statement, not an expression. Using it within an expression will result in a syntax error.
==== Native functions where the attribute is applied ====
As part of this RFC, the following native functions shall have the attribute applied:
* flock(): Ignoring the failure to obtain a lock might result in silent data corruption. Exception-based error handling cannot be used, due to the LOCK_NB flag, which is meant to gracefully handle files that are already locked. Furthermore issues in locking will only appear under heavy contention, making it hard to find issues by automated testing.
* DateTimeImmutable::set*(): The methods are badly named, because they do not actually set the updated value the DateTimeImmutable object itself, but return a new instance instead. An established naming scheme for this kind of method would nowadays be with*(). This naming can lead to subtle issues when forgetting to use the return value when migrating from DateTime to DateTimeImmutable.
==== Constraints ====
Adding the #[\NoDiscard] attribute will result in a compile-time error for the following cases:
* If the function is //explicitly// typed with the : void or : never return type.
* For magic methods that are required to be : void or not have a return type at all (e.g. __construct or __clone).
* For property hooks.
==== Implementation Details ====
The engine will verify whether the return value is used right //before// calling the function, and //after// evaluating the parameters. This means that using a throwing error handler will result in the function not being called. This is for two reasons:
* The primary reason is performance: Performing this check before calling the function allows:
* to merge the check with the deprecation check, and also
* to make use of “opcode specialization” to remove the check if the return value is known to be used.
* As the attribute is intended to be used on functions where not using the return value is “unsafe”, not calling the function if the return value is not used and a throwing error handler is active results in a “fail-closed” mechanism, implicitly doing the “safer” thing.
===== Examples =====
$items
* @return array
*/
#[\NoDiscard("as processing might fail for individual items")]
function bulk_process(array $items): array {
$results = [];
foreach ($items as $key => $item) {
if (\random_int(0, 9999) < 9999) {
// Pretend to do something useful with $item,
// which will succeed in 99.99% of cases.
echo "Processing {$item}", PHP_EOL;
$error = null;
} else {
$error = new \Exception("Failed to process {$item}.");
}
$results[$key] = $error;
}
return $results;
}
$items = [ 'foo', 'bar', 'baz' ];
// Warning: The return value of function bulk_process() is expected to be consumed, as processing might fail for individual items in test.php on line 34
bulk_process($items);
// No warning, because the return value is consumed by the assignment.
$results = bulk_process($items);
// No warning, because the (void) cast is used.
(void)bulk_process($items);
// No warning, because the return value is consumed by the cast (but the (bool) cast may be optimized away by OPcache).
(bool)bulk_process($items);
getMessage(), PHP_EOL;
}
==== Recommended usage ====
The attribute is intended to be used on functions where the user of the function is likely to forget using the return value by accident and where not using the return value will result in buggy behavior that is hard to detect during testing.
The above example of the bulk_process() function is a good use-case. It is a function with side-effects and it will correctly process all the items in the vast majority of the cases, making it easy to miss the failure case when testing the functionality.
str_contains() on the other hand is likely to be a bad use-case. It is unlikely for a developer to call str_contains() and not do anything with the result, as the function does not result in side-effects (other than implicitly casting the arguments to strings). If the developer forgets to use the return value anyways, it will just result in unnecessary processing without any further negative effects. The same reasoning applies to most “pure” functions.
===== Precedent =====
==== PHP ====
The PHPStorm IDE, as well as the static analyzers PHPStan and Psalm already support diagnostics when the return value of a //pure// function is unused. Thus they would detect the DateTimeImmutable::set*() gotcha. As far as the RFC authors are aware, there is no corresponding diagnostic for impure functions with important return values (such as the bulk_process() example).
==== Other programming languages ====
The naming of the attribute and the use of a (void) cast to suppress the warning is directly inspired by [[https://en.cppreference.com/w/c/language/attributes/nodiscard|C (since C23)]] and [[https://en.cppreference.com/w/cpp/language/attributes/nodiscard|C++ (since C++17)]], which support the ''[[nodiscard]]'' attribute.
[[https://doc.rust-lang.org/reference/attributes/diagnostics.html#the-must_use-attribute|Rust]] supports a ''#[must_use = "message"]'' attribute, which inspired the support for adding a message.
For Java there are several ecosystem-specific ''@CheckReturnValue'' annotations, but nothing in the standard.
Swift appears to emit a warning by default, the ''@discardableResult'' attribute can be used to opt-out of this warning for specific functions.
For [[https://github.com/golang/go/issues/20803|Golang exists a feature request]] to emit a diagnostic by default.
For [[https://github.com/microsoft/TypeScript/issues/8240|TypeScript exists a feature request]] for this kind of diagnostic. Within ESLint for TypeScript there [[https://typescript-eslint.io/rules/no-floating-promises/|already is a check that returned Promises are consumed]], which is a similar use-case to the bulk_process() example.
===== Backward Incompatible Changes =====
The class-name NoDiscard will no longer be available within the global namespace. A GitHub search for [[https://github.com/search?q=language%3APHP%20symbol%3ANoDiscard&type=code|language:PHP symbol:NoDiscard]] does not reveal any use of this class-name, making this a theoretical issue.
Functions having the attribute might emit a E_WARNING if they are used in an unsafe fashion. If a throwing error handler is used this may result in a breaking change. As the attribute shall only be applied to functions where not using the return value has a significant impact on correct functionality, we consider the benefit of pointing out the error to the user to outweigh the drawbacks, since suppressing the error by assigning to a dummy variable can be reliably performed in a way that is compatible with older PHP versions.
(void) currently is a valid access to a constant named void with (unnecessary) parentheses. A GitHub search reveals that there is code that defines constants called void, both namespaced and in the global namespace, but a search for (void) does not reveal any hits outside of strings and comments, making this unlikely to be a problem in practice.
===== Proposed PHP Version(s) =====
Next PHP 8.x (8.5).
===== RFC Impact =====
==== To SAPIs ====
None.
==== To Existing Extensions ====
None.
==== To Opcache ====
The attribute adds new semantics to a function or method call and thus requires changes to OPcache. The current implementation already passes all tests with OPcache enabled (including JIT).
==== New Constants ====
* T_VOID_CAST in ext/tokenizer (if the (void) cast is accepted).
==== php.ini Defaults ====
None.
===== Open Issues =====
None.
===== Unaffected PHP Functionality =====
This RFC only affects calls to functions and methods that have the attribute applied, this includes a limited number of native functions and methods that is listed above. Calls to other functions or methods and any other functionality is unaffected.
===== Future Scope =====
None.
===== Proposed Voting Choices =====
2/3 to accept the attribute.
* Yes
* No
2/3 to include the (void) cast. If this vote fails, the $_ variable will be special-cased so that it will not be optimized away by OPcache, even when the function is known not to return an object.
* Yes
* No
===== Patches and Tests =====
* The #[\NoDiscard] attribute: https://github.com/php/php-src/pull/17599
* The (void) cast: https://github.com/php/php-src/pull/18115
===== Implementation =====
* (void) cast: https://github.com/php/php-src/commit/8779e2a6031b78bb0e3cac980410eb038b9932c4
* #[\NoDiscard] attribute: https://github.com/php/php-src/commit/5544be7018fe64584945c49fffcda20402bece73
===== Errata =====
* The #[\NoDiscard] attribute was not implemented on flock(): https://github.com/php/php-src/issues/18235
* The (void) cast is allowed in for()’s statement lists (except the last condition statement): https://github.com/php/php-src/pull/18303
===== References =====
* Implementation: https://github.com/php/php-src/pull/17599
* C: https://en.cppreference.com/w/c/language/attributes/nodiscard
* Rust: https://doc.rust-lang.org/reference/attributes/diagnostics.html#the-must_use-attribute
===== Rejected Features =====
None.