rfc:marking_return_value_as_important

PHP RFC: Marking return values as important (#[\NoDiscard])

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: 

<?php
 
/**
 * Processes all the given items and returns an array with the results of the
 * operation for each item. `null` indicates success and an Exception indicates
 * an error. The keys of the result array match the keys of the $items array.
 * 
 * @param array<string> $items
 * @return array<null|Exception>
 */
#[\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 #[\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

<?php
 
/**
 * Processes all the given items and returns an array with the results of the
 * operation for each item. `null` indicates success and an Exception indicates
 * an error. The keys of the result array match the keys of the $items array.
 * 
 * @param array<string> $items
 * @return array<null|Exception>
 */
#[\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);
<?php
 
#[\NoDiscard("as processing might fail for individual items")]
function bulk_process(array $items): array {
	return [];
}
 
$items = [ 'foo', 'bar', 'baz' ];
 
// Parse error: syntax error, unexpected token "(void)" in test.php on line 11
if ((void)bulk_process($items)) {
	// …
}
<?php
 
set_error_handler(static function (int $errno, string $errstr, ?string $errfile = null, ?int $errline = null) {
	throw new \ErrorException($errstr, 0, $errno, $errfile, $errline);
});
 
#[\NoDiscard("as processing might fail for individual items")]
function bulk_process(array $items): array {
	echo __FUNCTION__, PHP_EOL;
 
	return [];
}
 
function get_items(): array {
	echo __FUNCTION__, PHP_EOL;
 
	return [ 'foo', 'bar', 'baz' ];
}
 
try {
	// Prints: get_items
	bulk_process(get_items());
} catch (\Exception $e) {
	// Prints: The return value of function bulk_process() is expected to be consumed, as processing might fail for individual items
	echo $e->getMessage(), PHP_EOL;
}
<?php
 
#[\NoDiscard]
function bulk_process(array $items): void { }
 
// Fatal error: A void function does not return a value, but #[\NoDiscard] requires a return value in test.php on line 4
<?php
 
#[\NoDiscard]
function bulk_process(array $items): never { }
 
// Fatal error: A never returning function does not return a value, but #[\NoDiscard] requires a return value in test.php on line 4

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 C (since C23) and C++ (since C++17), which support the [[nodiscard]] 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 Golang exists a feature request to emit a diagnostic by default.

For TypeScript exists a feature request for this kind of diagnostic. Within ESLint for TypeScript there 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 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.

Implement the #[\NoDiscard] attribute as described?
Real name Yes No
alcaeus (alcaeus)  
asgrim (asgrim)  
ashnazg (ashnazg)  
dharman (dharman)  
galvao (galvao)  
girgias (girgias)  
malferov (malferov)  
mbeccati (mbeccati)  
mcmic (mcmic)  
ocramius (ocramius)  
reywob (reywob)  
sebastian (sebastian)  
sergey (sergey)  
theodorejb (theodorejb)  
timwolla (timwolla)  
Count: 13 2

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.

Implement the (void) cast to reliably suppress #[\NoDiscard]?
Real name Yes No
alcaeus (alcaeus)  
asgrim (asgrim)  
ashnazg (ashnazg)  
dharman (dharman)  
galvao (galvao)  
girgias (girgias)  
kalle (kalle)  
mbeccati (mbeccati)  
mcmic (mcmic)  
ocramius (ocramius)  
reywob (reywob)  
sebastian (sebastian)  
sergey (sergey)  
theodorejb (theodorejb)  
timwolla (timwolla)  
Count: 13 2

Patches and Tests

https://github.com/php/php-src/pull/17599

Implementation

After the project is implemented, this section should contain

  1. the version(s) it was merged into
  2. a link to the git commit(s)
  3. a link to the PHP manual entry for the feature
  4. a link to the language specification section (if any)

References

Rejected Features

None.

rfc/marking_return_value_as_important.txt · Last modified: 2025/03/05 13:45 by timwolla

Page Tools



Table of Contents