diff --git a/src/Analyser/ExpressionResult.php b/src/Analyser/ExpressionResult.php index 3a937d6190..0ec7eb1597 100644 --- a/src/Analyser/ExpressionResult.php +++ b/src/Analyser/ExpressionResult.php @@ -54,6 +54,11 @@ public function getBeforeScope(): MutatingScope return $this->beforeScope; } + public function getExpr(): Expr + { + return $this->expr; + } + public function hasYield(): bool { return $this->hasYield; diff --git a/src/Rules/Variables/EmptyRule.php b/src/Rules/Variables/EmptyRule.php index b884cebddc..ca8bc4f9ed 100644 --- a/src/Rules/Variables/EmptyRule.php +++ b/src/Rules/Variables/EmptyRule.php @@ -3,9 +3,12 @@ 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\Node\EmptyExpressionNode; +use PHPStan\Rules\Comparison\ConstantConditionInTraitHelper; use PHPStan\Rules\IssetCheck; use PHPStan\Rules\Rule; use PHPStan\Type\Type; @@ -17,7 +20,10 @@ final class EmptyRule implements Rule { - public function __construct(private IssetCheck $issetCheck) + public function __construct( + private IssetCheck $issetCheck, + private ConstantConditionInTraitHelper $constantConditionInTraitHelper, + ) { } @@ -26,9 +32,10 @@ public function getNodeType(): string return EmptyExpressionNode::class; } - public function processNode(Node $node, Scope $scope): array + public function processNode(Node $node, Scope&NodeCallbackInvoker&CollectedDataEmitter $scope): array { - $error = $this->issetCheck->check($node->getExprResult(), $scope, 'in empty()', 'empty', static function (Type $type): ?string { + $exprResult = $node->getExprResult(); + $error = $this->issetCheck->check($exprResult, $scope, 'in empty()', 'empty', static function (Type $type): ?string { $isNull = $type->isNull(); if ($isNull->maybe()) { return null; @@ -61,6 +68,14 @@ public function processNode(Node $node, Scope $scope): array }); if ($error === null) { + $this->constantConditionInTraitHelper->emitNoError(self::class, $scope, $exprResult->getExpr()); + return []; + } + + if ($scope->isInTrait()) { + // IssetCheck's message already distinguishes the possible outcomes, + // so the contexts only need to be told apart by error/no error. + $this->constantConditionInTraitHelper->emitError(self::class, $scope, $exprResult->getExpr(), true, $error); return []; } diff --git a/src/Rules/Variables/IssetRule.php b/src/Rules/Variables/IssetRule.php index cd9300c2dc..1df1fa35db 100644 --- a/src/Rules/Variables/IssetRule.php +++ b/src/Rules/Variables/IssetRule.php @@ -3,9 +3,12 @@ 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\Node\IssetExpressionNode; +use PHPStan\Rules\Comparison\ConstantConditionInTraitHelper; use PHPStan\Rules\IssetCheck; use PHPStan\Rules\Rule; use PHPStan\Type\Type; @@ -17,7 +20,10 @@ final class IssetRule implements Rule { - public function __construct(private IssetCheck $issetCheck) + public function __construct( + private IssetCheck $issetCheck, + private ConstantConditionInTraitHelper $constantConditionInTraitHelper, + ) { } @@ -26,7 +32,7 @@ public function getNodeType(): string return IssetExpressionNode::class; } - public function processNode(Node $node, Scope $scope): array + public function processNode(Node $node, Scope&NodeCallbackInvoker&CollectedDataEmitter $scope): array { $messages = []; foreach ($node->getVarResults() as $varResult) { @@ -43,8 +49,17 @@ public function processNode(Node $node, Scope $scope): array return 'is not nullable'; }); if ($error === null) { + $this->constantConditionInTraitHelper->emitNoError(self::class, $scope, $varResult->getExpr()); continue; } + + if ($scope->isInTrait()) { + // IssetCheck's message already distinguishes the possible outcomes, + // so the contexts only need to be told apart by error/no error. + $this->constantConditionInTraitHelper->emitError(self::class, $scope, $varResult->getExpr(), true, $error); + continue; + } + $messages[] = $error; } diff --git a/src/Rules/Variables/NullCoalesceRule.php b/src/Rules/Variables/NullCoalesceRule.php index 409ae4e431..512b6f9bae 100644 --- a/src/Rules/Variables/NullCoalesceRule.php +++ b/src/Rules/Variables/NullCoalesceRule.php @@ -3,10 +3,13 @@ namespace PHPStan\Rules\Variables; use PhpParser\Node; +use PHPStan\Analyser\CollectedDataEmitter; +use PHPStan\Analyser\NodeCallbackInvoker; use PHPStan\Analyser\Scope; use PHPStan\DependencyInjection\AutowiredParameter; use PHPStan\DependencyInjection\RegisteredRule; use PHPStan\Node\CoalesceExpressionNode; +use PHPStan\Rules\Comparison\ConstantConditionInTraitHelper; use PHPStan\Rules\IdentifierRuleError; use PHPStan\Rules\IssetCheck; use PHPStan\Rules\Rule; @@ -23,6 +26,7 @@ final class NullCoalesceRule implements Rule public function __construct( private IssetCheck $issetCheck, + private ConstantConditionInTraitHelper $constantConditionInTraitHelper, #[AutowiredParameter(ref: '%featureToggles.unnecessaryNullCoalesce%')] private bool $unnecessaryNullCoalesce, ) @@ -34,10 +38,11 @@ public function getNodeType(): string return CoalesceExpressionNode::class; } - public function processNode(Node $node, Scope $scope): array + public function processNode(Node $node, Scope&NodeCallbackInvoker&CollectedDataEmitter $scope): array { + $subjectResult = $node->getSubjectResult(); $error = $this->issetCheck->check( - $node->getSubjectResult(), + $subjectResult, $scope, $node->getOperatorDescription(), 'nullCoalesce', @@ -53,18 +58,21 @@ static function (Type $type): ?string { return 'is not nullable'; }, - ); + ) ?? $this->checkUnnecessaryNullCoalesce($node, $scope); - if ($error !== null) { - return [$error]; + if ($error === null) { + $this->constantConditionInTraitHelper->emitNoError(self::class, $scope, $subjectResult->getExpr()); + return []; } - $unnecessaryError = $this->checkUnnecessaryNullCoalesce($node, $scope); - if ($unnecessaryError !== null) { - return [$unnecessaryError]; + if ($scope->isInTrait()) { + // The error messages already distinguish the possible outcomes, + // so the contexts only need to be told apart by error/no error. + $this->constantConditionInTraitHelper->emitError(self::class, $scope, $subjectResult->getExpr(), true, $error); + return []; } - return []; + return [$error]; } private function checkUnnecessaryNullCoalesce(CoalesceExpressionNode $node, Scope $scope): ?IdentifierRuleError diff --git a/tests/PHPStan/Rules/Variables/EmptyRuleTest.php b/tests/PHPStan/Rules/Variables/EmptyRuleTest.php index 6ab498be10..ea8b98474f 100644 --- a/tests/PHPStan/Rules/Variables/EmptyRuleTest.php +++ b/tests/PHPStan/Rules/Variables/EmptyRuleTest.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\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 { @@ -19,11 +22,18 @@ class EmptyRuleTest extends RuleTestCase protected function getRule(): Rule { - return new EmptyRule(new IssetCheck( - new PropertyDescriptor(), - true, - $this->treatPhpDocTypesAsCertain, - )); + // @phpstan-ignore argument.type + return new CompositeRule([ + new EmptyRule( + new IssetCheck( + new PropertyDescriptor(), + true, + $this->treatPhpDocTypesAsCertain, + ), + self::getContainer()->getByType(ConstantConditionInTraitHelper::class), + ), + new ConstantConditionInTraitRule(), + ]); } protected function shouldTreatPhpDocTypesAsCertain(): bool @@ -257,4 +267,24 @@ public function testNullCoalesceAssignRightSideScope(): void $this->analyse([__DIR__ . '/data/null-coalesce-assign-right-side-scope.php'], []); } + public function testInTrait(): void + { + $this->treatPhpDocTypesAsCertain = true; + + $this->analyse([__DIR__ . '/data/isset-in-trait.php'], [ + [ + 'Property IssetInTrait\FirstNonNullableProperty::$k (int<1, max>) in empty() is not falsy.', + 84, + ], + [ + 'Property IssetInTrait\SecondNonNullableProperty::$k (int<1, max>) in empty() is not falsy.', + 84, + ], + [ + 'Variable $s in empty() always exists and is not falsy.', + 121, + ], + ]); + } + } diff --git a/tests/PHPStan/Rules/Variables/IssetRuleTest.php b/tests/PHPStan/Rules/Variables/IssetRuleTest.php index db77f2aef5..bc95ddebb2 100644 --- a/tests/PHPStan/Rules/Variables/IssetRuleTest.php +++ b/tests/PHPStan/Rules/Variables/IssetRuleTest.php @@ -2,14 +2,17 @@ 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\Rule; +use PHPStan\Testing\CompositeRule; use PHPStan\Testing\RuleTestCase; use PHPUnit\Framework\Attributes\RequiresPhp; /** - * @extends RuleTestCase + * @extends RuleTestCase */ class IssetRuleTest extends RuleTestCase { @@ -18,11 +21,18 @@ class IssetRuleTest extends RuleTestCase protected function getRule(): Rule { - return new IssetRule(new IssetCheck( - new PropertyDescriptor(), - true, - $this->treatPhpDocTypesAsCertain, - )); + // @phpstan-ignore argument.type + return new CompositeRule([ + new IssetRule( + new IssetCheck( + new PropertyDescriptor(), + true, + $this->treatPhpDocTypesAsCertain, + ), + self::getContainer()->getByType(ConstantConditionInTraitHelper::class), + ), + new ConstantConditionInTraitRule(), + ]); } protected function shouldTreatPhpDocTypesAsCertain(): bool @@ -617,4 +627,31 @@ public function testNullCoalesceAssignRightSideScope(): void $this->analyse([__DIR__ . '/data/null-coalesce-assign-right-side-scope.php'], []); } + public function testInTrait(): void + { + $this->treatPhpDocTypesAsCertain = true; + + $this->analyse([__DIR__ . '/data/isset-in-trait.php'], [ + [ + 'Property IssetInTrait\FirstNonNullableProperty::$k (int<1, max>) in isset() is not nullable.', + 83, + ], + [ + 'Property IssetInTrait\SecondNonNullableProperty::$k (int<1, max>) in isset() is not nullable.', + 83, + ], + [ + 'Variable $s in isset() always exists and is not nullable.', + 120, + ], + ]); + } + + public function testBug14416(): void + { + $this->treatPhpDocTypesAsCertain = true; + + $this->analyse([__DIR__ . '/data/bug-14416.php'], []); + } + } diff --git a/tests/PHPStan/Rules/Variables/NullCoalesceRuleTest.php b/tests/PHPStan/Rules/Variables/NullCoalesceRuleTest.php index 1635fe7170..de3a2dff1d 100644 --- a/tests/PHPStan/Rules/Variables/NullCoalesceRuleTest.php +++ b/tests/PHPStan/Rules/Variables/NullCoalesceRuleTest.php @@ -2,26 +2,37 @@ 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\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(), - true, - $this->shouldTreatPhpDocTypesAsCertain(), - ), true); + // @phpstan-ignore argument.type + return new CompositeRule([ + new NullCoalesceRule( + new IssetCheck( + new PropertyDescriptor(), + true, + $this->shouldTreatPhpDocTypesAsCertain(), + ), + self::getContainer()->getByType(ConstantConditionInTraitHelper::class), + true, + ), + new ConstantConditionInTraitRule(), + ]); } public function testCoalesceRule(): void @@ -611,4 +622,22 @@ public function testBug15046(): void ]); } + public function testInTrait(): void + { + $this->analyse([__DIR__ . '/data/isset-in-trait.php'], [ + [ + 'Property IssetInTrait\FirstNonNullableProperty::$k (int<1, max>) on left side of ?? is not nullable.', + 82, + ], + [ + 'Property IssetInTrait\SecondNonNullableProperty::$k (int<1, max>) on left side of ?? is not nullable.', + 82, + ], + [ + 'Variable $s on left side of ?? always exists and is not nullable.', + 119, + ], + ]); + } + } diff --git a/tests/PHPStan/Rules/Variables/data/bug-14416.php b/tests/PHPStan/Rules/Variables/data/bug-14416.php new file mode 100644 index 0000000000..0a7ebe572d --- /dev/null +++ b/tests/PHPStan/Rules/Variables/data/bug-14416.php @@ -0,0 +1,23 @@ +i); + } +} + +class MyClass +{ + use MyTrait; + + public int $i = 10; +} + +class MyClass2 +{ + use MyTrait; +} 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 0000000000..19d9ba2a5e --- /dev/null +++ b/tests/PHPStan/Rules/Variables/data/isset-in-trait.php @@ -0,0 +1,138 @@ +i ?? -1); + var_dump(isset($this->i)); + var_dump(empty($this->i)); + } + +} + +class DeclaredProperty +{ + + use MaybeDeclaredPropertyTrait; + + /** @var positive-int */ + public int $i = 10; + +} + +class UndeclaredProperty +{ + + use MaybeDeclaredPropertyTrait; + +} + +/** + * The property is non-nullable in one class using the trait and nullable in the other, + * so nothing should be reported. + */ +trait DifferentPropertyTypeTrait +{ + + public function doFoo(): void + { + var_dump($this->j ?? -1); + var_dump(isset($this->j)); + var_dump(empty($this->j)); + } + +} + +class NonNullableProperty +{ + + use DifferentPropertyTypeTrait; + + /** @var positive-int */ + public int $j = 10; + +} + +class NullableProperty +{ + + use DifferentPropertyTypeTrait; + + public ?int $j = null; + +} + +/** + * The property is non-nullable in every class using the trait, so the errors are reported + * in the context of each of them. + */ +trait SamePropertyTypeTrait +{ + + public function doFoo(): void + { + var_dump($this->k ?? -1); + var_dump(isset($this->k)); + var_dump(empty($this->k)); + } + +} + +class FirstNonNullableProperty +{ + + use SamePropertyTypeTrait; + + /** @var positive-int */ + public int $k = 10; + +} + +class SecondNonNullableProperty +{ + + use SamePropertyTypeTrait; + + /** @var positive-int */ + public int $k = 20; + +} + +/** + * The checked expression does not depend on the class using the trait, so the errors are + * reported once, directly in the trait. + */ +trait ClassIndependentTrait +{ + + public function doFoo(): void + { + $s = 'foo'; + var_dump($s ?? -1); + var_dump(isset($s)); + var_dump(empty($s)); + } + +} + +class FirstClassIndependent +{ + + use ClassIndependentTrait; + +} + +class SecondClassIndependent +{ + + use ClassIndependentTrait; + +}