From 4f6765e7c98bdb5d1b67f1f914a93e10fd3d43d5 Mon Sep 17 00:00:00 2001 From: Martin Herndl Date: Wed, 27 Apr 2022 15:03:48 +0200 Subject: [PATCH 1/9] Use root expression when checking impossible types --- src/Analyser/SpecifiedTypes.php | 27 ++- src/Analyser/TypeSpecifier.php | 192 +++++++++++------- src/Process/Runnable/RunnableQueue.php | 6 + .../Comparison/ImpossibleCheckTypeHelper.php | 27 ++- .../ImpossibleCheckTypeMethodCallRuleTest.php | 4 + ...sibleCheckTypeStaticMethodCallRuleTest.php | 8 + 6 files changed, 172 insertions(+), 92 deletions(-) diff --git a/src/Analyser/SpecifiedTypes.php b/src/Analyser/SpecifiedTypes.php index aefc6214f2..83f19a7110 100644 --- a/src/Analyser/SpecifiedTypes.php +++ b/src/Analyser/SpecifiedTypes.php @@ -20,6 +20,7 @@ public function __construct( private array $sureNotTypes = [], private bool $overwrite = false, private array $newConditionalExpressionHolders = [], + private ?Expr $rootExpr = null, ) { } @@ -55,11 +56,17 @@ public function getNewConditionalExpressionHolders(): array return $this->newConditionalExpressionHolders; } + public function getRootExpr(): ?Expr + { + return $this->rootExpr; + } + /** @api */ public function intersectWith(SpecifiedTypes $other): self { $sureTypeUnion = []; $sureNotTypeUnion = []; + $rootExpr = $this->mergeRootExpr($this->rootExpr, $other->rootExpr); foreach ($this->sureTypes as $exprString => [$exprNode, $type]) { if (!isset($other->sureTypes[$exprString])) { @@ -83,7 +90,7 @@ public function intersectWith(SpecifiedTypes $other): self ]; } - return new self($sureTypeUnion, $sureNotTypeUnion); + return new self($sureTypeUnion, $sureNotTypeUnion, false, [], $rootExpr); } /** @api */ @@ -91,6 +98,7 @@ public function unionWith(SpecifiedTypes $other): self { $sureTypeUnion = $this->sureTypes + $other->sureTypes; $sureNotTypeUnion = $this->sureNotTypes + $other->sureNotTypes; + $rootExpr = $this->mergeRootExpr($this->rootExpr, $other->rootExpr); foreach ($this->sureTypes as $exprString => [$exprNode, $type]) { if (!isset($other->sureTypes[$exprString])) { @@ -114,7 +122,7 @@ public function unionWith(SpecifiedTypes $other): self ]; } - return new self($sureTypeUnion, $sureNotTypeUnion); + return new self($sureTypeUnion, $sureNotTypeUnion, false, [], $rootExpr); } public function normalize(Scope $scope): self @@ -130,7 +138,20 @@ public function normalize(Scope $scope): self $sureTypes[$exprString][1] = TypeCombinator::remove($sureTypes[$exprString][1], $sureNotType); } - return new self($sureTypes, [], $this->overwrite, $this->newConditionalExpressionHolders); + return new self($sureTypes, [], $this->overwrite, $this->newConditionalExpressionHolders, $this->rootExpr); + } + + private function mergeRootExpr(?Expr $rootExprA, ?Expr $rootExprB): ?Expr + { + if ($rootExprA === $rootExprB) { + return $rootExprA; + } + + if ($rootExprA === null || $rootExprB === null) { + return $rootExprA ?? $rootExprB; + } + + return null; } } diff --git a/src/Analyser/TypeSpecifier.php b/src/Analyser/TypeSpecifier.php index 715b5df326..0ae373cb1d 100644 --- a/src/Analyser/TypeSpecifier.php +++ b/src/Analyser/TypeSpecifier.php @@ -101,10 +101,13 @@ public function specifyTypesInCondition( Scope $scope, Expr $expr, TypeSpecifierContext $context, + ?Expr $rootExpr = null, ): SpecifiedTypes { + $rootExpr ??= $expr; + if ($expr instanceof Expr\CallLike && $expr->isFirstClassCallable()) { - return new SpecifiedTypes(); + return new SpecifiedTypes([], [], false, [], $rootExpr); } if ($expr instanceof Instanceof_) { @@ -128,7 +131,7 @@ public function specifyTypesInCondition( } else { $type = new ObjectType($className); } - return $this->create($exprNode, $type, $context, false, $scope); + return $this->create($exprNode, $type, $context, false, $scope, $rootExpr); } $classType = $scope->getType($expr->class); @@ -154,16 +157,16 @@ public function specifyTypesInCondition( $type, new ObjectWithoutClassType(), ); - return $this->create($exprNode, $type, $context, false, $scope); + return $this->create($exprNode, $type, $context, false, $scope, $rootExpr); } elseif ($context->false()) { $exprType = $scope->getType($expr->expr); if (!$type->isSuperTypeOf($exprType)->yes()) { - return $this->create($exprNode, $type, $context, false, $scope); + return $this->create($exprNode, $type, $context, false, $scope, $rootExpr); } } } if ($context->true()) { - return $this->create($exprNode, new ObjectWithoutClassType(), $context, false, $scope); + return $this->create($exprNode, new ObjectWithoutClassType(), $context, false, $scope, $rootExpr); } } elseif ($expr instanceof Node\Expr\BinaryOp\Identical) { $expressions = $this->findTypeExpressionsFromBinaryOperation($scope, $expr); @@ -173,25 +176,27 @@ public function specifyTypesInCondition( /** @var ConstantScalarType $constantType */ $constantType = $expressions[1]; if ($constantType->getValue() === false) { - $types = $this->create($exprNode, $constantType, $context, false, $scope); + $types = $this->create($exprNode, $constantType, $context, false, $scope, $rootExpr); return $types->unionWith($this->specifyTypesInCondition( $scope, $exprNode, $context->true() ? TypeSpecifierContext::createFalse() : TypeSpecifierContext::createFalse()->negate(), + $rootExpr, )); } if ($constantType->getValue() === true) { - $types = $this->create($exprNode, $constantType, $context, false, $scope); + $types = $this->create($exprNode, $constantType, $context, false, $scope, $rootExpr); return $types->unionWith($this->specifyTypesInCondition( $scope, $exprNode, $context->true() ? TypeSpecifierContext::createTrue() : TypeSpecifierContext::createTrue()->negate(), + $rootExpr, )); } if ($constantType->getValue() === null) { - return $this->create($exprNode, $constantType, $context, false, $scope); + return $this->create($exprNode, $constantType, $context, false, $scope, $rootExpr); } if ( @@ -209,8 +214,8 @@ public function specifyTypesInCondition( } $argType = $scope->getType($exprNode->getArgs()[0]->value); if ($argType->isArray()->yes()) { - $funcTypes = $this->create($exprNode, $constantType, $context, false, $scope); - $valueTypes = $this->create($exprNode->getArgs()[0]->value, new NonEmptyArrayType(), $newContext, false, $scope); + $funcTypes = $this->create($exprNode, $constantType, $context, false, $scope, $rootExpr); + $valueTypes = $this->create($exprNode->getArgs()[0]->value, new NonEmptyArrayType(), $newContext, false, $scope, $rootExpr); return $funcTypes->unionWith($valueTypes); } } @@ -231,8 +236,8 @@ public function specifyTypesInCondition( } $argType = $scope->getType($exprNode->getArgs()[0]->value); if ($argType instanceof StringType) { - $funcTypes = $this->create($exprNode, $constantType, $context, false, $scope); - $valueTypes = $this->create($exprNode->getArgs()[0]->value, new AccessoryNonEmptyStringType(), $newContext, false, $scope); + $funcTypes = $this->create($exprNode, $constantType, $context, false, $scope, $rootExpr); + $valueTypes = $this->create($exprNode->getArgs()[0]->value, new AccessoryNonEmptyStringType(), $newContext, false, $scope, $rootExpr); return $funcTypes->unionWith($valueTypes); } } @@ -255,6 +260,7 @@ public function specifyTypesInCondition( new Name($rightType->getValue()), ), $context, + $rootExpr, ); } if ($context->false()) { @@ -262,8 +268,8 @@ public function specifyTypesInCondition( if ($identicalType instanceof ConstantBooleanType) { $never = new NeverType(); $contextForTypes = $identicalType->getValue() ? $context->negate() : $context; - $leftTypes = $this->create($expr->left, $never, $contextForTypes, false, $scope); - $rightTypes = $this->create($expr->right, $never, $contextForTypes, false, $scope); + $leftTypes = $this->create($expr->left, $never, $contextForTypes, false, $scope, $rootExpr); + $rightTypes = $this->create($expr->right, $never, $contextForTypes, false, $scope, $rootExpr); return $leftTypes->unionWith($rightTypes); } } @@ -279,6 +285,7 @@ public function specifyTypesInCondition( $context, false, $scope, + $rootExpr, ); } } @@ -290,6 +297,7 @@ public function specifyTypesInCondition( $context, false, $scope, + $rootExpr, ); if ($types !== null) { $types = $types->unionWith($leftType); @@ -307,18 +315,18 @@ public function specifyTypesInCondition( $rightExprString = $this->printer->prettyPrintExpr($expr->right); if ($leftExprString === $rightExprString) { if (!$expr->left instanceof Expr\Variable || !$expr->right instanceof Expr\Variable) { - return new SpecifiedTypes(); + return new SpecifiedTypes([], [], false, [], $rootExpr); } } 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); + $leftTypes = $this->create($expr->left, $type, $context, false, $scope, $rootExpr); + $rightTypes = $this->create($expr->right, $type, $context, false, $scope, $rootExpr); 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)); + return $this->create($expr->left, $exprLeftType, $context, false, $scope, $rootExpr)->normalize($scope) + ->intersectWith($this->create($expr->right, $exprRightType, $context, false, $scope, $rootExpr)->normalize($scope)); } } elseif ($expr instanceof Node\Expr\BinaryOp\NotIdentical) { @@ -326,6 +334,7 @@ public function specifyTypesInCondition( $scope, new Node\Expr\BooleanNot(new Node\Expr\BinaryOp\Identical($expr->left, $expr->right)), $context, + $rootExpr, ); } elseif ($expr instanceof Node\Expr\BinaryOp\Equal) { $expressions = $this->findTypeExpressionsFromBinaryOperation($scope, $expr); @@ -339,6 +348,7 @@ public function specifyTypesInCondition( $scope, $exprNode, $context->true() ? TypeSpecifierContext::createFalsey() : TypeSpecifierContext::createFalsey()->negate(), + $rootExpr, ); } @@ -347,6 +357,7 @@ public function specifyTypesInCondition( $scope, $exprNode, $context->true() ? TypeSpecifierContext::createTruthy() : TypeSpecifierContext::createTruthy()->negate(), + $rootExpr, ); } } @@ -363,6 +374,7 @@ public function specifyTypesInCondition( $expr->right, ), $context, + $rootExpr, ); } @@ -375,6 +387,7 @@ public function specifyTypesInCondition( new ConstFetch(new Name($rightBooleanType->getValue() ? 'true' : 'false')), ), $context, + $rootExpr, ); } @@ -383,7 +396,7 @@ public function specifyTypesInCondition( && $rightType->isArray()->yes() && $leftType instanceof ConstantArrayType && $leftType->isEmpty() ) { - return $this->create($expr->right, new NonEmptyArrayType(), $context->negate(), false, $scope); + return $this->create($expr->right, new NonEmptyArrayType(), $context->negate(), false, $scope, $rootExpr); } if ( @@ -391,7 +404,7 @@ public function specifyTypesInCondition( && $leftType->isArray()->yes() && $rightType instanceof ConstantArrayType && $rightType->isEmpty() ) { - return $this->create($expr->left, new NonEmptyArrayType(), $context->negate(), false, $scope); + return $this->create($expr->left, new NonEmptyArrayType(), $context->negate(), false, $scope, $rootExpr); } if ( @@ -408,6 +421,7 @@ public function specifyTypesInCondition( new Name($rightType->getValue()), ), $context, + $rootExpr, ); } @@ -425,6 +439,7 @@ public function specifyTypesInCondition( new Name($leftType->getValue()), ), $context, + $rootExpr, ); } @@ -436,19 +451,19 @@ public function specifyTypesInCondition( || ($integerType->isSuperTypeOf($leftType)->yes() && $integerType->isSuperTypeOf($rightType)->yes()) || ($floatType->isSuperTypeOf($leftType)->yes() && $floatType->isSuperTypeOf($rightType)->yes()) ) { - return $this->specifyTypesInCondition($scope, new Expr\BinaryOp\Identical($expr->left, $expr->right), $context); + return $this->specifyTypesInCondition($scope, new Expr\BinaryOp\Identical($expr->left, $expr->right), $context, $rootExpr); } $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(); + return new SpecifiedTypes([], [], false, [], $rootExpr); } } - $leftTypes = $this->create($expr->left, $leftType, $context, false, $scope); - $rightTypes = $this->create($expr->right, $rightType, $context, false, $scope); + $leftTypes = $this->create($expr->left, $leftType, $context, false, $scope, $rootExpr); + $rightTypes = $this->create($expr->right, $rightType, $context, false, $scope, $rootExpr); return $context->true() ? $leftTypes->unionWith($rightTypes) @@ -458,6 +473,7 @@ public function specifyTypesInCondition( $scope, new Node\Expr\BooleanNot(new Node\Expr\BinaryOp\Equal($expr->left, $expr->right)), $context, + $rootExpr, ); } elseif ($expr instanceof Node\Expr\BinaryOp\Smaller || $expr instanceof Node\Expr\BinaryOp\SmallerOrEqual) { @@ -485,10 +501,11 @@ public function specifyTypesInCondition( $scope, new Node\Expr\BooleanNot($inverseOperator), $context, + $rootExpr, ); } - $result = new SpecifiedTypes(); + $result = new SpecifiedTypes([], [], false, [], $rootExpr); if ( !$context->null() @@ -504,7 +521,7 @@ public function specifyTypesInCondition( ) { $argType = $scope->getType($expr->right->getArgs()[0]->value); if ($argType->isArray()->yes()) { - $result = $result->unionWith($this->create($expr->right->getArgs()[0]->value, new NonEmptyArrayType(), $context, false, $scope)); + $result = $result->unionWith($this->create($expr->right->getArgs()[0]->value, new NonEmptyArrayType(), $context, false, $scope, $rootExpr)); } } } @@ -523,7 +540,7 @@ public function specifyTypesInCondition( ) { $argType = $scope->getType($expr->right->getArgs()[0]->value); if ($argType instanceof StringType) { - $result = $result->unionWith($this->create($expr->right->getArgs()[0]->value, new AccessoryNonEmptyStringType(), $context, false, $scope)); + $result = $result->unionWith($this->create($expr->right->getArgs()[0]->value, new AccessoryNonEmptyStringType(), $context, false, $scope, $rootExpr)); } } } @@ -531,18 +548,21 @@ public function specifyTypesInCondition( if ($leftType instanceof ConstantIntegerType) { if ($expr->right instanceof Expr\PostInc) { $result = $result->unionWith($this->createRangeTypes( + $rootExpr, $expr->right->var, IntegerRangeType::fromInterval($leftType->getValue(), null, $offset + 1), $context, )); } elseif ($expr->right instanceof Expr\PostDec) { $result = $result->unionWith($this->createRangeTypes( + $rootExpr, $expr->right->var, IntegerRangeType::fromInterval($leftType->getValue(), null, $offset - 1), $context, )); } elseif ($expr->right instanceof Expr\PreInc || $expr->right instanceof Expr\PreDec) { $result = $result->unionWith($this->createRangeTypes( + $rootExpr, $expr->right->var, IntegerRangeType::fromInterval($leftType->getValue(), null, $offset), $context, @@ -553,18 +573,21 @@ public function specifyTypesInCondition( if ($rightType instanceof ConstantIntegerType) { if ($expr->left instanceof Expr\PostInc) { $result = $result->unionWith($this->createRangeTypes( + $rootExpr, $expr->left->var, IntegerRangeType::fromInterval(null, $rightType->getValue(), -$offset + 1), $context, )); } elseif ($expr->left instanceof Expr\PostDec) { $result = $result->unionWith($this->createRangeTypes( + $rootExpr, $expr->left->var, IntegerRangeType::fromInterval(null, $rightType->getValue(), -$offset - 1), $context, )); } elseif ($expr->left instanceof Expr\PreInc || $expr->left instanceof Expr\PreDec) { $result = $result->unionWith($this->createRangeTypes( + $rootExpr, $expr->left->var, IntegerRangeType::fromInterval(null, $rightType->getValue(), -$offset), $context, @@ -581,6 +604,7 @@ public function specifyTypesInCondition( TypeSpecifierContext::createTruthy(), false, $scope, + $rootExpr, ), ); } @@ -592,6 +616,7 @@ public function specifyTypesInCondition( TypeSpecifierContext::createTruthy(), false, $scope, + $rootExpr, ), ); } @@ -604,6 +629,7 @@ public function specifyTypesInCondition( TypeSpecifierContext::createTruthy(), false, $scope, + $rootExpr, ), ); } @@ -615,6 +641,7 @@ public function specifyTypesInCondition( TypeSpecifierContext::createTruthy(), false, $scope, + $rootExpr, ), ); } @@ -623,10 +650,10 @@ public function specifyTypesInCondition( return $result; } elseif ($expr instanceof Node\Expr\BinaryOp\Greater) { - return $this->specifyTypesInCondition($scope, new Expr\BinaryOp\Smaller($expr->right, $expr->left), $context); + return $this->specifyTypesInCondition($scope, new Expr\BinaryOp\Smaller($expr->right, $expr->left), $context, $rootExpr); } elseif ($expr instanceof Node\Expr\BinaryOp\GreaterOrEqual) { - return $this->specifyTypesInCondition($scope, new Expr\BinaryOp\SmallerOrEqual($expr->right, $expr->left), $context); + return $this->specifyTypesInCondition($scope, new Expr\BinaryOp\SmallerOrEqual($expr->right, $expr->left), $context, $rootExpr); } elseif ($expr instanceof FuncCall && $expr->name instanceof Name) { if ($this->reflectionProvider->hasFunction($expr->name, $scope)) { @@ -640,7 +667,7 @@ public function specifyTypesInCondition( } } - return $this->handleDefaultTruthyOrFalseyContext($context, $expr, $scope); + return $this->handleDefaultTruthyOrFalseyContext($context, $rootExpr, $expr, $scope); } elseif ($expr instanceof MethodCall && $expr->name instanceof Node\Identifier) { $methodCalledOnType = $scope->getType($expr->var); $referencedClasses = TypeUtils::getDirectClassNames($methodCalledOnType); @@ -661,7 +688,7 @@ public function specifyTypesInCondition( } } - return $this->handleDefaultTruthyOrFalseyContext($context, $expr, $scope); + return $this->handleDefaultTruthyOrFalseyContext($context, $rootExpr, $expr, $scope); } elseif ($expr instanceof StaticCall && $expr->name instanceof Node\Identifier) { if ($expr->class instanceof Name) { $calleeType = $scope->resolveTypeByName($expr->class); @@ -687,14 +714,14 @@ public function specifyTypesInCondition( } } - return $this->handleDefaultTruthyOrFalseyContext($context, $expr, $scope); + return $this->handleDefaultTruthyOrFalseyContext($context, $rootExpr, $expr, $scope); } elseif ($expr instanceof BooleanAnd || $expr instanceof LogicalAnd) { if (!$scope instanceof MutatingScope) { throw new ShouldNotHappenException(); } - $leftTypes = $this->specifyTypesInCondition($scope, $expr->left, $context); + $leftTypes = $this->specifyTypesInCondition($scope, $expr->left, $context, $rootExpr); $rightScope = $scope->filterByTruthyValue($expr->left); - $rightTypes = $this->specifyTypesInCondition($rightScope, $expr->right, $context); + $rightTypes = $this->specifyTypesInCondition($rightScope, $expr->right, $context, $rootExpr); $types = $context->true() ? $leftTypes->unionWith($rightTypes) : $leftTypes->normalize($scope)->intersectWith($rightTypes->normalize($rightScope)); if ($context->false()) { return new SpecifiedTypes( @@ -705,6 +732,7 @@ public function specifyTypesInCondition( $this->processBooleanConditionalTypes($scope, $leftTypes, $rightTypes), $this->processBooleanConditionalTypes($scope, $rightTypes, $leftTypes), ), + $rootExpr, ); } @@ -713,9 +741,9 @@ public function specifyTypesInCondition( if (!$scope instanceof MutatingScope) { throw new ShouldNotHappenException(); } - $leftTypes = $this->specifyTypesInCondition($scope, $expr->left, $context); + $leftTypes = $this->specifyTypesInCondition($scope, $expr->left, $context, $rootExpr); $rightScope = $scope->filterByFalseyValue($expr->left); - $rightTypes = $this->specifyTypesInCondition($rightScope, $expr->right, $context); + $rightTypes = $this->specifyTypesInCondition($rightScope, $expr->right, $context, $rootExpr); $types = $context->true() ? $leftTypes->normalize($scope)->intersectWith($rightTypes->normalize($rightScope)) : $leftTypes->unionWith($rightTypes); if ($context->true()) { return new SpecifiedTypes( @@ -726,21 +754,22 @@ public function specifyTypesInCondition( $this->processBooleanConditionalTypes($scope, $leftTypes, $rightTypes), $this->processBooleanConditionalTypes($scope, $rightTypes, $leftTypes), ), + $rootExpr, ); } return $types; } elseif ($expr instanceof Node\Expr\BooleanNot && !$context->null()) { - return $this->specifyTypesInCondition($scope, $expr->expr, $context->negate()); + return $this->specifyTypesInCondition($scope, $expr->expr, $context->negate(), $rootExpr); } elseif ($expr instanceof Node\Expr\Assign) { if (!$scope instanceof MutatingScope) { throw new ShouldNotHappenException(); } if ($context->null()) { - return $this->specifyTypesInCondition($scope->exitFirstLevelStatements(), $expr->expr, $context); + return $this->specifyTypesInCondition($scope->exitFirstLevelStatements(), $expr->expr, $context, $rootExpr); } - return $this->specifyTypesInCondition($scope->exitFirstLevelStatements(), $expr->var, $context); + return $this->specifyTypesInCondition($scope->exitFirstLevelStatements(), $expr->var, $context, $rootExpr); } elseif ( $expr instanceof Expr\Isset_ && count($expr->vars) > 0 @@ -774,7 +803,7 @@ public function specifyTypesInCondition( foreach ($vars as $var) { if ($var instanceof Expr\Variable && is_string($var->name)) { if ($scope->hasVariableType($var->name)->no()) { - return new SpecifiedTypes([], []); + return new SpecifiedTypes([], [], false, [], $rootExpr); } } if ( @@ -788,11 +817,12 @@ public function specifyTypesInCondition( $context, false, $scope, + $rootExpr, )->unionWith( - $this->create($var, new NullType(), TypeSpecifierContext::createFalse(), false, $scope), + $this->create($var, new NullType(), TypeSpecifierContext::createFalse(), false, $scope, $rootExpr), ); } else { - $type = $this->create($var, new NullType(), TypeSpecifierContext::createFalse(), false, $scope); + $type = $this->create($var, new NullType(), TypeSpecifierContext::createFalse(), false, $scope, $rootExpr); } if ( @@ -802,7 +832,7 @@ public function specifyTypesInCondition( $type = $type->unionWith($this->create($var->var, new IntersectionType([ new ObjectWithoutClassType(), new HasPropertyType($var->name->toString()), - ]), TypeSpecifierContext::createTruthy(), false, $scope)); + ]), TypeSpecifierContext::createTruthy(), false, $scope, $rootExpr)); } elseif ( $var instanceof StaticPropertyFetch && $var->class instanceof Expr @@ -811,7 +841,7 @@ public function specifyTypesInCondition( $type = $type->unionWith($this->create($var->class, new IntersectionType([ new ObjectWithoutClassType(), new HasPropertyType($var->name->toString()), - ]), TypeSpecifierContext::createTruthy(), false, $scope)); + ]), TypeSpecifierContext::createTruthy(), false, $scope, $rootExpr)); } if ($types === null) { @@ -833,6 +863,7 @@ public function specifyTypesInCondition( TypeSpecifierContext::createFalse(), false, $scope, + $rootExpr, ); } elseif ( $expr instanceof Expr\Empty_ @@ -840,9 +871,9 @@ public function specifyTypesInCondition( return $this->specifyTypesInCondition($scope, new BooleanOr( new Expr\BooleanNot(new Expr\Isset_([$expr->expr])), new Expr\BooleanNot($expr->expr), - ), $context); + ), $context, $rootExpr); } elseif ($expr instanceof Expr\ErrorSuppress) { - return $this->specifyTypesInCondition($scope, $expr->expr, $context); + return $this->specifyTypesInCondition($scope, $expr->expr, $context, $rootExpr); } elseif ( $expr instanceof Expr\Ternary && !$context->null() @@ -853,7 +884,7 @@ public function specifyTypesInCondition( $conditionExpr = new BooleanAnd($conditionExpr, $expr->if); } - return $this->specifyTypesInCondition($scope, $conditionExpr, $context); + return $this->specifyTypesInCondition($scope, $conditionExpr, $context, $rootExpr); } elseif ($expr instanceof Expr\NullsafePropertyFetch && !$context->null()) { $types = $this->specifyTypesInCondition( @@ -863,9 +894,10 @@ public function specifyTypesInCondition( new PropertyFetch($expr->var, $expr->name), ), $context, + $rootExpr, ); - $nullSafeTypes = $this->handleDefaultTruthyOrFalseyContext($context, $expr, $scope); + $nullSafeTypes = $this->handleDefaultTruthyOrFalseyContext($context, $rootExpr, $expr, $scope); return $context->true() ? $types->unionWith($nullSafeTypes) : $types->normalize($scope)->intersectWith($nullSafeTypes->normalize($scope)); } elseif ($expr instanceof Expr\NullsafeMethodCall && !$context->null()) { $types = $this->specifyTypesInCondition( @@ -875,31 +907,32 @@ public function specifyTypesInCondition( new MethodCall($expr->var, $expr->name, $expr->args), ), $context, + $rootExpr, ); - $nullSafeTypes = $this->handleDefaultTruthyOrFalseyContext($context, $expr, $scope); + $nullSafeTypes = $this->handleDefaultTruthyOrFalseyContext($context, $rootExpr, $expr, $scope); return $context->true() ? $types->unionWith($nullSafeTypes) : $types->normalize($scope)->intersectWith($nullSafeTypes->normalize($scope)); } elseif (!$context->null()) { - return $this->handleDefaultTruthyOrFalseyContext($context, $expr, $scope); + return $this->handleDefaultTruthyOrFalseyContext($context, $rootExpr, $expr, $scope); } - return new SpecifiedTypes(); + return new SpecifiedTypes([], [], false, [], $rootExpr); } - private function handleDefaultTruthyOrFalseyContext(TypeSpecifierContext $context, Expr $expr, Scope $scope): SpecifiedTypes + private function handleDefaultTruthyOrFalseyContext(TypeSpecifierContext $context, ?Expr $rootExpr, Expr $expr, Scope $scope): SpecifiedTypes { if ($context->null()) { - return new SpecifiedTypes(); + return new SpecifiedTypes([], [], false, [], $rootExpr); } if (!$context->truthy()) { $type = StaticTypeFactory::truthy(); - return $this->create($expr, $type, TypeSpecifierContext::createFalse(), false, $scope); + return $this->create($expr, $type, TypeSpecifierContext::createFalse(), false, $scope, $rootExpr); } elseif (!$context->falsey()) { $type = StaticTypeFactory::falsey(); - return $this->create($expr, $type, TypeSpecifierContext::createFalse(), false, $scope); + return $this->create($expr, $type, TypeSpecifierContext::createFalse(), false, $scope, $rootExpr); } - return new SpecifiedTypes(); + return new SpecifiedTypes([], [], false, [], $rootExpr); } /** @@ -976,10 +1009,11 @@ public function create( TypeSpecifierContext $context, bool $overwrite = false, ?Scope $scope = null, + ?Expr $rootExpr = null, ): SpecifiedTypes { if ($expr instanceof Instanceof_ || $expr instanceof Expr\List_ || $expr instanceof VirtualNode) { - return new SpecifiedTypes(); + return new SpecifiedTypes([], [], false, [], $rootExpr); } while ($expr instanceof Expr\Assign) { @@ -1006,12 +1040,12 @@ public function create( $has = $this->reflectionProvider->hasFunction($expr->name, $scope); if (!$has) { // backwards compatibility with previous behaviour - return new SpecifiedTypes(); + return new SpecifiedTypes([], [], false, [], $rootExpr); } $functionReflection = $this->reflectionProvider->getFunction($expr->name, $scope); if ($functionReflection->hasSideEffects()->yes()) { - return new SpecifiedTypes(); + return new SpecifiedTypes([], [], false, [], $rootExpr); } } @@ -1025,10 +1059,10 @@ public function create( $methodReflection = $scope->getMethodReflection($calledOnType, $methodName); if ($methodReflection === null || $methodReflection->hasSideEffects()->yes()) { if (isset($resultType) && !TypeCombinator::containsNull($resultType)) { - return $this->createNullsafeTypes($originalExpr, $scope, $context, $type); + return $this->createNullsafeTypes($rootExpr, $originalExpr, $scope, $context, $type); } - return new SpecifiedTypes(); + return new SpecifiedTypes([], [], false, [], $rootExpr); } } @@ -1041,64 +1075,64 @@ public function create( $sureTypes[$exprString] = [$expr, $type]; } - $types = new SpecifiedTypes($sureTypes, $sureNotTypes, $overwrite); + $types = new SpecifiedTypes($sureTypes, $sureNotTypes, $overwrite, [], $rootExpr); if ($scope !== null && isset($resultType) && !TypeCombinator::containsNull($resultType)) { - return $this->createNullsafeTypes($originalExpr, $scope, $context, $type)->unionWith($types); + return $this->createNullsafeTypes($rootExpr, $originalExpr, $scope, $context, $type)->unionWith($types); } return $types; } - private function createNullsafeTypes(Expr $expr, Scope $scope, TypeSpecifierContext $context, ?Type $type): SpecifiedTypes + private function createNullsafeTypes(?Expr $rootExpr, Expr $expr, Scope $scope, TypeSpecifierContext $context, ?Type $type): SpecifiedTypes { if ($expr instanceof Expr\NullsafePropertyFetch) { if ($type !== null) { - $propertyFetchTypes = $this->create(new PropertyFetch($expr->var, $expr->name), $type, $context, false, $scope); + $propertyFetchTypes = $this->create(new PropertyFetch($expr->var, $expr->name), $type, $context, false, $scope, $rootExpr); } else { - $propertyFetchTypes = $this->create(new PropertyFetch($expr->var, $expr->name), new NullType(), TypeSpecifierContext::createFalse(), false, $scope); + $propertyFetchTypes = $this->create(new PropertyFetch($expr->var, $expr->name), new NullType(), TypeSpecifierContext::createFalse(), false, $scope, $rootExpr); } return $propertyFetchTypes->unionWith( - $this->create($expr->var, new NullType(), TypeSpecifierContext::createFalse(), false, $scope), + $this->create($expr->var, new NullType(), TypeSpecifierContext::createFalse(), false, $scope, $rootExpr), ); } if ($expr instanceof Expr\NullsafeMethodCall) { if ($type !== null) { - $methodCallTypes = $this->create(new MethodCall($expr->var, $expr->name, $expr->args), $type, $context, false, $scope); + $methodCallTypes = $this->create(new MethodCall($expr->var, $expr->name, $expr->args), $type, $context, false, $scope, $rootExpr); } else { - $methodCallTypes = $this->create(new MethodCall($expr->var, $expr->name, $expr->args), new NullType(), TypeSpecifierContext::createFalse(), false, $scope); + $methodCallTypes = $this->create(new MethodCall($expr->var, $expr->name, $expr->args), new NullType(), TypeSpecifierContext::createFalse(), false, $scope, $rootExpr); } return $methodCallTypes->unionWith( - $this->create($expr->var, new NullType(), TypeSpecifierContext::createFalse(), false, $scope), + $this->create($expr->var, new NullType(), TypeSpecifierContext::createFalse(), false, $scope, $rootExpr), ); } if ($expr instanceof Expr\PropertyFetch) { - return $this->createNullsafeTypes($expr->var, $scope, $context, null); + return $this->createNullsafeTypes($rootExpr, $expr->var, $scope, $context, null); } if ($expr instanceof Expr\MethodCall) { - return $this->createNullsafeTypes($expr->var, $scope, $context, null); + return $this->createNullsafeTypes($rootExpr, $expr->var, $scope, $context, null); } if ($expr instanceof Expr\ArrayDimFetch) { - return $this->createNullsafeTypes($expr->var, $scope, $context, null); + return $this->createNullsafeTypes($rootExpr, $expr->var, $scope, $context, null); } if ($expr instanceof Expr\StaticPropertyFetch && $expr->class instanceof Expr) { - return $this->createNullsafeTypes($expr->class, $scope, $context, null); + return $this->createNullsafeTypes($rootExpr, $expr->class, $scope, $context, null); } if ($expr instanceof Expr\StaticCall && $expr->class instanceof Expr) { - return $this->createNullsafeTypes($expr->class, $scope, $context, null); + return $this->createNullsafeTypes($rootExpr, $expr->class, $scope, $context, null); } - return new SpecifiedTypes(); + return new SpecifiedTypes([], [], false, [], $rootExpr); } - private function createRangeTypes(Expr $expr, Type $type, TypeSpecifierContext $context): SpecifiedTypes + private function createRangeTypes(?Expr $rootExpr, Expr $expr, Type $type, TypeSpecifierContext $context): SpecifiedTypes { $sureNotTypes = []; @@ -1112,7 +1146,7 @@ private function createRangeTypes(Expr $expr, Type $type, TypeSpecifierContext $ } } - return new SpecifiedTypes([], $sureNotTypes); + return new SpecifiedTypes([], $sureNotTypes, false, [], $rootExpr); } /** diff --git a/src/Process/Runnable/RunnableQueue.php b/src/Process/Runnable/RunnableQueue.php index 50cccba972..fc1042663e 100644 --- a/src/Process/Runnable/RunnableQueue.php +++ b/src/Process/Runnable/RunnableQueue.php @@ -27,6 +27,9 @@ public function __construct(private RunnableQueueLogger $logger, private int $ma $this->running = $running; } + /** + * @impure + */ public function getQueueSize(): int { $allSize = 0; @@ -37,6 +40,9 @@ public function getQueueSize(): int return $allSize; } + /** + * @impure + */ public function getRunningSize(): int { $allSize = 0; diff --git a/src/Rules/Comparison/ImpossibleCheckTypeHelper.php b/src/Rules/Comparison/ImpossibleCheckTypeHelper.php index 5781295bde..ca5984a83f 100644 --- a/src/Rules/Comparison/ImpossibleCheckTypeHelper.php +++ b/src/Rules/Comparison/ImpossibleCheckTypeHelper.php @@ -53,13 +53,11 @@ public function findSpecifiedType( Expr $node, ): ?bool { - if ( - $node instanceof FuncCall - && count($node->getArgs()) > 0 - ) { + if ($node instanceof FuncCall) { + $argsCount = count($node->getArgs()); if ($node->name instanceof Node\Name) { $functionName = strtolower((string) $node->name); - if ($functionName === 'assert') { + if ($functionName === 'assert' && $argsCount >= 1) { $assertValue = $scope->getType($node->getArgs()[0]->value)->toBoolean(); if (!$assertValue instanceof ConstantBooleanType) { return null; @@ -81,10 +79,7 @@ public function findSpecifiedType( return null; } elseif ($functionName === 'array_search') { return null; - } elseif ( - $functionName === 'in_array' - && count($node->getArgs()) >= 3 - ) { + } elseif ($functionName === 'in_array' && $argsCount >= 3) { $haystackType = $scope->getType($node->getArgs()[1]->value); if ($haystackType instanceof MixedType) { return null; @@ -143,7 +138,7 @@ public function findSpecifiedType( } } } - } elseif ($functionName === 'method_exists' && count($node->getArgs()) >= 2) { + } elseif ($functionName === 'method_exists' && $argsCount >= 2) { $objectType = $scope->getType($node->getArgs()[0]->value); $methodType = $scope->getType($node->getArgs()[1]->value); @@ -198,6 +193,18 @@ public function findSpecifiedType( ) && $scope->isSpecified($expr); }; + $rootExpr = $specifiedTypes->getRootExpr(); + if ($rootExpr !== null) { + if ($isSpecified($rootExpr)) { + return null; + } + + $rootExprType = $scope->getType($rootExpr); + if ($rootExprType instanceof ConstantBooleanType) { + return $rootExprType->getValue(); + } + } + if (count($sureTypes) === 1 && count($sureNotTypes) === 0) { $sureType = reset($sureTypes); if ($isSpecified($sureType[0])) { diff --git a/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeMethodCallRuleTest.php b/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeMethodCallRuleTest.php index 5db54072a4..83ed6f5e7b 100644 --- a/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeMethodCallRuleTest.php +++ b/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeMethodCallRuleTest.php @@ -76,6 +76,10 @@ 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 1 and null will always evaluate to false.', + 91, + ], [ 'Call to method ImpossibleMethodCall\Foo::isSame() with \'foo\' and \'foo\' will always evaluate to true.', 101, diff --git a/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeStaticMethodCallRuleTest.php b/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeStaticMethodCallRuleTest.php index 9115a5f9ce..6eda8aa35c 100644 --- a/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeStaticMethodCallRuleTest.php +++ b/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeStaticMethodCallRuleTest.php @@ -60,6 +60,14 @@ public function testRule(): void 'Call to static method PHPStan\Tests\AssertionClass::assertInt() with arguments 1, 2 and 3 will always evaluate to true.', 34, ], + [ + 'Call to static method PHPUnit\Framework\Assert::assertSame() with 302 and 200 will always evaluate to false.', + 40, + ], + [ + 'Call to static method PHPUnit\Framework\Assert::assertNotSame() with 302 and 200 will always evaluate to true.', + 47, + ], ]); } From fde975405a714f26f077326618a33bbaadbb3388 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Wed, 27 Apr 2022 17:28:05 +0200 Subject: [PATCH 2/9] This was missing --- src/Rules/Comparison/ImpossibleCheckTypeHelper.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Rules/Comparison/ImpossibleCheckTypeHelper.php b/src/Rules/Comparison/ImpossibleCheckTypeHelper.php index ca5984a83f..62b829ff52 100644 --- a/src/Rules/Comparison/ImpossibleCheckTypeHelper.php +++ b/src/Rules/Comparison/ImpossibleCheckTypeHelper.php @@ -203,6 +203,8 @@ public function findSpecifiedType( if ($rootExprType instanceof ConstantBooleanType) { return $rootExprType->getValue(); } + + return null; } if (count($sureTypes) === 1 && count($sureNotTypes) === 0) { From b84c54b026f25fcb6e3265b7104f37cf9de8fd60 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Wed, 27 Apr 2022 17:28:41 +0200 Subject: [PATCH 3/9] These should not be reported --- .../Comparison/ImpossibleCheckTypeMethodCallRuleTest.php | 4 ---- .../ImpossibleCheckTypeStaticMethodCallRuleTest.php | 8 -------- 2 files changed, 12 deletions(-) diff --git a/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeMethodCallRuleTest.php b/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeMethodCallRuleTest.php index 83ed6f5e7b..5db54072a4 100644 --- a/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeMethodCallRuleTest.php +++ b/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeMethodCallRuleTest.php @@ -76,10 +76,6 @@ 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 1 and null will always evaluate to false.', - 91, - ], [ 'Call to method ImpossibleMethodCall\Foo::isSame() with \'foo\' and \'foo\' will always evaluate to true.', 101, diff --git a/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeStaticMethodCallRuleTest.php b/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeStaticMethodCallRuleTest.php index 6eda8aa35c..9115a5f9ce 100644 --- a/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeStaticMethodCallRuleTest.php +++ b/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeStaticMethodCallRuleTest.php @@ -60,14 +60,6 @@ public function testRule(): void 'Call to static method PHPStan\Tests\AssertionClass::assertInt() with arguments 1, 2 and 3 will always evaluate to true.', 34, ], - [ - 'Call to static method PHPUnit\Framework\Assert::assertSame() with 302 and 200 will always evaluate to false.', - 40, - ], - [ - 'Call to static method PHPUnit\Framework\Assert::assertNotSame() with 302 and 200 will always evaluate to true.', - 47, - ], ]); } From ea053d8be1fba00ef058e36206df49218e925961 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Wed, 27 Apr 2022 17:29:07 +0200 Subject: [PATCH 4/9] This should not be necessary --- src/Process/Runnable/RunnableQueue.php | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/Process/Runnable/RunnableQueue.php b/src/Process/Runnable/RunnableQueue.php index fc1042663e..50cccba972 100644 --- a/src/Process/Runnable/RunnableQueue.php +++ b/src/Process/Runnable/RunnableQueue.php @@ -27,9 +27,6 @@ public function __construct(private RunnableQueueLogger $logger, private int $ma $this->running = $running; } - /** - * @impure - */ public function getQueueSize(): int { $allSize = 0; @@ -40,9 +37,6 @@ public function getQueueSize(): int return $allSize; } - /** - * @impure - */ public function getRunningSize(): int { $allSize = 0; From 08a27b064489f1d265276172d288854796e30721 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Wed, 27 Apr 2022 17:32:51 +0200 Subject: [PATCH 5/9] Simplify TypeSpecifier --- src/Analyser/TypeSpecifier.php | 44 ++++++++++++++++------------------ 1 file changed, 20 insertions(+), 24 deletions(-) diff --git a/src/Analyser/TypeSpecifier.php b/src/Analyser/TypeSpecifier.php index 0ae373cb1d..da3cbca409 100644 --- a/src/Analyser/TypeSpecifier.php +++ b/src/Analyser/TypeSpecifier.php @@ -278,32 +278,28 @@ public function specifyTypesInCondition( $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, - $context, - false, - $scope, - $rootExpr, - ); - } + $types = $this->create( + $expr->right, + $exprLeftType, + $context, + false, + $scope, + $rootExpr, + ); } 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, - $context, - false, - $scope, - $rootExpr, - ); - if ($types !== null) { - $types = $types->unionWith($leftType); - } else { - $types = $leftType; - } + $leftType = $this->create( + $expr->left, + $exprRightType, + $context, + false, + $scope, + $rootExpr, + ); + if ($types !== null) { + $types = $types->unionWith($leftType); + } else { + $types = $leftType; } } From ee16f736747072c1e48510257dd6fe1efea7e16f Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Wed, 27 Apr 2022 17:38:52 +0200 Subject: [PATCH 6/9] I hate this hack but it works --- src/Rules/Comparison/ImpossibleCheckTypeHelper.php | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/Rules/Comparison/ImpossibleCheckTypeHelper.php b/src/Rules/Comparison/ImpossibleCheckTypeHelper.php index 62b829ff52..e73e851fcd 100644 --- a/src/Rules/Comparison/ImpossibleCheckTypeHelper.php +++ b/src/Rules/Comparison/ImpossibleCheckTypeHelper.php @@ -199,6 +199,12 @@ public function findSpecifiedType( return null; } + if ($rootExpr instanceof Expr\BinaryOp) { + if ($isSpecified($rootExpr->left) || $isSpecified($rootExpr->right)) { + return null; + } + } + $rootExprType = $scope->getType($rootExpr); if ($rootExprType instanceof ConstantBooleanType) { return $rootExprType->getValue(); From e0827b9a95aaebd721a59c16e0f38ecd37baa02b Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Wed, 27 Apr 2022 17:42:28 +0200 Subject: [PATCH 7/9] Restructure and improve the hack --- .../Comparison/ImpossibleCheckTypeHelper.php | 57 ++++++++++--------- 1 file changed, 30 insertions(+), 27 deletions(-) diff --git a/src/Rules/Comparison/ImpossibleCheckTypeHelper.php b/src/Rules/Comparison/ImpossibleCheckTypeHelper.php index e73e851fcd..bfa17e1ab6 100644 --- a/src/Rules/Comparison/ImpossibleCheckTypeHelper.php +++ b/src/Rules/Comparison/ImpossibleCheckTypeHelper.php @@ -177,34 +177,12 @@ public function findSpecifiedType( $sureTypes = $specifiedTypes->getSureTypes(); $sureNotTypes = $specifiedTypes->getSureNotTypes(); - $isSpecified = static function (Expr $expr) use ($scope, $node): bool { - if ($expr === $node) { - return true; - } - - if ($expr instanceof Expr\Variable && is_string($expr->name) && !$scope->hasVariableType($expr->name)->yes()) { - return true; - } - - return ( - $node instanceof FuncCall - || $node instanceof MethodCall - || $node instanceof Expr\StaticCall - ) && $scope->isSpecified($expr); - }; - $rootExpr = $specifiedTypes->getRootExpr(); if ($rootExpr !== null) { - if ($isSpecified($rootExpr)) { + if (self::isSpecified($scope, $node, $rootExpr)) { return null; } - if ($rootExpr instanceof Expr\BinaryOp) { - if ($isSpecified($rootExpr->left) || $isSpecified($rootExpr->right)) { - return null; - } - } - $rootExprType = $scope->getType($rootExpr); if ($rootExprType instanceof ConstantBooleanType) { return $rootExprType->getValue(); @@ -215,7 +193,7 @@ public function findSpecifiedType( if (count($sureTypes) === 1 && count($sureNotTypes) === 0) { $sureType = reset($sureTypes); - if ($isSpecified($sureType[0])) { + if (self::isSpecified($scope, $node, $sureType[0])) { return null; } @@ -238,7 +216,7 @@ public function findSpecifiedType( return null; } elseif (count($sureNotTypes) === 1 && count($sureTypes) === 0) { $sureNotType = reset($sureNotTypes); - if ($isSpecified($sureNotType[0])) { + if (self::isSpecified($scope, $node, $sureNotType[0])) { return null; } @@ -263,7 +241,7 @@ public function findSpecifiedType( if (count($sureTypes) > 0) { foreach ($sureTypes as $sureType) { - if ($isSpecified($sureType[0])) { + if (self::isSpecified($scope, $node, $sureType[0])) { return null; } } @@ -277,7 +255,7 @@ public function findSpecifiedType( if (count($sureNotTypes) > 0) { foreach ($sureNotTypes as $sureNotType) { - if ($isSpecified($sureNotType[0])) { + if (self::isSpecified($scope, $node, $sureNotType[0])) { return null; } } @@ -292,6 +270,31 @@ public function findSpecifiedType( return null; } + private static function isSpecified(Scope $scope, Expr $node, Expr $expr): bool + { + if ($expr === $node) { + return true; + } + + if ($expr instanceof Expr\Variable && is_string($expr->name) && !$scope->hasVariableType($expr->name)->yes()) { + return true; + } + + if ($expr instanceof Expr\BooleanNot) { + return self::isSpecified($scope, $node, $expr->expr); + } + + if ($expr instanceof Expr\BinaryOp) { + return self::isSpecified($scope, $node, $expr->left) || self::isSpecified($scope, $node, $expr->right); + } + + return ( + $node instanceof FuncCall + || $node instanceof MethodCall + || $node instanceof Expr\StaticCall + ) && $scope->isSpecified($expr); + } + /** * @param Node\Arg[] $args */ From 036689b5f32418f06b02b888c176bceea0ceb3fb Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Wed, 27 Apr 2022 22:12:21 +0200 Subject: [PATCH 8/9] Fix test --- .../ImpossibleCheckTypeMethodCallRuleEqualsTest.php | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeMethodCallRuleEqualsTest.php b/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeMethodCallRuleEqualsTest.php index ef32567c9f..cf6ba5f085 100644 --- a/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeMethodCallRuleEqualsTest.php +++ b/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeMethodCallRuleEqualsTest.php @@ -84,6 +84,14 @@ public function testRule(): void '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 \'\' and \'\' will always evaluate to true.', 174, From 548e33ee1ba5a9e00a433fc470a6cfb43cc98943 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Wed, 27 Apr 2022 22:18:25 +0200 Subject: [PATCH 9/9] Further simplification --- .../Comparison/ImpossibleCheckTypeHelper.php | 32 ------------------- 1 file changed, 32 deletions(-) diff --git a/src/Rules/Comparison/ImpossibleCheckTypeHelper.php b/src/Rules/Comparison/ImpossibleCheckTypeHelper.php index bfa17e1ab6..d485dc6e73 100644 --- a/src/Rules/Comparison/ImpossibleCheckTypeHelper.php +++ b/src/Rules/Comparison/ImpossibleCheckTypeHelper.php @@ -18,11 +18,9 @@ use PHPStan\Type\NeverType; use PHPStan\Type\ObjectType; use PHPStan\Type\Type; -use PHPStan\Type\TypeCombinator; use PHPStan\Type\TypeUtils; use PHPStan\Type\TypeWithClassName; use PHPStan\Type\VerbosityLevel; -use function array_column; use function array_map; use function array_pop; use function count; @@ -235,36 +233,6 @@ public function findSpecifiedType( } elseif ($isSuperType->no()) { return true; } - - return null; - } - - if (count($sureTypes) > 0) { - foreach ($sureTypes as $sureType) { - if (self::isSpecified($scope, $node, $sureType[0])) { - return null; - } - } - $types = TypeCombinator::union( - ...array_column($sureTypes, 1), - ); - if ($types instanceof NeverType) { - return false; - } - } - - if (count($sureNotTypes) > 0) { - foreach ($sureNotTypes as $sureNotType) { - if (self::isSpecified($scope, $node, $sureNotType[0])) { - return null; - } - } - $types = TypeCombinator::union( - ...array_column($sureNotTypes, 1), - ); - if ($types instanceof NeverType) { - return true; - } } return null;