diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 733646773b2..8911da36868 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -300,12 +300,6 @@ parameters: count: 1 path: src/PhpDoc/PhpDocNodeResolver.php - - - rawMessage: 'Method PHPStan\PhpDoc\ResolvedPhpDocBlock::getNameScope() should return PHPStan\Analyser\NameScope but returns PHPStan\Analyser\NameScope|null.' - identifier: return.type - count: 1 - path: src/PhpDoc/ResolvedPhpDocBlock.php - - rawMessage: 'Doing instanceof PHPStan\Type\ArrayType is error-prone and deprecated. Use Type::isArray() or Type::getArrays() instead.' identifier: phpstanApi.instanceofType diff --git a/src/PhpDoc/ResolvedPhpDocBlock.php b/src/PhpDoc/ResolvedPhpDocBlock.php index be6da4fc46d..ecea68dd27c 100644 --- a/src/PhpDoc/ResolvedPhpDocBlock.php +++ b/src/PhpDoc/ResolvedPhpDocBlock.php @@ -37,6 +37,7 @@ use PHPStan\Type\TypeTraverser; use function array_key_exists; use function array_map; +use function assert; use function count; use function is_bool; use function substr; @@ -444,6 +445,8 @@ public function getFilename(): ?string private function getNameScope(): NameScope { + assert($this->nameScope !== null); + return $this->nameScope; } diff --git a/src/Rules/FunctionReturnTypeCheck.php b/src/Rules/FunctionReturnTypeCheck.php index e61965ca3a3..801c90f19ec 100644 --- a/src/Rules/FunctionReturnTypeCheck.php +++ b/src/Rules/FunctionReturnTypeCheck.php @@ -9,6 +9,7 @@ use PHPStan\DependencyInjection\AutowiredService; use PHPStan\Type\ErrorType; use PHPStan\Type\NeverType; +use PHPStan\Type\NullType; use PHPStan\Type\Type; use PHPStan\Type\TypeUtils; use PHPStan\Type\VerbosityLevel; @@ -35,17 +36,19 @@ public function checkReturnType( string $typeMismatchMessage, string $neverMessage, bool $isGenerator, + ?Type $nativeReturnType = null, ): array { $returnType = TypeUtils::resolveLateResolvableTypes($returnType); if ($returnType instanceof NeverType && $returnType->isExplicit()) { - return [ - RuleErrorBuilder::message($neverMessage) - ->line($returnNode->getStartLine()) - ->identifier('return.never') - ->build(), - ]; + $builder = RuleErrorBuilder::message($neverMessage) + ->line($returnNode->getStartLine()) + ->identifier('return.never'); + if ($nativeReturnType instanceof NeverType && $nativeReturnType->isExplicit()) { + $builder->nonIgnorable(); + } + return [$builder->build()]; } if ($isGenerator) { @@ -62,15 +65,16 @@ public function checkReturnType( return []; } - return [ - RuleErrorBuilder::message(sprintf( - $emptyReturnStatementMessage, - $returnType->describe($verbosityLevel), - )) - ->line($returnNode->getStartLine()) - ->identifier('return.empty') - ->build(), - ]; + $builder = RuleErrorBuilder::message(sprintf( + $emptyReturnStatementMessage, + $returnType->describe($verbosityLevel), + )) + ->line($returnNode->getStartLine()) + ->identifier('return.empty'); + if ($nativeReturnType !== null && $this->isNativeTypeViolated($nativeReturnType, new NullType(), $scope)) { + $builder->nonIgnorable(); + } + return [$builder->build()]; } if ($returnNode instanceof Expr\Yield_ || $returnNode instanceof Expr\YieldFrom) { @@ -81,33 +85,46 @@ public function checkReturnType( $verbosityLevel = VerbosityLevel::getRecommendedLevelByType($returnType, $returnValueType); if ($isVoidSuperType->yes()) { - return [ - RuleErrorBuilder::message(sprintf( - $voidMessage, - $returnValueType->describe($verbosityLevel), - )) - ->line($returnNode->getStartLine()) - ->identifier('return.void') - ->build(), - ]; + $builder = RuleErrorBuilder::message(sprintf( + $voidMessage, + $returnValueType->describe($verbosityLevel), + )) + ->line($returnNode->getStartLine()) + ->identifier('return.void'); + if ($nativeReturnType !== null && $nativeReturnType->isVoid()->yes()) { + $builder->nonIgnorable(); + } + return [$builder->build()]; } $accepts = $this->ruleLevelHelper->accepts($returnType, $returnValueType, $scope->isDeclareStrictTypes()); if (!$accepts->result) { - return [ - RuleErrorBuilder::message(sprintf( - $typeMismatchMessage, - $returnType->describe($verbosityLevel), - $returnValueType->describe($verbosityLevel), - )) - ->line($returnNode->getStartLine()) - ->identifier('return.type') - ->acceptsReasonsTip($accepts->reasons) - ->build(), - ]; + $builder = RuleErrorBuilder::message(sprintf( + $typeMismatchMessage, + $returnType->describe($verbosityLevel), + $returnValueType->describe($verbosityLevel), + )) + ->line($returnNode->getStartLine()) + ->identifier('return.type') + ->acceptsReasonsTip($accepts->reasons); + return [$builder->build()]; } return []; } + private function isNativeTypeViolated(Type $nativeReturnType, Type $nativeValueType, Scope $scope): bool + { + $accepts = $nativeReturnType->accepts($nativeValueType, $scope->isDeclareStrictTypes()); + if ($accepts->yes()) { + return false; + } + + if (!$scope->isDeclareStrictTypes() && $nativeReturnType->isScalar()->yes() && $nativeValueType->isScalar()->yes()) { + return false; + } + + return true; + } + } diff --git a/src/Rules/Functions/ArrowFunctionReturnTypeRule.php b/src/Rules/Functions/ArrowFunctionReturnTypeRule.php index ab1fecfe737..3a023bdd95c 100644 --- a/src/Rules/Functions/ArrowFunctionReturnTypeRule.php +++ b/src/Rules/Functions/ArrowFunctionReturnTypeRule.php @@ -12,6 +12,8 @@ use PHPStan\ShouldNotHappenException; use PHPStan\Type\NeverType; use PHPStan\Type\ObjectType; +use PHPStan\Type\ParserNodeTypeToPHPStanType; +use PHPStan\Type\Type; /** * @implements Rule @@ -54,6 +56,8 @@ public function processNode(Node $node, Scope $scope): array return []; } + $nativeReturnType = $this->resolveNativeReturnType($node, $scope); + return $this->returnTypeCheck->checkReturnType( $scope, $returnType, @@ -64,7 +68,18 @@ public function processNode(Node $node, Scope $scope): array 'Anonymous function should return %s but returns %s.', 'Anonymous function should never return but return statement found.', $generatorType->isSuperTypeOf($returnType)->yes(), + $nativeReturnType, ); } + private function resolveNativeReturnType(InArrowFunctionNode $node, Scope $scope): ?Type + { + $returnTypeNode = $node->getOriginalNode()->returnType; + if ($returnTypeNode === null) { + return null; + } + + return ParserNodeTypeToPHPStanType::resolve($returnTypeNode, $scope->getClassReflection()); + } + } diff --git a/src/Rules/Functions/ClosureReturnTypeRule.php b/src/Rules/Functions/ClosureReturnTypeRule.php index abeb7c3866d..d4604cfe59b 100644 --- a/src/Rules/Functions/ClosureReturnTypeRule.php +++ b/src/Rules/Functions/ClosureReturnTypeRule.php @@ -8,6 +8,8 @@ use PHPStan\Node\ClosureReturnStatementsNode; use PHPStan\Rules\FunctionReturnTypeCheck; use PHPStan\Rules\Rule; +use PHPStan\Type\ParserNodeTypeToPHPStanType; +use PHPStan\Type\Type; use PHPStan\Type\TypeCombinator; /** @@ -35,6 +37,7 @@ public function processNode(Node $node, Scope $scope): array $returnType = $scope->getAnonymousFunctionReturnType(); $containsNull = TypeCombinator::containsNull($returnType); $hasNativeTypehint = $node->getClosureExpr()->returnType !== null; + $nativeReturnType = $this->resolveNativeReturnType($node, $scope); $messages = []; foreach ($node->getReturnStatements() as $returnStatement) { @@ -53,6 +56,7 @@ public function processNode(Node $node, Scope $scope): array 'Anonymous function should return %s but returns %s.', 'Anonymous function should never return but return statement found.', $node->isGenerator(), + $nativeReturnType, ); foreach ($returnMessages as $returnMessage) { @@ -63,4 +67,14 @@ public function processNode(Node $node, Scope $scope): array return $messages; } + private function resolveNativeReturnType(ClosureReturnStatementsNode $node, Scope $scope): ?Type + { + $returnTypeNode = $node->getClosureExpr()->returnType; + if ($returnTypeNode === null) { + return null; + } + + return ParserNodeTypeToPHPStanType::resolve($returnTypeNode, $scope->getClassReflection()); + } + } diff --git a/src/Rules/Functions/ReturnTypeRule.php b/src/Rules/Functions/ReturnTypeRule.php index 157f03f2b27..74bec5f67b3 100644 --- a/src/Rules/Functions/ReturnTypeRule.php +++ b/src/Rules/Functions/ReturnTypeRule.php @@ -66,6 +66,7 @@ public function processNode(Node $node, Scope $scope): array $function->getName(), ), $function->isGenerator(), + $function->getNativeReturnType(), ); } diff --git a/src/Rules/Methods/ReturnTypeRule.php b/src/Rules/Methods/ReturnTypeRule.php index 9c9882fedf5..9fd82431ced 100644 --- a/src/Rules/Methods/ReturnTypeRule.php +++ b/src/Rules/Methods/ReturnTypeRule.php @@ -89,6 +89,7 @@ public function processNode(Node $node, Scope $scope): array $methodDescription, ), $method->isGenerator(), + $method->getNativeReturnType(), ); if ( diff --git a/tests/PHPStan/Rules/Functions/ArrowFunctionReturnTypeRuleTest.php b/tests/PHPStan/Rules/Functions/ArrowFunctionReturnTypeRuleTest.php index 23093600316..997fd0e1d3b 100644 --- a/tests/PHPStan/Rules/Functions/ArrowFunctionReturnTypeRuleTest.php +++ b/tests/PHPStan/Rules/Functions/ArrowFunctionReturnTypeRuleTest.php @@ -77,4 +77,40 @@ public function testBugFunctionMethodConstants(): void $this->analyse([__DIR__ . '/data/bug-anonymous-function-method-constant.php'], []); } + public function testBug9833(): void + { + $this->analyse([__DIR__ . '/data/bug-9833-arrow.php'], [ + [ + 'Anonymous function should return array but returns null.', + 5, + ], + [ + 'Anonymous function should return int but returns null.', + 11, + ], + ]); + } + + public function testBug9833NonIgnorable(): void + { + $errors = $this->gatherAnalyserErrors([__DIR__ . '/data/bug-9833-arrow.php']); + + $errorsByLine = []; + foreach ($errors as $error) { + $line = $error->getLine(); + if ($line === null) { + continue; + } + $errorsByLine[$line] = $error; + } + + // Native array return type violated (return.type) → ignorable + $this->assertArrayHasKey(5, $errorsByLine); + $this->assertTrue($errorsByLine[5]->canBeIgnored()); + + // Native int return type violated (return.type) → ignorable + $this->assertArrayHasKey(11, $errorsByLine); + $this->assertTrue($errorsByLine[11]->canBeIgnored()); + } + } diff --git a/tests/PHPStan/Rules/Functions/ClosureReturnTypeRuleTest.php b/tests/PHPStan/Rules/Functions/ClosureReturnTypeRuleTest.php index c363c194d36..cc699887b67 100644 --- a/tests/PHPStan/Rules/Functions/ClosureReturnTypeRuleTest.php +++ b/tests/PHPStan/Rules/Functions/ClosureReturnTypeRuleTest.php @@ -149,4 +149,40 @@ public function testBug13964(): void $this->analyse([__DIR__ . '/data/bug-13964.php'], []); } + public function testBug9833(): void + { + $this->analyse([__DIR__ . '/data/bug-9833-closure.php'], [ + [ + 'Anonymous function should return array but returns null.', + 6, + ], + [ + 'Anonymous function should return int but returns null.', + 16, + ], + ]); + } + + public function testBug9833NonIgnorable(): void + { + $errors = $this->gatherAnalyserErrors([__DIR__ . '/data/bug-9833-closure.php']); + + $errorsByLine = []; + foreach ($errors as $error) { + $line = $error->getLine(); + if ($line === null) { + continue; + } + $errorsByLine[$line] = $error; + } + + // Native array return type violated (return.type) → ignorable + $this->assertArrayHasKey(6, $errorsByLine); + $this->assertTrue($errorsByLine[6]->canBeIgnored()); + + // Native int return type violated (return.type) → ignorable + $this->assertArrayHasKey(16, $errorsByLine); + $this->assertTrue($errorsByLine[16]->canBeIgnored()); + } + } diff --git a/tests/PHPStan/Rules/Functions/ReturnTypeRuleTest.php b/tests/PHPStan/Rules/Functions/ReturnTypeRuleTest.php index 272fc1a39e9..5c7256a1ac9 100644 --- a/tests/PHPStan/Rules/Functions/ReturnTypeRuleTest.php +++ b/tests/PHPStan/Rules/Functions/ReturnTypeRuleTest.php @@ -438,4 +438,60 @@ public function testBug14428(): void $this->analyse([__DIR__ . '/../../Analyser/nsrt/bug-14428.php'], []); } + public function testBug9833(): void + { + $this->checkNullables = true; + $this->checkExplicitMixed = false; + $this->analyse([__DIR__ . '/data/bug-9833.php'], [ + [ + 'Function Bug9833Functions\nativeArrayReturnsNull() should return array but returns null.', + 8, + ], + [ + 'Function Bug9833Functions\phpDocOnlyReturnsNull() should return array but returns null.', + 17, + ], + [ + 'Function Bug9833Functions\nativeArrayReturnsWrongPhpDoc() should return array but returns array.', + 25, + ], + [ + 'Function Bug9833Functions\nativeIntReturnsNull() should return int but returns null.', + 30, + ], + ]); + } + + public function testBug9833NonIgnorable(): void + { + $this->checkNullables = true; + $this->checkExplicitMixed = false; + $errors = $this->gatherAnalyserErrors([__DIR__ . '/data/bug-9833.php']); + + $errorsByLine = []; + foreach ($errors as $error) { + $line = $error->getLine(); + if ($line === null) { + continue; + } + $errorsByLine[$line] = $error; + } + + // Native array return type violated (return.type) → ignorable + $this->assertArrayHasKey(8, $errorsByLine); + $this->assertTrue($errorsByLine[8]->canBeIgnored()); + + // PHPDoc-only return type violated → ignorable + $this->assertArrayHasKey(17, $errorsByLine); + $this->assertTrue($errorsByLine[17]->canBeIgnored()); + + // Only PHPDoc subtype violated, native type satisfied → ignorable + $this->assertArrayHasKey(25, $errorsByLine); + $this->assertTrue($errorsByLine[25]->canBeIgnored()); + + // Native int return type violated (return.type) → ignorable + $this->assertArrayHasKey(30, $errorsByLine); + $this->assertTrue($errorsByLine[30]->canBeIgnored()); + } + } diff --git a/tests/PHPStan/Rules/Functions/data/bug-9833-arrow.php b/tests/PHPStan/Rules/Functions/data/bug-9833-arrow.php new file mode 100644 index 00000000000..c0cbe630ad3 --- /dev/null +++ b/tests/PHPStan/Rules/Functions/data/bug-9833-arrow.php @@ -0,0 +1,11 @@ + null; + +$phpDocOnlyReturnsNull = + /** @return array */ + fn () => null; + +$nativeIntReturnsNull = fn (): int => null; diff --git a/tests/PHPStan/Rules/Functions/data/bug-9833-closure.php b/tests/PHPStan/Rules/Functions/data/bug-9833-closure.php new file mode 100644 index 00000000000..f54ef3193bc --- /dev/null +++ b/tests/PHPStan/Rules/Functions/data/bug-9833-closure.php @@ -0,0 +1,17 @@ + */ + function () { + return null; + }; + +$nativeIntReturnsNull = function (): int { + return null; +}; diff --git a/tests/PHPStan/Rules/Functions/data/bug-9833.php b/tests/PHPStan/Rules/Functions/data/bug-9833.php new file mode 100644 index 00000000000..48dcee9a741 --- /dev/null +++ b/tests/PHPStan/Rules/Functions/data/bug-9833.php @@ -0,0 +1,47 @@ + */ +function phpDocOnlyReturnsNull() +{ + if (rand(0, 1)) { + return null; + } + return []; +} + +/** @return array */ +function nativeArrayReturnsWrongPhpDoc(): array +{ + return ['a' => 'hello']; +} + +function nativeIntReturnsNull(): int +{ + return null; +} + +$closure = function (): array { + return null; +}; + +$closurePhpDocOnly = + /** @return array */ + function () { + return null; + }; + +$arrow = fn (): array => null; + +$arrowPhpDocOnly = + /** @return array */ + fn () => null; diff --git a/tests/PHPStan/Rules/Methods/ReturnTypeRuleTest.php b/tests/PHPStan/Rules/Methods/ReturnTypeRuleTest.php index a60a6a704a6..1cfbde0a335 100644 --- a/tests/PHPStan/Rules/Methods/ReturnTypeRuleTest.php +++ b/tests/PHPStan/Rules/Methods/ReturnTypeRuleTest.php @@ -1347,4 +1347,64 @@ public function testBug14553(): void $this->analyse([__DIR__ . '/../../Analyser/nsrt/bug-14553.php'], []); } + public function testBug9833(): void + { + $this->analyse([__DIR__ . '/data/bug-9833.php'], [ + [ + 'Method Bug9833\HelloWorld::nativeArrayReturnsNull() should return array but returns null.', + 10, + ], + [ + 'Method Bug9833\HelloWorld::phpDocOnlyReturnsNull() should return array but returns null.', + 19, + ], + [ + 'Method Bug9833\HelloWorld::nativeArrayReturnsWrongPhpDoc() should return array but returns array.', + 27, + ], + [ + 'Method Bug9833\HelloWorld::nativeIntReturnsNull() should return int but returns null.', + 32, + ], + [ + 'Method Bug9833\HelloWorld::nativeStringReturnsNull() should return string but returns null.', + 37, + ], + ]); + } + + public function testBug9833NonIgnorable(): void + { + $errors = $this->gatherAnalyserErrors([__DIR__ . '/data/bug-9833.php']); + + $errorsByLine = []; + foreach ($errors as $error) { + $line = $error->getLine(); + if ($line === null) { + continue; + } + $errorsByLine[$line] = $error; + } + + // Native array return type violated (return.type) → ignorable + $this->assertArrayHasKey(10, $errorsByLine); + $this->assertTrue($errorsByLine[10]->canBeIgnored()); + + // PHPDoc-only return type violated → ignorable + $this->assertArrayHasKey(19, $errorsByLine); + $this->assertTrue($errorsByLine[19]->canBeIgnored()); + + // Only PHPDoc subtype violated, native type satisfied → ignorable + $this->assertArrayHasKey(27, $errorsByLine); + $this->assertTrue($errorsByLine[27]->canBeIgnored()); + + // Native int return type violated (return.type) → ignorable + $this->assertArrayHasKey(32, $errorsByLine); + $this->assertTrue($errorsByLine[32]->canBeIgnored()); + + // Native string return type violated (return.type) → ignorable + $this->assertArrayHasKey(37, $errorsByLine); + $this->assertTrue($errorsByLine[37]->canBeIgnored()); + } + } diff --git a/tests/PHPStan/Rules/Methods/data/bug-9833.php b/tests/PHPStan/Rules/Methods/data/bug-9833.php new file mode 100644 index 00000000000..046cfba074d --- /dev/null +++ b/tests/PHPStan/Rules/Methods/data/bug-9833.php @@ -0,0 +1,39 @@ + */ + public function phpDocOnlyReturnsNull() + { + if (rand(0, 1)) { + return null; + } + return []; + } + + /** @return array */ + public function nativeArrayReturnsWrongPhpDoc(): array + { + return ['a' => 'hello']; + } + + public function nativeIntReturnsNull(): int + { + return null; + } + + public function nativeStringReturnsNull(): string + { + return null; + } +}