Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 0 additions & 6 deletions phpstan-baseline.neon
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions src/PhpDoc/ResolvedPhpDocBlock.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -444,6 +445,8 @@ public function getFilename(): ?string

private function getNameScope(): NameScope
{
assert($this->nameScope !== null);

return $this->nameScope;
}

Expand Down
90 changes: 55 additions & 35 deletions src/Rules/FunctionReturnTypeCheck.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -35,17 +36,19 @@
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) {
Expand All @@ -62,15 +65,16 @@
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) {
Expand All @@ -81,33 +85,49 @@
$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()) {

Check warning on line 94 in src/Rules/FunctionReturnTypeCheck.php

View workflow job for this annotation

GitHub Actions / Mutation Testing (8.4, ubuntu-latest)

Escaped Mutant for Mutator "PHPStan\Infection\TrinaryLogicMutator": @@ @@ )) ->line($returnNode->getStartLine()) ->identifier('return.void'); - if ($nativeReturnType !== null && $nativeReturnType->isVoid()->yes()) { + if ($nativeReturnType !== null && !$nativeReturnType->isVoid()->no()) { $builder->nonIgnorable(); } return [$builder->build()];

Check warning on line 94 in src/Rules/FunctionReturnTypeCheck.php

View workflow job for this annotation

GitHub Actions / Mutation Testing (8.3, ubuntu-latest)

Escaped Mutant for Mutator "PHPStan\Infection\TrinaryLogicMutator": @@ @@ )) ->line($returnNode->getStartLine()) ->identifier('return.void'); - if ($nativeReturnType !== null && $nativeReturnType->isVoid()->yes()) { + if ($nativeReturnType !== null && !$nativeReturnType->isVoid()->no()) { $builder->nonIgnorable(); } return [$builder->build()];
$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')
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I disagree about making return.type non-ignorable, it's too squishy and dependent on surrounding code. I still want people to be able to ignore this (for now).

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. Removed the non-ignorable marking for return.type errors per the reviewer's request. The other three identifiers (return.empty, return.void, return.never) still become non-ignorable when the native return type is violated. All tests pass and make phpstan reports no errors.

->acceptsReasonsTip($accepts->reasons);
if ($nativeReturnType !== null && $this->isNativeTypeViolated($nativeReturnType, $scope->getNativeType($returnValue), $scope)) {
$builder->nonIgnorable();
}
return [$builder->build()];
}

return [];
}

private function isNativeTypeViolated(Type $nativeReturnType, Type $nativeValueType, Scope $scope): bool
{
$accepts = $nativeReturnType->accepts($nativeValueType, $scope->isDeclareStrictTypes());
if ($accepts->yes()) {

Check warning on line 122 in src/Rules/FunctionReturnTypeCheck.php

View workflow job for this annotation

GitHub Actions / Mutation Testing (8.4, ubuntu-latest)

Escaped Mutant for Mutator "PHPStan\Infection\TrinaryLogicMutator": @@ @@ private function isNativeTypeViolated(Type $nativeReturnType, Type $nativeValueType, Scope $scope): bool { $accepts = $nativeReturnType->accepts($nativeValueType, $scope->isDeclareStrictTypes()); - if ($accepts->yes()) { + if (!$accepts->no()) { return false; }

Check warning on line 122 in src/Rules/FunctionReturnTypeCheck.php

View workflow job for this annotation

GitHub Actions / Mutation Testing (8.3, ubuntu-latest)

Escaped Mutant for Mutator "PHPStan\Infection\TrinaryLogicMutator": @@ @@ private function isNativeTypeViolated(Type $nativeReturnType, Type $nativeValueType, Scope $scope): bool { $accepts = $nativeReturnType->accepts($nativeValueType, $scope->isDeclareStrictTypes()); - if ($accepts->yes()) { + if (!$accepts->no()) { return false; }
return false;
}

if (!$scope->isDeclareStrictTypes() && $nativeReturnType->isScalar()->yes() && $nativeValueType->isScalar()->yes()) {

Check warning on line 126 in src/Rules/FunctionReturnTypeCheck.php

View workflow job for this annotation

GitHub Actions / Mutation Testing (8.4, ubuntu-latest)

Escaped Mutant for Mutator "PHPStan\Infection\TrinaryLogicMutator": @@ @@ return false; } - if (!$scope->isDeclareStrictTypes() && $nativeReturnType->isScalar()->yes() && $nativeValueType->isScalar()->yes()) { + if (!$scope->isDeclareStrictTypes() && $nativeReturnType->isScalar()->yes() && !$nativeValueType->isScalar()->no()) { return false; }

Check warning on line 126 in src/Rules/FunctionReturnTypeCheck.php

View workflow job for this annotation

GitHub Actions / Mutation Testing (8.4, ubuntu-latest)

Escaped Mutant for Mutator "PHPStan\Infection\TrinaryLogicMutator": @@ @@ return false; } - if (!$scope->isDeclareStrictTypes() && $nativeReturnType->isScalar()->yes() && $nativeValueType->isScalar()->yes()) { + if (!$scope->isDeclareStrictTypes() && !$nativeReturnType->isScalar()->no() && $nativeValueType->isScalar()->yes()) { return false; }

Check warning on line 126 in src/Rules/FunctionReturnTypeCheck.php

View workflow job for this annotation

GitHub Actions / Mutation Testing (8.3, ubuntu-latest)

Escaped Mutant for Mutator "PHPStan\Infection\TrinaryLogicMutator": @@ @@ return false; } - if (!$scope->isDeclareStrictTypes() && $nativeReturnType->isScalar()->yes() && $nativeValueType->isScalar()->yes()) { + if (!$scope->isDeclareStrictTypes() && $nativeReturnType->isScalar()->yes() && !$nativeValueType->isScalar()->no()) { return false; }

Check warning on line 126 in src/Rules/FunctionReturnTypeCheck.php

View workflow job for this annotation

GitHub Actions / Mutation Testing (8.3, ubuntu-latest)

Escaped Mutant for Mutator "PHPStan\Infection\TrinaryLogicMutator": @@ @@ return false; } - if (!$scope->isDeclareStrictTypes() && $nativeReturnType->isScalar()->yes() && $nativeValueType->isScalar()->yes()) { + if (!$scope->isDeclareStrictTypes() && !$nativeReturnType->isScalar()->no() && $nativeValueType->isScalar()->yes()) { return false; }
return false;
}

return true;
}

}
15 changes: 15 additions & 0 deletions src/Rules/Functions/ArrowFunctionReturnTypeRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -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<InArrowFunctionNode>
Expand Down Expand Up @@ -54,6 +56,8 @@ public function processNode(Node $node, Scope $scope): array
return [];
}

$nativeReturnType = $this->resolveNativeReturnType($node, $scope);

return $this->returnTypeCheck->checkReturnType(
$scope,
$returnType,
Expand All @@ -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());
}

}
14 changes: 14 additions & 0 deletions src/Rules/Functions/ClosureReturnTypeRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand Down Expand Up @@ -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) {
Expand All @@ -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) {
Expand All @@ -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());
}

}
1 change: 1 addition & 0 deletions src/Rules/Functions/ReturnTypeRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ public function processNode(Node $node, Scope $scope): array
$function->getName(),
),
$function->isGenerator(),
$function->getNativeReturnType(),
);
}

Expand Down
1 change: 1 addition & 0 deletions src/Rules/Methods/ReturnTypeRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ public function processNode(Node $node, Scope $scope): array
$methodDescription,
),
$method->isGenerator(),
$method->getNativeReturnType(),
);

if (
Expand Down
36 changes: 36 additions & 0 deletions tests/PHPStan/Rules/Functions/ArrowFunctionReturnTypeRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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 → non-ignorable
$this->assertArrayHasKey(5, $errorsByLine);
$this->assertFalse($errorsByLine[5]->canBeIgnored());

// Native int return type violated → non-ignorable
$this->assertArrayHasKey(11, $errorsByLine);
$this->assertFalse($errorsByLine[11]->canBeIgnored());
}

}
36 changes: 36 additions & 0 deletions tests/PHPStan/Rules/Functions/ClosureReturnTypeRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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 → non-ignorable
$this->assertArrayHasKey(6, $errorsByLine);
$this->assertFalse($errorsByLine[6]->canBeIgnored());

// Native int return type violated → non-ignorable
$this->assertArrayHasKey(16, $errorsByLine);
$this->assertFalse($errorsByLine[16]->canBeIgnored());
}

}
56 changes: 56 additions & 0 deletions tests/PHPStan/Rules/Functions/ReturnTypeRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, int> but returns null.',
17,
],
[
'Function Bug9833Functions\nativeArrayReturnsWrongPhpDoc() should return array<string, int> but returns array<string, string>.',
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 → non-ignorable
$this->assertArrayHasKey(8, $errorsByLine);
$this->assertFalse($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 → non-ignorable
$this->assertArrayHasKey(30, $errorsByLine);
$this->assertFalse($errorsByLine[30]->canBeIgnored());
}

}
Loading
Loading