Skip to content

Fix more TypeSpecifier issues regarding === #1058

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 1 commit into from
Mar 9, 2022
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 28 additions & 29 deletions src/Analyser/TypeSpecifier.php
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
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
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']) {
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
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
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
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;
}
}
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
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
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