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

Implement TypeSpecifierContext->getReturnType() #3878

Closed
wants to merge 17 commits into from
Closed
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
37 changes: 20 additions & 17 deletions src/Analyser/TypeSpecifier.php
Original file line number Diff line number Diff line change
Expand Up @@ -331,21 +331,6 @@ public function specifyTypesInCondition(
}
}

if (
!$context->null()
&& $expr->right instanceof FuncCall
&& count($expr->right->getArgs()) >= 3
&& $expr->right->name instanceof Name
&& in_array(strtolower((string) $expr->right->name), ['preg_match'], true)
&& IntegerRangeType::fromInterval(0, null)->isSuperTypeOf($leftType)->yes()
) {
return $this->specifyTypesInCondition(
$scope,
new Expr\BinaryOp\NotIdentical($expr->right, new ConstFetch(new Name('false'))),
$context,
)->setRootExpr($expr);
}

if (
!$context->null()
&& $expr->right instanceof FuncCall
Expand Down Expand Up @@ -466,6 +451,24 @@ public function specifyTypesInCondition(
}
}

if (
!$context->null()
&& $expr->right instanceof Expr\CallLike
) {
if (!$scope instanceof MutatingScope) {
throw new ShouldNotHappenException();
}
$newScope = $scope->filterBySpecifiedTypes($result);
$callType = $newScope->getType($expr->right);
$newContext = $context->true() ? TypeSpecifierContext::createTrue($callType) : TypeSpecifierContext::createTrue($callType)->negate();

$result = $result->unionWith($this->specifyTypesInCondition(
$scope,
$expr->right,
$newContext,
)->setRootExpr($expr));
}

return $result;

} elseif ($expr instanceof Node\Expr\BinaryOp\Greater) {
Expand Down Expand Up @@ -1173,7 +1176,7 @@ private function specifyTypesForConstantBinaryExpression(
return $types->unionWith($this->specifyTypesInCondition(
$scope,
$exprNode,
$context->true() ? TypeSpecifierContext::createTrue() : TypeSpecifierContext::createTrue()->negate(),
$context->true() ? TypeSpecifierContext::createTrue($context->getReturnType()) : TypeSpecifierContext::createTrue($context->getReturnType())->negate(),
)->setRootExpr($rootExpr));
}

Expand Down Expand Up @@ -1973,7 +1976,7 @@ public function resolveEqual(Expr\BinaryOp\Equal $expr, Scope $scope, TypeSpecif
return $this->specifyTypesInCondition(
$scope,
$exprNode,
$context->true() ? TypeSpecifierContext::createFalsey() : TypeSpecifierContext::createFalsey()->negate(),
$context->true() ? TypeSpecifierContext::createFalsey($context->getReturnType()) : TypeSpecifierContext::createFalsey($context->getReturnType())->negate(),
)->setRootExpr($expr);
}

Expand Down
47 changes: 35 additions & 12 deletions src/Analyser/TypeSpecifierContext.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@
namespace PHPStan\Analyser;

use PHPStan\ShouldNotHappenException;
use PHPStan\Type\GeneralizePrecision;
use PHPStan\Type\Type;
use PHPStan\Type\TypeCombinator;

/**
* @api
Expand All @@ -21,34 +24,42 @@ final class TypeSpecifierContext
/** @var self[] */
private static array $registry;

private function __construct(private ?int $value)
private function __construct(
private ?int $value,
private ?Type $returnType,
)
{
}

private static function create(?int $value): self
private static function create(?int $value, ?Type $returnType = null): self
{
self::$registry[$value] ??= new self($value);
if ($returnType !== null) {
// return type bound context is unique for each context and therefore not cacheable
return new self($value, $returnType);
}

self::$registry[$value] ??= new self($value, null);
return self::$registry[$value];
}

public static function createTrue(): self
public static function createTrue(?Type $returnType = null): self
{
return self::create(self::CONTEXT_TRUE);
return self::create(self::CONTEXT_TRUE, $returnType);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

todo: bit-field cleanup after the main-point of this PR works

About true/truthy - let’s get rid of the bit-masking, I think they will be implemented as getReturnType()->isTrue()->yes()
The other one as getReturnType()->toBoolean()->isTrue()->yes()

}

public static function createTruthy(): self
public static function createTruthy(?Type $returnType = null): self
{
return self::create(self::CONTEXT_TRUTHY);
return self::create(self::CONTEXT_TRUTHY, $returnType);
}

public static function createFalse(): self
public static function createFalse(?Type $returnType = null): self
{
return self::create(self::CONTEXT_FALSE);
return self::create(self::CONTEXT_FALSE, $returnType);
}

public static function createFalsey(): self
public static function createFalsey(?Type $returnType = null): self
{
return self::create(self::CONTEXT_FALSEY);
return self::create(self::CONTEXT_FALSEY, $returnType);
}

public static function createNull(): self
Expand All @@ -61,7 +72,14 @@ public function negate(): self
if ($this->value === null) {
throw new ShouldNotHappenException();
}
return self::create(~$this->value & self::CONTEXT_BITMASK);

$negatedReturnType = null;
if ($this->returnType !== null) {
$baseType = $this->returnType->generalize(GeneralizePrecision::lessSpecific());
$negatedReturnType = TypeCombinator::remove($baseType, $this->returnType);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This won't work for unions of int. We will need 2 two return types in the context and toggle between them on negate()

}

return self::create(~$this->value & self::CONTEXT_BITMASK, $negatedReturnType);
}

public function true(): bool
Expand Down Expand Up @@ -89,4 +107,9 @@ public function null(): bool
return $this->value === null;
}

public function getReturnType(): ?Type
{
return $this->returnType;
}

}
2 changes: 1 addition & 1 deletion src/Type/Php/InArrayFunctionTypeSpecifyingExtension.php
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ public function specifyTypes(FunctionReflection $functionReflection, FuncCall $n
$specifiedTypes = $specifiedTypes->unionWith($this->typeSpecifier->create(
$node->getArgs()[1]->value,
new ArrayType(new MixedType(), $arrayValueType),
TypeSpecifierContext::createTrue(),
TypeSpecifierContext::createTrue($context->getReturnType()),
$scope,
));
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
services:
-
class: PHPStan\Tests\TypeSpecifierContextReturnTypeExtension
tags:
- phpstan.typeSpecifier.methodTypeSpecifyingExtension
35 changes: 35 additions & 0 deletions tests/PHPStan/Analyser/TypeSpecifierContextReturnTypeTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
<?php declare(strict_types = 1);

namespace PHPStan\Analyser;

use PHPStan\Testing\TypeInferenceTestCase;

class TypeSpecifierContextReturnTypeTest extends TypeInferenceTestCase
{

public function dataContextReturnType(): iterable
{
yield from $this->gatherAssertTypes(__DIR__ . '/data/type-specifier-context-return-type.php');
}

/**
* @dataProvider dataContextReturnType
* @param mixed ...$args
*/
public function testContextReturnType(
string $assertType,
string $file,
...$args,
): void
{
$this->assertFileAsserts($assertType, $file, ...$args);
}

public static function getAdditionalConfigFiles(): array
{
return [
__DIR__ . '/TypeSpecifierContextReturnTypeExtension.neon',
];
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
<?php declare(strict_types = 1);

namespace PHPStan\Tests;

use PhpParser\Node\Expr\MethodCall;
use PHPStan\Analyser\Scope;
use PHPStan\Analyser\SpecifiedTypes;
use PHPStan\Analyser\TypeSpecifier;
use PHPStan\Analyser\TypeSpecifierAwareExtension;
use PHPStan\Analyser\TypeSpecifierContext;
use PHPStan\Reflection\MethodReflection;
use PHPStan\Type\MethodTypeSpecifyingExtension;
use TypeSpecifierContextReturnTypeTest\ContextReturnType;
use function str_starts_with;

class TypeSpecifierContextReturnTypeExtension implements MethodTypeSpecifyingExtension, TypeSpecifierAwareExtension
{

private TypeSpecifier $typeSpecifier;

public function setTypeSpecifier(TypeSpecifier $typeSpecifier): void
{
$this->typeSpecifier = $typeSpecifier;
}

public function getClass(): string
{
return ContextReturnType::class;
}

public function isMethodSupported(
MethodReflection $methodReflection,
MethodCall $node,
TypeSpecifierContext $context,
): bool
{
return str_starts_with($methodReflection->getName(), 'returns');
}

public function specifyTypes(
MethodReflection $methodReflection,
MethodCall $node,
Scope $scope,
TypeSpecifierContext $context,
): SpecifiedTypes
{
if ($context->null()) {
return new SpecifiedTypes();
}

return $this->typeSpecifier->create(
$node->getArgs()[0]->value,
$context->getReturnType(),
$context,
$scope,
);
}

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

namespace TypeSpecifierContextReturnTypeTest;

use function PHPStan\Testing\assertType;

class ContextReturnType {
public function returnsInt(int $specifiedContextReturnType): int {}

public function doFooLeft(int $i) {
assertType('int', $i);
if ($this->returnsInt($i) > 0) {
assertType('int<1, max>', $i);
} else {
assertType('int<min, 0>', $i);
}
assertType('int', $i);
}

public function doFooRight(int $i) {
assertType('int', $i);
if (0 < $this->returnsInt($i)) {
assertType('int<1, max>', $i);
} else {
assertType('int<min, 0>', $i);
}
assertType('int', $i);
}
}
Loading