diff --git a/src/Rules/Comparison/ConstantConditionRuleHelper.php b/src/Rules/Comparison/ConstantConditionRuleHelper.php index 30a08f665d..48fe9b0d84 100644 --- a/src/Rules/Comparison/ConstantConditionRuleHelper.php +++ b/src/Rules/Comparison/ConstantConditionRuleHelper.php @@ -57,7 +57,7 @@ private function shouldSkip(Scope $scope, Expr $expr): bool || $expr instanceof Expr\StaticCall ) && !$expr->isFirstClassCallable() ) { - $isAlways = $this->impossibleCheckTypeHelper->findSpecifiedType($scope, $expr); + $isAlways = $this->impossibleCheckTypeHelper->findSpecifiedType($scope, $expr, false); if ($isAlways !== null) { return true; } diff --git a/src/Rules/Comparison/ImpossibleCheckTypeFunctionCallRule.php b/src/Rules/Comparison/ImpossibleCheckTypeFunctionCallRule.php index 61e3b57c30..eaebc42672 100644 --- a/src/Rules/Comparison/ImpossibleCheckTypeFunctionCallRule.php +++ b/src/Rules/Comparison/ImpossibleCheckTypeFunctionCallRule.php @@ -43,7 +43,7 @@ public function processNode(Node $node, Scope $scope): array } $functionName = (string) $node->name; - $isAlways = $this->impossibleCheckTypeHelper->findSpecifiedType($scope, $node); + $isAlways = $this->impossibleCheckTypeHelper->findSpecifiedType($scope, $node, true); if ($isAlways === null) { return []; } @@ -53,7 +53,7 @@ public function processNode(Node $node, Scope $scope): array return $this->possiblyImpureTipHelper->addTip($scope, $node, $ruleErrorBuilder); } - $isAlways = $this->impossibleCheckTypeHelper->doNotTreatPhpDocTypesAsCertain()->findSpecifiedType($scope, $node); + $isAlways = $this->impossibleCheckTypeHelper->doNotTreatPhpDocTypesAsCertain()->findSpecifiedType($scope, $node, true); if ($isAlways !== null) { return $this->possiblyImpureTipHelper->addTip($scope, $node, $ruleErrorBuilder); } diff --git a/src/Rules/Comparison/ImpossibleCheckTypeHelper.php b/src/Rules/Comparison/ImpossibleCheckTypeHelper.php index 127cae01cd..39088e3c32 100644 --- a/src/Rules/Comparison/ImpossibleCheckTypeHelper.php +++ b/src/Rules/Comparison/ImpossibleCheckTypeHelper.php @@ -61,6 +61,7 @@ public function __construct( public function findSpecifiedType( Scope $scope, Expr $node, + bool $ignoreTraitContext, ): ?bool { if ($node instanceof FuncCall) { @@ -198,6 +199,27 @@ public function findSpecifiedType( } } elseif ($functionName === 'method_exists' && $argsCount >= 2) { $objectArg = $args[0]->value; + + if ( + $ignoreTraitContext + && $this->isExpressionDependentOnTraitContext($scope, $objectArg) + ) { + $traitReflection = $scope->getTraitReflection(); + if ($traitReflection === null) { + return null; + } + $methodArgValue = $args[1]->value; + $methodArgType = $this->treatPhpDocTypesAsCertain ? $scope->getType($methodArgValue) : $scope->getNativeType($methodArgValue); + $constantMethodNames = $methodArgType->getConstantStrings(); + if (count($constantMethodNames) === 0) { + return null; + } + foreach ($constantMethodNames as $constantMethodName) { + if (!$traitReflection->hasNativeMethod($constantMethodName->getValue())) { + return null; + } + } + } $objectType = $this->treatPhpDocTypesAsCertain ? $scope->getType($objectArg) : $scope->getNativeType($objectArg); if ($objectType instanceof ConstantStringType @@ -310,6 +332,14 @@ public function findSpecifiedType( continue; } + if ( + $ignoreTraitContext + && $this->isExpressionDependentOnTraitContext($scope, $sureType[0]) + ) { + $results[] = TrinaryLogic::createMaybe(); + continue; + } + if ($this->treatPhpDocTypesAsCertain) { $argumentType = $scope->getType($sureType[0]); } else { @@ -336,6 +366,14 @@ public function findSpecifiedType( continue; } + if ( + $ignoreTraitContext + && $this->isExpressionDependentOnTraitContext($scope, $sureNotType[0]) + ) { + $results[] = TrinaryLogic::createMaybe(); + continue; + } + if ($this->treatPhpDocTypesAsCertain) { $argumentType = $scope->getType($sureNotType[0]); } else { @@ -356,6 +394,57 @@ public function findSpecifiedType( return $result->maybe() ? null : $result->yes(); } + private function isExpressionDependentOnTraitContext(Scope $scope, Expr $expr): bool + { + if (!$scope->isInTrait()) { + return false; + } + + if (self::isExpressionDependentOnThis($expr)) { + return true; + } + + $classReflection = $scope->getClassReflection(); + if ($classReflection === null) { + return false; + } + + $type = $this->treatPhpDocTypesAsCertain ? $scope->getType($expr) : $scope->getNativeType($expr); + foreach ($type->getObjectClassNames() as $className) { + if ($className === $classReflection->getName()) { + return true; + } + } + + return false; + } + + public static function isExpressionDependentOnThis(Expr $expr): bool + { + if ($expr instanceof Expr\Variable && $expr->name === 'this') { + return true; + } + + if ($expr instanceof Expr\PropertyFetch || $expr instanceof Expr\NullsafePropertyFetch) { + return self::isExpressionDependentOnThis($expr->var); + } + + if ($expr instanceof Expr\MethodCall || $expr instanceof Expr\NullsafeMethodCall) { + return self::isExpressionDependentOnThis($expr->var); + } + + if ($expr instanceof Expr\StaticPropertyFetch || $expr instanceof Expr\StaticCall) { + if ($expr->class instanceof Expr) { + return self::isExpressionDependentOnThis($expr->class); + } + + $className = $expr->class->toString(); + return in_array($className, ['self', 'static', 'parent'], true); + } + + return false; + } + private static function isSpecified(Scope $scope, Expr $node, Expr $expr): bool { if ($expr === $node) { diff --git a/src/Rules/Comparison/ImpossibleCheckTypeMethodCallRule.php b/src/Rules/Comparison/ImpossibleCheckTypeMethodCallRule.php index bc8284d111..92265b5051 100644 --- a/src/Rules/Comparison/ImpossibleCheckTypeMethodCallRule.php +++ b/src/Rules/Comparison/ImpossibleCheckTypeMethodCallRule.php @@ -45,7 +45,7 @@ public function processNode(Node $node, Scope $scope): array return []; } - $isAlways = $this->impossibleCheckTypeHelper->findSpecifiedType($scope, $node); + $isAlways = $this->impossibleCheckTypeHelper->findSpecifiedType($scope, $node, true); if ($isAlways === null) { return []; } @@ -55,7 +55,7 @@ public function processNode(Node $node, Scope $scope): array return $this->possiblyImpureTipHelper->addTip($scope, $node, $ruleErrorBuilder); } - $isAlways = $this->impossibleCheckTypeHelper->doNotTreatPhpDocTypesAsCertain()->findSpecifiedType($scope, $node); + $isAlways = $this->impossibleCheckTypeHelper->doNotTreatPhpDocTypesAsCertain()->findSpecifiedType($scope, $node, true); if ($isAlways !== null) { return $this->possiblyImpureTipHelper->addTip($scope, $node, $ruleErrorBuilder); } diff --git a/src/Rules/Comparison/ImpossibleCheckTypeStaticMethodCallRule.php b/src/Rules/Comparison/ImpossibleCheckTypeStaticMethodCallRule.php index 3c24b38176..647c89ab23 100644 --- a/src/Rules/Comparison/ImpossibleCheckTypeStaticMethodCallRule.php +++ b/src/Rules/Comparison/ImpossibleCheckTypeStaticMethodCallRule.php @@ -45,7 +45,7 @@ public function processNode(Node $node, Scope $scope): array return []; } - $isAlways = $this->impossibleCheckTypeHelper->findSpecifiedType($scope, $node); + $isAlways = $this->impossibleCheckTypeHelper->findSpecifiedType($scope, $node, true); if ($isAlways === null) { return []; } @@ -55,7 +55,7 @@ public function processNode(Node $node, Scope $scope): array return $this->possiblyImpureTipHelper->addTip($scope, $node, $ruleErrorBuilder); } - $isAlways = $this->impossibleCheckTypeHelper->doNotTreatPhpDocTypesAsCertain()->findSpecifiedType($scope, $node); + $isAlways = $this->impossibleCheckTypeHelper->doNotTreatPhpDocTypesAsCertain()->findSpecifiedType($scope, $node, true); if ($isAlways !== null) { return $this->possiblyImpureTipHelper->addTip($scope, $node, $ruleErrorBuilder); } diff --git a/src/Type/Php/TypeSpecifyingFunctionsDynamicReturnTypeExtension.php b/src/Type/Php/TypeSpecifyingFunctionsDynamicReturnTypeExtension.php index 8ed3c6b605..81f1af6b7d 100644 --- a/src/Type/Php/TypeSpecifyingFunctionsDynamicReturnTypeExtension.php +++ b/src/Type/Php/TypeSpecifyingFunctionsDynamicReturnTypeExtension.php @@ -66,6 +66,7 @@ public function getTypeFromFunctionCall( $isAlways = $this->getHelper()->findSpecifiedType( $scope, $functionCall, + false, ); if ($isAlways === null) { return null; diff --git a/tests/PHPStan/Rules/Arrays/InvalidKeyInArrayDimFetchRuleTest.php b/tests/PHPStan/Rules/Arrays/InvalidKeyInArrayDimFetchRuleTest.php index e61164f515..c7b2144c9f 100644 --- a/tests/PHPStan/Rules/Arrays/InvalidKeyInArrayDimFetchRuleTest.php +++ b/tests/PHPStan/Rules/Arrays/InvalidKeyInArrayDimFetchRuleTest.php @@ -144,6 +144,12 @@ public function testBug12273(): void ]); } + #[RequiresPhp('>= 8.1')] + public function testBug12798(): void + { + $this->analyse([__DIR__ . '/../Comparison/data/bug-12798.php'], []); + } + public function testBug12981(): void { $this->analyse([__DIR__ . '/data/bug-12981.php'], [ diff --git a/tests/PHPStan/Rules/Comparison/BooleanNotConstantConditionRuleTest.php b/tests/PHPStan/Rules/Comparison/BooleanNotConstantConditionRuleTest.php index 3cf3f2a8bb..f265e02db8 100644 --- a/tests/PHPStan/Rules/Comparison/BooleanNotConstantConditionRuleTest.php +++ b/tests/PHPStan/Rules/Comparison/BooleanNotConstantConditionRuleTest.php @@ -236,4 +236,10 @@ public function testBug6702(): void $this->analyse([__DIR__ . '/data/bug-6702.php'], []); } + public function testBug13023(): void + { + $this->treatPhpDocTypesAsCertain = true; + $this->analyse([__DIR__ . '/data/bug-13023.php'], []); + } + } diff --git a/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeFunctionCallRuleTest.php b/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeFunctionCallRuleTest.php index 69e992139d..34fdb5c4b1 100644 --- a/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeFunctionCallRuleTest.php +++ b/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeFunctionCallRuleTest.php @@ -187,14 +187,6 @@ public function testImpossibleCheckTypeFunctionCall(): void 'Call to function method_exists() with $this(CheckTypeFunctionCall\MethodExistsWithTrait) and \'method\' will always evaluate to true.', 650, ], - [ - 'Call to function method_exists() with $this(CheckTypeFunctionCall\MethodExistsWithTrait) and \'someAnother\' will always evaluate to true.', - 653, - ], - [ - 'Call to function method_exists() with $this(CheckTypeFunctionCall\MethodExistsWithTrait) and \'unknown\' will always evaluate to false.', - 656, - ], [ 'Call to function method_exists() with \'CheckTypeFunctionCall\\\\MethodExistsWithTrait\' and \'method\' will always evaluate to true.', 659, @@ -1207,4 +1199,42 @@ public function testBug13799(): void ]); } + public function testBug13023(): void + { + $this->treatPhpDocTypesAsCertain = true; + $this->analyse([__DIR__ . '/data/bug-13023.php'], []); + } + + #[RequiresPhp('>= 8.1')] + public function testBug7599(): void + { + $this->treatPhpDocTypesAsCertain = true; + $this->analyse([__DIR__ . '/data/bug-7599.php'], []); + } + + public function testBug9095(): void + { + $this->treatPhpDocTypesAsCertain = true; + $this->analyse([__DIR__ . '/data/bug-9095.php'], []); + } + + public function testBug13474(): void + { + $this->treatPhpDocTypesAsCertain = true; + $this->analyse([__DIR__ . '/data/bug-13474.php'], []); + } + + public function testBug13687(): void + { + $this->treatPhpDocTypesAsCertain = true; + $this->analyse([__DIR__ . '/data/bug-13687.php'], []); + } + + #[RequiresPhp('>= 8.1')] + public function testBug12798(): void + { + $this->treatPhpDocTypesAsCertain = true; + $this->analyse([__DIR__ . '/data/bug-12798.php'], []); + } + } diff --git a/tests/PHPStan/Rules/Comparison/data/bug-12798.php b/tests/PHPStan/Rules/Comparison/data/bug-12798.php new file mode 100644 index 0000000000..7bc404d240 --- /dev/null +++ b/tests/PHPStan/Rules/Comparison/data/bug-12798.php @@ -0,0 +1,50 @@ += 8.1 + +namespace Bug12798; + +interface Colorable +{ + public function color(): string; +} + +trait HasColors +{ + /** @return array */ + public static function colors(): array { + return array_reduce(self::cases(), function (array $colors, self $case) { + $key = is_subclass_of($case, \BackedEnum::class) ? $case->value : $case->name; + $color = is_subclass_of($case, Colorable::class) ? $case->color() : 'gray'; + + $colors[$key] = $color; + $colors[$color] = $color; + return $colors; + }, []); + } +} + +enum AlertLevelBacked: int implements Colorable +{ + use HasColors; + + case Low = 1; + case Medium = 2; + case Critical = 3; + + public function color(): string + { + return match ($this) { + self::Low => 'green', + self::Medium => 'yellow', + self::Critical => 'red', + }; + } +} + +enum AlertLevel +{ + use HasColors; + + case Low; + case Medium; + case Critical; +} diff --git a/tests/PHPStan/Rules/Comparison/data/bug-13023.php b/tests/PHPStan/Rules/Comparison/data/bug-13023.php new file mode 100644 index 0000000000..2183704383 --- /dev/null +++ b/tests/PHPStan/Rules/Comparison/data/bug-13023.php @@ -0,0 +1,168 @@ += 8.0 + +namespace Bug13023; + +class SomeClass +{ + use MyTrait; +} + +class SomeClass2 +{ + use MyTrait; +} + +trait MyTrait +{ + public function getRandom(): int + { + $value = random_int(1, 100); + if (is_a($this, SomeClass::class)) { + return $value * $value; + } + return $value; + } +} + +class SomeClass3 +{ + use MyTrait2; + + public string $foo = 'foo'; +} + +class SomeClass4 +{ + use MyTrait2; + + public int $foo = 1; +} + +trait MyTrait2 +{ + public function getRandom(): int + { + $value = random_int(1, 100); + if (\is_int($this->foo)) { + return $value * $value; + } + + return $value; + } +} + +class SomeClass5 +{ + use MyTrait3; + + public static string $bar = 'bar'; +} + +class SomeClass6 +{ + use MyTrait3; + + public static int $bar = 1; +} + +trait MyTrait3 +{ + public function getRandom(): int + { + $value = random_int(1, 100); + if (\is_int(self::$bar)) { + return $value * $value; + } + if (\is_int(static::$bar)) { + return $value * $value; + } + if (\is_int($this::$bar)) { + return $value * $value; + } + + return $value; + } +} + +class SomeClass7 +{ + use MyTrait4; + + public ?string $baz = 'baz'; +} + +class SomeClass8 +{ + use MyTrait4; + + public ?int $baz = 1; +} + +trait MyTrait4 +{ + public function getRandom(): int + { + $value = random_int(1, 100); + if (\is_int($this?->baz)) { + return $value * $value; + } + + return $value; + } +} + +class HelloWorld +{ + use SomeTrait; + + public string $message = 'Hello'; + + public function foo(): void + { + $this->bar(); + } +} + +class EmptyClass { + use SomeTrait; +} + +trait SomeTrait { + public function bar(): void + { + if (property_exists($this, 'message')) { + if (! is_string($this->message)) { + return; + } + + echo $this->message . "\n"; + } + } +} + +class SomeClass9 +{ + use MyTrait5; + + public string $prop = 'foo'; +} + +class SomeClass10 +{ + use MyTrait5; + + public int $prop = 1; +} + +trait MyTrait5 +{ + public function getRandom(): int + { + $value = random_int(1, 100); + if (!\is_int($this->prop)) { + return $value; + } + + return $value * $value; + } +} diff --git a/tests/PHPStan/Rules/Comparison/data/bug-13474.php b/tests/PHPStan/Rules/Comparison/data/bug-13474.php new file mode 100644 index 0000000000..db93fd400f --- /dev/null +++ b/tests/PHPStan/Rules/Comparison/data/bug-13474.php @@ -0,0 +1,86 @@ += 8.0 + +namespace Bug13474; + +/** + * @template TValue of mixed + */ +interface ModelInterface { + /** + * @return TValue + */ + public function getValue(): mixed; +} + +/** + * @implements ModelInterface + */ +class ModelA implements ModelInterface +{ + public function getValue(): int + { + return 0; + } +} + +/** + * @implements ModelInterface + */ +class ModelB implements ModelInterface +{ + public function getValue(): string + { + return 'foo'; + } +} + +/** + * @template T of ModelInterface + */ +trait ModelTrait +{ + /** + * @return T + */ + abstract function model(): ModelInterface; + + /** + * @return template-type + */ + public function getValue(): mixed + { + return $this->model()->getValue(); + } + + public function test(): void + { + if (is_string($this->getValue())) { + echo 'string'; + return; + } + + echo 'other'; + } +} + +class TestA +{ + /** @use ModelTrait */ + use ModelTrait; + + function model(): ModelA + { + return new ModelA(); + } +} + +class TestB +{ + /** @use ModelTrait */ + use ModelTrait; + + function model(): ModelB + { + return new ModelB(); + } +} diff --git a/tests/PHPStan/Rules/Comparison/data/bug-13687.php b/tests/PHPStan/Rules/Comparison/data/bug-13687.php new file mode 100644 index 0000000000..08927d78c6 --- /dev/null +++ b/tests/PHPStan/Rules/Comparison/data/bug-13687.php @@ -0,0 +1,29 @@ +bar(); + } + + if (property_exists($this, 'baz')) { + $a = $this->baz; + } + } +} + +class A { + use MyTrait; + + public string $baz = 'baz'; +} + +class B { + use MyTrait; + + public function bar(): void { + echo 'bar'; + } +} diff --git a/tests/PHPStan/Rules/Comparison/data/bug-7599.php b/tests/PHPStan/Rules/Comparison/data/bug-7599.php new file mode 100644 index 0000000000..c4fa4b3fd7 --- /dev/null +++ b/tests/PHPStan/Rules/Comparison/data/bug-7599.php @@ -0,0 +1,39 @@ += 8.1 + +namespace Bug7599; + +trait TraitForEnum +{ + /** + * @return array + */ + public static function fooMethod(): array + { + return array_map( + fn(self $enum): string => method_exists($enum, 'barMethod') + ? $enum->barMethod() + : $enum->name, + static::cases() + ); + } +} + +enum TestEnum: string +{ + use TraitForEnum; + + case Foo = 'foo'; + case Bar = 'bar'; +} + +enum SecondEnum: string +{ + use TraitForEnum; + + case Baz = 'baz'; + + public function barMethod(): string + { + return 'blah'; + } +} diff --git a/tests/PHPStan/Rules/Comparison/data/bug-9095.php b/tests/PHPStan/Rules/Comparison/data/bug-9095.php new file mode 100644 index 0000000000..4b9b84f858 --- /dev/null +++ b/tests/PHPStan/Rules/Comparison/data/bug-9095.php @@ -0,0 +1,23 @@ +createdAt) && + $this->createdAt instanceof \DateTimeInterface + ) { + return $this->createdAt; + } + return null; + } +} + +final class Event +{ + use EventTrait; +}