Skip to content

Commit

Permalink
Fix more TypeSpecifier issues regarding ===
Browse files Browse the repository at this point in the history
  • Loading branch information
@ondrejmirtes
ondrejmirtes committed Mar 9, 2022
1 parent 10b295f commit 6117640
Show file tree
Hide file tree
Showing 10 changed files with 274 additions and 34 deletions.
57 changes: 28 additions & 29 deletions src/Analyser/TypeSpecifier.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
use PhpParser\Node\Expr\FuncCall;
use PhpParser\Node\Expr\Instanceof_;
use PhpParser\Node\Expr\MethodCall;
use PhpParser\Node\Expr\New_;
use PhpParser\Node\Expr\PropertyFetch;
use PhpParser\Node\Expr\StaticCall;
use PhpParser\Node\Expr\StaticPropertyFetch;
Expand Down Expand Up @@ -258,14 +257,7 @@ public function specifyTypesInCondition(
$context,
);
}

if ($context->true()) {
$type = TypeCombinator::intersect($scope->getType($expr->right), $scope->getType($expr->left));
$leftTypes = $this->create($expr->left, $type, $context, false, $scope);
$rightTypes = $this->create($expr->right, $type, $context, false, $scope);
return $leftTypes->unionWith($rightTypes);

} elseif ($context->false()) {
if ($context->false()) {
$identicalType = $scope->getType($expr);
if ($identicalType instanceof ConstantBooleanType) {
$never = new NeverType();
Expand All @@ -274,18 +266,13 @@ public function specifyTypesInCondition(
$rightTypes = $this->create($expr->right, $never, $contextForTypes, false, $scope);
return $leftTypes->unionWith($rightTypes);
}
}

$exprLeftType = $scope->getType($expr->left);
$exprRightType = $scope->getType($expr->right);

$types = null;

if (
(
$exprLeftType instanceof ConstantType
&& !$expr->right instanceof Node\Scalar
) || $exprLeftType instanceof EnumCaseObjectType
) {
$types = null;
$exprLeftType = $scope->getType($expr->left);
$exprRightType = $scope->getType($expr->right);
if ($exprLeftType instanceof ConstantType || $exprLeftType instanceof EnumCaseObjectType) {
if (!$expr->right instanceof Node\Scalar && !$expr->right instanceof Expr\Array_) {
$types = $this->create(
$expr->right,
$exprLeftType,
Expand All @@ -294,12 +281,9 @@ public function specifyTypesInCondition(
$scope,
);
}
if (
(
$exprRightType instanceof ConstantType
&& !$expr->left instanceof Node\Scalar
) || $exprRightType instanceof EnumCaseObjectType
) {
}
if ($exprRightType instanceof ConstantType || $exprRightType instanceof EnumCaseObjectType) {
if ($types === null || (!$expr->left instanceof Node\Scalar && !$expr->left instanceof Expr\Array_)) {
$leftType = $this->create(
$expr->left,
$exprRightType,
Expand All @@ -313,11 +297,26 @@ public function specifyTypesInCondition(
$types = $leftType;
}
}
}

if ($types !== null) {
return $types;
}

if ($types !== null) {
return $types;
$leftExprString = $this->printer->prettyPrintExpr($expr->left);
$rightExprString = $this->printer->prettyPrintExpr($expr->right);
if ($leftExprString === $rightExprString) {
if (!$expr->left instanceof Expr\Variable || !$expr->right instanceof Expr\Variable) {
return new SpecifiedTypes();
}
}

if ($context->true()) {
$type = TypeCombinator::intersect($scope->getType($expr->right), $scope->getType($expr->left));
$leftTypes = $this->create($expr->left, $type, $context, false, $scope);
$rightTypes = $this->create($expr->right, $type, $context, false, $scope);
return $leftTypes->unionWith($rightTypes);
} elseif ($context->false()) {
return $this->create($expr->left, $exprLeftType, $context, false, $scope)->normalize($scope)
->intersectWith($this->create($expr->right, $exprRightType, $context, false, $scope)->normalize($scope));
}
Expand Down Expand Up @@ -981,7 +980,7 @@ public function create(
?Scope $scope = null,
): SpecifiedTypes
{
if ($expr instanceof New_ || $expr instanceof Instanceof_ || $expr instanceof Expr\List_ || $expr instanceof VirtualNode) {
if ($expr instanceof Instanceof_ || $expr instanceof Expr\List_ || $expr instanceof VirtualNode) {
return new SpecifiedTypes();
}

Expand Down
1 change: 0 additions & 1 deletion tests/PHPStan/Analyser/TypeSpecifierTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -554,7 +554,6 @@ public function dataCondition(): array
),
[
'$foo' => '123',
123 => '123',
],
['$foo' => '~123'],
],
Expand Down
10 changes: 10 additions & 0 deletions tests/PHPStan/Analyser/data/equal.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,16 @@ public function stdClass(\stdClass $a, \stdClass $b): void
assertType('stdClass', $b);
}

/**
* @param array{a: string, b: array{c: string|null}} $a
*/
public function arrayOffset(array $a): void
{
if (strlen($a['a']) > 0 && $a['a'] === $a['b']['c']) {

This comment has been minimized.

Sign in to view

Copy link
@staabm

staabm Mar 9, 2022

Contributor

Should this be == instead?

This comment has been minimized.

Sign in to view

Copy link
@ondrejmirtes

ondrejmirtes Mar 9, 2022

Author Member

Nope, this is just a regression test for issue in Composer source.
assertType('array{a: non-empty-string, b: array{c: non-empty-string}}', $a);
}
}

}

class Bar
Expand Down
10 changes: 10 additions & 0 deletions tests/PHPStan/Analyser/data/identical.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,16 @@ public function foo(\stdClass $a, \stdClass $b): void
assertType('stdClass', $b);
}

/**
* @param array{a: string, b: array{c: string|null}} $a
*/
public function arrayOffset(array $a): void
{
if (strlen($a['a']) > 0 && $a['a'] === $a['b']['c']) {
assertType('array{a: non-empty-string, b: array{c: non-empty-string}}', $a);
}
}

}

class Bar
Expand Down
4 changes: 4 additions & 0 deletions tests/PHPStan/Rules/Arrays/NonexistentOffsetInArrayDimFetchRuleTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,10 @@ public function testRule(): void
'Cannot access offset \'foo\' on array|int.',
443,
],
[
'Offset \'feature_pretty…\' does not exist on array{version: non-empty-string, commit: string|null, pretty_version: string|null, feature_version: non-empty-string, feature_pretty_version?: string|null}.',
504,
],
]);
}

Expand Down
23 changes: 23 additions & 0 deletions tests/PHPStan/Rules/Arrays/data/nonexistent-offset.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -485,3 +485,26 @@ function test($array): void {
}

}

/**
* @phpstan-type Version array{version: string, commit: string|null, pretty_version: string|null, feature_version?: string|null, feature_pretty_version?: string|null}
*/
class VersionGuesser
{
/**
* @param array $versionData
*
* @phpstan-param Version $versionData
*
* @return array
* @phpstan-return Version
*/
private function postprocess(array $versionData): array
{
if (!empty($versionData['feature_version']) && $versionData['feature_version'] === $versionData['version'] && $versionData['feature_pretty_version'] === $versionData['pretty_version']) {
unset($versionData['feature_version'], $versionData['feature_pretty_version']);
}

return $versionData;
}
}
32 changes: 32 additions & 0 deletions tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeMethodCallRuleEqualsTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,38 @@ public function testRule(): void
'Call to method ImpossibleMethodCall\Foo::isNotSame() with stdClass and stdClass will always evaluate to false.',
81,
],
[
'Call to method ImpossibleMethodCall\Foo::isSame() with \'foo\' and \'foo\' will always evaluate to true.',
101,
],
[
'Call to method ImpossibleMethodCall\Foo::isNotSame() with \'foo\' and \'foo\' will always evaluate to false.',
104,
],
[
'Call to method ImpossibleMethodCall\Foo::isSame() with array{} and array{} will always evaluate to true.',
113,
],
[
'Call to method ImpossibleMethodCall\Foo::isNotSame() with array{} and array{} will always evaluate to false.',
116,
],
[
'Call to method ImpossibleMethodCall\Foo::isSame() with \'\' and \'\' will always evaluate to true.',
174,
],
[
'Call to method ImpossibleMethodCall\Foo::isNotSame() with \'\' and \'\' will always evaluate to false.',
175,
],
[
'Call to method ImpossibleMethodCall\Foo::isSame() with 1 and 1 will always evaluate to true.',
191,
],
[
'Call to method ImpossibleMethodCall\Foo::isNotSame() with 2 and 2 will always evaluate to false.',
194,
],
]);
}

Expand Down
72 changes: 70 additions & 2 deletions tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeMethodCallRuleTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,76 @@ public function testRule(): void
81,
],
[
'Call to method ImpossibleMethodCall\Foo::isSame() with *NEVER* and stdClass will always evaluate to false.',
84,
'Call to method ImpossibleMethodCall\Foo::isSame() with \'foo\' and \'foo\' will always evaluate to true.',
101,
],
[
'Call to method ImpossibleMethodCall\Foo::isNotSame() with \'foo\' and \'foo\' will always evaluate to false.',
104,
],
[
'Call to method ImpossibleMethodCall\Foo::isSame() with array{} and array{} will always evaluate to true.',
113,
],
[
'Call to method ImpossibleMethodCall\Foo::isNotSame() with array{} and array{} will always evaluate to false.',
116,
],
[
'Call to method ImpossibleMethodCall\Foo::isSame() with array{1, 3} and array{1, 3} will always evaluate to true.',
119,
],
[
'Call to method ImpossibleMethodCall\Foo::isNotSame() with array{1, 3} and array{1, 3} will always evaluate to false.',
122,
],
[
'Call to method ImpossibleMethodCall\Foo::isSame() with 1 and stdClass will always evaluate to false.',
126,
],
[
'Call to method ImpossibleMethodCall\Foo::isNotSame() with 1 and stdClass will always evaluate to true.',
130,
],
[
'Call to method ImpossibleMethodCall\Foo::isSame() with \'1\' and stdClass will always evaluate to false.',
133,
],
[
'Call to method ImpossibleMethodCall\Foo::isNotSame() with \'1\' and stdClass will always evaluate to true.',
136,
],
[
'Call to method ImpossibleMethodCall\Foo::isSame() with array{\'a\', \'b\'} and array{1, 2} will always evaluate to false.',
139,
],
[
'Call to method ImpossibleMethodCall\Foo::isNotSame() with array{\'a\', \'b\'} and array{1, 2} will always evaluate to true.',
142,
],
[
'Call to method ImpossibleMethodCall\Foo::isSame() with stdClass and \'1\' will always evaluate to false.',
145,
],
[
'Call to method ImpossibleMethodCall\Foo::isNotSame() with stdClass and \'1\' will always evaluate to true.',
148,
],
[
'Call to method ImpossibleMethodCall\Foo::isSame() with \'\' and \'\' will always evaluate to true.',
174,
],
[
'Call to method ImpossibleMethodCall\Foo::isNotSame() with \'\' and \'\' will always evaluate to false.',
175,
],
[
'Call to method ImpossibleMethodCall\Foo::isSame() with 1 and 1 will always evaluate to true.',
191,
],
[
'Call to method ImpossibleMethodCall\Foo::isNotSame() with 2 and 2 will always evaluate to false.',
194,
],
]);
}
Expand Down
4 changes: 2 additions & 2 deletions tests/PHPStan/Rules/Comparison/data/TestMethodTypeSpecifyingExtensions.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ public function specifyTypes(
$node->args[0]->value,
$node->args[1]->value
),
TypeSpecifierContext::createTruthy()
$context
);
}

Expand Down Expand Up @@ -144,7 +144,7 @@ public function specifyTypes(
$node->args[0]->value,
$node->args[1]->value
),
TypeSpecifierContext::createTruthy()
$context
);
}

Expand Down
Loading

0 comments on commit 6117640

Please sign in to comment.