Skip to content

Commit bade709

Browse files
peter17peter17
committed
Avoid warning about non-nullable class-property in traits.
1 parent 735d28a commit bade709

File tree

8 files changed

+104
-13
lines changed

8 files changed

+104
-13
lines changed

‎src/Rules/IssetCheck.php‎

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,13 @@
44

55
use PhpParser\Node;
66
use PhpParser\Node\Expr;
7+
use PHPStan\Analyser\CollectedDataEmitter;
8+
use PHPStan\Analyser\NodeCallbackInvoker;
79
use PHPStan\Analyser\Scope;
810
use PHPStan\DependencyInjection\AutowiredParameter;
911
use PHPStan\DependencyInjection\AutowiredService;
1012
use PHPStan\Node\Expr\PropertyInitializationExpr;
13+
use PHPStan\Rules\Comparison\ConstantConditionInTraitHelper;
1114
use PHPStan\Rules\Properties\PropertyDescriptor;
1215
use PHPStan\Rules\Properties\PropertyReflectionFinder;
1316
use PHPStan\Type\NeverType;
@@ -27,6 +30,7 @@ final class IssetCheck
2730
public function __construct(
2831
private PropertyDescriptor $propertyDescriptor,
2932
private PropertyReflectionFinder $propertyReflectionFinder,
33+
private ConstantConditionInTraitHelper $constantConditionInTraitHelper,
3034
#[AutowiredParameter]
3135
private bool $checkAdvancedIsset,
3236
#[AutowiredParameter]
@@ -36,10 +40,11 @@ public function __construct(
3640
}
3741

3842
/**
43+
* @param class-string<Rule<covariant Node>> $ruleName
3944
* @param ErrorIdentifier $identifier
4045
* @param callable(Type): ?string $typeMessageCallback
4146
*/
42-
public function check(Expr $expr, Scope $scope, string $operatorDescription, string $identifier, callable $typeMessageCallback, ?IdentifierRuleError $error = null): ?IdentifierRuleError
47+
public function check(Expr $expr, Scope&NodeCallbackInvoker&CollectedDataEmitter $scope, string $operatorDescription, string $identifier, callable $typeMessageCallback, string $ruleName, Expr $originalExpr, ?IdentifierRuleError $error = null): ?IdentifierRuleError
4348
{
4449
// mirrored in PHPStan\Analyser\MutatingScope::issetCheck()
4550
if ($expr instanceof Node\Expr\Variable && is_string($expr->name)) {
@@ -114,7 +119,7 @@ public function check(Expr $expr, Scope $scope, string $operatorDescription, str
114119
), $typeMessageCallback, $identifier, 'offset');
115120

116121
if ($error !== null) {
117-
return $this->check($expr->var, $scope, $operatorDescription, $identifier, $typeMessageCallback, $error);
122+
return $this->check($expr->var, $scope, $operatorDescription, $identifier, $typeMessageCallback, $ruleName, $originalExpr, $error);
118123
}
119124
}
120125

@@ -216,11 +221,11 @@ static function (Type $type) use ($typeMessageCallback): ?string {
216221

217222
if ($error !== null) {
218223
if ($expr instanceof Node\Expr\PropertyFetch) {
219-
return $this->check($expr->var, $scope, $operatorDescription, $identifier, $typeMessageCallback, $error);
224+
return $this->check($expr->var, $scope, $operatorDescription, $identifier, $typeMessageCallback, $ruleName, $originalExpr, $error);
220225
}
221226

222227
if ($expr->class instanceof Expr) {
223-
return $this->check($expr->class, $scope, $operatorDescription, $identifier, $typeMessageCallback, $error);
228+
return $this->check($expr->class, $scope, $operatorDescription, $identifier, $typeMessageCallback, $ruleName, $originalExpr, $error);
224229
}
225230
}
226231

@@ -261,6 +266,27 @@ static function (Type $type) use ($typeMessageCallback): ?string {
261266
return null;
262267
}
263268

269+
/**
270+
* @param class-string<Rule<covariant Node>> $ruleName
271+
* @param ErrorIdentifier $identifier
272+
* @param callable(Type): ?string $typeMessageCallback
273+
*/
274+
public function checkWithTraitHandling(Expr $expr, Scope&NodeCallbackInvoker&CollectedDataEmitter $scope, string $operatorDescription, string $identifier, callable $typeMessageCallback, string $ruleName): ?IdentifierRuleError
275+
{
276+
$error = $this->check($expr, $scope, $operatorDescription, $identifier, $typeMessageCallback, $ruleName, $expr);
277+
278+
if ($scope->isInTrait()) {
279+
if ($error !== null) {
280+
$this->constantConditionInTraitHelper->emitError($ruleName, $scope, $expr, true, $error);
281+
return null;
282+
}
283+
$this->constantConditionInTraitHelper->emitNoError($ruleName, $scope, $expr);
284+
return null;
285+
}
286+
287+
return $error;
288+
}
289+
264290
/**
265291
* @param ErrorIdentifier $identifier
266292
*/

‎src/Rules/Variables/EmptyRule.php‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
namespace PHPStan\Rules\Variables;
44

55
use PhpParser\Node;
6+
use PHPStan\Analyser\CollectedDataEmitter;
7+
use PHPStan\Analyser\NodeCallbackInvoker;
68
use PHPStan\Analyser\Scope;
79
use PHPStan\DependencyInjection\RegisteredRule;
810
use PHPStan\Rules\IssetCheck;
@@ -25,9 +27,9 @@ public function getNodeType(): string
2527
return Node\Expr\Empty_::class;
2628
}
2729

28-
public function processNode(Node $node, Scope $scope): array
30+
public function processNode(Node $node, Scope&NodeCallbackInvoker&CollectedDataEmitter $scope): array
2931
{
30-
$error = $this->issetCheck->check($node->expr, $scope, 'in empty()', 'empty', static function (Type $type): ?string {
32+
$error = $this->issetCheck->checkWithTraitHandling($node->expr, $scope, 'in empty()', 'empty', static function (Type $type): ?string {
3133
$isNull = $type->isNull();
3234
if ($isNull->maybe()) {
3335
return null;
@@ -57,7 +59,7 @@ public function processNode(Node $node, Scope $scope): array
5759
}
5860

5961
return 'is not nullable';
60-
});
62+
}, self::class);
6163

6264
if ($error === null) {
6365
return [];

‎src/Rules/Variables/IssetRule.php‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
namespace PHPStan\Rules\Variables;
44

55
use PhpParser\Node;
6+
use PHPStan\Analyser\CollectedDataEmitter;
7+
use PHPStan\Analyser\NodeCallbackInvoker;
68
use PHPStan\Analyser\Scope;
79
use PHPStan\DependencyInjection\RegisteredRule;
810
use PHPStan\Rules\IssetCheck;
@@ -25,11 +27,11 @@ public function getNodeType(): string
2527
return Node\Expr\Isset_::class;
2628
}
2729

28-
public function processNode(Node $node, Scope $scope): array
30+
public function processNode(Node $node, Scope&NodeCallbackInvoker&CollectedDataEmitter $scope): array
2931
{
3032
$messages = [];
3133
foreach ($node->vars as $var) {
32-
$error = $this->issetCheck->check($var, $scope, 'in isset()', 'isset', static function (Type $type): ?string {
34+
$error = $this->issetCheck->checkWithTraitHandling($var, $scope, 'in isset()', 'isset', static function (Type $type): ?string {
3335
$isNull = $type->isNull();
3436
if ($isNull->maybe()) {
3537
return null;
@@ -40,7 +42,7 @@ public function processNode(Node $node, Scope $scope): array
4042
}
4143

4244
return 'is not nullable';
43-
});
45+
}, self::class);
4446
if ($error === null) {
4547
continue;
4648
}

‎src/Rules/Variables/NullCoalesceRule.php‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
namespace PHPStan\Rules\Variables;
44

55
use PhpParser\Node;
6+
use PHPStan\Analyser\CollectedDataEmitter;
7+
use PHPStan\Analyser\NodeCallbackInvoker;
68
use PHPStan\Analyser\Scope;
79
use PHPStan\DependencyInjection\RegisteredRule;
810
use PHPStan\Rules\IssetCheck;
@@ -25,7 +27,7 @@ public function getNodeType(): string
2527
return Node\Expr::class;
2628
}
2729

28-
public function processNode(Node $node, Scope $scope): array
30+
public function processNode(Node $node, Scope&NodeCallbackInvoker&CollectedDataEmitter $scope): array
2931
{
3032
$typeMessageCallback = static function (Type $type): ?string {
3133
$isNull = $type->isNull();
@@ -41,9 +43,9 @@ public function processNode(Node $node, Scope $scope): array
4143
};
4244

4345
if ($node instanceof Node\Expr\BinaryOp\Coalesce) {
44-
$error = $this->issetCheck->check($node->left, $scope, 'on left side of ??', 'nullCoalesce', $typeMessageCallback);
46+
$error = $this->issetCheck->checkWithTraitHandling($node->left, $scope, 'on left side of ??', 'nullCoalesce', $typeMessageCallback, self::class);
4547
} elseif ($node instanceof Node\Expr\AssignOp\Coalesce) {
46-
$error = $this->issetCheck->check($node->var, $scope, 'on left side of ??=', 'nullCoalesce', $typeMessageCallback);
48+
$error = $this->issetCheck->checkWithTraitHandling($node->var, $scope, 'on left side of ??=', 'nullCoalesce', $typeMessageCallback, self::class);
4749
} else {
4850
return [];
4951
}

‎tests/PHPStan/Rules/Variables/EmptyRuleTest.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
namespace PHPStan\Rules\Variables;
44

5+
use PHPStan\Rules\Comparison\ConstantConditionInTraitHelper;
56
use PHPStan\Rules\IssetCheck;
67
use PHPStan\Rules\Properties\PropertyDescriptor;
78
use PHPStan\Rules\Properties\PropertyReflectionFinder;
@@ -23,6 +24,7 @@ protected function getRule(): Rule
2324
return new EmptyRule(new IssetCheck(
2425
new PropertyDescriptor(),
2526
new PropertyReflectionFinder(),
27+
self::getContainer()->getByType(ConstantConditionInTraitHelper::class),
2628
true,
2729
$this->treatPhpDocTypesAsCertain,
2830
));

‎tests/PHPStan/Rules/Variables/IssetRuleTest.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
namespace PHPStan\Rules\Variables;
44

5+
use PHPStan\Rules\Comparison\ConstantConditionInTraitHelper;
56
use PHPStan\Rules\IssetCheck;
67
use PHPStan\Rules\Properties\PropertyDescriptor;
78
use PHPStan\Rules\Properties\PropertyReflectionFinder;
@@ -22,6 +23,7 @@ protected function getRule(): Rule
2223
return new IssetRule(new IssetCheck(
2324
new PropertyDescriptor(),
2425
new PropertyReflectionFinder(),
26+
self::getContainer()->getByType(ConstantConditionInTraitHelper::class),
2527
true,
2628
$this->treatPhpDocTypesAsCertain,
2729
));

‎tests/PHPStan/Rules/Variables/NullCoalesceRuleTest.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
namespace PHPStan\Rules\Variables;
44

5+
use PHPStan\Rules\Comparison\ConstantConditionInTraitHelper;
56
use PHPStan\Rules\IssetCheck;
67
use PHPStan\Rules\Properties\PropertyDescriptor;
78
use PHPStan\Rules\Properties\PropertyReflectionFinder;
@@ -21,6 +22,7 @@ protected function getRule(): Rule
2122
return new NullCoalesceRule(new IssetCheck(
2223
new PropertyDescriptor(),
2324
new PropertyReflectionFinder(),
25+
self::getContainer()->getByType(ConstantConditionInTraitHelper::class),
2426
true,
2527
$this->shouldTreatPhpDocTypesAsCertain(),
2628
));

‎tests/PHPStan/Rules/Variables/data/isset-in-trait.php‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
<?php
2+
3+
namespace IssetInTrait;
4+
5+
// Trait where property type differs across classes (should suppress errors)
6+
trait IssetTrait
7+
{
8+
public function doFoo(): void
9+
{
10+
var_dump($this->i ?? -1);
11+
var_dump(isset($this->i));
12+
var_dump(empty($this->i));
13+
}
14+
}
15+
16+
class MyClass
17+
{
18+
use IssetTrait;
19+
20+
public int $i = 10;
21+
}
22+
23+
class MyClassNullable
24+
{
25+
use IssetTrait;
26+
27+
public ?int $i = null;
28+
}
29+
30+
// Trait where property type is the same in all classes (should still report errors)
31+
trait AlwaysNonNullableTrait
32+
{
33+
public function doFoo(): void
34+
{
35+
var_dump($this->j ?? -1);
36+
var_dump(isset($this->j));
37+
var_dump(empty($this->j));
38+
}
39+
}
40+
41+
class ClassA
42+
{
43+
use AlwaysNonNullableTrait;
44+
45+
public int $j = 10;
46+
}
47+
48+
class ClassB
49+
{
50+
use AlwaysNonNullableTrait;
51+
52+
public int $j = 20;
53+
}

0 commit comments

Comments
 (0)