Skip to content

Commit 9058c27

Browse files
committed
refactored rule to use reflection provider to clear up whitelist of built in types
1 parent 25e1155 commit 9058c27

File tree

2 files changed

+76
-61
lines changed

2 files changed

+76
-61
lines changed

rules/RequireAbstractionInDependenciesRule.php

Lines changed: 73 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -11,93 +11,106 @@
1111
use PhpParser\Node;
1212
use PhpParser\Node\Expr\Error;
1313
use PHPStan\Analyser\Scope;
14+
use PHPStan\Reflection\ReflectionProvider;
1415
use PHPStan\Rules\Rule;
16+
use PHPStan\Rules\RuleError;
1517
use PHPStan\Rules\RuleErrorBuilder;
1618

1719
/**
1820
* @implements \PHPStan\Rules\Rule<\PhpParser\Node\Stmt\ClassMethod>
1921
*/
2022
final class RequireAbstractionInDependenciesRule implements Rule
2123
{
24+
private ReflectionProvider $reflectionProvider;
25+
26+
public function __construct(
27+
ReflectionProvider $reflectionProvider
28+
) {
29+
$this->reflectionProvider = $reflectionProvider;
30+
}
31+
2232
public function getNodeType(): string
2333
{
2434
return Node\Stmt\ClassMethod::class;
2535
}
2636

2737
public function processNode(Node $node, Scope $scope): array
2838
{
29-
$errors = [];
30-
3139
if (!$node->params) {
3240
return [];
3341
}
3442

35-
foreach ($node->params as $param) {
36-
if (!$param->type instanceof Node\Name) {
37-
continue;
38-
}
39-
if ($param->var instanceof Error) {
40-
continue;
41-
}
42-
$typeName = $param->type->toString();
43-
44-
// Skip built-in types and primitives
45-
if ($this->isBuiltInType($typeName)) {
46-
continue;
47-
}
48-
49-
// Skip interfaces - they are always acceptable
50-
if (interface_exists($typeName)) {
51-
continue;
52-
}
43+
$errors = [];
5344

54-
// Check if it's a class
55-
if (class_exists($typeName)) {
56-
$reflection = new \ReflectionClass($typeName);
57-
58-
// Skip abstract classes - they are acceptable
59-
if ($reflection->isAbstract()) {
60-
continue;
61-
}
62-
63-
// This is a concrete class - check if it has interfaces or extends an abstract class
64-
$interfaces = class_implements($typeName);
65-
$parentClass = $reflection->getParentClass();
66-
$hasAbstractParent = $parentClass && $parentClass->isAbstract();
67-
68-
if (!empty($interfaces) || $hasAbstractParent) {
69-
$suggestions = [];
70-
71-
if (!empty($interfaces)) {
72-
$suggestions[] = 'Available interfaces: ' . implode(', ', $interfaces);
73-
}
74-
75-
if ($hasAbstractParent) {
76-
$suggestions[] = 'Abstract parent: ' . $parentClass->getName();
77-
}
78-
79-
$errors[] = RuleErrorBuilder::message(
80-
sprintf(
81-
'Parameter $%s uses concrete class %s instead of an interface or abstract class. %s',
82-
is_string($param->var->name) ? $param->var->name : $param->var->name->getType(),
83-
$typeName,
84-
implode('. ', $suggestions)
85-
)
86-
)->build();
87-
}
45+
foreach ($node->params as $param) {
46+
$error = $this->validateParameter($param);
47+
if ($error !== null) {
48+
$errors[] = $error;
8849
}
8950
}
9051

9152
return $errors;
9253
}
9354

94-
private function isBuiltInType(string $type): bool
55+
private function validateParameter(Node\Param $param): ?RuleError
9556
{
96-
$builtInTypes = [
97-
'string', 'int', 'float', 'bool', 'array', 'object',
98-
'callable', 'iterable', 'mixed', 'void', 'never',
99-
];
57+
if (!$param->type instanceof Node\Name) {
58+
return null;
59+
}
60+
61+
if ($param->var instanceof Error) {
62+
return null;
63+
}
64+
65+
$typeName = $param->type->toString();
66+
67+
// Skip if the type doesn't exist in reflection
68+
if (!$this->reflectionProvider->hasClass($typeName)) {
69+
return null;
70+
}
71+
72+
$classReflection = $this->reflectionProvider->getClass($typeName);
73+
74+
// Skip interfaces - they are always acceptable
75+
if ($classReflection->isInterface()) {
76+
return null;
77+
}
78+
79+
// Skip abstract classes - they are acceptable
80+
if ($classReflection->isAbstract()) {
81+
return null;
82+
}
83+
84+
$reflection = $classReflection->getNativeReflection();
85+
86+
// This is a concrete class - check if it has interfaces or extends an abstract class
87+
$interfaces = class_implements($typeName);
88+
$parentClass = $reflection->getParentClass();
89+
$hasAbstractParent = $parentClass && $parentClass->isAbstract();
90+
91+
// If there are no interfaces and no abstract parent, it's acceptable (no violation)
92+
if (empty($interfaces) && !$hasAbstractParent) {
93+
return null;
94+
}
95+
96+
// Build error with suggestions
97+
$suggestions = [];
98+
99+
if (!empty($interfaces)) {
100+
$suggestions[] = 'Available interfaces: ' . implode(', ', $interfaces);
101+
}
102+
103+
if ($hasAbstractParent) {
104+
$suggestions[] = 'Abstract parent: ' . $parentClass->getName();
105+
}
100106

101-
return in_array(strtolower($type), $builtInTypes);
107+
return RuleErrorBuilder::message(
108+
sprintf(
109+
'Parameter $%s uses concrete class %s instead of an interface or abstract class. %s',
110+
is_string($param->var->name) ? $param->var->name : $param->var->name->getType(),
111+
$typeName,
112+
implode('. ', $suggestions)
113+
)
114+
)->build();
102115
}
103116
}

tests/rules/RequireAbstractionInDependenciesRuleTest.php

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,9 @@ final class RequireAbstractionInDependenciesRuleTest extends RuleTestCase
1919
{
2020
protected function getRule(): Rule
2121
{
22-
return new RequireAbstractionInDependenciesRule();
22+
return new RequireAbstractionInDependenciesRule(
23+
$this->createReflectionProvider()
24+
);
2325
}
2426

2527
public function testRule(): void

0 commit comments

Comments
 (0)