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: 6 additions & 0 deletions phpstan-baseline.neon
Original file line number Diff line number Diff line change
Expand Up @@ -708,6 +708,12 @@ parameters:
count: 2
path: src/Rules/RuleErrorBuilder.php

-
rawMessage: Doing instanceof PHPStan\Type\Generic\GenericObjectType is error-prone and deprecated.
identifier: phpstanApi.instanceofType
count: 1
path: src/Rules/RuleLevelHelper.php

-
rawMessage: Doing instanceof PHPStan\Type\IntersectionType is error-prone and deprecated.
identifier: phpstanApi.instanceofType
Expand Down
25 changes: 23 additions & 2 deletions src/Rules/RuleLevelHelper.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
use PHPStan\Type\CallableType;
use PHPStan\Type\ClosureType;
use PHPStan\Type\ErrorType;
use PHPStan\Type\Generic\GenericObjectType;
use PHPStan\Type\Generic\TemplateMixedType;
use PHPStan\Type\IntersectionType;
use PHPStan\Type\MixedType;
Expand Down Expand Up @@ -49,6 +50,22 @@ public function __construct(
{
}

/**
* Skip the mapper on this node and map its children instead. For a generic
* object that avoids applying the nullability relaxation to invariant type
* arguments; GenericObjectType::traverse() rebuilds the same subclass.
*
* @param callable(Type): Type $traverse
*/
private function traverseWithoutMapping(Type $type, callable $traverse): Type
{
if ($type instanceof GenericObjectType) {
return $type->traverse($traverse);

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.

This looks promising but I don't think it's that easy.

Currently when you have @template-covariant and you're trying to pass Foo<int|null> into Foo<int> below level 8, it doesn't report an error.

But on the other hand, what we're trying to fix is passing @template (invariant) Foo<int|null> into Foo<int|null> below level 8, it SHOULD NOT report an error.

And also, passing Foo<int> into Foo<int|null> should not be allowed for @template below level 8.

Please verify my statements, I'm not sure all of that is still true. And on which levels these start or stop being reported.

That's why in our experiments we were trying to solve it with this #4210

}

return $traverse($type);
}

/** @api */
public function isThis(Expr $expression): bool
{
Expand Down Expand Up @@ -95,7 +112,7 @@ private function transformAcceptedType(Type $acceptingType, Type $acceptedType):

return new CallableType(
$acceptedType->getParameters(),
$traverse($acceptedType->getReturnType()),
$this->traverseWithoutMapping($acceptedType->getReturnType(), $traverse),
$acceptedType->isVariadic(),
$acceptedType->getTemplateTypeMap(),
$acceptedType->getResolvedTemplateTypeMap(),
Expand All @@ -111,7 +128,7 @@ private function transformAcceptedType(Type $acceptingType, Type $acceptedType):

return new ClosureType(
$acceptedType->getParameters(),
$traverse($acceptedType->getReturnType()),
$this->traverseWithoutMapping($acceptedType->getReturnType(), $traverse),
$acceptedType->isVariadic(),
$acceptedType->getTemplateTypeMap(),
$acceptedType->getResolvedTemplateTypeMap(),
Expand All @@ -127,6 +144,10 @@ private function transformAcceptedType(Type $acceptingType, Type $acceptedType):
);
}

if ($acceptedType instanceof GenericObjectType) {
return $acceptedType->traverse($traverse);
}

if (
!$this->checkNullables
&& !$acceptingType instanceof NullType
Expand Down
15 changes: 15 additions & 0 deletions tests/PHPStan/Rules/Methods/CallMethodsRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -3023,6 +3023,21 @@ public function testCallablesWithoutCheckNullables(bool $checkNullables, bool $c
$this->analyse([__DIR__ . '/data/callables-without-check-nullables.php'], $expectedErrors);
}

public static function dataGenericArgumentNullability(): iterable
{
yield [false];
yield [true];
}

#[DataProvider('dataGenericArgumentNullability')]
public function testGenericArgumentNullability(bool $checkNullables): void
{
$this->checkThisOnly = false;
$this->checkNullables = $checkNullables;
$this->checkUnionTypes = true;
$this->analyse([__DIR__ . '/data/generic-argument-nullability.php'], []);
}

#[RequiresPhp('>= 8.0.0')]
public function testBug8713(): void
{
Expand Down
57 changes: 57 additions & 0 deletions tests/PHPStan/Rules/Methods/data/generic-argument-nullability.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
<?php declare(strict_types = 1);

namespace GenericArgumentNullability;

/**
* @template TKey of array-key
* @template TValue
*/
class Collection
{

}

class Foo
{

/**
* @template T
* @param callable(): T $cb
* @return T
*/
public function grab(callable $cb)
{
return $cb();
}

/**
* @template T
* @param callable(mixed, array<string, mixed>): T $cb
* @return T
*/
public function grabTwoArgs(callable $cb)
{
return $cb(null, []);
}

/** @param Collection<string, int|null> $collection */
public function acceptNullable(Collection $collection): void
{
}

}

/**
* @param Collection<string, int> $plain
* @param Collection<string, int|null> $nullable
* @param array<string, int|null> $array
*/
function test(Foo $foo, Collection $plain, Collection $nullable, array $array, ?int $scalar): void
{
$foo->grab(fn () => $plain);
$foo->grab(fn () => $nullable);
$foo->grab(fn () => $array);
$foo->grab(fn () => $scalar);
$foo->grabTwoArgs(fn () => $nullable);
$foo->acceptNullable($nullable);
}
Loading