diff --git a/src/Rules/IssetCheck.php b/src/Rules/IssetCheck.php index 594d01dd259..dfa746a90f6 100644 --- a/src/Rules/IssetCheck.php +++ b/src/Rules/IssetCheck.php @@ -4,10 +4,13 @@ use PhpParser\Node; use PhpParser\Node\Expr; +use PHPStan\Analyser\CollectedDataEmitter; +use PHPStan\Analyser\NodeCallbackInvoker; use PHPStan\Analyser\Scope; use PHPStan\DependencyInjection\AutowiredParameter; use PHPStan\DependencyInjection\AutowiredService; use PHPStan\Node\Expr\PropertyInitializationExpr; +use PHPStan\Rules\Comparison\ConstantConditionInTraitHelper; use PHPStan\Rules\Properties\PropertyDescriptor; use PHPStan\Rules\Properties\PropertyReflectionFinder; use PHPStan\Type\NeverType; @@ -27,6 +30,7 @@ final class IssetCheck public function __construct( private PropertyDescriptor $propertyDescriptor, private PropertyReflectionFinder $propertyReflectionFinder, + private ConstantConditionInTraitHelper $constantConditionInTraitHelper, #[AutowiredParameter] private bool $checkAdvancedIsset, #[AutowiredParameter] @@ -36,10 +40,32 @@ public function __construct( } /** + * @param class-string>|null $ruleName * @param ErrorIdentifier $identifier * @param callable(Type): ?string $typeMessageCallback */ - public function check(Expr $expr, Scope $scope, string $operatorDescription, string $identifier, callable $typeMessageCallback, ?IdentifierRuleError $error = null): ?IdentifierRuleError + public function check(Expr $expr, Scope&NodeCallbackInvoker&CollectedDataEmitter $scope, string $operatorDescription, string $identifier, callable $typeMessageCallback, ?string $ruleName = null, ?IdentifierRuleError $error = null): ?IdentifierRuleError + { + $result = $this->checkInternal($expr, $scope, $operatorDescription, $identifier, $typeMessageCallback, $ruleName, $expr, $error); + + if ($ruleName !== null && $scope->isInTrait()) { + if ($result !== null) { + $this->constantConditionInTraitHelper->emitError($ruleName, $scope, $expr, true, $result); + return null; + } + $this->constantConditionInTraitHelper->emitNoError($ruleName, $scope, $expr); + return null; + } + + return $result; + } + + /** + * @param class-string>|null $ruleName + * @param ErrorIdentifier $identifier + * @param callable(Type): ?string $typeMessageCallback + */ + private function checkInternal(Expr $expr, Scope&NodeCallbackInvoker&CollectedDataEmitter $scope, string $operatorDescription, string $identifier, callable $typeMessageCallback, ?string $ruleName, Expr $originalExpr, ?IdentifierRuleError $error = null): ?IdentifierRuleError { // mirrored in PHPStan\Analyser\MutatingScope::issetCheck() if ($expr instanceof Node\Expr\Variable && is_string($expr->name)) { @@ -114,7 +140,7 @@ public function check(Expr $expr, Scope $scope, string $operatorDescription, str ), $typeMessageCallback, $identifier, 'offset'); if ($error !== null) { - return $this->check($expr->var, $scope, $operatorDescription, $identifier, $typeMessageCallback, $error); + return $this->checkInternal($expr->var, $scope, $operatorDescription, $identifier, $typeMessageCallback, $ruleName, $originalExpr, $error); } } @@ -220,11 +246,11 @@ static function (Type $type) use ($typeMessageCallback): ?string { if ($error !== null) { if ($expr instanceof Node\Expr\PropertyFetch) { - return $this->check($expr->var, $scope, $operatorDescription, $identifier, $typeMessageCallback, $error); + return $this->checkInternal($expr->var, $scope, $operatorDescription, $identifier, $typeMessageCallback, $ruleName, $originalExpr, $error); } if ($expr->class instanceof Expr) { - return $this->check($expr->class, $scope, $operatorDescription, $identifier, $typeMessageCallback, $error); + return $this->checkInternal($expr->class, $scope, $operatorDescription, $identifier, $typeMessageCallback, $ruleName, $originalExpr, $error); } } diff --git a/src/Rules/Variables/EmptyRule.php b/src/Rules/Variables/EmptyRule.php index d1656a3be27..fce2ceec013 100644 --- a/src/Rules/Variables/EmptyRule.php +++ b/src/Rules/Variables/EmptyRule.php @@ -3,6 +3,8 @@ namespace PHPStan\Rules\Variables; use PhpParser\Node; +use PHPStan\Analyser\CollectedDataEmitter; +use PHPStan\Analyser\NodeCallbackInvoker; use PHPStan\Analyser\Scope; use PHPStan\DependencyInjection\RegisteredRule; use PHPStan\Rules\IssetCheck; @@ -25,7 +27,7 @@ public function getNodeType(): string return Node\Expr\Empty_::class; } - public function processNode(Node $node, Scope $scope): array + public function processNode(Node $node, Scope&NodeCallbackInvoker&CollectedDataEmitter $scope): array { $error = $this->issetCheck->check($node->expr, $scope, 'in empty()', 'empty', static function (Type $type): ?string { $isNull = $type->isNull(); @@ -57,7 +59,7 @@ public function processNode(Node $node, Scope $scope): array } return 'is not nullable'; - }); + }, self::class); if ($error === null) { return []; diff --git a/src/Rules/Variables/IssetRule.php b/src/Rules/Variables/IssetRule.php index 839241a169b..f73d379418d 100644 --- a/src/Rules/Variables/IssetRule.php +++ b/src/Rules/Variables/IssetRule.php @@ -3,6 +3,8 @@ namespace PHPStan\Rules\Variables; use PhpParser\Node; +use PHPStan\Analyser\CollectedDataEmitter; +use PHPStan\Analyser\NodeCallbackInvoker; use PHPStan\Analyser\Scope; use PHPStan\DependencyInjection\RegisteredRule; use PHPStan\Rules\IssetCheck; @@ -25,7 +27,7 @@ public function getNodeType(): string return Node\Expr\Isset_::class; } - public function processNode(Node $node, Scope $scope): array + public function processNode(Node $node, Scope&NodeCallbackInvoker&CollectedDataEmitter $scope): array { $messages = []; foreach ($node->vars as $var) { @@ -40,7 +42,7 @@ public function processNode(Node $node, Scope $scope): array } return 'is not nullable'; - }); + }, self::class); if ($error === null) { continue; } diff --git a/src/Rules/Variables/NullCoalesceRule.php b/src/Rules/Variables/NullCoalesceRule.php index 79941d0293d..7a74ec0fe07 100644 --- a/src/Rules/Variables/NullCoalesceRule.php +++ b/src/Rules/Variables/NullCoalesceRule.php @@ -3,6 +3,8 @@ namespace PHPStan\Rules\Variables; use PhpParser\Node; +use PHPStan\Analyser\CollectedDataEmitter; +use PHPStan\Analyser\NodeCallbackInvoker; use PHPStan\Analyser\Scope; use PHPStan\DependencyInjection\RegisteredRule; use PHPStan\Rules\IssetCheck; @@ -25,7 +27,7 @@ public function getNodeType(): string return Node\Expr::class; } - public function processNode(Node $node, Scope $scope): array + public function processNode(Node $node, Scope&NodeCallbackInvoker&CollectedDataEmitter $scope): array { $typeMessageCallback = static function (Type $type): ?string { $isNull = $type->isNull(); @@ -41,9 +43,9 @@ public function processNode(Node $node, Scope $scope): array }; if ($node instanceof Node\Expr\BinaryOp\Coalesce) { - $error = $this->issetCheck->check($node->left, $scope, 'on left side of ??', 'nullCoalesce', $typeMessageCallback); + $error = $this->issetCheck->check($node->left, $scope, 'on left side of ??', 'nullCoalesce', $typeMessageCallback, self::class); } elseif ($node instanceof Node\Expr\AssignOp\Coalesce) { - $error = $this->issetCheck->check($node->var, $scope, 'on left side of ??=', 'nullCoalesce', $typeMessageCallback); + $error = $this->issetCheck->check($node->var, $scope, 'on left side of ??=', 'nullCoalesce', $typeMessageCallback, self::class); } else { return []; } diff --git a/tests/PHPStan/Rules/Variables/EmptyRuleTest.php b/tests/PHPStan/Rules/Variables/EmptyRuleTest.php index d14d6c24d84..62ddf808125 100644 --- a/tests/PHPStan/Rules/Variables/EmptyRuleTest.php +++ b/tests/PHPStan/Rules/Variables/EmptyRuleTest.php @@ -2,16 +2,19 @@ namespace PHPStan\Rules\Variables; +use PHPStan\Rules\Comparison\ConstantConditionInTraitHelper; +use PHPStan\Rules\Comparison\ConstantConditionInTraitRule; use PHPStan\Rules\IssetCheck; use PHPStan\Rules\Properties\PropertyDescriptor; use PHPStan\Rules\Properties\PropertyReflectionFinder; use PHPStan\Rules\Rule; +use PHPStan\Testing\CompositeRule; use PHPStan\Testing\RuleTestCase; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\RequiresPhp; /** - * @extends RuleTestCase + * @extends RuleTestCase */ class EmptyRuleTest extends RuleTestCase { @@ -20,12 +23,17 @@ class EmptyRuleTest extends RuleTestCase protected function getRule(): Rule { - return new EmptyRule(new IssetCheck( - new PropertyDescriptor(), - new PropertyReflectionFinder(), - true, - $this->treatPhpDocTypesAsCertain, - )); + // @phpstan-ignore argument.type + return new CompositeRule([ + new EmptyRule(new IssetCheck( + new PropertyDescriptor(), + new PropertyReflectionFinder(), + self::getContainer()->getByType(ConstantConditionInTraitHelper::class), + true, + $this->treatPhpDocTypesAsCertain, + )), + new ConstantConditionInTraitRule(), + ]); } protected function shouldTreatPhpDocTypesAsCertain(): bool @@ -252,4 +260,10 @@ public function testIssetAfterRememberedConstructor(): void ]); } + public function testInTrait(): void + { + $this->treatPhpDocTypesAsCertain = true; + $this->analyse([__DIR__ . '/data/isset-in-trait.php'], []); + } + } diff --git a/tests/PHPStan/Rules/Variables/IssetRuleTest.php b/tests/PHPStan/Rules/Variables/IssetRuleTest.php index fd841e49b16..9827526c9b7 100644 --- a/tests/PHPStan/Rules/Variables/IssetRuleTest.php +++ b/tests/PHPStan/Rules/Variables/IssetRuleTest.php @@ -2,15 +2,18 @@ namespace PHPStan\Rules\Variables; +use PHPStan\Rules\Comparison\ConstantConditionInTraitHelper; +use PHPStan\Rules\Comparison\ConstantConditionInTraitRule; use PHPStan\Rules\IssetCheck; use PHPStan\Rules\Properties\PropertyDescriptor; use PHPStan\Rules\Properties\PropertyReflectionFinder; use PHPStan\Rules\Rule; +use PHPStan\Testing\CompositeRule; use PHPStan\Testing\RuleTestCase; use PHPUnit\Framework\Attributes\RequiresPhp; /** - * @extends RuleTestCase + * @extends RuleTestCase */ class IssetRuleTest extends RuleTestCase { @@ -19,12 +22,17 @@ class IssetRuleTest extends RuleTestCase protected function getRule(): Rule { - return new IssetRule(new IssetCheck( - new PropertyDescriptor(), - new PropertyReflectionFinder(), - true, - $this->treatPhpDocTypesAsCertain, - )); + // @phpstan-ignore argument.type + return new CompositeRule([ + new IssetRule(new IssetCheck( + new PropertyDescriptor(), + new PropertyReflectionFinder(), + self::getContainer()->getByType(ConstantConditionInTraitHelper::class), + true, + $this->treatPhpDocTypesAsCertain, + )), + new ConstantConditionInTraitRule(), + ]); } protected function shouldTreatPhpDocTypesAsCertain(): bool @@ -562,4 +570,19 @@ public function testBug14393(): void ]); } + public function testInTrait(): void + { + $this->treatPhpDocTypesAsCertain = true; + $this->analyse([__DIR__ . '/data/isset-in-trait.php'], [ + [ + 'Property IssetInTrait\ClassA::$j (int) in isset() is not nullable.', + 36, + ], + [ + 'Property IssetInTrait\ClassB::$j (int) in isset() is not nullable.', + 36, + ], + ]); + } + } diff --git a/tests/PHPStan/Rules/Variables/NullCoalesceRuleTest.php b/tests/PHPStan/Rules/Variables/NullCoalesceRuleTest.php index 9abe4cf394c..06e5018c4e2 100644 --- a/tests/PHPStan/Rules/Variables/NullCoalesceRuleTest.php +++ b/tests/PHPStan/Rules/Variables/NullCoalesceRuleTest.php @@ -2,28 +2,36 @@ namespace PHPStan\Rules\Variables; +use PHPStan\Rules\Comparison\ConstantConditionInTraitHelper; +use PHPStan\Rules\Comparison\ConstantConditionInTraitRule; use PHPStan\Rules\IssetCheck; use PHPStan\Rules\Properties\PropertyDescriptor; use PHPStan\Rules\Properties\PropertyReflectionFinder; use PHPStan\Rules\Rule; +use PHPStan\Testing\CompositeRule; use PHPStan\Testing\RuleTestCase; use PHPUnit\Framework\Attributes\RequiresPhp; use const PHP_VERSION_ID; /** - * @extends RuleTestCase + * @extends RuleTestCase */ class NullCoalesceRuleTest extends RuleTestCase { protected function getRule(): Rule { - return new NullCoalesceRule(new IssetCheck( - new PropertyDescriptor(), - new PropertyReflectionFinder(), - true, - $this->shouldTreatPhpDocTypesAsCertain(), - )); + // @phpstan-ignore argument.type + return new CompositeRule([ + new NullCoalesceRule(new IssetCheck( + new PropertyDescriptor(), + new PropertyReflectionFinder(), + self::getContainer()->getByType(ConstantConditionInTraitHelper::class), + true, + $this->shouldTreatPhpDocTypesAsCertain(), + )), + new ConstantConditionInTraitRule(), + ]); } public function testCoalesceRule(): void @@ -448,4 +456,18 @@ public function testBug14393(): void ]); } + public function testInTrait(): void + { + $this->analyse([__DIR__ . '/data/isset-in-trait.php'], [ + [ + 'Property IssetInTrait\ClassA::$j (int) on left side of ?? is not nullable.', + 35, + ], + [ + 'Property IssetInTrait\ClassB::$j (int) on left side of ?? is not nullable.', + 35, + ], + ]); + } + } diff --git a/tests/PHPStan/Rules/Variables/data/isset-in-trait.php b/tests/PHPStan/Rules/Variables/data/isset-in-trait.php new file mode 100644 index 00000000000..fcfa57875a1 --- /dev/null +++ b/tests/PHPStan/Rules/Variables/data/isset-in-trait.php @@ -0,0 +1,53 @@ +i ?? -1); + var_dump(isset($this->i)); + var_dump(empty($this->i)); + } +} + +class MyClass +{ + use IssetTrait; + + public int $i = 10; +} + +class MyClassNullable +{ + use IssetTrait; + + public ?int $i = null; +} + +// Trait where property type is the same in all classes (should still report errors) +trait AlwaysNonNullableTrait +{ + public function doFoo(): void + { + var_dump($this->j ?? -1); + var_dump(isset($this->j)); + var_dump(empty($this->j)); + } +} + +class ClassA +{ + use AlwaysNonNullableTrait; + + public int $j = 10; +} + +class ClassB +{ + use AlwaysNonNullableTrait; + + public int $j = 20; +}