Skip to content

Commit a2834bc

Browse files
ondrejmirtesondrejmirtes
committed
RuleTestCase and TypeInferenceTestCase looking for wrongly configured autoloading - show the hint only if there are other failures
1 parent e5b2baf commit a2834bc

File tree

6 files changed

+200
-35
lines changed

6 files changed

+200
-35
lines changed

Diff for: ‎src/Testing/DelayedRule.php

+57
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\Testing;
4+
5+
use PhpParser\Node;
6+
use PHPStan\Analyser\Scope;
7+
use PHPStan\Rules\DirectRegistry;
8+
use PHPStan\Rules\IdentifierRuleError;
9+
use PHPStan\Rules\Registry;
10+
use PHPStan\Rules\Rule;
11+
use function get_class;
12+
13+
/**
14+
* @implements Rule<Node>
15+
*/
16+
final class DelayedRule implements Rule
17+
{
18+
19+
private Registry $registry;
20+
21+
/** @var list<IdentifierRuleError> */
22+
private array $errors = [];
23+
24+
/**
25+
* @param Rule<covariant Node> $rule
26+
*/
27+
public function __construct(Rule $rule)
28+
{
29+
$this->registry = new DirectRegistry([$rule]);
30+
}
31+
32+
public function getNodeType(): string
33+
{
34+
return Node::class;
35+
}
36+
37+
/**
38+
* @return list<IdentifierRuleError>
39+
*/
40+
public function getDelayedErrors(): array
41+
{
42+
return $this->errors;
43+
}
44+
45+
public function processNode(Node $node, Scope $scope): array
46+
{
47+
$nodeType = get_class($node);
48+
foreach ($this->registry->getRules($nodeType) as $rule) {
49+
foreach ($rule->processNode($node, $scope) as $error) {
50+
$this->errors[] = $error;
51+
}
52+
}
53+
54+
return [];
55+
}
56+
57+
}

Diff for: ‎src/Testing/RuleTestCase.php

+44-5
Original file line numberDiff line numberDiff line change
@@ -27,12 +27,14 @@
2727
use PHPStan\Reflection\InitializerExprTypeResolver;
2828
use PHPStan\Reflection\SignatureMap\SignatureMapProvider;
2929
use PHPStan\Rules\DirectRegistry as DirectRuleRegistry;
30+
use PHPStan\Rules\IdentifierRuleError;
3031
use PHPStan\Rules\Properties\DirectReadWritePropertiesExtensionProvider;
3132
use PHPStan\Rules\Properties\ReadWritePropertiesExtension;
3233
use PHPStan\Rules\Properties\ReadWritePropertiesExtensionProvider;
3334
use PHPStan\Rules\Rule;
3435
use PHPStan\Type\FileTypeMapper;
3536
use function array_map;
37+
use function array_merge;
3638
use function count;
3739
use function implode;
3840
use function sprintf;
@@ -136,7 +138,7 @@ private function getAnalyser(DirectRuleRegistry $ruleRegistry): Analyser
136138
*/
137139
public function analyse(array $files, array $expectedErrors): void
138140
{
139-
$actualErrors = $this->gatherAnalyserErrors($files);
141+
[$actualErrors, $delayedErrors] = $this->gatherAnalyserErrorsWithDelayedErrors($files);
140142
$strictlyTypedSprintf = static function (int $line, string $message, ?string $tip): string {
141143
$message = sprintf('%02d: %s', $line, $message);
142144
if ($tip !== null) {
@@ -162,20 +164,54 @@ static function (Error $error) use ($strictlyTypedSprintf): string {
162164
$actualErrors,
163165
);
164166

165-
$this->assertSame(implode("\n", $expectedErrors) . "\n", implode("\n", $actualErrors) . "\n");
167+
$expectedErrorsString = implode("\n", $expectedErrors) . "\n";
168+
$actualErrorsString = implode("\n", $actualErrors) . "\n";
169+
170+
if (count($delayedErrors) === 0) {
171+
$this->assertSame($expectedErrorsString, $actualErrorsString);
172+
return;
173+
}
174+
175+
if ($expectedErrorsString === $actualErrorsString) {
176+
$this->assertSame($expectedErrorsString, $actualErrorsString);
177+
return;
178+
}
179+
180+
$actualErrorsString .= sprintf(
181+
"\n%s might be reported because of the following misconfiguration %s:\n\n",
182+
count($actualErrors) === 1 ? 'This error' : 'These errors',
183+
count($delayedErrors) === 1 ? 'issue' : 'issues',
184+
);
185+
186+
foreach ($delayedErrors as $delayedError) {
187+
$actualErrorsString .= sprintf("* %s\n", $delayedError->getMessage());
188+
}
189+
190+
$this->assertSame($expectedErrorsString, $actualErrorsString);
166191
}
167192

168193
/**
169194
* @param string[] $files
170195
* @return list<Error>
171196
*/
172197
public function gatherAnalyserErrors(array $files): array
198+
{
199+
return $this->gatherAnalyserErrorsWithDelayedErrors($files)[0];
200+
}
201+
202+
/**
203+
* @param string[] $files
204+
* @return array{list<Error>, list<IdentifierRuleError>}
205+
*/
206+
private function gatherAnalyserErrorsWithDelayedErrors(array $files): array
173207
{
174208
$reflectionProvider = $this->createReflectionProvider();
209+
$classRule = new DelayedRule(new NonexistentAnalysedClassRule($reflectionProvider));
210+
$traitRule = new DelayedRule(new NonexistentAnalysedTraitRule($reflectionProvider));
175211
$ruleRegistry = new DirectRuleRegistry([
176212
$this->getRule(),
177-
new NonexistentAnalysedClassRule($reflectionProvider),
178-
new NonexistentAnalysedTraitRule($reflectionProvider),
213+
$classRule,
214+
$traitRule,
179215
]);
180216
$files = array_map([$this->getFileHelper(), 'normalizePath'], $files);
181217
$analyserResult = $this->getAnalyser($ruleRegistry)->analyse(
@@ -204,7 +240,10 @@ public function gatherAnalyserErrors(array $files): array
204240
true,
205241
);
206242

207-
return $finalizer->finalize($analyserResult, false, true)->getAnalyserResult()->getUnorderedErrors();
243+
return [
244+
$finalizer->finalize($analyserResult, false, true)->getAnalyserResult()->getUnorderedErrors(),
245+
array_merge($classRule->getDelayedErrors(), $traitRule->getDelayedErrors()),
246+
];
208247
}
209248

210249
protected function shouldPolluteScopeWithLoopInitialAssignments(): bool

Diff for: ‎src/Testing/TypeInferenceTestCase.php

+45-19
Original file line numberDiff line numberDiff line change
@@ -126,21 +126,45 @@ public function assertFileAsserts(
126126
$actual = $args[1];
127127
}
128128

129+
$failureMessage = sprintf('Expected type %s, got type %s in %s on line %d.', $expected, $actual, $file, $args[2]);
130+
131+
$delayedErrors = $args[3] ?? [];
132+
if (count($delayedErrors) > 0) {
133+
$failureMessage .= sprintf(
134+
"\n\nThis failure might be reported because of the following misconfiguration %s:\n\n",
135+
count($delayedErrors) === 1 ? 'issue' : 'issues',
136+
);
137+
foreach ($delayedErrors as $delayedError) {
138+
$failureMessage .= sprintf("* %s\n", $delayedError);
139+
}
140+
}
141+
129142
$this->assertSame(
130143
$expected,
131144
$actual,
132-
sprintf('Expected type %s, got type %s in %s on line %d.', $expected, $actual, $file, $args[2]),
145+
$failureMessage,
133146
);
134147
} elseif ($assertType === 'variableCertainty') {
135148
$expectedCertainty = $args[0];
136149
$actualCertainty = $args[1];
137150
$variableName = $args[2];
151+
152+
$failureMessage = sprintf('Expected %s, actual certainty of %s is %s in %s on line %d.', $expectedCertainty->describe(), $variableName, $actualCertainty->describe(), $file, $args[3]);
153+
$delayedErrors = $args[4] ?? [];
154+
if (count($delayedErrors) > 0) {
155+
$failureMessage .= sprintf(
156+
"\n\nThis failure might be reported because of the following misconfiguration %s:\n\n",
157+
count($delayedErrors) === 1 ? 'issue' : 'issues',
158+
);
159+
foreach ($delayedErrors as $delayedError) {
160+
$failureMessage .= sprintf("* %s\n", $delayedError);
161+
}
162+
}
163+
138164
$this->assertTrue(
139165
$expectedCertainty->equals($actualCertainty),
140-
sprintf('Expected %s, actual certainty of %s is %s in %s on line %d.', $expectedCertainty->describe(), $variableName, $actualCertainty->describe(), $file, $args[3]),
166+
$failureMessage,
141167
);
142-
} elseif ($assertType === 'error') {
143-
$this->fail($args[0]);
144168
}
145169
}
146170

@@ -158,30 +182,23 @@ public static function gatherAssertTypes(string $file): array
158182
$file = $fileHelper->normalizePath($file);
159183

160184
$asserts = [];
161-
self::processFile($file, static function (Node $node, Scope $scope) use (&$asserts, $file, $relativePathHelper, $reflectionProvider): void {
185+
$delayedErrors = [];
186+
self::processFile($file, static function (Node $node, Scope $scope) use (&$asserts, &$delayedErrors, $file, $relativePathHelper, $reflectionProvider): void {
162187
if ($node instanceof InClassNode) {
163188
if (!$reflectionProvider->hasClass($node->getClassReflection()->getName())) {
164-
$asserts[$file . ':' . $node->getStartLine()] = [
165-
'error',
189+
$delayedErrors[] = sprintf(
190+
'%s %s in %s not found in ReflectionProvider. Configure "autoload-dev" section in composer.json to include your tests directory.',
191+
$node->getClassReflection()->getClassTypeDescription(),
192+
$node->getClassReflection()->getName(),
166193
$file,
167-
sprintf(
168-
'%s %s in %s not found in ReflectionProvider. Configure "autoload-dev" section in composer.json to include your tests directory.',
169-
$node->getClassReflection()->getClassTypeDescription(),
170-
$node->getClassReflection()->getName(),
171-
$file,
172-
),
173-
];
194+
);
174195
}
175196
} elseif ($node instanceof Node\Stmt\Trait_) {
176197
if ($node->namespacedName === null) {
177198
throw new ShouldNotHappenException();
178199
}
179200
if (!$reflectionProvider->hasClass($node->namespacedName->toString())) {
180-
$asserts[$file . ':' . $node->getStartLine()] = [
181-
'error',
182-
$file,
183-
sprintf('Trait %s not found in ReflectionProvider. Configure "autoload-dev" section in composer.json to include your tests directory.', $node->namespacedName->toString()),
184-
];
201+
$delayedErrors[] = sprintf('Trait %s not found in ReflectionProvider. Configure "autoload-dev" section in composer.json to include your tests directory.', $node->namespacedName->toString());
185202
}
186203
}
187204
if (!$node instanceof Node\Expr\FuncCall) {
@@ -303,6 +320,15 @@ public static function gatherAssertTypes(string $file): array
303320
self::fail(sprintf('File %s does not contain any asserts', $file));
304321
}
305322

323+
if (count($delayedErrors) === 0) {
324+
return $asserts;
325+
}
326+
327+
foreach ($asserts as $i => $assert) {
328+
$assert[] = $delayedErrors;
329+
$asserts[$i] = $assert;
330+
}
331+
306332
return $asserts;
307333
}
308334

Diff for: ‎tests/PHPStan/Testing/NonexistentAnalysedClassRuleTest.php

+25-10
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66
use PhpParser\Node\Expr\FuncCall;
77
use PHPStan\Analyser\Scope;
88
use PHPStan\Rules\Rule;
9+
use PHPStan\Rules\RuleErrorBuilder;
10+
use PHPUnit\Framework\ExpectationFailedException;
911

1012
/**
1113
* @extends RuleTestCase<Rule<FuncCall>>
@@ -24,6 +26,15 @@ public function getNodeType(): string
2426

2527
public function processNode(Node $node, Scope $scope): array
2628
{
29+
if ($node->name instanceof Node\Name && $node->name->toString() === 'error') {
30+
return [
31+
RuleErrorBuilder::message('Error call')
32+
->identifier('test.errorCall')
33+
->nonIgnorable()
34+
->build(),
35+
];
36+
}
37+
2738
return [];
2839
}
2940

@@ -32,16 +43,20 @@ public function processNode(Node $node, Scope $scope): array
3243

3344
public function testRule(): void
3445
{
35-
$this->analyse([__DIR__ . '/../../notAutoloaded/nonexistentClasses.php'], [
36-
[
37-
'Class NamespaceForNonexistentClasses\Foo not found in ReflectionProvider. Configure "autoload-dev" section in composer.json to include your tests directory.',
38-
7,
39-
],
40-
[
41-
'Trait NamespaceForNonexistentClasses\FooTrait not found in ReflectionProvider. Configure "autoload-dev" section in composer.json to include your tests directory.',
42-
17,
43-
],
44-
]);
46+
$this->analyse([__DIR__ . '/../../notAutoloaded/nonexistentClasses.php'], []);
47+
}
48+
49+
public function testRuleWithError(): void
50+
{
51+
try {
52+
$this->analyse([__DIR__ . '/../../notAutoloaded/nonexistentClasses-error.php'], []);
53+
$this->fail('Should have failed');
54+
} catch (ExpectationFailedException $e) {
55+
if ($e->getComparisonFailure() === null) {
56+
throw $e;
57+
}
58+
$this->assertStringContainsString('not found in ReflectionProvider', $e->getComparisonFailure()->getDiff());
59+
}
4560
}
4661

4762
}

Diff for: ‎tests/PHPStan/Testing/TypeInferenceTestCaseTest.php

+8-1
Original file line numberDiff line numberDiff line change
@@ -102,9 +102,16 @@ public function testVariableOrOffsetDescription(): void
102102
}
103103

104104
public function testNonexistentClassInAnalysedFile(): void
105+
{
106+
foreach ($this->gatherAssertTypes(__DIR__ . '/../../notAutoloaded/nonexistentClasses.php') as $data) {
107+
$this->assertFileAsserts(...$data);
108+
}
109+
}
110+
111+
public function testNonexistentClassInAnalysedFileWithError(): void
105112
{
106113
try {
107-
foreach ($this->gatherAssertTypes(__DIR__ . '/../../notAutoloaded/nonexistentClasses.php') as $data) {
114+
foreach ($this->gatherAssertTypes(__DIR__ . '/../../notAutoloaded/nonexistentClasses-error.php') as $data) {
108115
$this->assertFileAsserts(...$data);
109116
}
110117

Diff for: ‎tests/notAutoloaded/nonexistentClasses-error.php

+21
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace NamespaceForNonexistentClassesError;
4+
5+
use function PHPStan\Testing\assertType;
6+
7+
class Foo
8+
{
9+
10+
public function doFoo(): void
11+
{
12+
assertType('string', 1);
13+
error();
14+
}
15+
16+
}
17+
18+
trait FooTrait
19+
{
20+
21+
}

0 commit comments

Comments
 (0)