Skip to content

Native type does not know anything about purity #3797

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Feb 26, 2025
Merged
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
88 changes: 71 additions & 17 deletions src/Rules/PhpDoc/VarTagTypeRuleHelper.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,16 @@

use PhpParser\Node;
use PhpParser\Node\Expr;
use PHPStan\Analyser\NameScope;
use PHPStan\Analyser\Scope;
use PHPStan\Node\Expr\GetOffsetValueTypeExpr;
use PHPStan\PhpDoc\NameScopeAlreadyBeingCreatedException;
use PHPStan\PhpDoc\Tag\VarTag;
use PHPStan\PhpDoc\TypeNodeResolver;
use PHPStan\Rules\IdentifierRuleError;
use PHPStan\Rules\RuleErrorBuilder;
use PHPStan\Type\ArrayType;
use PHPStan\Type\FileTypeMapper;
use PHPStan\Type\Generic\GenericObjectType;
use PHPStan\Type\MixedType;
use PHPStan\Type\ObjectType;
Expand All @@ -24,7 +28,12 @@
final class VarTagTypeRuleHelper
{

public function __construct(private bool $checkTypeAgainstPhpDocType, private bool $strictWideningCheck)
public function __construct(
private TypeNodeResolver $typeNodeResolver,
private FileTypeMapper $fileTypeMapper,
private bool $checkTypeAgainstPhpDocType,
private bool $strictWideningCheck,
)
{
}

Expand Down Expand Up @@ -76,7 +85,7 @@ public function checkExprType(Scope $scope, Node\Expr $expr, Type $varTagType):
$errors = [];
$exprNativeType = $scope->getNativeType($expr);
$containsPhpStanType = $this->containsPhpStanType($varTagType);
if ($this->shouldVarTagTypeBeReported($expr, $exprNativeType, $varTagType)) {
if ($this->shouldVarTagTypeBeReported($scope, $expr, $exprNativeType, $varTagType)) {
$verbosity = VerbosityLevel::getRecommendedLevelByType($exprNativeType, $varTagType);
$errors[] = RuleErrorBuilder::message(sprintf(
'PHPDoc tag @var with type %s is not subtype of native type %s.',
Expand All @@ -86,7 +95,7 @@ public function checkExprType(Scope $scope, Node\Expr $expr, Type $varTagType):
} else {
$exprType = $scope->getType($expr);
if (
$this->shouldVarTagTypeBeReported($expr, $exprType, $varTagType)
$this->shouldVarTagTypeBeReported($scope, $expr, $exprType, $varTagType)
&& ($this->checkTypeAgainstPhpDocType || $containsPhpStanType)
) {
$verbosity = VerbosityLevel::getRecommendedLevelByType($exprType, $varTagType);
Expand Down Expand Up @@ -127,22 +136,22 @@ private function containsPhpStanType(Type $type): bool
return false;
}

private function shouldVarTagTypeBeReported(Node\Expr $expr, Type $type, Type $varTagType): bool
private function shouldVarTagTypeBeReported(Scope $scope, Node\Expr $expr, Type $type, Type $varTagType): bool
{
if ($expr instanceof Expr\Array_) {
if ($expr->items === []) {
$type = new ArrayType(new MixedType(), new MixedType());
}

return $type->isSuperTypeOf($varTagType)->no();
return !$this->isAtLeastMaybeSuperTypeOfVarType($scope, $type, $varTagType);
}

if ($expr instanceof Expr\ConstFetch) {
return $type->isSuperTypeOf($varTagType)->no();
return !$this->isAtLeastMaybeSuperTypeOfVarType($scope, $type, $varTagType);
}

if ($expr instanceof Node\Scalar) {
return $type->isSuperTypeOf($varTagType)->no();
return !$this->isAtLeastMaybeSuperTypeOfVarType($scope, $type, $varTagType);
}

if ($expr instanceof Expr\New_) {
Expand All @@ -151,42 +160,87 @@ private function shouldVarTagTypeBeReported(Node\Expr $expr, Type $type, Type $v
}
}

return $this->checkType($type, $varTagType);
return $this->checkType($scope, $type, $varTagType);
}

private function checkType(Type $type, Type $varTagType, int $depth = 0): bool
private function checkType(Scope $scope, Type $type, Type $varTagType, int $depth = 0): bool
{
if ($this->strictWideningCheck) {
return !$type->isSuperTypeOf($varTagType)->yes();
return !$this->isSuperTypeOfVarType($scope, $type, $varTagType);
}

if ($type->isConstantArray()->yes()) {
if ($type->isIterableAtLeastOnce()->no()) {
$type = new ArrayType(new MixedType(), new MixedType());
return $type->isSuperTypeOf($varTagType)->no();
return !$this->isAtLeastMaybeSuperTypeOfVarType($scope, $type, $varTagType);
}
}

if ($type->isIterable()->yes() && $varTagType->isIterable()->yes()) {
if ($type->isSuperTypeOf($varTagType)->no()) {
if (!$this->isAtLeastMaybeSuperTypeOfVarType($scope, $type, $varTagType)) {
return true;
}

$innerType = $type->getIterableValueType();
$innerVarTagType = $varTagType->getIterableValueType();

if ($type->equals($innerType) || $varTagType->equals($innerVarTagType)) {
return !$innerType->isSuperTypeOf($innerVarTagType)->yes();
return !$this->isSuperTypeOfVarType($scope, $innerType, $innerVarTagType);
}

return $this->checkType($innerType, $innerVarTagType, $depth + 1);
return $this->checkType($scope, $innerType, $innerVarTagType, $depth + 1);
}

if ($type->isConstantValue()->yes() && $depth === 0) {
return $type->isSuperTypeOf($varTagType)->no();
if ($depth === 0 && $type->isConstantValue()->yes()) {
return !$this->isAtLeastMaybeSuperTypeOfVarType($scope, $type, $varTagType);
}

return !$type->isSuperTypeOf($varTagType)->yes();
return !$this->isSuperTypeOfVarType($scope, $type, $varTagType);
}

private function isSuperTypeOfVarType(Scope $scope, Type $type, Type $varTagType): bool
{
if ($type->isSuperTypeOf($varTagType)->yes()) {
return true;
}

try {
$type = $this->typeNodeResolver->resolve($type->toPhpDocNode(), $this->createNameScope($scope));
} catch (NameScopeAlreadyBeingCreatedException) {
return true;
}

return $type->isSuperTypeOf($varTagType)->yes();
}

private function isAtLeastMaybeSuperTypeOfVarType(Scope $scope, Type $type, Type $varTagType): bool
{
if (!$type->isSuperTypeOf($varTagType)->no()) {
return true;
}

try {
$type = $this->typeNodeResolver->resolve($type->toPhpDocNode(), $this->createNameScope($scope));
} catch (NameScopeAlreadyBeingCreatedException) {
return true;
}

return !$type->isSuperTypeOf($varTagType)->no();
}

/**
* @throws NameScopeAlreadyBeingCreatedException
*/
private function createNameScope(Scope $scope): NameScope
{
$function = $scope->getFunction();

return $this->fileTypeMapper->getNameScope(
$scope->getFile(),
$scope->isInClass() ? $scope->getClassReflection()->getName() : null,
$scope->isInTrait() ? $scope->getTraitReflection()->getName() : null,
$function !== null ? $function->getName() : null,
);
}

}
4 changes: 1 addition & 3 deletions tests/PHPStan/Analyser/AnalyserIntegrationTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -1198,9 +1198,7 @@ public function testBug5091(): void
public function testBug9459(): void
{
$errors = $this->runAnalyse(__DIR__ . '/data/bug-9459.php');
$this->assertCount(1, $errors);
$this->assertSame('PHPDoc tag @var with type callable(): array is not subtype of native type Closure(): array{}.', $errors[0]->getMessage());
$this->assertSame(10, $errors[0]->getLine());
$this->assertCount(0, $errors);
}

public function testBug9573(): void
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@

namespace PHPStan\Rules\PhpDoc;

use PHPStan\PhpDoc\TypeNodeResolver;
use PHPStan\Rules\Rule;
use PHPStan\Testing\RuleTestCase;
use PHPStan\Type\FileTypeMapper;

/**
* @extends RuleTestCase<VarTagChangedExpressionTypeRule>
Expand All @@ -13,7 +15,12 @@ class VarTagChangedExpressionTypeRuleTest extends RuleTestCase

protected function getRule(): Rule
{
return new VarTagChangedExpressionTypeRule(new VarTagTypeRuleHelper(true, true));
return new VarTagChangedExpressionTypeRule(new VarTagTypeRuleHelper(
self::getContainer()->getByType(TypeNodeResolver::class),
self::getContainer()->getByType(FileTypeMapper::class),
true,
true,
));
}

public function testRule(): void
Expand Down
17 changes: 16 additions & 1 deletion tests/PHPStan/Rules/PhpDoc/WrongVariableNameInVarTagRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

namespace PHPStan\Rules\PhpDoc;

use PHPStan\PhpDoc\TypeNodeResolver;
use PHPStan\Rules\Rule;
use PHPStan\Testing\RuleTestCase;
use PHPStan\Type\FileTypeMapper;
Expand All @@ -23,7 +24,12 @@ protected function getRule(): Rule
{
return new WrongVariableNameInVarTagRule(
self::getContainer()->getByType(FileTypeMapper::class),
new VarTagTypeRuleHelper($this->checkTypeAgainstPhpDocType, $this->strictWideningCheck),
new VarTagTypeRuleHelper(
self::getContainer()->getByType(TypeNodeResolver::class),
self::getContainer()->getByType(FileTypeMapper::class),
$this->checkTypeAgainstPhpDocType,
$this->strictWideningCheck,
),
$this->checkTypeAgainstNativeType,
);
}
Expand Down Expand Up @@ -182,6 +188,15 @@ public function testBug4505(): void
$this->analyse([__DIR__ . '/data/bug-4505.php'], []);
}

public function testBug12458(): void
{
$this->checkTypeAgainstNativeType = true;
$this->checkTypeAgainstPhpDocType = true;
$this->strictWideningCheck = true;

$this->analyse([__DIR__ . '/data/bug-12458.php'], []);
}

public function testEnums(): void
{
if (PHP_VERSION_ID < 80100) {
Expand Down
29 changes: 29 additions & 0 deletions tests/PHPStan/Rules/PhpDoc/data/bug-12458.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
<?php declare(strict_types = 1);

namespace Bug12458;

class HelloWorld
{
/**
* @param list<HelloWorld> $a
*/
public function test(array $a): void
{
/** @var \Closure(): list<HelloWorld> $c */
$c = function () use ($a): array {
return $a;
};
}

/**
* @template T of HelloWorld
* @param list<T> $a
*/
public function testGeneric(array $a): void
{
/** @var \Closure(): list<T> $c */
$c = function () use ($a): array {
return $a;
};
}
}
Loading