Skip to content

Commit 0fcdcc7

Browse files
peter17peter17
committed
Fix tests. Improve design of check method.
1 parent 3b8318f commit 0fcdcc7

7 files changed

Lines changed: 107 additions & 54 deletions

File tree

‎src/Rules/IssetCheck.php‎

Lines changed: 26 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -40,11 +40,32 @@ public function __construct(
4040
}
4141

4242
/**
43-
* @param class-string<Rule<covariant Node>> $ruleName
43+
* @param class-string<Rule<covariant Node>>|null $ruleName
4444
* @param ErrorIdentifier $identifier
4545
* @param callable(Type): ?string $typeMessageCallback
4646
*/
47-
public function check(Expr $expr, Scope&NodeCallbackInvoker&CollectedDataEmitter $scope, string $operatorDescription, string $identifier, callable $typeMessageCallback, string $ruleName, Expr $originalExpr, ?IdentifierRuleError $error = null): ?IdentifierRuleError
47+
public function check(Expr $expr, Scope&NodeCallbackInvoker&CollectedDataEmitter $scope, string $operatorDescription, string $identifier, callable $typeMessageCallback, ?string $ruleName = null, ?IdentifierRuleError $error = null): ?IdentifierRuleError
48+
{
49+
$result = $this->checkInternal($expr, $scope, $operatorDescription, $identifier, $typeMessageCallback, $ruleName, $expr, $error);
50+
51+
if ($ruleName !== null && $scope->isInTrait()) {
52+
if ($result !== null) {
53+
$this->constantConditionInTraitHelper->emitError($ruleName, $scope, $expr, true, $result);
54+
return null;
55+
}
56+
$this->constantConditionInTraitHelper->emitNoError($ruleName, $scope, $expr);
57+
return null;
58+
}
59+
60+
return $result;
61+
}
62+
63+
/**
64+
* @param class-string<Rule<covariant Node>>|null $ruleName
65+
* @param ErrorIdentifier $identifier
66+
* @param callable(Type): ?string $typeMessageCallback
67+
*/
68+
private function checkInternal(Expr $expr, Scope&NodeCallbackInvoker&CollectedDataEmitter $scope, string $operatorDescription, string $identifier, callable $typeMessageCallback, ?string $ruleName, Expr $originalExpr, ?IdentifierRuleError $error = null): ?IdentifierRuleError
4869
{
4970
// mirrored in PHPStan\Analyser\MutatingScope::issetCheck()
5071
if ($expr instanceof Node\Expr\Variable && is_string($expr->name)) {
@@ -119,7 +140,7 @@ public function check(Expr $expr, Scope&NodeCallbackInvoker&CollectedDataEmitter
119140
), $typeMessageCallback, $identifier, 'offset');
120141

121142
if ($error !== null) {
122-
return $this->check($expr->var, $scope, $operatorDescription, $identifier, $typeMessageCallback, $ruleName, $originalExpr, $error);
143+
return $this->checkInternal($expr->var, $scope, $operatorDescription, $identifier, $typeMessageCallback, $ruleName, $originalExpr, $error);
123144
}
124145
}
125146

@@ -225,11 +246,11 @@ static function (Type $type) use ($typeMessageCallback): ?string {
225246

226247
if ($error !== null) {
227248
if ($expr instanceof Node\Expr\PropertyFetch) {
228-
return $this->check($expr->var, $scope, $operatorDescription, $identifier, $typeMessageCallback, $ruleName, $originalExpr, $error);
249+
return $this->checkInternal($expr->var, $scope, $operatorDescription, $identifier, $typeMessageCallback, $ruleName, $originalExpr, $error);
229250
}
230251

231252
if ($expr->class instanceof Expr) {
232-
return $this->check($expr->class, $scope, $operatorDescription, $identifier, $typeMessageCallback, $ruleName, $originalExpr, $error);
253+
return $this->checkInternal($expr->class, $scope, $operatorDescription, $identifier, $typeMessageCallback, $ruleName, $originalExpr, $error);
233254
}
234255
}
235256

@@ -270,27 +291,6 @@ static function (Type $type) use ($typeMessageCallback): ?string {
270291
return null;
271292
}
272293

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

‎src/Rules/Variables/EmptyRule.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ public function getNodeType(): string
2929

3030
public function processNode(Node $node, Scope&NodeCallbackInvoker&CollectedDataEmitter $scope): array
3131
{
32-
$error = $this->issetCheck->checkWithTraitHandling($node->expr, $scope, 'in empty()', 'empty', static function (Type $type): ?string {
32+
$error = $this->issetCheck->check($node->expr, $scope, 'in empty()', 'empty', static function (Type $type): ?string {
3333
$isNull = $type->isNull();
3434
if ($isNull->maybe()) {
3535
return null;

‎src/Rules/Variables/IssetRule.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ public function processNode(Node $node, Scope&NodeCallbackInvoker&CollectedDataE
3131
{
3232
$messages = [];
3333
foreach ($node->vars as $var) {
34-
$error = $this->issetCheck->checkWithTraitHandling($var, $scope, 'in isset()', 'isset', static function (Type $type): ?string {
34+
$error = $this->issetCheck->check($var, $scope, 'in isset()', 'isset', static function (Type $type): ?string {
3535
$isNull = $type->isNull();
3636
if ($isNull->maybe()) {
3737
return null;

‎src/Rules/Variables/NullCoalesceRule.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,9 +43,9 @@ public function processNode(Node $node, Scope&NodeCallbackInvoker&CollectedDataE
4343
};
4444

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

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

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

55
use PHPStan\Rules\Comparison\ConstantConditionInTraitHelper;
6+
use PHPStan\Rules\Comparison\ConstantConditionInTraitRule;
67
use PHPStan\Rules\IssetCheck;
78
use PHPStan\Rules\Properties\PropertyDescriptor;
89
use PHPStan\Rules\Properties\PropertyReflectionFinder;
910
use PHPStan\Rules\Rule;
11+
use PHPStan\Testing\CompositeRule;
1012
use PHPStan\Testing\RuleTestCase;
1113
use PHPUnit\Framework\Attributes\DataProvider;
1214
use PHPUnit\Framework\Attributes\RequiresPhp;
1315

1416
/**
15-
* @extends RuleTestCase<EmptyRule>
17+
* @extends RuleTestCase<CompositeRule>
1618
*/
1719
class EmptyRuleTest extends RuleTestCase
1820
{
@@ -21,13 +23,17 @@ class EmptyRuleTest extends RuleTestCase
2123

2224
protected function getRule(): Rule
2325
{
24-
return new EmptyRule(new IssetCheck(
25-
new PropertyDescriptor(),
26-
new PropertyReflectionFinder(),
27-
self::getContainer()->getByType(ConstantConditionInTraitHelper::class),
28-
true,
29-
$this->treatPhpDocTypesAsCertain,
30-
));
26+
// @phpstan-ignore argument.type
27+
return new CompositeRule([
28+
new EmptyRule(new IssetCheck(
29+
new PropertyDescriptor(),
30+
new PropertyReflectionFinder(),
31+
self::getContainer()->getByType(ConstantConditionInTraitHelper::class),
32+
true,
33+
$this->treatPhpDocTypesAsCertain,
34+
)),
35+
new ConstantConditionInTraitRule(),
36+
]);
3137
}
3238

3339
protected function shouldTreatPhpDocTypesAsCertain(): bool
@@ -254,4 +260,10 @@ public function testIssetAfterRememberedConstructor(): void
254260
]);
255261
}
256262

263+
public function testInTrait(): void
264+
{
265+
$this->treatPhpDocTypesAsCertain = true;
266+
$this->analyse([__DIR__ . '/data/isset-in-trait.php'], []);
267+
}
268+
257269
}

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

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

55
use PHPStan\Rules\Comparison\ConstantConditionInTraitHelper;
6+
use PHPStan\Rules\Comparison\ConstantConditionInTraitRule;
67
use PHPStan\Rules\IssetCheck;
78
use PHPStan\Rules\Properties\PropertyDescriptor;
89
use PHPStan\Rules\Properties\PropertyReflectionFinder;
910
use PHPStan\Rules\Rule;
11+
use PHPStan\Testing\CompositeRule;
1012
use PHPStan\Testing\RuleTestCase;
1113
use PHPUnit\Framework\Attributes\RequiresPhp;
1214

1315
/**
14-
* @extends RuleTestCase<IssetRule>
16+
* @extends RuleTestCase<CompositeRule>
1517
*/
1618
class IssetRuleTest extends RuleTestCase
1719
{
@@ -20,13 +22,17 @@ class IssetRuleTest extends RuleTestCase
2022

2123
protected function getRule(): Rule
2224
{
23-
return new IssetRule(new IssetCheck(
24-
new PropertyDescriptor(),
25-
new PropertyReflectionFinder(),
26-
self::getContainer()->getByType(ConstantConditionInTraitHelper::class),
27-
true,
28-
$this->treatPhpDocTypesAsCertain,
29-
));
25+
// @phpstan-ignore argument.type
26+
return new CompositeRule([
27+
new IssetRule(new IssetCheck(
28+
new PropertyDescriptor(),
29+
new PropertyReflectionFinder(),
30+
self::getContainer()->getByType(ConstantConditionInTraitHelper::class),
31+
true,
32+
$this->treatPhpDocTypesAsCertain,
33+
)),
34+
new ConstantConditionInTraitRule(),
35+
]);
3036
}
3137

3238
protected function shouldTreatPhpDocTypesAsCertain(): bool
@@ -564,4 +570,19 @@ public function testBug14393(): void
564570
]);
565571
}
566572

573+
public function testInTrait(): void
574+
{
575+
$this->treatPhpDocTypesAsCertain = true;
576+
$this->analyse([__DIR__ . '/data/isset-in-trait.php'], [
577+
[
578+
'Property IssetInTrait\ClassA::$j (int) in isset() is not nullable.',
579+
36,
580+
],
581+
[
582+
'Property IssetInTrait\ClassB::$j (int) in isset() is not nullable.',
583+
36,
584+
],
585+
]);
586+
}
587+
567588
}

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

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

55
use PHPStan\Rules\Comparison\ConstantConditionInTraitHelper;
6+
use PHPStan\Rules\Comparison\ConstantConditionInTraitRule;
67
use PHPStan\Rules\IssetCheck;
78
use PHPStan\Rules\Properties\PropertyDescriptor;
89
use PHPStan\Rules\Properties\PropertyReflectionFinder;
910
use PHPStan\Rules\Rule;
11+
use PHPStan\Testing\CompositeRule;
1012
use PHPStan\Testing\RuleTestCase;
1113
use PHPUnit\Framework\Attributes\RequiresPhp;
1214
use const PHP_VERSION_ID;
1315

1416
/**
15-
* @extends RuleTestCase<NullCoalesceRule>
17+
* @extends RuleTestCase<CompositeRule>
1618
*/
1719
class NullCoalesceRuleTest extends RuleTestCase
1820
{
1921

2022
protected function getRule(): Rule
2123
{
22-
return new NullCoalesceRule(new IssetCheck(
23-
new PropertyDescriptor(),
24-
new PropertyReflectionFinder(),
25-
self::getContainer()->getByType(ConstantConditionInTraitHelper::class),
26-
true,
27-
$this->shouldTreatPhpDocTypesAsCertain(),
28-
));
24+
// @phpstan-ignore argument.type
25+
return new CompositeRule([
26+
new NullCoalesceRule(new IssetCheck(
27+
new PropertyDescriptor(),
28+
new PropertyReflectionFinder(),
29+
self::getContainer()->getByType(ConstantConditionInTraitHelper::class),
30+
true,
31+
$this->shouldTreatPhpDocTypesAsCertain(),
32+
)),
33+
new ConstantConditionInTraitRule(),
34+
]);
2935
}
3036

3137
public function testCoalesceRule(): void
@@ -450,4 +456,18 @@ public function testBug14393(): void
450456
]);
451457
}
452458

459+
public function testInTrait(): void
460+
{
461+
$this->analyse([__DIR__ . '/data/isset-in-trait.php'], [
462+
[
463+
'Property IssetInTrait\ClassA::$j (int) on left side of ?? is not nullable.',
464+
35,
465+
],
466+
[
467+
'Property IssetInTrait\ClassB::$j (int) on left side of ?? is not nullable.',
468+
35,
469+
],
470+
]);
471+
}
472+
453473
}

0 commit comments

Comments
 (0)