Skip to content

Commit 42d594e

Browse files
Boegiemglaman
Boegie
and
mglaman
authored
False positive when (one of) the callback function name(s) in #date_date_callbacks/#date_time_callbacks is a variable. (#558)
Co-authored-by: Matt Glaman <[email protected]>
1 parent c9f11c2 commit 42d594e

File tree

3 files changed

+148
-4
lines changed

3 files changed

+148
-4
lines changed

‎src/Rules/Drupal/RenderCallbackRule.php‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,8 @@ public function processNode(Node $node, Scope $scope): array
133133
*/
134134
private function doProcessNode(Node\Expr $node, Scope $scope, string $keyChecked, int $pos): array
135135
{
136+
$checkIsCallable = true;
137+
136138
$trustedCallbackType = new UnionType([
137139
new ObjectType(TrustedCallbackInterface::class),
138140
new ObjectType(RenderCallbackInterface::class),
@@ -172,6 +174,20 @@ private function doProcessNode(Node\Expr $node, Scope $scope, string $keyChecked
172174

173175
foreach ($type->getConstantArrays() as $constantArrayType) {
174176
if (!$constantArrayType->isCallable()->yes()) {
177+
// If the right-hand side of the array is a variable, we cannot
178+
// determine if it is callable. Bail now.
179+
$itemType = $constantArrayType->getItemType();
180+
if ($itemType instanceof UnionType) {
181+
$unionConstantStrings = array_merge(...array_map(static function (Type $type) {
182+
return $type->getConstantStrings();
183+
}, $itemType->getTypes()));
184+
if (count($unionConstantStrings) === 0) {
185+
// Right-hand side of UnionType is not a constant string. We cannot determine if the dynamic
186+
// value is callable or not.
187+
$checkIsCallable = false;
188+
break;
189+
}
190+
}
175191
$errors[] = RuleErrorBuilder::message(
176192
sprintf("%s callback %s at key '%s' is not callable.", $keyChecked, $constantArrayType->describe(VerbosityLevel::value()), $pos)
177193
)->line($errorLine)->build();
@@ -232,7 +248,7 @@ function (ClassReflection $reflection) use ($typeAndMethodName) {
232248
}
233249
}
234250

235-
if (count($errors) === 0 && !$type->isCallable()->yes()) {
251+
if (count($errors) === 0 && ($checkIsCallable && !$type->isCallable()->yes())) {
236252
$errors[] = RuleErrorBuilder::message(
237253
sprintf("%s value '%s' at key '%s' is invalid.", $keyChecked, $type->describe(VerbosityLevel::value()), $pos)
238254
)->line($errorLine)->build();

‎tests/src/Rules/RenderCallbackRuleTest.php‎

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,14 @@
66
use mglaman\PHPStanDrupal\Drupal\ServiceMap;
77
use mglaman\PHPStanDrupal\Tests\DrupalRuleTestCase;
88
use mglaman\PHPStanDrupal\Rules\Drupal\RenderCallbackRule;
9+
use PHPStan\Rules\Rule;
910

1011
final class RenderCallbackRuleTest extends DrupalRuleTestCase {
1112

12-
protected function getRule(): \PHPStan\Rules\Rule
13+
protected function getRule(): Rule
1314
{
1415
return new RenderCallbackRule(
15-
$this->createReflectionProvider(),
16+
self::createReflectionProvider(),
1617
self::getContainer()->getByType(ServiceMap::class)
1718
);
1819
}
@@ -158,11 +159,23 @@ public static function fileData(): \Generator
158159
],
159160
];
160161
}
161-
162162
yield [
163163
__DIR__ . '/data/bug-543.php',
164164
[]
165165
];
166+
yield [
167+
__DIR__ . '/data/bug-554.php',
168+
[
169+
[
170+
'#date_date_callbacks callback array{$this(Bug554\\TestClass), \'notExisting\'} at key \'2\' is not callable.',
171+
79
172+
],
173+
[
174+
'#date_time_callbacks callback array{$this(Bug554\\TestClass), \'notExisting\'} at key \'2\' is not callable.',
175+
84
176+
],
177+
]
178+
];
166179
}
167180

168181

‎tests/src/Rules/data/bug-554.php‎

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,115 @@
1+
<?php
2+
3+
namespace Bug554;
4+
5+
use Drupal\Core\Datetime\DrupalDateTime;
6+
use Drupal\Core\Form\FormInterface;
7+
use Drupal\Core\Form\FormStateInterface;
8+
use Drupal\Core\Security\TrustedCallbackInterface;
9+
10+
/**
11+
* Tests callbacks for '#date_date_callbacks/' and '#date_time_callbacks'.
12+
*
13+
* Code snippets based on \Drupal\KernelTests\Core\Datetime\DatetimeElementFormTest.
14+
*/
15+
class TestClass implements FormInterface, TrustedCallbackInterface {
16+
17+
/**
18+
* {@inheritdoc}
19+
*/
20+
public function getFormId() {
21+
return 'test_datelist_element';
22+
}
23+
24+
/**
25+
* {@inheritdoc}
26+
*/
27+
public function datetimeDateCallbackTrusted(array &$element, FormStateInterface $form_state, DrupalDateTime $date = NULL) {
28+
$element['datetimeDateCallbackExecuted'] = [
29+
'#value' => TRUE,
30+
];
31+
$form_state->set('datetimeDateCallbackExecuted', TRUE);
32+
}
33+
34+
/**
35+
* {@inheritdoc}
36+
*/
37+
public function datetimeTimeCallbackTrusted(array &$element, FormStateInterface $form_state, DrupalDateTime $date = NULL) {
38+
$element['timeCallbackExecuted'] = [
39+
'#value' => TRUE,
40+
];
41+
$form_state->set('timeCallbackExecuted', TRUE);
42+
}
43+
44+
/**
45+
* {@inheritdoc}
46+
*/
47+
public function submitForm(array &$form, FormStateInterface $form_state) {}
48+
49+
/**
50+
* Form validation handler.
51+
*
52+
* @param array $form
53+
* An associative array containing the structure of the form.
54+
* @param \Drupal\Core\Form\FormStateInterface $form_state
55+
* The current state of the form.
56+
*/
57+
public function validateForm(array &$form, FormStateInterface $form_state) {}
58+
59+
/**
60+
* {@inheritdoc}
61+
*/
62+
public function buildForm(array $form, FormStateInterface $form_state, string $date_callback = 'datetimeDateCallbackTrusted', string $time_callback = 'datetimeTimeCallbackTrusted') {
63+
$form['datetime_element'] = [
64+
'#title' => 'datelist test',
65+
'#type' => 'datetime',
66+
'#default_value' => new DrupalDateTime('2000-01-01 00:00:00'),
67+
'#date_date_format' => 'Y-m-d',
68+
'#date_time_format' => 'H:i:s',
69+
'#date_date_element' => 'HTML Date',
70+
'#date_time_element' => 'HTML Time',
71+
'#date_increment' => 1,
72+
// We test callbacks:
73+
// - by calling it by method name.
74+
// - by calling it using a variable.
75+
// - which don't exist.
76+
'#date_date_callbacks' => [
77+
[$this, 'datetimeDateCallbackTrusted'],
78+
[$this, $date_callback],
79+
[$this, 'notExisting']
80+
],
81+
'#date_time_callbacks' => [
82+
[$this, 'datetimeTimeCallbackTrusted'],
83+
[$this, $time_callback],
84+
[$this, 'notExisting']
85+
],
86+
];
87+
88+
// Element without specifying the default value.
89+
$form['simple_datetime_element'] = [
90+
'#type' => 'datetime',
91+
'#date_date_format' => 'Y-m-d',
92+
'#date_time_format' => 'H:i:s',
93+
'#date_date_element' => 'HTML Date',
94+
'#date_time_element' => 'HTML Time',
95+
];
96+
97+
$form['submit'] = [
98+
'#type' => 'submit',
99+
'#value' => t('Submit'),
100+
];
101+
102+
return $form;
103+
}
104+
105+
/**
106+
* {@inheritdoc}
107+
*/
108+
public static function trustedCallbacks() {
109+
return [
110+
'datetimeDateCallbackTrusted',
111+
'datetimeTimeCallbackTrusted',
112+
];
113+
}
114+
115+
}

0 commit comments

Comments
 (0)