diff --git a/docs/running_psalm/issues/UnusedFunctionCall.md b/docs/running_psalm/issues/UnusedFunctionCall.md index f44ff0da2a3..b42af21502d 100644 --- a/docs/running_psalm/issues/UnusedFunctionCall.md +++ b/docs/running_psalm/issues/UnusedFunctionCall.md @@ -9,3 +9,5 @@ $a = strlen("hello"); strlen("goodbye"); // unused echo $a; ``` + +It is also emitted, regardless of `--find-dead-code`, when the return value of a function annotated with PHP 8.5's `#[\NoDiscard]` attribute is discarded. Casting the call to `(void)` intentionally discards the value without triggering the issue. diff --git a/docs/running_psalm/issues/UnusedMethodCall.md b/docs/running_psalm/issues/UnusedMethodCall.md index f7a4a862f58..d84d248838c 100644 --- a/docs/running_psalm/issues/UnusedMethodCall.md +++ b/docs/running_psalm/issues/UnusedMethodCall.md @@ -20,3 +20,5 @@ final class A { $a = new A("hello"); $a->getFoo(); ``` + +It is also emitted, regardless of `--find-dead-code`, when the return value of a method annotated with PHP 8.5's `#[\NoDiscard]` attribute is discarded. Casting the call to `(void)` intentionally discards the value without triggering the issue. diff --git a/src/Psalm/Internal/Analyzer/AttributesAnalyzer.php b/src/Psalm/Internal/Analyzer/AttributesAnalyzer.php index fd4dd7174bf..0bf9a0f686d 100644 --- a/src/Psalm/Internal/Analyzer/AttributesAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/AttributesAnalyzer.php @@ -22,6 +22,7 @@ use Psalm\Issue\UndefinedClass; use Psalm\IssueBuffer; use Psalm\Storage\ClassLikeStorage; +use Psalm\Storage\FunctionLikeStorage; use Psalm\Storage\HasAttributesInterface; use Psalm\Type\Atomic\TLiteralString; use Psalm\Type\Union; @@ -119,6 +120,23 @@ public static function analyze( $suppressed_issues, ); } + + // PHP 8.5 rejects #[\NoDiscard] at declaration time when there is no return value + // to discard (a fatal error for void/never native return types). + if ($fq_attribute_name === 'NoDiscard' + && $storage instanceof FunctionLikeStorage + && $storage->signature_return_type !== null + && ($storage->signature_return_type->isVoid() || $storage->signature_return_type->isNever()) + ) { + IssueBuffer::maybeAdd( + new InvalidAttribute( + "Attribute {$attribute_name} cannot be used on a function or method with a " + . 'void or never return type', + $attribute_name_location, + ), + $suppressed_issues, + ); + } } } diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/FunctionCallAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/FunctionCallAnalyzer.php index 18db16871fc..85b8c60b60b 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/FunctionCallAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/FunctionCallAnalyzer.php @@ -357,6 +357,14 @@ public static function analyze( $context, ); + self::checkFunctionNoDiscard( + $statements_analyzer, + $stmt, + $function_name, + $function_call_info, + $context, + ); + if ($function_call_info->function_storage) { if ($function_call_info->function_storage->assertions && $function_name instanceof PhpParser\Node\Name) { self::applyAssertionsToContext( @@ -1121,6 +1129,58 @@ private static function checkFunctionCallPurity( } } + /** + * Reports the return value of a `#[\NoDiscard]` function being discarded at a call site. + * + * Unlike the pure-call check in {@see self::checkFunctionCallPurity()}, this runs + * regardless of purity and the find-unused-variables setting, matching PHP 8.5's + * runtime behaviour. A `(void)` cast is the documented escape hatch: it analyses its + * operand with `inside_general_use`, so `$context->insideUse()` is already true there. + * + * `#[\NoDiscard]` is only enforced by PHP from 8.5, where the `(void)` escape hatch also + * becomes available, so the report is gated on the analysis PHP version. Below 8.5 the + * attribute is inert at runtime and `@psalm-suppress` remains the only way to silence it. + */ + private static function checkFunctionNoDiscard( + StatementsAnalyzer $statements_analyzer, + PhpParser\Node\Expr\FuncCall $stmt, + PhpParser\Node $function_name, + FunctionCallInfo $function_call_info, + Context $context, + ): void { + if ($context->collect_initializations + || $context->collect_mutations + || $stmt->isFirstClassCallable() + || $statements_analyzer->getCodebase()->analysis_php_version_id < 8_05_00 + || $function_call_info->function_id === null + || $function_call_info->function_storage === null + || !$function_call_info->function_storage->no_discard + ) { + return; + } + + if ($context->inside_unset || $context->insideUse()) { + return; + } + + // A void/never native return has nothing to discard. PHP rejects such a declaration + // outright (reported as InvalidAttribute), so this is just a defensive call-site guard. + $return_type = $function_call_info->function_storage->signature_return_type; + + if ($return_type !== null && ($return_type->isVoid() || $return_type->isNever())) { + return; + } + + IssueBuffer::maybeAdd( + new UnusedFunctionCall( + 'The call to ' . $function_call_info->function_id . ' is not used', + new CodeLocation($statements_analyzer, $function_name), + $function_call_info->function_id, + ), + $statements_analyzer->getSuppressedIssues(), + ); + } + private static function callUsesByReferenceArguments( FunctionCallInfo $function_call_info, PhpParser\Node\Expr\FuncCall $stmt, diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/Method/MethodCallPurityAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/Method/MethodCallPurityAnalyzer.php index 9da3c08543c..c8140de6c01 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/Method/MethodCallPurityAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/Method/MethodCallPurityAnalyzer.php @@ -133,6 +133,37 @@ public static function analyze( } } + // PHP 8.5's #[\NoDiscard] requires the return value to be used. Unlike the + // pure/mutation-free check above, this runs regardless of purity and the + // find-unused-variables setting. It is gated on PHP 8.5 (where the attribute is + // enforced and the (void) escape hatch exists). PHP does not inherit the attribute + // onto an implementation, so an abstract/interface declaration alone never fires; + // a void/never native return has nothing to discard. + if ($method_storage->no_discard + && !$method_storage->abstract + && !$class_storage->is_interface + && $codebase->analysis_php_version_id >= 8_05_00 + && !$context->collect_initializations + && !$context->collect_mutations + && !$stmt->isFirstClassCallable() + && !$context->inside_unset + && !$context->insideUse() + && !( + $method_storage->signature_return_type + && ($method_storage->signature_return_type->isVoid() + || $method_storage->signature_return_type->isNever()) + ) + ) { + IssueBuffer::maybeAdd( + new UnusedMethodCall( + 'The call to ' . $cased_method_id . ' is not used', + new CodeLocation($statements_analyzer, $stmt->name), + (string) $method_id, + ), + $statements_analyzer->getSuppressedIssues(), + ); + } + if ($statements_analyzer->getSource() instanceof FunctionLikeAnalyzer && $statements_analyzer->getSource()->track_mutations && !$method_storage->mutation_free diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/StaticMethod/ExistingAtomicStaticCallAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/StaticMethod/ExistingAtomicStaticCallAnalyzer.php index c7dc29376ad..3544005fba1 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/StaticMethod/ExistingAtomicStaticCallAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/StaticMethod/ExistingAtomicStaticCallAnalyzer.php @@ -28,6 +28,7 @@ use Psalm\Internal\TypeVisitor\ContainsStaticVisitor; use Psalm\Issue\AbstractMethodCall; use Psalm\Issue\ImpureMethodCall; +use Psalm\Issue\UnusedMethodCall; use Psalm\IssueBuffer; use Psalm\Plugin\EventHandler\Event\AfterMethodCallAnalysisEvent; use Psalm\Storage\ClassLikeStorage; @@ -317,6 +318,37 @@ public static function analyze( } } + // PHP 8.5's #[\NoDiscard] requires the return value to be used, including for + // static calls. Runs regardless of purity and the find-unused-variables setting, + // but is gated on PHP 8.5 (where the attribute is enforced and the (void) escape + // hatch exists). PHP does not inherit the attribute onto an implementation, so an + // abstract declaration alone never fires; a void/never native return has nothing + // to discard. + if ($method_storage->no_discard + && !$method_storage->abstract + && !$class_storage->is_interface + && $codebase->analysis_php_version_id >= 8_05_00 + && !$context->collect_initializations + && !$context->collect_mutations + && !$stmt->isFirstClassCallable() + && !$context->inside_unset + && !$context->insideUse() + && !( + $method_storage->signature_return_type + && ($method_storage->signature_return_type->isVoid() + || $method_storage->signature_return_type->isNever()) + ) + ) { + IssueBuffer::maybeAdd( + new UnusedMethodCall( + 'The call to ' . $cased_method_id . ' is not used', + new CodeLocation($statements_analyzer, $stmt_name), + (string) $method_id, + ), + $statements_analyzer->getSuppressedIssues(), + ); + } + $assertionsResolver = new AssertionsFromInheritanceResolver($codebase); $assertions = $assertionsResolver->resolve( $method_storage, diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/CastAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/CastAnalyzer.php index dce9bd1e3a0..be60e870ba3 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/CastAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/CastAnalyzer.php @@ -298,6 +298,31 @@ public static function analyze( return true; } + if ($stmt instanceof PhpParser\Node\Expr\Cast\Void_) { + // PHP 8.5's (void) cast evaluates its operand and explicitly discards the result. + // It is only valid as a statement; consuming its result is a compile error in PHP + // (the parser nikic/php-parser accepts more leniently), so reject it in any + // value-consuming position. Otherwise it is the documented escape hatch for + // #[\NoDiscard]: analysing the operand under inside_general_use marks it as used, so + // no discarded-return-value issue is raised. The parser only produces this node when + // targeting PHP 8.5+, so no version guard is needed here. + if ($context->insideUse()) { + IssueBuffer::maybeAdd( + new InvalidCast( + 'The (void) cast can only be used as a statement, not as an expression', + new CodeLocation($statements_analyzer->getSource(), $stmt), + ), + $statements_analyzer->getSuppressedIssues(), + ); + } + + $expression_result = self::checkExprGeneralUse($statements_analyzer, $stmt, $context); + + $statements_analyzer->node_data->setType($stmt, Type::getVoid()); + + return $expression_result; + } + IssueBuffer::maybeAdd( new UnrecognizedExpression( 'Psalm does not understand the cast ' . $stmt::class, diff --git a/src/Psalm/Internal/PhpVisitor/Reflector/FunctionLikeNodeScanner.php b/src/Psalm/Internal/PhpVisitor/Reflector/FunctionLikeNodeScanner.php index 073a595d1ca..92607b504d1 100644 --- a/src/Psalm/Internal/PhpVisitor/Reflector/FunctionLikeNodeScanner.php +++ b/src/Psalm/Internal/PhpVisitor/Reflector/FunctionLikeNodeScanner.php @@ -683,6 +683,10 @@ public function start( } } + if ($attribute->fq_class_name === 'NoDiscard') { + $storage->no_discard = true; + } + if ($attribute->fq_class_name === 'Psalm\\Deprecated' || $attribute->fq_class_name === 'Deprecated' || $attribute->fq_class_name === 'JetBrains\\PhpStorm\\Deprecated' diff --git a/src/Psalm/Storage/FunctionLikeStorage.php b/src/Psalm/Storage/FunctionLikeStorage.php index 384d0ecbd48..54be3e1ee3e 100644 --- a/src/Psalm/Storage/FunctionLikeStorage.php +++ b/src/Psalm/Storage/FunctionLikeStorage.php @@ -143,6 +143,15 @@ abstract class FunctionLikeStorage implements HasAttributesInterface, Stringable public bool $pure = false; + /** + * Whether the return value of this function/method must be used by callers. + * + * Set when the function-like is annotated with PHP 8.5's `#[\NoDiscard]` attribute. + * When true, Psalm reports the return value being discarded at a call site + * independently of purity and the find-unused-variables setting. + */ + public bool $no_discard = false; + /** * Whether or not the function output is dependent solely on input - a function can be * impure but still have this property (e.g. var_export). Useful for taint analysis. diff --git a/tests/CastTest.php b/tests/CastTest.php index 1147ccd70db..cc50cf972cf 100644 --- a/tests/CastTest.php +++ b/tests/CastTest.php @@ -14,6 +14,17 @@ final class CastTest extends TestCase #[Override] public function providerValidCodeParse(): iterable { + yield 'voidCastDiscardsValueWithoutError' => [ + 'code' => ' [], + 'ignored_issues' => [], + 'php_version' => '8.5', + ]; yield 'SKIPPED-castFalseOrIntToInt' => [ 'code' => ' */ diff --git a/tests/NoDiscardTest.php b/tests/NoDiscardTest.php new file mode 100644 index 00000000000..2719a39048b --- /dev/null +++ b/tests/NoDiscardTest.php @@ -0,0 +1,497 @@ +project_analyzer->setPhpVersion('8.5', 'tests'); + $codebase = $this->project_analyzer->getCodebase(); + $codebase->find_unused_variables = true; + $codebase->config->throw_exception = false; + + $this->addFile( + 'somefile.php', + 'analyzeFile('somefile.php', new Context()); + + $unused_calls = array_filter( + IssueBuffer::getIssuesData()['somefile.php'] ?? [], + static fn($issue): bool => $issue->type === 'UnusedFunctionCall', + ); + + $this->assertCount(1, $unused_calls); + } + + /** + * A mutation-free `#[\NoDiscard]` method is matched by the mutation-free unused-call check + * and the NoDiscard check. Both emit an identical UnusedMethodCall, so IssueBuffer must + * deduplicate them into a single report. + */ + public function testMutationFreeNoDiscardMethodIsReportedOnce(): void + { + $this->project_analyzer->setPhpVersion('8.5', 'tests'); + $codebase = $this->project_analyzer->getCodebase(); + $codebase->find_unused_variables = true; + $codebase->config->throw_exception = false; + + $this->addFile( + 'somefile.php', + 'bar(); + ', + ); + + $this->analyzeFile('somefile.php', new Context()); + + $unused_calls = array_filter( + IssueBuffer::getIssuesData()['somefile.php'] ?? [], + static fn($issue): bool => $issue->type === 'UnusedMethodCall', + ); + + $this->assertCount(1, $unused_calls); + } + + #[Override] + public function providerValidCodeParse(): iterable + { + yield 'noDiscardFunctionReturnValueAssigned' => [ + 'code' => ' [], + 'ignored_issues' => [], + 'php_version' => '8.5', + ]; + + yield 'noDiscardFunctionReturnValueReturned' => [ + 'code' => ' [], + 'ignored_issues' => [], + 'php_version' => '8.5', + ]; + + yield 'noDiscardFunctionReturnValueUsedAsCondition' => [ + 'code' => ' [], + 'ignored_issues' => [], + 'php_version' => '8.5', + ]; + + yield 'noDiscardFunctionReturnValuePassedAsArgument' => [ + 'code' => ' [], + 'ignored_issues' => [], + 'php_version' => '8.5', + ]; + + yield 'noDiscardFunctionResultDiscardedViaVoidCast' => [ + 'code' => ' [], + 'ignored_issues' => [], + 'php_version' => '8.5', + ]; + + yield 'noDiscardWithMessageArgumentStillResolves' => [ + 'code' => ' [], + 'ignored_issues' => [], + 'php_version' => '8.5', + ]; + + yield 'noDiscardMethodReturnValueUsed' => [ + 'code' => 'bar(); + $y = Foo::baz(); + echo $x + $y; + ', + 'assertions' => [], + 'ignored_issues' => [], + 'php_version' => '8.5', + ]; + + yield 'noDiscardOnInterfaceMethodNotInherited' => [ + 'code' => 'bar(); + } + ', + 'assertions' => [], + 'ignored_issues' => [], + 'php_version' => '8.5', + ]; + + yield 'noDiscardOnAbstractMethodNotInherited' => [ + 'code' => 'bar(); + } + ', + 'assertions' => [], + 'ignored_issues' => [], + 'php_version' => '8.5', + ]; + + yield 'noDiscardResultDiscardedViaVoidCastOnMethods' => [ + 'code' => 'bar(); + (void) Foo::baz(); + echo "still reachable"; + ', + 'assertions' => [], + 'ignored_issues' => [], + 'php_version' => '8.5', + ]; + + yield 'noDiscardSuppressedViaAnnotation' => [ + 'code' => ' [], + 'ignored_issues' => [], + 'php_version' => '8.5', + ]; + + yield 'noDiscardResultUsedInMatchAndTernaryAndBinaryOp' => [ + 'code' => ' f() }; + echo $x + $y + f(); + } + ', + 'assertions' => [], + 'ignored_issues' => [], + 'php_version' => '8.5', + ]; + + yield 'noDiscardNamedMessageArgumentUsed' => [ + 'code' => ' [], + 'ignored_issues' => [], + 'php_version' => '8.5', + ]; + + yield 'noDiscardAttributeNotEnforcedBelowPhp85' => [ + 'code' => ' [], + 'ignored_issues' => [], + 'php_version' => '8.4', + ]; + + yield 'noDiscardOverrideThatDropsAttribute' => [ + 'code' => 'foo(); + ', + 'assertions' => [], + 'ignored_issues' => [], + 'php_version' => '8.5', + ]; + + yield 'noDiscardFirstClassCallableCreation' => [ + 'code' => ' [], + 'ignored_issues' => [], + 'php_version' => '8.5', + ]; + + yield 'noDiscardNullsafeMethodReturnValueUsed' => [ + 'code' => 'bar(); + echo $x ?? 0; + } + ', + 'assertions' => [], + 'ignored_issues' => [], + 'php_version' => '8.5', + ]; + } + + #[Override] + public function providerInvalidCodeParse(): iterable + { + yield 'noDiscardFunctionReturnValueDiscarded' => [ + 'code' => ' 'UnusedFunctionCall', + 'ignored_issues' => [], + 'php_version' => '8.5', + ]; + + yield 'noDiscardFunctionWithMessageDiscarded' => [ + 'code' => ' 'UnusedFunctionCall', + 'ignored_issues' => [], + 'php_version' => '8.5', + ]; + + yield 'noDiscardInstanceMethodReturnValueDiscarded' => [ + 'code' => 'bar(); + ', + 'error_message' => 'UnusedMethodCall', + 'ignored_issues' => [], + 'php_version' => '8.5', + ]; + + yield 'noDiscardStaticMethodReturnValueDiscarded' => [ + 'code' => ' 'UnusedMethodCall', + 'ignored_issues' => [], + 'php_version' => '8.5', + ]; + + yield 'noDiscardInheritedMethodReturnValueDiscarded' => [ + 'code' => 'foo(); + ', + 'error_message' => 'UnusedMethodCall', + 'ignored_issues' => [], + 'php_version' => '8.5', + ]; + + yield 'noDiscardNullsafeMethodReturnValueDiscarded' => [ + 'code' => 'bar(); + } + ', + 'error_message' => 'UnusedMethodCall', + 'ignored_issues' => [], + 'php_version' => '8.5', + ]; + + yield 'noDiscardTraitMethodReturnValueDiscarded' => [ + 'code' => 'bar(); + ', + 'error_message' => 'UnusedMethodCall', + 'ignored_issues' => [], + 'php_version' => '8.5', + ]; + + yield 'noDiscardOnVoidReturnIsInvalidDeclaration' => [ + 'code' => ' 'InvalidAttribute', + 'ignored_issues' => [], + 'php_version' => '8.5', + ]; + + yield 'noDiscardOnNeverReturnIsInvalidDeclaration' => [ + 'code' => ' 'InvalidAttribute', + 'ignored_issues' => [], + 'php_version' => '8.5', + ]; + + yield 'voidCastInValuePositionIsInvalid' => [ + 'code' => ' 'InvalidCast', + 'ignored_issues' => [], + 'php_version' => '8.5', + ]; + + yield 'noDiscardNamedMessageArgumentDiscarded' => [ + 'code' => ' 'UnusedFunctionCall', + 'ignored_issues' => [], + 'php_version' => '8.5', + ]; + } +}