Skip to content

Commit ce4f652

Browse files
mglamanmglaman
authored
Support TrustedCallback attribute for RenderCallbackRule (#529)
1 parent 821957a commit ce4f652

File tree

5 files changed

+241
-131
lines changed

5 files changed

+241
-131
lines changed

‎.github/workflows/php.yml‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,10 @@ jobs:
5050
drupal: "^9.0"
5151
experimental: true
5252
- php-version: "8.1"
53-
drupal: "10.0.x-dev"
53+
drupal: "^10.0"
54+
experimental: false
55+
- php-version: "8.1"
56+
drupal: "10.1.x-dev"
5457
experimental: true
5558
steps:
5659
- name: "Checkout"
@@ -67,7 +70,7 @@ jobs:
6770
if: ${{ matrix.drupal == '^8.9' }}
6871
- name: "Upgrade to drupal/core:10.0.x and drush/drush:^11.0"
6972
run: "composer require drupal/core-recommended:${{ matrix.drupal }} drush/drush:^11.0 --with-all-dependencies --dev --no-update"
70-
if: ${{ matrix.drupal == '10.0.x-dev' }}
73+
if: ${{ matrix.drupal == '10.0.x-dev' || matrix.drupal == '10.1.x-dev' }}
7174
- name: "Add phpspec/prophecy-phpunit"
7275
run: "composer require phpspec/prophecy-phpunit:^2 --dev --no-update"
7376
if: ${{ matrix.drupal != '^8.9' }}

‎.github/workflows/phpstan-dev.yml‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,8 @@ jobs:
1313
strategy:
1414
matrix:
1515
phpstan:
16-
- '1.9.x-dev'
16+
- '1.10.x-dev'
17+
- '1.11.x-dev'
1718
steps:
1819
- name: "Checkout"
1920
uses: "actions/checkout@v3"

‎src/Rules/Drupal/RenderCallbackRule.php‎

Lines changed: 120 additions & 90 deletions
Original file line numberDiff line numberDiff line change
@@ -2,16 +2,20 @@
22

33
namespace mglaman\PHPStanDrupal\Rules\Drupal;
44

5+
use Drupal\Core\Render\Element\RenderCallbackInterface;
56
use Drupal\Core\Render\PlaceholderGenerator;
67
use Drupal\Core\Render\Renderer;
8+
use Drupal\Core\Security\Attribute\TrustedCallback;
9+
use Drupal\Core\Security\TrustedCallbackInterface;
710
use mglaman\PHPStanDrupal\Drupal\ServiceMap;
811
use PhpParser\Node;
912
use PhpParser\Node\Name;
1013
use PHPStan\Analyser\Scope;
14+
use PHPStan\Reflection\ClassReflection;
1115
use PHPStan\Reflection\ReflectionProvider;
1216
use PHPStan\Rules\Rule;
13-
use PHPStan\Rules\RuleError;
1417
use PHPStan\Rules\RuleErrorBuilder;
18+
use PHPStan\TrinaryLogic;
1519
use PHPStan\Type\ClosureType;
1620
use PHPStan\Type\Constant\ConstantArrayType;
1721
use PHPStan\Type\Constant\ConstantIntegerType;
@@ -20,17 +24,23 @@
2024
use PHPStan\Type\IntersectionType;
2125
use PHPStan\Type\ObjectType;
2226
use PHPStan\Type\StaticType;
23-
use PHPStan\Type\ThisType;
2427
use PHPStan\Type\Type;
2528
use PHPStan\Type\UnionType;
2629
use PHPStan\Type\VerbosityLevel;
2730

2831
final class RenderCallbackRule implements Rule
2932
{
3033

31-
protected ReflectionProvider $reflectionProvider;
34+
private ReflectionProvider $reflectionProvider;
3235

33-
protected ServiceMap $serviceMap;
36+
private ServiceMap $serviceMap;
37+
38+
private array $supportedKeys = [
39+
'#pre_render',
40+
'#post_render',
41+
'#access_callback',
42+
'#lazy_builder',
43+
];
3444

3545
public function __construct(ReflectionProvider $reflectionProvider, ServiceMap $serviceMap)
3646
{
@@ -51,20 +61,18 @@ public function processNode(Node $node, Scope $scope): array
5161
return [];
5262
}
5363

54-
// @todo this should be 3 rules.
5564
// @see https://www.drupal.org/node/2966725
56-
$keysToCheck = ['#pre_render', '#post_render', '#access_callback', '#lazy_builder'];
57-
$keySearch = array_search($key->value, $keysToCheck, true);
65+
$keySearch = array_search($key->value, $this->supportedKeys, true);
5866
if ($keySearch === false) {
5967
return [];
6068
}
61-
$keyChecked = $keysToCheck[$keySearch];
62-
69+
$keyChecked = $this->supportedKeys[$keySearch];
6370
$value = $node->value;
6471

65-
$errors = [];
72+
if ($keyChecked === '#access_callback') {
73+
return $this->doProcessNode($node->value, $scope, $keyChecked, 0);
74+
}
6675

67-
// @todo Move into its own rule.
6876
if ($keyChecked === '#lazy_builder') {
6977
if ($scope->isInClass()) {
7078
$classReflection = $scope->getClassReflection();
@@ -73,15 +81,13 @@ public function processNode(Node $node, Scope $scope): array
7381
// PHPStan 1.6, nodes do not track their parent/next/prev which
7482
// saves a lot of memory. But makes it harder to detect if we're
7583
// in a call to array_intersect_key. This is an easier workaround.
76-
$allowedTypes = [
77-
PlaceholderGenerator::class,
78-
Renderer::class,
79-
'Drupal\Tests\Core\Render\RendererPlaceholdersTest',
80-
];
81-
foreach ($allowedTypes as $allowedType) {
82-
if ($classType->isInstanceOf($allowedType)->yes()) {
83-
return [];
84-
}
84+
$allowedTypes = new UnionType([
85+
new ObjectType(PlaceholderGenerator::class),
86+
new ObjectType(Renderer::class),
87+
new ObjectType('Drupal\Tests\Core\Render\RendererPlaceholdersTest'),
88+
]);
89+
if ($allowedTypes->isSuperTypeOf($classType)->yes()) {
90+
return [];
8591
}
8692
}
8793

@@ -98,107 +104,131 @@ public function processNode(Node $node, Scope $scope): array
98104
return [];
99105
}
100106
// @todo take $value->items[1] and validate parameters against the callback.
101-
$errors[] = $this->doProcessNode($value->items[0]->value, $scope, $keyChecked, 0);
102-
} elseif ($keyChecked === '#access_callback') {
103-
// @todo move into its own rule.
104-
$errors[] = $this->doProcessNode($value, $scope, $keyChecked, 0);
105-
} else {
106-
// @todo keep here.
107-
if (!$value instanceof Node\Expr\Array_) {
108-
return [
109-
RuleErrorBuilder::message(sprintf('The "%s" render array value expects an array of callbacks.', $keyChecked))
110-
->line($node->getLine())->build()
111-
];
112-
}
113-
if (count($value->items) === 0) {
114-
return [];
115-
}
116-
foreach ($value->items as $pos => $item) {
117-
if (!$item instanceof Node\Expr\ArrayItem) {
118-
continue;
119-
}
120-
$errors[] = $this->doProcessNode($item->value, $scope, $keyChecked, $pos);
107+
return $this->doProcessNode($value->items[0]->value, $scope, $keyChecked, 0);
108+
}
109+
110+
if (!$value instanceof Node\Expr\Array_) {
111+
return [
112+
RuleErrorBuilder::message(sprintf('The "%s" render array value expects an array of callbacks.', $keyChecked))
113+
->line($node->getLine())->build()
114+
];
115+
}
116+
if (count($value->items) === 0) {
117+
return [];
118+
}
119+
$errors = [];
120+
foreach ($value->items as $pos => $item) {
121+
if (!$item instanceof Node\Expr\ArrayItem) {
122+
continue;
121123
}
124+
$errors[] = $this->doProcessNode($item->value, $scope, $keyChecked, $pos);
122125
}
123-
return array_filter($errors);
126+
return array_merge(...$errors);
124127
}
125128

126-
private function doProcessNode(Node\Expr $node, Scope $scope, string $keyChecked, int $pos): ?RuleError
129+
/**
130+
@return (string|\PHPStan\Rules\RuleError)[] errors
131+
*/
132+
private function doProcessNode(Node\Expr $node, Scope $scope, string $keyChecked, int $pos): array
127133
{
128134
$trustedCallbackType = new UnionType([
129-
new ObjectType('Drupal\Core\Security\TrustedCallbackInterface'),
130-
new ObjectType('Drupal\Core\Render\Element\RenderCallbackInterface'),
135+
new ObjectType(TrustedCallbackInterface::class),
136+
new ObjectType(RenderCallbackInterface::class),
131137
]);
132138

139+
$errors = [];
133140
$errorLine = $node->getLine();
134141
$type = $this->getType($node, $scope);
135142

136-
if ($type instanceof ConstantStringType) {
137-
if (!$type->isCallable()->yes()) {
138-
return RuleErrorBuilder::message(
139-
sprintf("%s callback %s at key '%s' is not callable.", $keyChecked, $type->describe(VerbosityLevel::value()), $pos)
143+
foreach ($type->getConstantStrings() as $constantStringType) {
144+
if (!$constantStringType->isCallable()->yes()) {
145+
$errors[] = RuleErrorBuilder::message(
146+
sprintf("%s callback %s at key '%s' is not callable.", $keyChecked, $constantStringType->describe(VerbosityLevel::value()), $pos)
140147
)->line($errorLine)->build();
141-
}
142-
// We can determine if the callback is callable through the type system. However, we cannot determine
143-
// if it is just a function or a static class call (MyClass::staticFunc).
144-
if ($this->reflectionProvider->hasFunction(new Name($type->getValue()), null)) {
145-
return RuleErrorBuilder::message(
146-
sprintf("%s callback %s at key '%s' is not trusted.", $keyChecked, $type->describe(VerbosityLevel::value()), $pos)
148+
} elseif ($this->reflectionProvider->hasFunction(new Name($constantStringType->getValue()), null)) {
149+
// We can determine if the callback is callable through the type system. However, we cannot determine
150+
// if it is just a function or a static class call (MyClass::staticFunc).
151+
$errors[] = RuleErrorBuilder::message(
152+
sprintf("%s callback %s at key '%s' is not trusted.", $keyChecked, $constantStringType->describe(VerbosityLevel::value()), $pos)
147153
)->line($errorLine)
148154
->tip('Change record: https://www.drupal.org/node/2966725.')
149155
->build();
150-
}
151-
152-
if (!$trustedCallbackType->isSuperTypeOf($type)->yes()) {
153-
return RuleErrorBuilder::message(
154-
sprintf("%s callback class %s at key '%s' does not implement Drupal\Core\Security\TrustedCallbackInterface.", $keyChecked, $type->describe(VerbosityLevel::value()), $pos)
156+
} elseif (!$trustedCallbackType->isSuperTypeOf($type)->yes()) {
157+
$errors[] = RuleErrorBuilder::message(
158+
sprintf("%s callback class %s at key '%s' does not implement Drupal\Core\Security\TrustedCallbackInterface.", $keyChecked, $constantStringType->describe(VerbosityLevel::value()), $pos)
155159
)->line($errorLine)->tip('Change record: https://www.drupal.org/node/2966725.')->build();
156160
}
157-
} elseif ($type instanceof ConstantArrayType) {
158-
if (!$type->isCallable()->yes()) {
159-
return RuleErrorBuilder::message(
160-
sprintf("%s callback %s at key '%s' is not callable.", $keyChecked, $type->describe(VerbosityLevel::value()), $pos)
161+
}
162+
163+
foreach ($type->getConstantArrays() as $constantArrayType) {
164+
if (!$constantArrayType->isCallable()->yes()) {
165+
$errors[] = RuleErrorBuilder::message(
166+
sprintf("%s callback %s at key '%s' is not callable.", $keyChecked, $constantArrayType->describe(VerbosityLevel::value()), $pos)
161167
)->line($errorLine)->build();
168+
continue;
162169
}
163-
$typeAndMethodNames = $type->findTypeAndMethodNames();
170+
$typeAndMethodNames = $constantArrayType->findTypeAndMethodNames();
164171
if ($typeAndMethodNames === []) {
165-
throw new \PHPStan\ShouldNotHappenException();
172+
continue;
166173
}
167174

168175
foreach ($typeAndMethodNames as $typeAndMethodName) {
169-
if (!$trustedCallbackType->isSuperTypeOf($typeAndMethodName->getType())->yes()) {
170-
return RuleErrorBuilder::message(
171-
sprintf("%s callback class '%s' at key '%s' does not implement Drupal\Core\Security\TrustedCallbackInterface.", $keyChecked, $typeAndMethodName->getType()->describe(VerbosityLevel::value()), $pos)
172-
)->line($errorLine)->tip('Change record: https://www.drupal.org/node/2966725.')->build();
173-
}
174-
}
175-
} elseif ($type instanceof ClosureType) {
176-
if ($scope->isInClass()) {
177-
$classReflection = $scope->getClassReflection();
178-
$classType = new ObjectType($classReflection->getName());
179-
$formType = new ObjectType('\Drupal\Core\Form\FormInterface');
180-
if ($formType->isSuperTypeOf($classType)->yes()) {
181-
return RuleErrorBuilder::message(
182-
sprintf("%s may not contain a closure at key '%s' as forms may be serialized and serialization of closures is not allowed.", $keyChecked, $pos)
183-
)->line($errorLine)->build();
176+
$isTrustedCallbackAttribute = TrinaryLogic::createNo()->lazyOr(
177+
$typeAndMethodName->getType()->getObjectClassReflections(),
178+
function (ClassReflection $reflection) use ($typeAndMethodName) {
179+
if (!class_exists(TrustedCallback::class)) {
180+
return TrinaryLogic::createNo();
181+
}
182+
$hasAttribute = $reflection->getNativeReflection()
183+
->getMethod($typeAndMethodName->getMethod())
184+
->getAttributes(TrustedCallback::class);
185+
return TrinaryLogic::createFromBoolean(count($hasAttribute) > 0);
186+
}
187+
);
188+
189+
$isTrustedCallbackInterfaceType = $trustedCallbackType->isSuperTypeOf($typeAndMethodName->getType())->yes();
190+
if (!$isTrustedCallbackInterfaceType && !$isTrustedCallbackAttribute->yes()) {
191+
if (class_exists(TrustedCallback::class)) {
192+
$errors[] = RuleErrorBuilder::message(
193+
sprintf(
194+
"%s callback method '%s' at key '%s' does not implement attribute \Drupal\Core\Security\Attribute\TrustedCallback.",
195+
$keyChecked,
196+
$constantArrayType->describe(VerbosityLevel::value()),
197+
$pos
198+
)
199+
)->line($errorLine)->tip('Change record: https://www.drupal.org/node/3349470')->build();
200+
} else {
201+
$errors[] = RuleErrorBuilder::message(
202+
sprintf(
203+
"%s callback class '%s' at key '%s' does not implement Drupal\Core\Security\TrustedCallbackInterface.",
204+
$keyChecked,
205+
$typeAndMethodName->getType()->describe(VerbosityLevel::value()),
206+
$pos
207+
)
208+
)->line($errorLine)->tip('Change record: https://www.drupal.org/node/2966725.')->build();
209+
}
184210
}
185211
}
186-
} elseif ($type instanceof ThisType) {
187-
if (!$type->isCallable()->yes()) {
188-
return RuleErrorBuilder::message(
189-
sprintf("%s callback %s at key '%s' is not callable.", $keyChecked, $type->describe(VerbosityLevel::value()), $pos)
212+
}
213+
// @todo move to its own rule for 1.2.0, FormClosureSerializationRule.
214+
if (($type instanceof ClosureType) && $scope->isInClass()) {
215+
$classReflection = $scope->getClassReflection();
216+
$classType = new ObjectType($classReflection->getName());
217+
$formType = new ObjectType('\Drupal\Core\Form\FormInterface');
218+
if ($formType->isSuperTypeOf($classType)->yes()) {
219+
$errors[] = RuleErrorBuilder::message(
220+
sprintf("%s may not contain a closure at key '%s' as forms may be serialized and serialization of closures is not allowed.", $keyChecked, $pos)
190221
)->line($errorLine)->build();
191222
}
192-
} elseif ($type->isCallable()->yes()) {
193-
// If the value has been marked as callable or callable-string, we cannot resolve the callable, trust it.
194-
return null;
195-
} else {
196-
return RuleErrorBuilder::message(
223+
}
224+
225+
if (count($errors) === 0 && !$type->isCallable()->yes()) {
226+
$errors[] = RuleErrorBuilder::message(
197227
sprintf("%s value '%s' at key '%s' is invalid.", $keyChecked, $type->describe(VerbosityLevel::value()), $pos)
198228
)->line($errorLine)->build();
199229
}
200230

201-
return null;
231+
return $errors;
202232
}
203233

204234
// @todo move to a helper, as Drupal uses `service:method` references a lot.

0 commit comments

Comments
 (0)