Skip to content

Commit ca0a7e9

Browse files
ondrejmirtesondrejmirtes
committed
Bleeding edge - added absent type checks to AssertRuleHelper
1 parent dbe8b74 commit ca0a7e9

File tree

7 files changed

+376
-22
lines changed

7 files changed

+376
-22
lines changed

‎conf/config.neon

+4
Original file line numberDiff line numberDiff line change
@@ -1083,6 +1083,10 @@ services:
10831083

10841084
-
10851085
class: PHPStan\Rules\PhpDoc\AssertRuleHelper
1086+
arguments:
1087+
checkMissingTypehints: %checkMissingTypehints%
1088+
checkClassCaseSensitivity: %checkClassCaseSensitivity%
1089+
absentTypeChecks: %featureToggles.absentTypeChecks%
10861090

10871091
-
10881092
class: PHPStan\Rules\PhpDoc\UnresolvableTypeHelper

‎src/Rules/PhpDoc/AssertRuleHelper.php

+142-18
Original file line numberDiff line numberDiff line change
@@ -2,39 +2,63 @@
22

33
namespace PHPStan\Rules\PhpDoc;
44

5+
use PhpParser\Node\Stmt\ClassMethod;
6+
use PhpParser\Node\Stmt\Function_;
57
use PHPStan\Node\Expr\TypeExpr;
8+
use PHPStan\PhpDoc\Tag\AssertTag;
69
use PHPStan\Reflection\ExtendedMethodReflection;
710
use PHPStan\Reflection\FunctionReflection;
811
use PHPStan\Reflection\InitializerExprContext;
912
use PHPStan\Reflection\InitializerExprTypeResolver;
1013
use PHPStan\Reflection\ParametersAcceptor;
14+
use PHPStan\Reflection\ReflectionProvider;
15+
use PHPStan\Rules\ClassNameCheck;
16+
use PHPStan\Rules\ClassNameNodePair;
17+
use PHPStan\Rules\Generics\GenericObjectTypeCheck;
1118
use PHPStan\Rules\IdentifierRuleError;
19+
use PHPStan\Rules\MissingTypehintCheck;
1220
use PHPStan\Rules\RuleErrorBuilder;
1321
use PHPStan\Type\ErrorType;
1422
use PHPStan\Type\ObjectType;
1523
use PHPStan\Type\VerbosityLevel;
1624
use function array_key_exists;
25+
use function array_merge;
26+
use function implode;
1727
use function sprintf;
1828
use function substr;
1929

2030
final class AssertRuleHelper
2131
{
2232

23-
public function __construct(private InitializerExprTypeResolver $initializerExprTypeResolver)
33+
public function __construct(
34+
private InitializerExprTypeResolver $initializerExprTypeResolver,
35+
private ReflectionProvider $reflectionProvider,
36+
private UnresolvableTypeHelper $unresolvableTypeHelper,
37+
private ClassNameCheck $classCheck,
38+
private MissingTypehintCheck $missingTypehintCheck,
39+
private GenericObjectTypeCheck $genericObjectTypeCheck,
40+
private bool $absentTypeChecks,
41+
private bool $checkClassCaseSensitivity,
42+
private bool $checkMissingTypehints,
43+
)
2444
{
2545
}
2646

2747
/**
2848
* @return list<IdentifierRuleError>
2949
*/
30-
public function check(ExtendedMethodReflection|FunctionReflection $reflection, ParametersAcceptor $acceptor): array
50+
public function check(
51+
Function_|ClassMethod $node,
52+
ExtendedMethodReflection|FunctionReflection $reflection,
53+
ParametersAcceptor $acceptor,
54+
): array
3155
{
3256
$parametersByName = [];
3357
foreach ($acceptor->getParameters() as $parameter) {
3458
$parametersByName[$parameter->getName()] = $parameter->getType();
3559
}
3660

37-
if ($reflection instanceof ExtendedMethodReflection) {
61+
if ($reflection instanceof ExtendedMethodReflection && !$reflection->isStatic()) {
3862
$class = $reflection->getDeclaringClass();
3963
$parametersByName['this'] = new ObjectType($class->getName(), null, $class);
4064
}
@@ -57,38 +81,138 @@ public function check(ExtendedMethodReflection|FunctionReflection $reflection, P
5781

5882
$assertedExpr = $assert->getParameter()->getExpr(new TypeExpr($parametersByName[$parameterName]));
5983
$assertedExprType = $this->initializerExprTypeResolver->getType($assertedExpr, $context);
84+
$assertedExprString = $assert->getParameter()->describe();
6085
if ($assertedExprType instanceof ErrorType) {
86+
if ($this->absentTypeChecks) {
87+
$errors[] = RuleErrorBuilder::message(sprintf('Assert references unknown %s.', $assertedExprString))
88+
->identifier('assert.unknownExpr')
89+
->build();
90+
}
6191
continue;
6292
}
6393

6494
$assertedType = $assert->getType();
6595

96+
$tagName = [
97+
AssertTag::NULL => '@phpstan-assert',
98+
AssertTag::IF_TRUE => '@phpstan-assert-if-true',
99+
AssertTag::IF_FALSE => '@phpstan-assert-if-false',
100+
][$assert->getIf()];
101+
102+
if ($this->absentTypeChecks) {
103+
if ($this->unresolvableTypeHelper->containsUnresolvableType($assertedType)) {
104+
$errors[] = RuleErrorBuilder::message(sprintf(
105+
'PHPDoc tag %s for %s contains unresolvable type.',
106+
$tagName,
107+
$assertedExprString,
108+
))->identifier('assert.unresolvableType')->build();
109+
continue;
110+
}
111+
}
112+
66113
$isSuperType = $assertedType->isSuperTypeOf($assertedExprType);
67-
if ($isSuperType->maybe()) {
114+
if (!$isSuperType->maybe()) {
115+
if ($assert->isNegated() ? $isSuperType->yes() : $isSuperType->no()) {
116+
$errors[] = RuleErrorBuilder::message(sprintf(
117+
'Asserted %stype %s for %s with type %s can never happen.',
118+
$assert->isNegated() ? 'negated ' : '',
119+
$assertedType->describe(VerbosityLevel::precise()),
120+
$assertedExprString,
121+
$assertedExprType->describe(VerbosityLevel::precise()),
122+
))->identifier('assert.impossibleType')->build();
123+
} elseif ($assert->isNegated() ? $isSuperType->no() : $isSuperType->yes()) {
124+
$errors[] = RuleErrorBuilder::message(sprintf(
125+
'Asserted %stype %s for %s with type %s does not narrow down the type.',
126+
$assert->isNegated() ? 'negated ' : '',
127+
$assertedType->describe(VerbosityLevel::precise()),
128+
$assertedExprString,
129+
$assertedExprType->describe(VerbosityLevel::precise()),
130+
))->identifier('assert.alreadyNarrowedType')->build();
131+
}
132+
}
133+
134+
if (!$this->absentTypeChecks) {
68135
continue;
69136
}
70137

71-
$assertedExprString = $assert->getParameter()->describe();
138+
foreach ($assertedType->getReferencedClasses() as $class) {
139+
if (!$this->reflectionProvider->hasClass($class)) {
140+
$errors[] = RuleErrorBuilder::message(sprintf(
141+
'PHPDoc tag %s for %s contains unknown class %s.',
142+
$tagName,
143+
$assertedExprString,
144+
$class,
145+
))->identifier('class.notFound')->build();
146+
continue;
147+
}
148+
149+
$classReflection = $this->reflectionProvider->getClass($class);
150+
if ($classReflection->isTrait()) {
151+
$errors[] = RuleErrorBuilder::message(sprintf(
152+
'PHPDoc tag %s for %s contains invalid type %s.',
153+
$tagName,
154+
$assertedExprString,
155+
$class,
156+
))->identifier('assert.trait')->build();
157+
continue;
158+
}
159+
160+
$errors = array_merge(
161+
$errors,
162+
$this->classCheck->checkClassNames([
163+
new ClassNameNodePair($class, $node),
164+
], $this->checkClassCaseSensitivity),
165+
);
166+
}
167+
168+
$errors = array_merge($errors, $this->genericObjectTypeCheck->check(
169+
$assertedType,
170+
sprintf('PHPDoc tag %s for %s contains generic type %%s but %%s %%s is not generic.', $tagName, $assertedExprString),
171+
sprintf('Generic type %%s in PHPDoc tag %s for %s does not specify all template types of %%s %%s: %%s', $tagName, $assertedExprString),
172+
sprintf('Generic type %%s in PHPDoc tag %s for %s specifies %%d template types, but %%s %%s supports only %%d: %%s', $tagName, $assertedExprString),
173+
sprintf('Type %%s in generic type %%s in PHPDoc tag %s for %s is not subtype of template type %%s of %%s %%s.', $tagName, $assertedExprString),
174+
sprintf('Call-site variance of %%s in generic type %%s in PHPDoc tag %s for %s is in conflict with %%s template type %%s of %%s %%s.', $tagName, $assertedExprString),
175+
sprintf('Call-site variance of %%s in generic type %%s in PHPDoc tag %s for %s is redundant, template type %%s of %%s %%s has the same variance.', $tagName, $assertedExprString),
176+
));
177+
178+
if (!$this->checkMissingTypehints) {
179+
continue;
180+
}
72181

73-
if ($assert->isNegated() ? $isSuperType->yes() : $isSuperType->no()) {
182+
foreach ($this->missingTypehintCheck->getIterableTypesWithMissingValueTypehint($assertedType) as $iterableType) {
183+
$iterableTypeDescription = $iterableType->describe(VerbosityLevel::typeOnly());
74184
$errors[] = RuleErrorBuilder::message(sprintf(
75-
'Asserted %stype %s for %s with type %s can never happen.',
76-
$assert->isNegated() ? 'negated ' : '',
77-
$assertedType->describe(VerbosityLevel::precise()),
185+
'PHPDoc tag %s for %s has no value type specified in iterable type %s.',
186+
$tagName,
78187
$assertedExprString,
79-
$assertedExprType->describe(VerbosityLevel::precise()),
80-
))->identifier('assert.impossibleType')->build();
81-
} elseif ($assert->isNegated() ? $isSuperType->no() : $isSuperType->yes()) {
188+
$iterableTypeDescription,
189+
))
190+
->tip(MissingTypehintCheck::MISSING_ITERABLE_VALUE_TYPE_TIP)
191+
->identifier('missingType.iterableValue')
192+
->build();
193+
}
194+
195+
foreach ($this->missingTypehintCheck->getNonGenericObjectTypesWithGenericClass($assertedType) as [$innerName, $genericTypeNames]) {
82196
$errors[] = RuleErrorBuilder::message(sprintf(
83-
'Asserted %stype %s for %s with type %s does not narrow down the type.',
84-
$assert->isNegated() ? 'negated ' : '',
85-
$assertedType->describe(VerbosityLevel::precise()),
197+
'PHPDoc tag %s for %s contains generic %s but does not specify its types: %s',
198+
$tagName,
86199
$assertedExprString,
87-
$assertedExprType->describe(VerbosityLevel::precise()),
88-
))->identifier('assert.alreadyNarrowedType')->build();
200+
$innerName,
201+
implode(', ', $genericTypeNames),
202+
))
203+
->identifier('missingType.generics')
204+
->build();
89205
}
90-
}
91206

207+
foreach ($this->missingTypehintCheck->getCallablesWithMissingSignature($assertedType) as $callableType) {
208+
$errors[] = RuleErrorBuilder::message(sprintf(
209+
'PHPDoc tag %s for %s has no signature specified for %s.',
210+
$tagName,
211+
$assertedExprString,
212+
$callableType->describe(VerbosityLevel::typeOnly()),
213+
))->identifier('missingType.callable')->build();
214+
}
215+
}
92216
return $errors;
93217
}
94218

‎src/Rules/PhpDoc/FunctionAssertRule.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ public function processNode(Node $node, Scope $scope): array
3131
return [];
3232
}
3333

34-
return $this->helper->check($function, $variants[0]);
34+
return $this->helper->check($node->getOriginalNode(), $function, $variants[0]);
3535
}
3636

3737
}

‎src/Rules/PhpDoc/MethodAssertRule.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ public function processNode(Node $node, Scope $scope): array
3131
return [];
3232
}
3333

34-
return $this->helper->check($method, $variants[0]);
34+
return $this->helper->check($node->getOriginalNode(), $method, $variants[0]);
3535
}
3636

3737
}

‎tests/PHPStan/Rules/PhpDoc/FunctionAssertRuleTest.php

+17-1
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,11 @@
33
namespace PHPStan\Rules\PhpDoc;
44

55
use PHPStan\Reflection\InitializerExprTypeResolver;
6+
use PHPStan\Rules\ClassCaseSensitivityCheck;
7+
use PHPStan\Rules\ClassForbiddenNameCheck;
8+
use PHPStan\Rules\ClassNameCheck;
9+
use PHPStan\Rules\Generics\GenericObjectTypeCheck;
10+
use PHPStan\Rules\MissingTypehintCheck;
611
use PHPStan\Rules\Rule;
712
use PHPStan\Testing\RuleTestCase;
813

@@ -15,7 +20,18 @@ class FunctionAssertRuleTest extends RuleTestCase
1520
protected function getRule(): Rule
1621
{
1722
$initializerExprTypeResolver = self::getContainer()->getByType(InitializerExprTypeResolver::class);
18-
return new FunctionAssertRule(new AssertRuleHelper($initializerExprTypeResolver));
23+
$reflectionProvider = $this->createReflectionProvider();
24+
return new FunctionAssertRule(new AssertRuleHelper(
25+
$initializerExprTypeResolver,
26+
$reflectionProvider,
27+
new UnresolvableTypeHelper(),
28+
new ClassNameCheck(new ClassCaseSensitivityCheck($reflectionProvider, true), new ClassForbiddenNameCheck(self::getContainer())),
29+
new MissingTypehintCheck(true, true, true, true, []),
30+
new GenericObjectTypeCheck(),
31+
true,
32+
true,
33+
true,
34+
));
1935
}
2036

2137
public function testRule(): void

‎tests/PHPStan/Rules/PhpDoc/MethodAssertRuleTest.php

+78-1
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,11 @@
33
namespace PHPStan\Rules\PhpDoc;
44

55
use PHPStan\Reflection\InitializerExprTypeResolver;
6+
use PHPStan\Rules\ClassCaseSensitivityCheck;
7+
use PHPStan\Rules\ClassForbiddenNameCheck;
8+
use PHPStan\Rules\ClassNameCheck;
9+
use PHPStan\Rules\Generics\GenericObjectTypeCheck;
10+
use PHPStan\Rules\MissingTypehintCheck;
611
use PHPStan\Rules\Rule;
712
use PHPStan\Testing\RuleTestCase;
813

@@ -15,7 +20,18 @@ class MethodAssertRuleTest extends RuleTestCase
1520
protected function getRule(): Rule
1621
{
1722
$initializerExprTypeResolver = self::getContainer()->getByType(InitializerExprTypeResolver::class);
18-
return new MethodAssertRule(new AssertRuleHelper($initializerExprTypeResolver));
23+
$reflectionProvider = $this->createReflectionProvider();
24+
return new MethodAssertRule(new AssertRuleHelper(
25+
$initializerExprTypeResolver,
26+
$reflectionProvider,
27+
new UnresolvableTypeHelper(),
28+
new ClassNameCheck(new ClassCaseSensitivityCheck($reflectionProvider, true), new ClassForbiddenNameCheck(self::getContainer())),
29+
new MissingTypehintCheck(true, true, true, true, []),
30+
new GenericObjectTypeCheck(),
31+
true,
32+
true,
33+
true,
34+
));
1935
}
2036

2137
public function testRule(): void
@@ -54,6 +70,67 @@ public function testRule(): void
5470
'Asserted negated type string for $i with type int does not narrow down the type.',
5571
72,
5672
],
73+
[
74+
'PHPDoc tag @phpstan-assert for $this->fooProp contains unresolvable type.',
75+
94,
76+
],
77+
[
78+
'PHPDoc tag @phpstan-assert-if-true for $a contains unresolvable type.',
79+
94,
80+
],
81+
[
82+
'PHPDoc tag @phpstan-assert for $a contains unknown class MethodAssert\Nonexistent.',
83+
105,
84+
],
85+
[
86+
'PHPDoc tag @phpstan-assert for $b contains invalid type MethodAssert\FooTrait.',
87+
105,
88+
],
89+
[
90+
'Class MethodAssert\Foo referenced with incorrect case: MethodAssert\fOO.',
91+
105,
92+
],
93+
[
94+
'Assert references unknown $this->barProp.',
95+
105,
96+
],
97+
[
98+
'Assert references unknown parameter $this.',
99+
113,
100+
],
101+
[
102+
'PHPDoc tag @phpstan-assert for $m contains generic type Exception<int, float> but class Exception is not generic.',
103+
131,
104+
],
105+
[
106+
'Generic type MethodAssert\FooBar<mixed> in PHPDoc tag @phpstan-assert for $m does not specify all template types of class MethodAssert\FooBar: T, TT',
107+
138,
108+
],
109+
[
110+
'Type mixed in generic type MethodAssert\FooBar<mixed> in PHPDoc tag @phpstan-assert for $m is not subtype of template type T of int of class MethodAssert\FooBar.',
111+
138,
112+
],
113+
[
114+
'Generic type MethodAssert\FooBar<int> in PHPDoc tag @phpstan-assert for $m does not specify all template types of class MethodAssert\FooBar: T, TT',
115+
145,
116+
],
117+
[
118+
'Generic type MethodAssert\FooBar<int, string, float> in PHPDoc tag @phpstan-assert for $m specifies 3 template types, but class MethodAssert\FooBar supports only 2: T, TT',
119+
152,
120+
],
121+
[
122+
'PHPDoc tag @phpstan-assert for $m has no value type specified in iterable type array.',
123+
194,
124+
'See: https://phpstan.org/blog/solving-phpstan-no-value-type-specified-in-iterable-type',
125+
],
126+
[
127+
'PHPDoc tag @phpstan-assert for $m contains generic class MethodAssert\FooBar but does not specify its types: T, TT',
128+
202,
129+
],
130+
[
131+
'PHPDoc tag @phpstan-assert for $m has no signature specified for callable.',
132+
210,
133+
],
57134
]);
58135
}
59136

0 commit comments

Comments
 (0)