Skip to content
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

Experiment - do not generalize template types, except when in GenericObjectType #2818

Merged
merged 4 commits into from
Dec 11, 2023
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
22 changes: 19 additions & 3 deletions src/Analyser/MutatingScope.php
Original file line number Diff line number Diff line change
Expand Up @@ -5052,10 +5052,26 @@ private function exactInstantiation(New_ $node, string $className): ?Type
);

if ($this->explicitMixedInUnknownGenericNew) {
return new GenericObjectType(
$resolvedTemplateTypeMap = $parametersAcceptor->getResolvedTemplateTypeMap();
return TypeTraverser::map(new GenericObjectType(
$resolvedClassName,
$classReflection->typeMapToList($parametersAcceptor->getResolvedTemplateTypeMap()),
);
$classReflection->typeMapToList($classReflection->getTemplateTypeMap()),
), static function (Type $type, callable $traverse) use ($resolvedTemplateTypeMap): Type {
if ($type instanceof TemplateType && !$type->isArgument()) {
$newType = $resolvedTemplateTypeMap->getType($type->getName());
if ($newType === null || $newType instanceof ErrorType) {
return $type->getBound();
}

if ($newType->isConstantValue()->yes() && (!$type->getBound()->isScalar()->yes() || $type->getBound()->describe(VerbosityLevel::precise()) === '(int|string)')) {
$newType = $newType->generalize(GeneralizePrecision::templateArgument());
}

return $newType;
}

return $traverse($type);
});
}

$resolvedPhpDoc = $classReflection->getResolvedPhpDoc();
Expand Down
50 changes: 49 additions & 1 deletion src/Reflection/ResolvedFunctionVariant.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,12 @@

namespace PHPStan\Reflection;

use PHPStan\DependencyInjection\BleedingEdgeToggle;
use PHPStan\Reflection\Php\DummyParameterWithPhpDocs;
use PHPStan\Type\ConditionalTypeForParameter;
use PHPStan\Type\ErrorType;
use PHPStan\Type\GeneralizePrecision;
use PHPStan\Type\Generic\GenericObjectType;
use PHPStan\Type\Generic\TemplateType;
use PHPStan\Type\Generic\TemplateTypeHelper;
use PHPStan\Type\Generic\TemplateTypeMap;
Expand All @@ -14,6 +17,7 @@
use PHPStan\Type\Type;
use PHPStan\Type\TypeTraverser;
use PHPStan\Type\TypeUtils;
use PHPStan\Type\VerbosityLevel;
use function array_key_exists;
use function array_map;

Expand Down Expand Up @@ -173,7 +177,51 @@ private function resolveResolvableTemplateTypes(Type $type, TemplateTypeVariance
{
$references = $type->getReferencedTemplateTypes($positionVariance);

return TypeTraverser::map($type, function (Type $type, callable $traverse) use ($references): Type {
$objectCb = function (Type $type, callable $traverse) use ($references): Type {
if ($type instanceof TemplateType && !$type->isArgument()) {
$newType = $this->resolvedTemplateTypeMap->getType($type->getName());
if ($newType === null || $newType instanceof ErrorType) {
return $traverse($type);
}

if ($newType->isConstantValue()->yes() && (!$type->getBound()->isScalar()->yes() || $type->getBound()->describe(VerbosityLevel::precise()) === '(int|string)')) {
$newType = $newType->generalize(GeneralizePrecision::templateArgument());
}

$variance = TemplateTypeVariance::createInvariant();
foreach ($references as $reference) {
// this uses identity to distinguish between different occurrences of the same template type
// see https://github.com/phpstan/phpstan-src/pull/2485#discussion_r1328555397 for details
if ($reference->getType() === $type) {
$variance = $reference->getPositionVariance();
break;
}
}

$callSiteVariance = $this->callSiteVarianceMap->getVariance($type->getName());
if ($callSiteVariance === null || $callSiteVariance->invariant()) {
return $newType;
}

if (!$callSiteVariance->covariant() && $variance->covariant()) {
return $traverse($type->getBound());
}

if (!$callSiteVariance->contravariant() && $variance->contravariant()) {
return new NonAcceptingNeverType();
}

return $newType;
}

return $traverse($type);
};

return TypeTraverser::map($type, function (Type $type, callable $traverse) use ($references, $objectCb): Type {
if (BleedingEdgeToggle::isBleedingEdge() && $type instanceof GenericObjectType) {
return TypeTraverser::map($type, $objectCb);
}

if ($type instanceof TemplateType && !$type->isArgument()) {
$newType = $this->resolvedTemplateTypeMap->getType($type->getName());
if ($newType === null || $newType instanceof ErrorType) {
Expand Down
6 changes: 4 additions & 2 deletions src/Type/AcceptsResult.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
use PHPStan\TrinaryLogic;
use function array_map;
use function array_merge;
use function array_unique;
use function array_values;

/** @api */
class AcceptsResult
Expand Down Expand Up @@ -60,15 +62,15 @@ public function and(self $other): self
{
return new self(
$this->result->and($other->result),
array_merge($this->reasons, $other->reasons),
array_values(array_unique(array_merge($this->reasons, $other->reasons))),
);
}

public function or(self $other): self
{
return new self(
$this->result->or($other->result),
array_merge($this->reasons, $other->reasons),
array_values(array_unique(array_merge($this->reasons, $other->reasons))),
);
}

Expand Down
4 changes: 3 additions & 1 deletion src/Type/Generic/TemplateTypeTrait.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

namespace PHPStan\Type\Generic;

use PHPStan\DependencyInjection\BleedingEdgeToggle;
use PHPStan\PhpDocParser\Ast\Type\IdentifierTypeNode;
use PHPStan\PhpDocParser\Ast\Type\TypeNode;
use PHPStan\TrinaryLogic;
Expand Down Expand Up @@ -265,12 +266,13 @@ public function inferTemplateTypes(Type $receivedType): TemplateTypeMap
TemplateTypeVariance::createStatic(),
));
if ($resolvedBound->isSuperTypeOf($receivedType)->yes()) {
if ($this->shouldGeneralizeInferredType()) {
if (!BleedingEdgeToggle::isBleedingEdge() && $this->shouldGeneralizeInferredType()) {
$generalizedType = $receivedType->generalize(GeneralizePrecision::templateArgument());
if ($resolvedBound->isSuperTypeOf($generalizedType)->yes()) {
$receivedType = $generalizedType;
}
}

return (new TemplateTypeMap([
$this->name => $receivedType,
]))->union($map);
Expand Down
6 changes: 3 additions & 3 deletions tests/PHPStan/Analyser/AnalyserIntegrationTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -782,11 +782,11 @@ public function testDiscussion7124(): void

$errors = $this->runAnalyse(__DIR__ . '/data/discussion-7124.php');
$this->assertCount(4, $errors);
$this->assertSame('Parameter #2 $callback of function Discussion7124\filter expects callable(bool, int=): bool, Closure(int, bool): bool given.', $errors[0]->getMessage());
$this->assertSame('Parameter #2 $callback of function Discussion7124\filter expects callable(bool, 0|1|2=): bool, Closure(int, bool): bool given.', $errors[0]->getMessage());
$this->assertSame(38, $errors[0]->getLine());
$this->assertSame('Parameter #2 $callback of function Discussion7124\filter expects callable(bool, int=): bool, Closure(int): bool given.', $errors[1]->getMessage());
$this->assertSame('Parameter #2 $callback of function Discussion7124\filter expects callable(bool, 0|1|2=): bool, Closure(int): bool given.', $errors[1]->getMessage());
$this->assertSame(45, $errors[1]->getLine());
$this->assertSame('Parameter #2 $callback of function Discussion7124\filter expects callable(int): bool, Closure(bool): bool given.', $errors[2]->getMessage());
$this->assertSame('Parameter #2 $callback of function Discussion7124\filter expects callable(0|1|2): bool, Closure(bool): bool given.', $errors[2]->getMessage());
$this->assertSame(52, $errors[2]->getLine());
$this->assertSame('Parameter #2 $callback of function Discussion7124\filter expects callable(bool): bool, Closure(int): bool given.', $errors[3]->getMessage());
$this->assertSame(59, $errors[3]->getLine());
Expand Down
12 changes: 12 additions & 0 deletions tests/PHPStan/Analyser/NodeScopeResolverTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,17 @@ public function dataFileAsserts(): iterable
yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-3997.php');

yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-4016.php');
yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-8127.php');
yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-7944.php');
yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-6196.php');
yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-7301.php');
yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-9472.php');
yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-9764.php');
yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-10092.php');

if (PHP_VERSION_ID >= 80100) {
yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-9084.php');
}

yield from $this->gatherAssertTypes(__DIR__ . '/data/promoted-properties-types.php');

Expand All @@ -230,6 +241,7 @@ public function dataFileAsserts(): iterable
yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-3915.php');

yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-2378.php');
yield from $this->gatherAssertTypes(__DIR__ . '/data/generics-do-not-generalize.php');
yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-9985.php');
yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-6294.php');
yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-6462.php');
Expand Down
43 changes: 43 additions & 0 deletions tests/PHPStan/Analyser/data/bug-10092.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
<?php

namespace Bug10092;

use function PHPStan\Testing\assertType;

/** @template-covariant T */
interface TypeInterface {}

/** @return TypeInterface<int> */
function int() { }

/** @return TypeInterface<0> */
function zero() { }

/** @return TypeInterface<int<1, max>> */
function positive_int() { }

/** @return TypeInterface<numeric-string> */
function numeric_string() { }


/**
* @template T
*
* @param TypeInterface<T> $first
* @param TypeInterface<T> $second
* @param TypeInterface<T> ...$rest
*
* @return TypeInterface<T>
*/
function union(
TypeInterface $first,
TypeInterface $second,
TypeInterface ...$rest
) {

}

function (): void {
assertType('Bug10092\TypeInterface<int|numeric-string>', union(int(), numeric_string()));
assertType('Bug10092\TypeInterface<int<0, max>>', union(positive_int(), zero()));
};
2 changes: 1 addition & 1 deletion tests/PHPStan/Analyser/data/bug-3351.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ public function sayHello(): void
$b = [1, 2, 3];

$c = $this->combine($a, $b);
assertType('array<string, int>|false', $c);
assertType("array<'a'|'b'|'c', 1|2|3>|false", $c);

assertType('array{a: 1, b: 2, c: 3}', array_combine($a, $b));
}
Expand Down
2 changes: 1 addition & 1 deletion tests/PHPStan/Analyser/data/bug-4545.php
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ function compareMaps(Map $firstMap, Map $secondMap, Closure $comparator): Set
foreach ($intersect as $key) {
assertType('TValue1 (method Bug4545\Foo::compareMaps(), argument)', $firstMap->get($key));
assertType('TValue2 (method Bug4545\Foo::compareMaps(), argument)', $secondMap->get($key));
assertType('int|TValue2 (method Bug4545\Foo::compareMaps(), argument)', $secondMap->get($key, 1));
assertType('1|TValue2 (method Bug4545\Foo::compareMaps(), argument)', $secondMap->get($key, 1));
}

return $keys;
Expand Down
27 changes: 27 additions & 0 deletions tests/PHPStan/Analyser/data/bug-6196.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
<?php

namespace Bug6196;

use function PHPStan\Testing\assertType;

final class ErrorToExceptionHandler{
private function __construct(){

}

/**
* @phpstan-template TReturn
* @phpstan-param \Closure() : TReturn $closure
*
* @phpstan-return TReturn
* @throws \ErrorException
*/
public static function trap(\Closure $closure){
return $closure();
}
}

function (): void {
assertType('string|false', zlib_decode("aaaaaaa"));
assertType('string|false', ErrorToExceptionHandler::trap(fn() => zlib_decode("aaaaaaa")));
};
2 changes: 1 addition & 1 deletion tests/PHPStan/Analyser/data/bug-6433.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ class Foo
{

function x(): void {
assertType('Ds\Set<Bug6433\E>', new Set([E::A, E::B]));
assertType('Ds\Set<Bug6433\E::A|Bug6433\E::B>', new Set([E::A, E::B]));
}

}
Expand Down
2 changes: 1 addition & 1 deletion tests/PHPStan/Analyser/data/bug-6695.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ enum Foo: int

public function toCollection(): void
{
assertType('Bug6695\Collection<int, Bug6695\Foo>', $this->collect(self::cases()));
assertType('Bug6695\Collection<int, Bug6695\Foo::BAR|Bug6695\Foo::BAZ>', $this->collect(self::cases()));
}

/**
Expand Down
4 changes: 2 additions & 2 deletions tests/PHPStan/Analyser/data/bug-7068.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ function merge(array ...$arrays): array {

public function doFoo(): void
{
assertType('array<int>', $this->merge([1, 2], [3, 4], [5]));
assertType('array<int|string>', $this->merge([1, 2], ['foo', 'bar']));
assertType('array<1|2|3|4|5>', $this->merge([1, 2], [3, 4], [5]));
assertType('array<1|2|\'bar\'|\'foo\'>', $this->merge([1, 2], ['foo', 'bar']));
}

}
29 changes: 29 additions & 0 deletions tests/PHPStan/Analyser/data/bug-7301.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
<?php

namespace Bug7301;

use Closure;
use function PHPStan\Testing\assertType;

/**
* @template TReturn
* @param Closure(): TReturn $closure
* @return TReturn
*/
function templated($closure)
{
return $closure();
}

function (): void {
/**
* @var Closure(): array<non-empty-string, mixed>
*/
$arg = function () {
return ['key' => 'value'];
};

$result = templated($arg);

assertType('array<non-empty-string, mixed>', $result);
};
31 changes: 31 additions & 0 deletions tests/PHPStan/Analyser/data/bug-7944.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
<?php

namespace Bug7944;

use function PHPStan\Testing\assertType;

/**
* @template TValue
*/
final class Value
{
/** @var TValue */
public readonly mixed $value;

/**
* @param TValue $value
*/
public function __construct(mixed $value)
{
$this->value = $value;
}
}

/**
* @param non-empty-string $p
*/
function test($p): void {
$value = new Value($p);
assertType('Bug7944\\Value<non-empty-string>', $value);
};