Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
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
87 changes: 52 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,46 @@
$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.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()];

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()];
$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);
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 119 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; }

Check warning on line 119 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; }
return false;
}

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

Check warning on line 123 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 123 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; }

Check warning on line 123 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 123 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; }
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 (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());
}

}
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 (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());
}

}
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 (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());
}

}
Loading
Loading