From e0a4341d51adb0de2edf927b843c620608299eaa Mon Sep 17 00:00:00 2001 From: Vincent Langlet Date: Fri, 9 Aug 2024 14:32:21 +0200 Subject: [PATCH 1/3] Handle condition with always true/false --- src/Analyser/TypeSpecifier.php | 58 +++++++++++++++++-- ...nexistentOffsetInArrayDimFetchRuleTest.php | 6 ++ tests/PHPStan/Rules/Arrays/data/bug-11276.php | 40 +++++++++++++ 3 files changed, 98 insertions(+), 6 deletions(-) create mode 100644 tests/PHPStan/Rules/Arrays/data/bug-11276.php diff --git a/src/Analyser/TypeSpecifier.php b/src/Analyser/TypeSpecifier.php index 276d219dce..b047423e3f 100644 --- a/src/Analyser/TypeSpecifier.php +++ b/src/Analyser/TypeSpecifier.php @@ -575,10 +575,33 @@ public function specifyTypesInCondition( if (!$scope instanceof MutatingScope) { throw new ShouldNotHappenException(); } - $leftTypes = $this->specifyTypesInCondition($scope, $expr->left, $context, $rootExpr); + + if ($expr instanceof BooleanAnd && $scope->getType($expr->left)->isTrue()->yes()) { + $leftTypes = null; + } else { + $leftTypes = $this->specifyTypesInCondition($scope, $expr->left, $context, $rootExpr); + } + $rightScope = $scope->filterByTruthyValue($expr->left); - $rightTypes = $this->specifyTypesInCondition($rightScope, $expr->right, $context, $rootExpr); - $types = $context->true() ? $leftTypes->unionWith($rightTypes) : $leftTypes->normalize($scope)->intersectWith($rightTypes->normalize($rightScope)); + if ($expr instanceof BooleanAnd && $rightScope->getType($expr->right)->isTrue()->yes()) { + $rightTypes = null; + } else { + $rightTypes = $this->specifyTypesInCondition($rightScope, $expr->right, $context, $rootExpr); + } + + if (null === $leftTypes && null === $rightTypes) { + return new SpecifiedTypes([], [], false, [], $expr); + } + if (null === $leftTypes) { + return $context->true() ? $rightTypes : $rightTypes->normalize($rightScope); + } + if (null === $rightTypes) { + return $context->true() ? $leftTypes : $leftTypes->normalize($scope); + } + + $types = $context->true() + ? $leftTypes->unionWith($rightTypes) + : $leftTypes->normalize($scope)->intersectWith($rightTypes->normalize($rightScope)); if ($context->false()) { return new SpecifiedTypes( $types->getSureTypes(), @@ -599,10 +622,33 @@ public function specifyTypesInCondition( if (!$scope instanceof MutatingScope) { throw new ShouldNotHappenException(); } - $leftTypes = $this->specifyTypesInCondition($scope, $expr->left, $context, $rootExpr); + + if ($expr instanceof BooleanOr && $scope->getType($expr->left)->isFalse()->yes()) { + $leftTypes = null; + } else { + $leftTypes = $this->specifyTypesInCondition($scope, $expr->left, $context, $rootExpr); + } + $rightScope = $scope->filterByFalseyValue($expr->left); - $rightTypes = $this->specifyTypesInCondition($rightScope, $expr->right, $context, $rootExpr); - $types = $context->true() ? $leftTypes->normalize($scope)->intersectWith($rightTypes->normalize($rightScope)) : $leftTypes->unionWith($rightTypes); + if ($expr instanceof BooleanOr && $rightScope->getType($expr->right)->isFalse()->yes()) { + $rightTypes = null; + } else { + $rightTypes = $this->specifyTypesInCondition($rightScope, $expr->right, $context, $rootExpr); + } + + if (null === $leftTypes && null === $rightTypes) { + return new SpecifiedTypes([], [], false, [], $expr); + } + if (null === $leftTypes) { + return $context->true() ? $rightTypes->normalize($rightScope) : $rightTypes; + } + if (null === $rightTypes) { + return $context->true() ? $leftTypes->normalize($scope) : $leftTypes; + } + + $types = $context->true() + ? $leftTypes->normalize($scope)->intersectWith($rightTypes->normalize($rightScope)) + : $leftTypes->unionWith($rightTypes); if ($context->true()) { return new SpecifiedTypes( $types->getSureTypes(), diff --git a/tests/PHPStan/Rules/Arrays/NonexistentOffsetInArrayDimFetchRuleTest.php b/tests/PHPStan/Rules/Arrays/NonexistentOffsetInArrayDimFetchRuleTest.php index deacff9371..d4c70900b4 100644 --- a/tests/PHPStan/Rules/Arrays/NonexistentOffsetInArrayDimFetchRuleTest.php +++ b/tests/PHPStan/Rules/Arrays/NonexistentOffsetInArrayDimFetchRuleTest.php @@ -864,4 +864,10 @@ public function testBug10997(): void ]); } + public function testBug11276(): void + { + $this->reportPossiblyNonexistentConstantArrayOffset = true; + $this->analyse([__DIR__ . '/data/bug-11276.php'], []); + } + } diff --git a/tests/PHPStan/Rules/Arrays/data/bug-11276.php b/tests/PHPStan/Rules/Arrays/data/bug-11276.php new file mode 100644 index 0000000000..7594f72659 --- /dev/null +++ b/tests/PHPStan/Rules/Arrays/data/bug-11276.php @@ -0,0 +1,40 @@ +[a-z]{2})-(?[a-z]{1}[a-z0-9]{1})\/#', $url, $matches); + + foreach ($expected as $key => $value) { + if ($matches instanceof ArrayAccess || \array_key_exists($key, $matches)) { + $matches[$key]; + } + } + + foreach ($expected as $key => $value) { + if (\array_key_exists($key, $matches) || $matches instanceof ArrayAccess) { + $matches[$key]; + } + } + + foreach ($expected as $key => $value) { + if (!$matches instanceof ArrayAccess && !\array_key_exists($key, $matches)) { + } else { + $matches[$key]; + } + } + + foreach ($expected as $key => $value) { + if (!\array_key_exists($key, $matches) && !$matches instanceof ArrayAccess) { + } else { + $matches[$key]; + } + } + } +} From 87b36844d8724b48ffe8fab9e20cff58012a9600 Mon Sep 17 00:00:00 2001 From: Vincent Langlet Date: Fri, 9 Aug 2024 21:11:06 +0200 Subject: [PATCH 2/3] Rework --- src/Analyser/TypeSpecifier.php | 53 ++++++++++--------- tests/PHPStan/Analyser/TypeSpecifierTest.php | 34 ++++++++++-- .../Classes/ImpossibleInstanceOfRuleTest.php | 8 --- .../BooleanAndConstantConditionRuleTest.php | 15 ------ 4 files changed, 60 insertions(+), 50 deletions(-) diff --git a/src/Analyser/TypeSpecifier.php b/src/Analyser/TypeSpecifier.php index b047423e3f..44ef486189 100644 --- a/src/Analyser/TypeSpecifier.php +++ b/src/Analyser/TypeSpecifier.php @@ -576,43 +576,48 @@ public function specifyTypesInCondition( throw new ShouldNotHappenException(); } - if ($expr instanceof BooleanAnd && $scope->getType($expr->left)->isTrue()->yes()) { + if ($context->falsey() && $scope->getType($expr->left)->isTrue()->yes()) { $leftTypes = null; } else { $leftTypes = $this->specifyTypesInCondition($scope, $expr->left, $context, $rootExpr); } $rightScope = $scope->filterByTruthyValue($expr->left); - if ($expr instanceof BooleanAnd && $rightScope->getType($expr->right)->isTrue()->yes()) { + if ($context->falsey() && $scope->getType($expr->right)->isTrue()->yes()) { $rightTypes = null; } else { $rightTypes = $this->specifyTypesInCondition($rightScope, $expr->right, $context, $rootExpr); } - if (null === $leftTypes && null === $rightTypes) { - return new SpecifiedTypes([], [], false, [], $expr); - } - if (null === $leftTypes) { - return $context->true() ? $rightTypes : $rightTypes->normalize($rightScope); - } - if (null === $rightTypes) { - return $context->true() ? $leftTypes : $leftTypes->normalize($scope); + if ($leftTypes === null && $rightTypes === null) { + $types = new SpecifiedTypes([], [], false, [], $expr); + } elseif ($leftTypes === null) { + $types = $context->true() ? $rightTypes : $rightTypes->normalize($rightScope); + } elseif ($rightTypes === null) { + $types = $context->true() ? $leftTypes : $leftTypes->normalize($scope); + } else { + $types = $context->true() + ? $leftTypes->unionWith($rightTypes) + : $leftTypes->normalize($scope)->intersectWith($rightTypes->normalize($rightScope)); } - $types = $context->true() - ? $leftTypes->unionWith($rightTypes) - : $leftTypes->normalize($scope)->intersectWith($rightTypes->normalize($rightScope)); if ($context->false()) { - return new SpecifiedTypes( - $types->getSureTypes(), - $types->getSureNotTypes(), - false, - array_merge( + if ($leftTypes === null || $rightTypes === null) { + $conditionalTypes = []; + } else { + $conditionalTypes = array_merge( $this->processBooleanNotSureConditionalTypes($scope, $leftTypes, $rightTypes), $this->processBooleanNotSureConditionalTypes($scope, $rightTypes, $leftTypes), $this->processBooleanSureConditionalTypes($scope, $leftTypes, $rightTypes), $this->processBooleanSureConditionalTypes($scope, $rightTypes, $leftTypes), - ), + ); + } + + return new SpecifiedTypes( + $types->getSureTypes(), + $types->getSureNotTypes(), + false, + $conditionalTypes, $rootExpr, ); } @@ -623,26 +628,26 @@ public function specifyTypesInCondition( throw new ShouldNotHappenException(); } - if ($expr instanceof BooleanOr && $scope->getType($expr->left)->isFalse()->yes()) { + if ($context->truthy() && $scope->getType($expr->left)->isFalse()->yes()) { $leftTypes = null; } else { $leftTypes = $this->specifyTypesInCondition($scope, $expr->left, $context, $rootExpr); } $rightScope = $scope->filterByFalseyValue($expr->left); - if ($expr instanceof BooleanOr && $rightScope->getType($expr->right)->isFalse()->yes()) { + if ($context->truthy() && $scope->getType($expr->right)->isFalse()->yes()) { $rightTypes = null; } else { $rightTypes = $this->specifyTypesInCondition($rightScope, $expr->right, $context, $rootExpr); } - if (null === $leftTypes && null === $rightTypes) { + if ($leftTypes === null && $rightTypes === null) { return new SpecifiedTypes([], [], false, [], $expr); } - if (null === $leftTypes) { + if ($leftTypes === null) { return $context->true() ? $rightTypes->normalize($rightScope) : $rightTypes; } - if (null === $rightTypes) { + if ($rightTypes === null) { return $context->true() ? $leftTypes->normalize($scope) : $leftTypes; } diff --git a/tests/PHPStan/Analyser/TypeSpecifierTest.php b/tests/PHPStan/Analyser/TypeSpecifierTest.php index c2b21e92c4..492bc16ef7 100644 --- a/tests/PHPStan/Analyser/TypeSpecifierTest.php +++ b/tests/PHPStan/Analyser/TypeSpecifierTest.php @@ -70,7 +70,9 @@ protected function setUp(): void $this->scope = $this->scope->assignVariable('barOrFalse', new UnionType([new ObjectType('Bar'), new ConstantBooleanType(false)]), new UnionType([new ObjectType('Bar'), new ConstantBooleanType(false)])); $this->scope = $this->scope->assignVariable('stringOrFalse', new UnionType([new StringType(), new ConstantBooleanType(false)]), new UnionType([new StringType(), new ConstantBooleanType(false)])); $this->scope = $this->scope->assignVariable('array', new ArrayType(new MixedType(), new MixedType()), new ArrayType(new MixedType(), new MixedType())); + $this->scope = $this->scope->assignVariable('arrayOrNull', new UnionType([new ArrayType(new MixedType(), new MixedType()), new NullType()]), new UnionType([new ArrayType(new MixedType(), new MixedType()), new NullType()])); $this->scope = $this->scope->assignVariable('foo', new MixedType(), new MixedType()); + $this->scope = $this->scope->assignVariable('mixed', new MixedType(), new MixedType()); $this->scope = $this->scope->assignVariable('classString', new ClassStringType(), new ClassStringType()); $this->scope = $this->scope->assignVariable('genericClassString', new GenericClassStringType(new ObjectType('Bar')), new GenericClassStringType(new ObjectType('Bar'))); $this->scope = $this->scope->assignVariable('object', new ObjectWithoutClassType(), new ObjectWithoutClassType()); @@ -349,9 +351,17 @@ public function dataCondition(): iterable $this->createFunctionCall('is_int', 'foo'), $this->createFunctionCall('is_string', 'bar'), ), - [], + ['$foo' => 'int'], ['$foo' => '~int', '$bar' => '~string'], ], + [ + new Expr\BinaryOp\BooleanOr( + $this->createFunctionCall('is_int', 'foo'), + $this->createFunctionCall('is_string', 'mixed'), + ), + [], + ['$foo' => '~int', '$mixed' => '~string'], + ], [ new Expr\BinaryOp\BooleanAnd( new Expr\BinaryOp\BooleanOr( @@ -648,19 +658,37 @@ public function dataCondition(): iterable [ new Expr\Empty_(new Variable('array')), [ - '$array' => 'array{}|null', + '$array' => 'array{}', ], [ '$array' => '~0|0.0|\'\'|\'0\'|array{}|false|null', ], ], + [ + new Expr\Empty_(new Variable('arrayOrNull')), + [ + '$arrayOrNull' => 'array{}|null', + ], + [ + '$arrayOrNull' => '~0|0.0|\'\'|\'0\'|array{}|false|null', + ], + ], [ new BooleanNot(new Expr\Empty_(new Variable('array'))), [ '$array' => '~0|0.0|\'\'|\'0\'|array{}|false|null', ], [ - '$array' => 'array{}|null', + '$array' => 'array{}', + ], + ], + [ + new BooleanNot(new Expr\Empty_(new Variable('arrayOrNull'))), + [ + '$arrayOrNull' => '~0|0.0|\'\'|\'0\'|array{}|false|null', + ], + [ + '$arrayOrNull' => 'array{}|null', ], ], [ diff --git a/tests/PHPStan/Rules/Classes/ImpossibleInstanceOfRuleTest.php b/tests/PHPStan/Rules/Classes/ImpossibleInstanceOfRuleTest.php index 562401245b..a141050b1e 100644 --- a/tests/PHPStan/Rules/Classes/ImpossibleInstanceOfRuleTest.php +++ b/tests/PHPStan/Rules/Classes/ImpossibleInstanceOfRuleTest.php @@ -119,10 +119,6 @@ public function testInstanceof(): void 232, $tipText, ], - [ - 'Instanceof between *NEVER* and ImpossibleInstanceOf\Foo will always evaluate to false.', - 234, - ], [ 'Instanceof between ImpossibleInstanceOf\Bar&ImpossibleInstanceOf\Foo and ImpossibleInstanceOf\Foo will always evaluate to true.', 238, @@ -232,10 +228,6 @@ public function testInstanceofWithoutAlwaysTrue(): void 'Instanceof between *NEVER* and ImpossibleInstanceOf\Lorem will always evaluate to false.', 228, ], - [ - 'Instanceof between *NEVER* and ImpossibleInstanceOf\Foo will always evaluate to false.', - 234, - ], [ 'Instanceof between *NEVER* and ImpossibleInstanceOf\Bar will always evaluate to false.', 240, diff --git a/tests/PHPStan/Rules/Comparison/BooleanAndConstantConditionRuleTest.php b/tests/PHPStan/Rules/Comparison/BooleanAndConstantConditionRuleTest.php index d8a3d75a45..8fa4188e24 100644 --- a/tests/PHPStan/Rules/Comparison/BooleanAndConstantConditionRuleTest.php +++ b/tests/PHPStan/Rules/Comparison/BooleanAndConstantConditionRuleTest.php @@ -97,11 +97,6 @@ public function testRule(): void 64, //$tipText, ], - [ - 'Result of && is always false.', - 66, - //$tipText, - ], [ 'Result of && is always false.', 125, @@ -190,11 +185,6 @@ public function testRuleLogicalAnd(): void 64, //$tipText, ], - [ - 'Result of && is always false.', - 66, - //$tipText, - ], [ 'Result of && is always false.', 125, @@ -274,11 +264,6 @@ public function testRuleLogicalAndBleedingEdge(): void 64, //$tipText, ], - [ - 'Result of and is always false.', - 66, - //$tipText, - ], [ 'Result of and is always false.', 125, From 8763286a647e2b87a04da5cf508695ee286e87ce Mon Sep 17 00:00:00 2001 From: Vincent Langlet Date: Fri, 9 Aug 2024 21:50:23 +0200 Subject: [PATCH 3/3] Rework --- src/Analyser/TypeSpecifier.php | 85 +++++++------------ .../Classes/ImpossibleInstanceOfRuleTest.php | 8 ++ .../BooleanAndConstantConditionRuleTest.php | 15 ++++ 3 files changed, 56 insertions(+), 52 deletions(-) diff --git a/src/Analyser/TypeSpecifier.php b/src/Analyser/TypeSpecifier.php index 44ef486189..9af30f6191 100644 --- a/src/Analyser/TypeSpecifier.php +++ b/src/Analyser/TypeSpecifier.php @@ -576,48 +576,36 @@ public function specifyTypesInCondition( throw new ShouldNotHappenException(); } - if ($context->falsey() && $scope->getType($expr->left)->isTrue()->yes()) { - $leftTypes = null; - } else { - $leftTypes = $this->specifyTypesInCondition($scope, $expr->left, $context, $rootExpr); - } - + $leftTypes = $this->specifyTypesInCondition($scope, $expr->left, $context, $rootExpr); $rightScope = $scope->filterByTruthyValue($expr->left); - if ($context->falsey() && $scope->getType($expr->right)->isTrue()->yes()) { - $rightTypes = null; - } else { - $rightTypes = $this->specifyTypesInCondition($rightScope, $expr->right, $context, $rootExpr); - } + $rightTypes = $this->specifyTypesInCondition($rightScope, $expr->right, $context, $rootExpr); - if ($leftTypes === null && $rightTypes === null) { - $types = new SpecifiedTypes([], [], false, [], $expr); - } elseif ($leftTypes === null) { - $types = $context->true() ? $rightTypes : $rightTypes->normalize($rightScope); - } elseif ($rightTypes === null) { - $types = $context->true() ? $leftTypes : $leftTypes->normalize($scope); - } else { - $types = $context->true() - ? $leftTypes->unionWith($rightTypes) - : $leftTypes->normalize($scope)->intersectWith($rightTypes->normalize($rightScope)); - } + if ($context->falsey()) { + $leftIsAlwaysTrue = $scope->getType($expr->left)->isTrue()->yes(); + $rightIsAlwaysTrue = $scope->getType($expr->right)->isTrue()->yes(); - if ($context->false()) { - if ($leftTypes === null || $rightTypes === null) { - $conditionalTypes = []; - } else { - $conditionalTypes = array_merge( - $this->processBooleanNotSureConditionalTypes($scope, $leftTypes, $rightTypes), - $this->processBooleanNotSureConditionalTypes($scope, $rightTypes, $leftTypes), - $this->processBooleanSureConditionalTypes($scope, $leftTypes, $rightTypes), - $this->processBooleanSureConditionalTypes($scope, $rightTypes, $leftTypes), - ); + if ($leftIsAlwaysTrue && !$rightIsAlwaysTrue) { + return $context->true() ? $rightTypes : $rightTypes->normalize($rightScope); + } + if ($rightIsAlwaysTrue && !$leftIsAlwaysTrue) { + return $context->true() ? $leftTypes : $leftTypes->normalize($scope); } + } + $types = $context->true() + ? $leftTypes->unionWith($rightTypes) + : $leftTypes->normalize($scope)->intersectWith($rightTypes->normalize($rightScope)); + if ($context->false()) { return new SpecifiedTypes( $types->getSureTypes(), $types->getSureNotTypes(), false, - $conditionalTypes, + array_merge( + $this->processBooleanNotSureConditionalTypes($scope, $leftTypes, $rightTypes), + $this->processBooleanNotSureConditionalTypes($scope, $rightTypes, $leftTypes), + $this->processBooleanSureConditionalTypes($scope, $leftTypes, $rightTypes), + $this->processBooleanSureConditionalTypes($scope, $rightTypes, $leftTypes), + ), $rootExpr, ); } @@ -628,27 +616,20 @@ public function specifyTypesInCondition( throw new ShouldNotHappenException(); } - if ($context->truthy() && $scope->getType($expr->left)->isFalse()->yes()) { - $leftTypes = null; - } else { - $leftTypes = $this->specifyTypesInCondition($scope, $expr->left, $context, $rootExpr); - } - + $leftTypes = $this->specifyTypesInCondition($scope, $expr->left, $context, $rootExpr); $rightScope = $scope->filterByFalseyValue($expr->left); - if ($context->truthy() && $scope->getType($expr->right)->isFalse()->yes()) { - $rightTypes = null; - } else { - $rightTypes = $this->specifyTypesInCondition($rightScope, $expr->right, $context, $rootExpr); - } + $rightTypes = $this->specifyTypesInCondition($rightScope, $expr->right, $context, $rootExpr); - if ($leftTypes === null && $rightTypes === null) { - return new SpecifiedTypes([], [], false, [], $expr); - } - if ($leftTypes === null) { - return $context->true() ? $rightTypes->normalize($rightScope) : $rightTypes; - } - if ($rightTypes === null) { - return $context->true() ? $leftTypes->normalize($scope) : $leftTypes; + if ($context->truthy()) { + $leftIsAlwaysFalse = $scope->getType($expr->left)->isFalse()->yes(); + $rightIsAlwaysFalse = $scope->getType($expr->right)->isFalse()->yes(); + + if ($leftIsAlwaysFalse && !$rightIsAlwaysFalse) { + return $context->true() ? $rightTypes->normalize($rightScope) : $rightTypes; + } + if ($rightIsAlwaysFalse && !$leftIsAlwaysFalse) { + return $context->true() ? $leftTypes->normalize($scope) : $leftTypes; + } } $types = $context->true() diff --git a/tests/PHPStan/Rules/Classes/ImpossibleInstanceOfRuleTest.php b/tests/PHPStan/Rules/Classes/ImpossibleInstanceOfRuleTest.php index a141050b1e..562401245b 100644 --- a/tests/PHPStan/Rules/Classes/ImpossibleInstanceOfRuleTest.php +++ b/tests/PHPStan/Rules/Classes/ImpossibleInstanceOfRuleTest.php @@ -119,6 +119,10 @@ public function testInstanceof(): void 232, $tipText, ], + [ + 'Instanceof between *NEVER* and ImpossibleInstanceOf\Foo will always evaluate to false.', + 234, + ], [ 'Instanceof between ImpossibleInstanceOf\Bar&ImpossibleInstanceOf\Foo and ImpossibleInstanceOf\Foo will always evaluate to true.', 238, @@ -228,6 +232,10 @@ public function testInstanceofWithoutAlwaysTrue(): void 'Instanceof between *NEVER* and ImpossibleInstanceOf\Lorem will always evaluate to false.', 228, ], + [ + 'Instanceof between *NEVER* and ImpossibleInstanceOf\Foo will always evaluate to false.', + 234, + ], [ 'Instanceof between *NEVER* and ImpossibleInstanceOf\Bar will always evaluate to false.', 240, diff --git a/tests/PHPStan/Rules/Comparison/BooleanAndConstantConditionRuleTest.php b/tests/PHPStan/Rules/Comparison/BooleanAndConstantConditionRuleTest.php index 8fa4188e24..d8a3d75a45 100644 --- a/tests/PHPStan/Rules/Comparison/BooleanAndConstantConditionRuleTest.php +++ b/tests/PHPStan/Rules/Comparison/BooleanAndConstantConditionRuleTest.php @@ -97,6 +97,11 @@ public function testRule(): void 64, //$tipText, ], + [ + 'Result of && is always false.', + 66, + //$tipText, + ], [ 'Result of && is always false.', 125, @@ -185,6 +190,11 @@ public function testRuleLogicalAnd(): void 64, //$tipText, ], + [ + 'Result of && is always false.', + 66, + //$tipText, + ], [ 'Result of && is always false.', 125, @@ -264,6 +274,11 @@ public function testRuleLogicalAndBleedingEdge(): void 64, //$tipText, ], + [ + 'Result of and is always false.', + 66, + //$tipText, + ], [ 'Result of and is always false.', 125,