Skip to content

Commit cc6655f

Browse files
ondrejmirtesondrejmirtes
committed
Fix another BooleanOr issue
1 parent a483c11 commit cc6655f

File tree

7 files changed

+46
-28
lines changed

7 files changed

+46
-28
lines changed

‎src/Analyser/SpecifiedTypes.php

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -58,31 +58,28 @@ public function getNewConditionalExpressionHolders(): array
5858
/** @api */
5959
public function intersectWith(SpecifiedTypes $other): self
6060
{
61-
$normalized = $this->normalize();
62-
$otherNormalized = $other->normalize();
63-
6461
$sureTypeUnion = [];
6562
$sureNotTypeUnion = [];
6663

67-
foreach ($normalized->sureTypes as $exprString => [$exprNode, $type]) {
68-
if (!isset($otherNormalized->sureTypes[$exprString])) {
64+
foreach ($this->sureTypes as $exprString => [$exprNode, $type]) {
65+
if (!isset($other->sureTypes[$exprString])) {
6966
continue;
7067
}
7168

7269
$sureTypeUnion[$exprString] = [
7370
$exprNode,
74-
TypeCombinator::union($type, $otherNormalized->sureTypes[$exprString][1]),
71+
TypeCombinator::union($type, $other->sureTypes[$exprString][1]),
7572
];
7673
}
7774

78-
foreach ($normalized->sureNotTypes as $exprString => [$exprNode, $type]) {
79-
if (!isset($otherNormalized->sureNotTypes[$exprString])) {
75+
foreach ($this->sureNotTypes as $exprString => [$exprNode, $type]) {
76+
if (!isset($other->sureNotTypes[$exprString])) {
8077
continue;
8178
}
8279

8380
$sureNotTypeUnion[$exprString] = [
8481
$exprNode,
85-
TypeCombinator::intersect($type, $otherNormalized->sureNotTypes[$exprString][1]),
82+
TypeCombinator::intersect($type, $other->sureNotTypes[$exprString][1]),
8683
];
8784
}
8885

@@ -120,21 +117,20 @@ public function unionWith(SpecifiedTypes $other): self
120117
return new self($sureTypeUnion, $sureNotTypeUnion);
121118
}
122119

123-
private function normalize(): self
120+
public function normalize(Scope $scope): self
124121
{
125122
$sureTypes = $this->sureTypes;
126-
$sureNotTypes = [];
127123

128124
foreach ($this->sureNotTypes as $exprString => [$exprNode, $sureNotType]) {
129125
if (!isset($sureTypes[$exprString])) {
130-
$sureNotTypes[$exprString] = [$exprNode, $sureNotType];
126+
$sureTypes[$exprString] = [$exprNode, TypeCombinator::remove($scope->getType($exprNode), $sureNotType)];
131127
continue;
132128
}
133129

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

137-
return new self($sureTypes, $sureNotTypes, $this->overwrite, $this->newConditionalExpressionHolders);
133+
return new self($sureTypes, [], $this->overwrite, $this->newConditionalExpressionHolders);
138134
}
139135

140136
}

‎src/Analyser/TypeSpecifier.php

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -674,8 +674,9 @@ public function specifyTypesInCondition(
674674
throw new ShouldNotHappenException();
675675
}
676676
$leftTypes = $this->specifyTypesInCondition($scope, $expr->left, $context);
677-
$rightTypes = $this->specifyTypesInCondition($scope->filterByTruthyValue($expr->left), $expr->right, $context);
678-
$types = $context->true() ? $leftTypes->unionWith($rightTypes) : $leftTypes->intersectWith($rightTypes);
677+
$rightScope = $scope->filterByTruthyValue($expr->left);
678+
$rightTypes = $this->specifyTypesInCondition($rightScope, $expr->right, $context);
679+
$types = $context->true() ? $leftTypes->unionWith($rightTypes) : $leftTypes->normalize($scope)->intersectWith($rightTypes->normalize($rightScope));
679680
if ($context->false()) {
680681
return new SpecifiedTypes(
681682
$types->getSureTypes(),
@@ -694,8 +695,9 @@ public function specifyTypesInCondition(
694695
throw new ShouldNotHappenException();
695696
}
696697
$leftTypes = $this->specifyTypesInCondition($scope, $expr->left, $context);
697-
$rightTypes = $this->specifyTypesInCondition($scope->filterByFalseyValue($expr->left), $expr->right, $context);
698-
$types = $context->true() ? $leftTypes->intersectWith($rightTypes) : $leftTypes->unionWith($rightTypes);
698+
$rightScope = $scope->filterByFalseyValue($expr->left);
699+
$rightTypes = $this->specifyTypesInCondition($rightScope, $expr->right, $context);
700+
$types = $context->true() ? $leftTypes->normalize($scope)->intersectWith($rightTypes->normalize($rightScope)) : $leftTypes->unionWith($rightTypes);
699701
if ($context->true()) {
700702
return new SpecifiedTypes(
701703
$types->getSureTypes(),
@@ -849,7 +851,7 @@ public function specifyTypesInCondition(
849851
);
850852

851853
$nullSafeTypes = $this->handleDefaultTruthyOrFalseyContext($context, $expr, $scope);
852-
return $context->true() ? $types->unionWith($nullSafeTypes) : $types->intersectWith($nullSafeTypes);
854+
return $context->true() ? $types->unionWith($nullSafeTypes) : $types->normalize($scope)->intersectWith($nullSafeTypes->normalize($scope));
853855
} elseif ($expr instanceof Expr\NullsafeMethodCall && !$context->null()) {
854856
$types = $this->specifyTypesInCondition(
855857
$scope,
@@ -861,7 +863,7 @@ public function specifyTypesInCondition(
861863
);
862864

863865
$nullSafeTypes = $this->handleDefaultTruthyOrFalseyContext($context, $expr, $scope);
864-
return $context->true() ? $types->unionWith($nullSafeTypes) : $types->intersectWith($nullSafeTypes);
866+
return $context->true() ? $types->unionWith($nullSafeTypes) : $types->normalize($scope)->intersectWith($nullSafeTypes->normalize($scope));
865867
} elseif (!$context->null()) {
866868
return $this->handleDefaultTruthyOrFalseyContext($context, $expr, $scope);
867869
}

‎tests/PHPStan/Analyser/NodeScopeResolverTest.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -720,6 +720,8 @@ public function dataFileAsserts(): iterable
720720

721721
require_once __DIR__ . '/data/countable.php';
722722
yield from $this->gatherAssertTypes(__DIR__ . '/data/countable.php');
723+
724+
yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-6696.php');
723725
}
724726

725727
/**

‎tests/PHPStan/Analyser/TypeSpecifierTest.php

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -310,7 +310,7 @@ public function dataCondition(): array
310310
$this->createFunctionCall('random'),
311311
),
312312
[],
313-
['$foo' => '~*NEVER*'],
313+
['$foo' => 'mixed'],
314314
],
315315
[
316316
new Expr\BinaryOp\BooleanOr(
@@ -342,7 +342,7 @@ public function dataCondition(): array
342342
),
343343
$this->createFunctionCall('random'),
344344
),
345-
['$foo' => '~*NEVER*'],
345+
['$foo' => 'mixed'],
346346
[],
347347
],
348348

@@ -975,7 +975,7 @@ public function dataCondition(): array
975975
new Identical(new Expr\ConstFetch(new Name('null')), new Variable('a')),
976976
),
977977
['$a' => 'non-empty-string|null'],
978-
['$a' => '~null'],
978+
['$a' => 'mixed~non-empty-string & ~null'],
979979
],
980980
[
981981
new Expr\BinaryOp\BooleanOr(
@@ -989,7 +989,7 @@ public function dataCondition(): array
989989
new Identical(new Expr\ConstFetch(new Name('null')), new Variable('a')),
990990
),
991991
['$a' => 'non-empty-string|null'],
992-
['$a' => '~non-empty-string|null'],
992+
['$a' => 'mixed~non-empty-string & ~null'],
993993
],
994994
[
995995
new Expr\BinaryOp\BooleanOr(
@@ -1003,7 +1003,7 @@ public function dataCondition(): array
10031003
new Identical(new Expr\ConstFetch(new Name('null')), new Variable('a')),
10041004
),
10051005
['$a' => 'non-empty-array|null'],
1006-
['$a' => '~non-empty-array|null'],
1006+
['$a' => 'mixed~non-empty-array & ~null'],
10071007
],
10081008
[
10091009
new Expr\BinaryOp\BooleanAnd(
@@ -1072,7 +1072,7 @@ public function dataCondition(): array
10721072
'strlen($foo)' => '~0',
10731073
],
10741074
[
1075-
'$foo' => '~non-empty-string',
1075+
'$foo' => 'mixed~non-empty-string',
10761076
],
10771077
],
10781078
];

‎tests/PHPStan/Analyser/data/bug-3009.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ public function createRedirectRequest(string $redirectUri): ?string
1111
{
1212
$redirectUrlParts = parse_url($redirectUri);
1313
if (false === is_array($redirectUrlParts) || true === array_key_exists('host', $redirectUrlParts)) {
14-
assertType('array{scheme?: string, host?: string, port?: int, user?: string, pass?: string, path?: string, query?: string, fragment?: string}|false', $redirectUrlParts);
14+
assertType('array{scheme?: string, host: string, port?: int, user?: string, pass?: string, path?: string, query?: string, fragment?: string}|false', $redirectUrlParts);
1515
return null;
1616
}
1717

‎tests/PHPStan/Analyser/data/bug-6696.php

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
<?php
2+
3+
namespace Bug6696;
4+
5+
use function PHPStan\Testing\assertType;
6+
7+
class HelloWorld
8+
{
9+
public function sayHello(?string $s): void
10+
{
11+
assert($s !== '');
12+
assertType('non-empty-string|null', $s);
13+
}
14+
15+
public function sayHello2(?string $s): void
16+
{
17+
assert($s === null || $s !== '');
18+
assertType('non-empty-string|null', $s);
19+
}
20+
}

‎tests/PHPStan/Rules/Classes/ImpossibleInstanceOfRuleTest.php

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,6 @@ public function testInstanceof(): void
119119
[
120120
'Instanceof between *NEVER* and ImpossibleInstanceOf\Foo will always evaluate to false.',
121121
234,
122-
$tipText,
123122
],
124123
[
125124
'Instanceof between ImpossibleInstanceOf\Bar&ImpossibleInstanceOf\Foo and ImpossibleInstanceOf\Foo will always evaluate to true.',
@@ -227,7 +226,6 @@ public function testInstanceofWithoutAlwaysTrue(): void
227226
[
228227
'Instanceof between *NEVER* and ImpossibleInstanceOf\Foo will always evaluate to false.',
229228
234,
230-
$tipText,
231229
],
232230
[
233231
'Instanceof between *NEVER* and ImpossibleInstanceOf\Bar will always evaluate to false.',

0 commit comments

Comments
 (0)