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

Normalize specified types before intersection #1016

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
37 changes: 31 additions & 6 deletions src/Analyser/SpecifiedTypes.php
Expand Up @@ -58,28 +58,31 @@ public function getNewConditionalExpressionHolders(): array
/** @api */
public function intersectWith(SpecifiedTypes $other): self
{
$normalized = $this->normalize();
$otherNormalized = $other->normalize();

$sureTypeUnion = [];
$sureNotTypeUnion = [];

foreach ($this->sureTypes as $exprString => [$exprNode, $type]) {
if (!isset($other->sureTypes[$exprString])) {
foreach ($normalized->sureTypes as $exprString => [$exprNode, $type]) {
if (!isset($otherNormalized->sureTypes[$exprString])) {
continue;
}

$sureTypeUnion[$exprString] = [
$exprNode,
TypeCombinator::union($type, $other->sureTypes[$exprString][1]),
TypeCombinator::union($type, $otherNormalized->sureTypes[$exprString][1]),
];
}

foreach ($this->sureNotTypes as $exprString => [$exprNode, $type]) {
if (!isset($other->sureNotTypes[$exprString])) {
foreach ($normalized->sureNotTypes as $exprString => [$exprNode, $type]) {
if (!isset($otherNormalized->sureNotTypes[$exprString])) {
continue;
}

$sureNotTypeUnion[$exprString] = [
$exprNode,
TypeCombinator::intersect($type, $other->sureNotTypes[$exprString][1]),
TypeCombinator::intersect($type, $otherNormalized->sureNotTypes[$exprString][1]),
];
}

Expand Down Expand Up @@ -117,4 +120,26 @@ public function unionWith(SpecifiedTypes $other): self
return new self($sureTypeUnion, $sureNotTypeUnion);
}

public function inverse(): self
{
return new self($this->sureNotTypes, $this->sureTypes, $this->overwrite, $this->newConditionalExpressionHolders);
}

private function normalize(): self
{
$sureTypes = $this->sureTypes;
$sureNotTypes = [];

foreach ($this->sureNotTypes as $exprString => [$exprNode, $sureNotType]) {
if (!isset($sureTypes[$exprString])) {
$sureNotTypes[$exprString] = [$exprNode, $sureNotType];
continue;
}

$sureTypes[$exprString][1] = TypeCombinator::remove($sureTypes[$exprString][1], $sureNotType);
}

return new self($sureTypes, $sureNotTypes, $this->overwrite, $this->newConditionalExpressionHolders);
}

}
12 changes: 10 additions & 2 deletions src/Analyser/TypeSpecifier.php
Expand Up @@ -666,8 +666,12 @@ public function specifyTypesInCondition(

return $this->handleDefaultTruthyOrFalseyContext($context, $expr, $scope);
} elseif ($expr instanceof BooleanAnd || $expr instanceof LogicalAnd) {
if (!$scope instanceof MutatingScope) {
throw new ShouldNotHappenException();
}
$leftTypes = $this->specifyTypesInCondition($scope, $expr->left, $context);
$rightTypes = $this->specifyTypesInCondition($scope, $expr->right, $context);
$leftScope = $scope->filterBySpecifiedTypes($context->true() ? $leftTypes : $leftTypes->inverse());
$rightTypes = $this->specifyTypesInCondition($leftScope, $expr->right, $context);
$types = $context->true() ? $leftTypes->unionWith($rightTypes) : $leftTypes->intersectWith($rightTypes);
if ($context->false()) {
return new SpecifiedTypes(
Expand All @@ -683,8 +687,12 @@ public function specifyTypesInCondition(

return $types;
} elseif ($expr instanceof BooleanOr || $expr instanceof LogicalOr) {
if (!$scope instanceof MutatingScope) {
throw new ShouldNotHappenException();
}
$leftTypes = $this->specifyTypesInCondition($scope, $expr->left, $context);
$rightTypes = $this->specifyTypesInCondition($scope, $expr->right, $context);
$leftScope = $scope->filterBySpecifiedTypes($context->true() ? $leftTypes->inverse() : $leftTypes);
$rightTypes = $this->specifyTypesInCondition($leftScope, $expr->right, $context);
$types = $context->true() ? $leftTypes->intersectWith($rightTypes) : $leftTypes->unionWith($rightTypes);
if ($context->true()) {
return new SpecifiedTypes(
Expand Down
1 change: 1 addition & 0 deletions tests/PHPStan/Analyser/NodeScopeResolverTest.php
Expand Up @@ -692,6 +692,7 @@ public function dataFileAsserts(): iterable
if (PHP_VERSION_ID >= 80000) {
yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-6308.php');
}
yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-6329.php');

if (PHP_VERSION_ID >= 70400) {
yield from $this->gatherAssertTypes(__DIR__ . '/../Rules/Comparison/data/bug-6473.php');
Expand Down
57 changes: 57 additions & 0 deletions tests/PHPStan/Analyser/TypeSpecifierTest.php
Expand Up @@ -8,6 +8,7 @@
use PhpParser\Node\Expr\BinaryOp\Identical;
use PhpParser\Node\Expr\BinaryOp\NotIdentical;
use PhpParser\Node\Expr\BooleanNot;
use PhpParser\Node\Expr\ConstFetch;
use PhpParser\Node\Expr\FuncCall;
use PhpParser\Node\Expr\PropertyFetch;
use PhpParser\Node\Expr\Variable;
Expand Down Expand Up @@ -965,6 +966,62 @@ public function dataCondition(): array
],
[],
],
[
new Expr\BinaryOp\BooleanOr(
new Expr\BinaryOp\BooleanAnd(
$this->createFunctionCall('is_string', 'a'),
new NotIdentical(new String_(''), new Variable('a')),
),
new Identical(new Expr\ConstFetch(new Name('null')), new Variable('a')),
),
['$a' => 'non-empty-string|null'],
['$a' => '~null'],
],
[
new Expr\BinaryOp\BooleanOr(
new Expr\BinaryOp\BooleanAnd(
$this->createFunctionCall('is_string', 'a'),
new Expr\BinaryOp\Greater(
$this->createFunctionCall('strlen', 'a'),
new LNumber(0),
),
),
new Identical(new Expr\ConstFetch(new Name('null')), new Variable('a')),
),
['$a' => 'non-empty-string|null'],
['$a' => '~non-empty-string|null'],
],
[
new Expr\BinaryOp\BooleanOr(
new Expr\BinaryOp\BooleanAnd(
$this->createFunctionCall('is_array', 'a'),
new Expr\BinaryOp\Greater(
$this->createFunctionCall('count', 'a'),
herndlm marked this conversation as resolved.
Show resolved Hide resolved
new LNumber(0),
),
),
new Identical(new Expr\ConstFetch(new Name('null')), new Variable('a')),
),
['$a' => 'non-empty-array|null'],
['$a' => '~non-empty-array|null'],
],
[
new Expr\BinaryOp\BooleanAnd(
$this->createFunctionCall('is_array', 'foo'),
new Identical(
new FuncCall(
new Name('array_filter'),
[new Arg(new Variable('foo')), new Arg(new String_('is_string')), new Arg(new ConstFetch(new Name('ARRAY_FILTER_USE_KEY')))],
),
new Variable('foo'),
),
),
[
'$foo' => 'array<string, mixed>',
'array_filter($foo, \'is_string\', ARRAY_FILTER_USE_KEY)' => 'array<string, mixed>',
],
[],
],
];
}

Expand Down
185 changes: 185 additions & 0 deletions tests/PHPStan/Analyser/data/bug-6329.php
@@ -0,0 +1,185 @@
<?php

namespace Bug6329;

use function PHPStan\Testing\assertType;

/**
* @param mixed $a
*/
function nonEmptyString1($a): void
{
if (is_string($a) && '' !== $a || null === $a) {
assertType('non-empty-string|null', $a);
}

if ('' !== $a && is_string($a) || null === $a) {
assertType('non-empty-string|null', $a);
}

if (null === $a || is_string($a) && '' !== $a) {
assertType('non-empty-string|null', $a);
}

if (null === $a || '' !== $a && is_string($a)) {
assertType('non-empty-string|null', $a);
}
}

/**
* @param mixed $a
*/
function nonEmptyString2($a): void
{
if (is_string($a) && strlen($a) > 0 || null === $a) {
assertType('non-empty-string|null', $a);
}

if (null === $a || is_string($a) && strlen($a) > 0) {
assertType('non-empty-string|null', $a);
}
}


/**
* @param mixed $a
*/
function int1($a): void
{
if (is_int($a) && 0 !== $a || null === $a) {
assertType('int<min, -1>|int<1, max>|null', $a);
}

if (0 !== $a && is_int($a) || null === $a) {
assertType('int<min, -1>|int<1, max>|null', $a);
}

if (null === $a || is_int($a) && 0 !== $a) {
assertType('int<min, -1>|int<1, max>|null', $a);
}

if (null === $a || 0 !== $a && is_int($a)) {
assertType('int<min, -1>|int<1, max>|null', $a);
}
}

/**
* @param mixed $a
*/
function int2($a): void
{
if (is_int($a) && $a > 0 || null === $a) {
assertType('int<1, max>|null', $a);
}

if (null === $a || is_int($a) && $a > 0) {
assertType('int<1, max>|null', $a);
}
}


/**
* @param mixed $a
*/
function true($a): void
{
if (is_bool($a) && false !== $a || null === $a) {
assertType('true|null', $a);
}

if (false !== $a && is_bool($a) || null === $a) {
assertType('true|null', $a);
}

if (null === $a || is_bool($a) && false !== $a) {
assertType('true|null', $a);
}

if (null === $a || false !== $a && is_bool($a)) {
assertType('true|null', $a);
}
}

/**
* @param mixed $a
*/
function nonEmptyArray1($a): void
{
if (is_array($a) && [] !== $a || null === $a) {
herndlm marked this conversation as resolved.
Show resolved Hide resolved
assertType('non-empty-array|null', $a);
}

if ([] !== $a && is_array($a) || null === $a) {
assertType('non-empty-array|null', $a);
}

if (null === $a || is_array($a) && [] !== $a) {
assertType('non-empty-array|null', $a);
}

if (null === $a || [] !== $a && is_array($a)) {
assertType('non-empty-array|null', $a);
}
}

/**
* @param mixed $a
*/
function nonEmptyArray2($a): void
{
if (is_array($a) && count($a) > 0 || null === $a) {
assertType('non-empty-array|null', $a);
}

if (null === $a || is_array($a) && count($a) > 0) {
assertType('non-empty-array|null', $a);
}
}

/**
* @param mixed $a
* @param mixed $b
* @param mixed $c
*/
function inverse($a, $b, $c): void
{
if ((!is_string($a) || '' === $a) && null !== $a) {
} else {
assertType('non-empty-string|null', $a);
}

if ((!is_int($b) || $b <= 0) && null !== $b) {
} else {
assertType('int<1, max>|null', $b);
}

if (null !== $c && (!is_array($c) || count($c) <= 0)) {
} else {
assertType('non-empty-array|null', $c);
}
}

/**
* @param mixed $a
* @param mixed $b
* @param mixed $c
* @param mixed $d
*/
function combinations($a, $b, $c, $d): void
{
if (is_string($a) && '' !== $a || is_int($a) && $a > 0 || null === $a) {
assertType('int<1, max>|non-empty-string|null', $a);
}
if ((!is_string($b) || '' === $b) && (!is_int($b) || $b <= 0) && null !== $b) {
} else {
assertType('int<1, max>|non-empty-string|null', $b);
}

if (is_array($c) && $c === array_filter($c, 'is_string', ARRAY_FILTER_USE_KEY) || null === $c) {
assertType('array<string, mixed>|null', $c);
}
if ((!is_array($d) || $d !== array_filter($d, 'is_string', ARRAY_FILTER_USE_KEY)) && null !== $d) {
} else {
assertType('array<string, mixed>|null', $d);
}
}