Skip to content

Make ParametersAcceptorSelector::selectSingle() return union-ed conditional types #1159

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 9 commits into from
Apr 2, 2022
Merged
20 changes: 19 additions & 1 deletion src/Reflection/FunctionVariant.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,10 @@

use PHPStan\Type\Generic\TemplateTypeMap;
use PHPStan\Type\Type;
use PHPStan\Type\TypeUtils;

/** @api */
class FunctionVariant implements ParametersAcceptor
class FunctionVariant implements ParametersAcceptor, SingleParametersAcceptor
{

/**
Expand Down Expand Up @@ -51,4 +52,21 @@ public function getReturnType(): Type
return $this->returnType;
}

/**
* @return static
*/
public function flattenConditionalsInReturnType(): SingleParametersAcceptor
{
/** @var static $result */
$result = new self(
$this->templateTypeMap,
$this->resolvedTemplateTypeMap,
$this->parameters,
$this->isVariadic,
TypeUtils::flattenConditionals($this->returnType),
);

return $result;
}

}
20 changes: 20 additions & 0 deletions src/Reflection/FunctionVariantWithPhpDocs.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

use PHPStan\Type\Generic\TemplateTypeMap;
use PHPStan\Type\Type;
use PHPStan\Type\TypeUtils;

/** @api */
class FunctionVariantWithPhpDocs extends FunctionVariant implements ParametersAcceptorWithPhpDocs
Expand Down Expand Up @@ -53,4 +54,23 @@ public function getNativeReturnType(): Type
return $this->nativeReturnType;
}

/**
* @return static
*/
public function flattenConditionalsInReturnType(): SingleParametersAcceptor
{
/** @var static $result */
$result = new self(
$this->getTemplateTypeMap(),
$this->getResolvedTemplateTypeMap(),
$this->getParameters(),
$this->isVariadic(),
TypeUtils::flattenConditionals($this->getReturnType()),
TypeUtils::flattenConditionals($this->phpDocReturnType),
$this->nativeReturnType,
);

return $result;
}

}
8 changes: 7 additions & 1 deletion src/Reflection/ParametersAcceptorSelector.php
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,13 @@ public static function selectSingle(
throw new ShouldNotHappenException('Multiple variants - use selectFromArgs() instead.');
}

return $parametersAcceptors[0];
$parametersAcceptor = $parametersAcceptors[0];

if ($parametersAcceptor instanceof SingleParametersAcceptor) {
$parametersAcceptor = $parametersAcceptor->flattenConditionalsInReturnType();
}

return $parametersAcceptor;
}

/**
Expand Down
19 changes: 18 additions & 1 deletion src/Reflection/ResolvedFunctionVariant.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,11 @@
use PHPStan\Type\Generic\TemplateTypeMap;
use PHPStan\Type\Type;
use PHPStan\Type\TypeTraverser;
use PHPStan\Type\TypeUtils;
use function array_key_exists;
use function array_map;

class ResolvedFunctionVariant implements ParametersAcceptor
class ResolvedFunctionVariant implements ParametersAcceptor, SingleParametersAcceptor
{

/** @var ParameterReflection[]|null */
Expand Down Expand Up @@ -88,6 +89,22 @@ public function getReturnType(): Type
return $type;
}

/**
* @return static
*/
public function flattenConditionalsInReturnType(): SingleParametersAcceptor
{
/** @var static $result */
$result = new self(
$this->parametersAcceptor,
$this->resolvedTemplateTypeMap,
$this->passedArgs,
);
$result->returnType = TypeUtils::flattenConditionals($this->getReturnType());

return $result;
}

private function resolveConditionalTypes(Type $type): Type
{
return TypeTraverser::map($type, function (Type $type, callable $traverse): Type {
Expand Down
13 changes: 13 additions & 0 deletions src/Reflection/SingleParametersAcceptor.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<?php declare(strict_types = 1);

namespace PHPStan\Reflection;

interface SingleParametersAcceptor
{

/**
* @return static
*/
public function flattenConditionalsInReturnType(): self;

}
2 changes: 1 addition & 1 deletion src/Type/ConditionalType.php
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ private function resolve(): Type
}

if ($this->isResolved()) {
return TypeCombinator::union($this->if, $this->else);
return $this->getResult();
}

return $this;
Expand Down
2 changes: 1 addition & 1 deletion src/Type/Traits/ConditionalTypeTrait.php
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,7 @@ public function isGreaterThanOrEqual(Type $otherType): TrinaryLogic
return $otherType->isSmallerThanOrEqual($result);
}

private function getResult(): Type
public function getResult(): Type
{
if ($this->result === null) {
return $this->result = TypeCombinator::union($this->if, $this->else);
Expand Down
11 changes: 11 additions & 0 deletions src/Type/TypeUtils.php
Original file line number Diff line number Diff line change
Expand Up @@ -328,4 +328,15 @@ public static function containsCallable(Type $type): bool
return false;
}

public static function flattenConditionals(Type $type): Type
{
return TypeTraverser::map($type, static function (Type $type, callable $traverse) {
while ($type instanceof ConditionalType || $type instanceof ConditionalTypeForParameter) {
$type = $type->getResult();
}

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

}
2 changes: 1 addition & 1 deletion tests/PHPStan/Analyser/AnalyserIntegrationTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -622,7 +622,7 @@ public function testBug6896(): void
}

$errors = $this->runAnalyse(__DIR__ . '/data/bug-6896.php');
$this->assertCount(4, $errors);
$this->assertCount(2, $errors);
}

public function testBug6940(): void
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ public function dataAsserts(): iterable
{
yield from $this->gatherAssertTypes(__DIR__ . '/data/dynamic-method-return-types.php');
yield from $this->gatherAssertTypes(__DIR__ . '/data/dynamic-method-return-compound-types.php');
yield from $this->gatherAssertTypes(__DIR__ . '/data/dynamic-method-return-getsingle-conditional.php');
}

/**
Expand Down
20 changes: 20 additions & 0 deletions tests/PHPStan/Analyser/data/TestDynamicReturnTypeExtensions.php
Original file line number Diff line number Diff line change
Expand Up @@ -189,3 +189,23 @@ public function getTypeFromMethodCall(MethodReflection $methodReflection, Method
}

}


class ConditionalGetSingle implements DynamicMethodReturnTypeExtension {

public function getClass(): string
{
return \DynamicMethodReturnGetSingleConditional\Foo::class;
}

public function isMethodSupported(MethodReflection $methodReflection): bool
{
return $methodReflection->getName() === 'get';
}

public function getTypeFromMethodCall(MethodReflection $methodReflection, MethodCall $methodCall, Scope $scope): Type
{
return ParametersAcceptorSelector::selectSingle($methodReflection->getVariants())->getReturnType();
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
<?php

namespace DynamicMethodReturnGetSingleConditional;

use function PHPStan\Testing\assertType;

abstract class Foo
{
/**
* @return ($input is 1 ? true : false)
*/
abstract public function get(int $input): mixed;

public function doFoo(): void
{
assertType('bool', $this->get(0));
assertType('bool', $this->get(1));
assertType('bool', $this->get(2));
Comment on lines +16 to +18
Copy link
Contributor

Choose a reason for hiding this comment

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

do we expect a more precise type here?

Suggested change
assertType('bool', $this->get(0));
assertType('bool', $this->get(1));
assertType('bool', $this->get(2));
assertType('false', $this->get(0));
assertType('true', $this->get(1));
assertType('false', $this->get(2));

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No - there is return type extension that just returns ParametersAcceptorSelector::selectSingle() to verify that it provides the resulting union type instead of the conditional type.

}
}
4 changes: 4 additions & 0 deletions tests/PHPStan/Analyser/dynamic-return-type.neon
Original file line number Diff line number Diff line change
Expand Up @@ -27,3 +27,7 @@ services:
class: PHPStan\Tests\FooGetSelf
tags:
- phpstan.broker.dynamicMethodReturnTypeExtension
-
class: PHPStan\Tests\ConditionalGetSingle
tags:
- phpstan.broker.dynamicMethodReturnTypeExtension