Skip to content

Commit fda0f67

Browse files
authored
Rework allowInMethodsWithAttributes in signatures to use a node visitor (#395)
PHPStan's `$scope->getFunction()` returns `null` in param return types because scope has not entered the function or method body yet. The previous workaround #315 (fixing #314) used a stateful object and four helper rules that set/unset the current method or function name, relying on a rule execution order being strictly top-down - an undocumented implementation detail, not a contract. `TypeHintContextVisitor` (ex-`TypeHintPositionVisitor`) already annotated type nodes at parse time with position information. Extending it to also carry the enclosing function's attribute names solves the same problem at the right level, with no ordering assumptions and no shared state.
2 parents 4ca9a07 + 353de84 commit fda0f67

15 files changed

Lines changed: 341 additions & 543 deletions

extension.neon

Lines changed: 2 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -499,13 +499,12 @@ parametersSchema:
499499

500500
services:
501501
-
502-
class: Spaze\PHPStan\Rules\Disallowed\Type\TypeHintPositionVisitor
502+
class: Spaze\PHPStan\Rules\Disallowed\Type\TypeHintContextVisitor
503503
tags:
504504
- phpstan.parser.richParserNodeVisitor
505505
- Spaze\PHPStan\Rules\Disallowed\Allowed\Allowed
506506
- Spaze\PHPStan\Rules\Disallowed\Allowed\AllowedConfigFactory
507507
- Spaze\PHPStan\Rules\Disallowed\Allowed\AllowedPath
508-
- Spaze\PHPStan\Rules\Disallowed\Allowed\GetAttributesWhenInSignature
509508
- Spaze\PHPStan\Rules\Disallowed\DisallowedAttributeFactory
510509
- Spaze\PHPStan\Rules\Disallowed\DisallowedCallFactory
511510
- Spaze\PHPStan\Rules\Disallowed\DisallowedConstantFactory
@@ -690,19 +689,4 @@ services:
690689
factory: Spaze\PHPStan\Rules\Disallowed\Usages\StaticPropertyUsages(disallowedProperties: %disallowedProperties%)
691690
tags:
692691
- phpstan.rules.rule
693-
-
694-
factory: Spaze\PHPStan\Rules\Disallowed\HelperRules\SetCurrentClassMethodNameHelperRule
695-
tags:
696-
- phpstan.rules.rule
697-
-
698-
factory: Spaze\PHPStan\Rules\Disallowed\HelperRules\UnsetCurrentClassMethodNameHelperRule
699-
tags:
700-
- phpstan.rules.rule
701-
-
702-
factory: Spaze\PHPStan\Rules\Disallowed\HelperRules\SetCurrentFunctionNameHelperRule
703-
tags:
704-
- phpstan.rules.rule
705-
-
706-
factory: Spaze\PHPStan\Rules\Disallowed\HelperRules\UnsetCurrentFunctionNameHelperRule
707-
tags:
708-
- phpstan.rules.rule
692+

src/Allowed/Allowed.php

Lines changed: 34 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,6 @@
88
use PhpParser\Node\Stmt\ClassMethod;
99
use PhpParser\Node\Stmt\Function_;
1010
use PHPStan\Analyser\Scope;
11-
use PHPStan\BetterReflection\Reflection\Adapter\FakeReflectionAttribute;
12-
use PHPStan\BetterReflection\Reflection\Adapter\ReflectionAttribute;
13-
use PHPStan\BetterReflection\Reflection\ReflectionAttribute as BetterReflectionAttribute;
1411
use PHPStan\BetterReflection\Reflector\Reflector;
1512
use PHPStan\Reflection\FunctionReflection;
1613
use PHPStan\Reflection\MethodReflection;
@@ -23,6 +20,7 @@
2320
use Spaze\PHPStan\Rules\Disallowed\Formatter\Formatter;
2421
use Spaze\PHPStan\Rules\Disallowed\Identifier\Identifier;
2522
use Spaze\PHPStan\Rules\Disallowed\Params\Param;
23+
use Spaze\PHPStan\Rules\Disallowed\Type\TypeHintContextVisitor;
2624

2725
class Allowed
2826
{
@@ -33,22 +31,18 @@ class Allowed
3331

3432
private Identifier $identifier;
3533

36-
private GetAttributesWhenInSignature $attributesWhenInSignature;
37-
3834
private AllowedPath $allowedPath;
3935

4036

4137
public function __construct(
4238
Formatter $formatter,
4339
Reflector $reflector,
4440
Identifier $identifier,
45-
GetAttributesWhenInSignature $attributesWhenInSignature,
4641
AllowedPath $allowedPath
4742
) {
4843
$this->formatter = $formatter;
4944
$this->reflector = $reflector;
5045
$this->identifier = $identifier;
51-
$this->attributesWhenInSignature = $attributesWhenInSignature;
5246
$this->allowedPath = $allowedPath;
5347
}
5448

@@ -220,18 +214,14 @@ private function hasAllowedParamsInAllowed(Scope $scope, ?array $args, Disallowe
220214

221215

222216
/**
223-
* @param list<FakeReflectionAttribute|ReflectionAttribute|BetterReflectionAttribute> $attributes
217+
* @param list<string> $attributeNames
224218
* @param list<string> $allowConfig
225219
* @return bool
226220
*/
227-
private function hasAllowedAttribute(array $attributes, array $allowConfig): bool
221+
private function hasAllowedAttribute(array $attributeNames, array $allowConfig): bool
228222
{
229-
$names = [];
230-
foreach ($attributes as $attribute) {
231-
$names[] = $attribute->getName();
232-
}
233223
foreach ($allowConfig as $allowAttribute) {
234-
foreach ($names as $name) {
224+
foreach ($attributeNames as $name) {
235225
if ($this->identifier->matches($allowAttribute, $name)) {
236226
return true;
237227
}
@@ -264,35 +254,50 @@ private function getArgType(array $args, Scope $scope, Param $param): ?Type
264254

265255
/**
266256
* @param Scope $scope
267-
* @return list<FakeReflectionAttribute>|list<ReflectionAttribute>
257+
* @return list<string>
268258
*/
269259
private function getAttributes(Scope $scope): array
270260
{
271-
return $scope->isInClass() ? $scope->getClassReflection()->getNativeReflection()->getAttributes() : [];
261+
if (!$scope->isInClass()) {
262+
return [];
263+
}
264+
return array_map(static fn($a) => $a->getName(), $scope->getClassReflection()->getNativeReflection()->getAttributes());
272265
}
273266

274267

275268
/**
276269
* @param Node|null $node
277270
* @param Scope $scope
278-
* @return list<FakeReflectionAttribute|ReflectionAttribute|BetterReflectionAttribute>
271+
* @return list<string>
279272
*/
280273
private function getCallAttributes(?Node $node, Scope $scope): array
281274
{
282275
$function = $scope->getFunction();
283276
if ($function instanceof MethodReflection) {
284-
return $scope->isInClass() ? $scope->getClassReflection()->getNativeReflection()->getMethod($function->getName())->getAttributes() : [];
277+
if (!$scope->isInClass()) {
278+
return [];
279+
}
280+
return array_map(static fn($a) => $a->getName(), $scope->getClassReflection()->getNativeReflection()->getMethod($function->getName())->getAttributes());
285281
} elseif ($function instanceof FunctionReflection) {
286-
return $this->reflector->reflectFunction($function->getName())->getAttributes();
282+
return array_map(static fn($a) => $a->getName(), $this->reflector->reflectFunction($function->getName())->getAttributes());
287283
} elseif ($function === null) {
288284
if ($node instanceof ClassMethod && $scope->isInClass()) {
289-
return $scope->getClassReflection()->getNativeReflection()->getMethod($node->name->name)->getAttributes();
285+
return array_map(static fn($a) => $a->getName(), $scope->getClassReflection()->getNativeReflection()->getMethod($node->name->name)->getAttributes());
290286
} elseif ($node instanceof Function_) {
291-
return $this->reflector->reflectFunction($node->name->name)->getAttributes();
287+
return array_map(static fn($a) => $a->getName(), $this->reflector->reflectFunction(($node->namespacedName ?? $node->name)->toString())->getAttributes());
292288
}
293-
$attributes = $this->attributesWhenInSignature->get($scope);
294-
if ($attributes !== null) {
295-
return $attributes;
289+
// $scope->getFunction() is null in param/return type hints, use the enclosing function attributes stored by TypeHintContextVisitor
290+
if ($node !== null) {
291+
$names = $node->getAttribute(TypeHintContextVisitor::ATTRIBUTE_ENCLOSING_FUNCTION_ATTR_NAMES);
292+
if (is_array($names)) {
293+
$result = [];
294+
foreach ($names as $name) {
295+
if (is_string($name)) {
296+
$result[] = $name;
297+
}
298+
}
299+
return $result;
300+
}
296301
}
297302
}
298303
return [];
@@ -301,21 +306,20 @@ private function getCallAttributes(?Node $node, Scope $scope): array
301306

302307
/**
303308
* @param Scope $scope
304-
* @return list<FakeReflectionAttribute>|list<ReflectionAttribute>
309+
* @return list<string>
305310
*/
306311
private function getAllMethodAttributes(Scope $scope): array
307312
{
308313
if (!$scope->isInClass()) {
309314
return [];
310315
}
311-
$attributes = [];
316+
$names = [];
312317
foreach ($scope->getClassReflection()->getNativeReflection()->getMethods() as $method) {
313-
$methodAttributes = $method->getAttributes();
314-
if ($methodAttributes !== []) {
315-
$attributes = array_merge($attributes, $methodAttributes);
318+
foreach ($method->getAttributes() as $attribute) {
319+
$names[] = $attribute->getName();
316320
}
317321
}
318-
return $attributes;
322+
return $names;
319323
}
320324

321325
}

src/Allowed/GetAttributesWhenInSignature.php

Lines changed: 0 additions & 92 deletions
This file was deleted.

src/HelperRules/SetCurrentClassMethodNameHelperRule.php

Lines changed: 0 additions & 46 deletions
This file was deleted.

src/HelperRules/SetCurrentFunctionNameHelperRule.php

Lines changed: 0 additions & 46 deletions
This file was deleted.

0 commit comments

Comments
 (0)