Skip to content

Commit 5b69dc0

Browse files
spazespaze
authored
First-class callables with parameter conditions (#418)
PHPStan detects first-class callable syntax at the point of `func(...)`, where no arguments are present - they are supplied only when the callable is later invoked, which means parameter conditions configured on the disallowed call cannot be evaluated at the detection point. This PR corrects the behavior across all param condition directives. Conditions that require a matching param value to allow a call - `allowParamsAnywhere`, `allowParamsInAllowed`, and their _`AnyValue`_ and _`Flags`_ variants - can never be satisfied without arguments, so first-class callables are always reported when any of these is configured, regardless of zone. Conditions that require a matching param value to disallow a call - `allowExceptParamsAnywhere`, `allowExceptParamsInAllowed`, and their variants and aliases - can never be triggered without arguments, so first-class callables are never reported in either `allowIn*` or `allowExceptIn*` zones when these are configured. Previously, `allowParamsAnywhere` incorrectly allowed first-class callables because the null-args path in `hasAllowedParams` unconditionally returned `true`. `allowExceptParamsInAllowed` incorrectly disallowed first-class callables in allowed zones because the null-args path in `hasAllowedParamsInAllowed` treated it identically to `allowParamsInAllowed`. Both are fixed. A new section in `docs/allow-with-parameters.md` documents the complete behavior for all param condition directives. Closes #415
2 parents d0af76f + aecdc11 commit 5b69dc0

9 files changed

Lines changed: 300 additions & 10 deletions

‎composer.json‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,8 @@
4141
},
4242
"scripts": {
4343
"lint": "vendor/bin/parallel-lint --colors src/ tests/",
44-
"lint-7.x": "vendor/bin/parallel-lint --colors src/ tests/ --exclude tests/src/TypesEverywhere.php --exclude tests/src/AttributesEverywhere.php --exclude tests/src/disallowed/functionCallsClassPatternParams.php --exclude tests/src/disallowed/functionCallsNamedParams.php --exclude tests/src/disallowed-allow/functionCallsNamedParams.php --exclude tests/src/disallowed/attributeUsages.php --exclude tests/src/disallowed-allow/attributeUsages.php --exclude tests/src/disallowed/constantDynamicUsages.php --exclude tests/src/disallowed-allow/constantDynamicUsages.php --exclude tests/src/AttributeClass.php --exclude tests/src/Bar.php --exclude tests/src/Enums.php --exclude tests/src/Functions.php --exclude tests/src/disallowed/controlStructures.php --exclude tests/src/disallowed-allow/controlStructures.php --exclude tests/src/disallowed/firstClassCallable.php --exclude tests/src/disallowed-allow/firstClassCallable.php --exclude tests/src/disallowed/callableParameters.php --exclude tests/src/disallowed-allow/callableParameters.php",
45-
"lint-8.0": "vendor/bin/parallel-lint --colors src/ tests/ --exclude tests/src/TypesEverywhere.php --exclude tests/src/AttributesEverywhere.php --exclude tests/src/disallowed/constantDynamicUsages.php --exclude tests/src/disallowed-allow/constantDynamicUsages.php --exclude tests/src/Enums.php --exclude tests/src/disallowed/firstClassCallable.php --exclude tests/src/disallowed-allow/firstClassCallable.php",
44+
"lint-7.x": "vendor/bin/parallel-lint --colors src/ tests/ --exclude tests/src/TypesEverywhere.php --exclude tests/src/AttributesEverywhere.php --exclude tests/src/disallowed/functionCallsClassPatternParams.php --exclude tests/src/disallowed/functionCallsNamedParams.php --exclude tests/src/disallowed-allow/functionCallsNamedParams.php --exclude tests/src/disallowed/attributeUsages.php --exclude tests/src/disallowed-allow/attributeUsages.php --exclude tests/src/disallowed/constantDynamicUsages.php --exclude tests/src/disallowed-allow/constantDynamicUsages.php --exclude tests/src/AttributeClass.php --exclude tests/src/Bar.php --exclude tests/src/Enums.php --exclude tests/src/Functions.php --exclude tests/src/disallowed/controlStructures.php --exclude tests/src/disallowed-allow/controlStructures.php --exclude tests/src/disallowed/firstClassCallable.php --exclude tests/src/disallowed-allow/firstClassCallable.php --exclude tests/src/disallowed/callableParameters.php --exclude tests/src/disallowed-allow/callableParameters.php --exclude tests/src/FirstClassCallableParamsAnywhere.php --exclude tests/src/RoyaleAllowInFirstClassCallable.php --exclude tests/src/RoyaleExceptFirstClassCallable.php",
45+
"lint-8.0": "vendor/bin/parallel-lint --colors src/ tests/ --exclude tests/src/TypesEverywhere.php --exclude tests/src/AttributesEverywhere.php --exclude tests/src/disallowed/constantDynamicUsages.php --exclude tests/src/disallowed-allow/constantDynamicUsages.php --exclude tests/src/Enums.php --exclude tests/src/disallowed/firstClassCallable.php --exclude tests/src/disallowed-allow/firstClassCallable.php --exclude tests/src/FirstClassCallableParamsAnywhere.php --exclude tests/src/RoyaleAllowInFirstClassCallable.php --exclude tests/src/RoyaleExceptFirstClassCallable.php",
4646
"lint-8.1": "vendor/bin/parallel-lint --colors src/ tests/ --exclude tests/src/AttributesEverywhere.php --exclude tests/src/disallowed/constantDynamicUsages.php --exclude tests/src/disallowed-allow/constantDynamicUsages.php --exclude tests/src/disallowed/firstClassCallable.php --exclude tests/src/disallowed-allow/firstClassCallable.php",
4747
"lint-8.2": "vendor/bin/parallel-lint --colors src/ tests/ --exclude tests/src/disallowed/constantDynamicUsages.php --exclude tests/src/disallowed-allow/constantDynamicUsages.php",
4848
"lint-neon": "vendor/bin/neon-lint .",

‎docs/allow-with-parameters.md‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -208,6 +208,14 @@ parameters:
208208

209209
But because the "positional _or_ named" limitation described above applies here as well, I generally don't recommend using these shortcuts and instead recommend specifying both `position` and `name` keys.
210210

211+
### First-class callables
212+
213+
When a function or method is used as a [first-class callable](https://www.php.net/functions.first_class_callable_syntax) (`strlen(...)`), no arguments are present at the call site - the callable is invoked later with whatever arguments the caller passes. Because parameter conditions that restrict which calls are *allowed* (`allowParamsInAllowed`, `allowParamsInAllowedAnyValue`, `allowParamFlagsInAllowed`, `allowParamsAnywhere`, `allowParamsAnywhereAnyValue`, `allowParamFlagsAnywhere`) cannot be evaluated without arguments, any first-class callable is always reported when such a condition is configured, no matter where in the code it appears.
214+
215+
Conditions that restrict which calls are *disallowed* behave differently - the forbidden parameter condition cannot be triggered without arguments. For the `*Anywhere` variants (`allowExceptParamsAnywhere`, `allowExceptParamsAnyValue`, `allowExceptParamFlags`, `allowExceptCaseInsensitiveParams`, and their aliases), first-class callables are never reported. For the `*InAllowed` variants (`allowExceptParamsInAllowed`, `allowExceptParamFlagsInAllowed`, and their aliases), first-class callables are not reported inside the relevant zone; outside it, the zone rule alone determines whether the call is reported.
216+
217+
To allow a first-class callable of a disallowed function, use a zone-based directive without a parameter condition, for example `allowIn`, `allowInMethods`, or `allowInInstanceOf`. Alternatively, use an anonymous function that calls the function with the required argument: `fn($x) => hash('sha256', $x)` instead of `hash(...)`.
218+
211219
### PHPDoc type strings
212220

213221
Instead of the `value` directive, you can use the `typeString` directive which allows you to specify arrays, unions, and anything that can be expressed with PHPDoc:

‎src/Allowed/Allowed.php‎

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ public function isAllowed(?Node $node, Scope $scope, ?array $args, Disallowed $d
6161
$hasParams = $disallowed instanceof DisallowedWithParams;
6262
foreach ($disallowed->getAllowInCalls() as $call) {
6363
if ($this->callMatches($scope, $call)) {
64-
return !$hasParams || $this->hasAllowedParamsInAllowed($scope, $args, $disallowed);
64+
return !$hasParams || $this->hasAllowedParamsInAllowed($scope, $args, $disallowed, true);
6565
}
6666
}
6767
if ($disallowed->getAllowExceptInCalls()) {
@@ -74,7 +74,7 @@ public function isAllowed(?Node $node, Scope $scope, ?array $args, Disallowed $d
7474
}
7575
foreach ($disallowed->getAllowIn() as $allowedPath) {
7676
if ($this->allowedPath->matches($scope, $allowedPath)) {
77-
return !$hasParams || $this->hasAllowedParamsInAllowed($scope, $args, $disallowed);
77+
return !$hasParams || $this->hasAllowedParamsInAllowed($scope, $args, $disallowed, true);
7878
}
7979
}
8080
if ($disallowed->getAllowExceptIn()) {
@@ -104,7 +104,7 @@ public function isAllowed(?Node $node, Scope $scope, ?array $args, Disallowed $d
104104
if (!$this->isInstanceOf($scope, $disallowed->getAllowInInstanceOf())) {
105105
return false;
106106
}
107-
return !$hasParams || $this->hasAllowedParamsInAllowed($scope, $args, $disallowed);
107+
return !$hasParams || $this->hasAllowedParamsInAllowed($scope, $args, $disallowed, true);
108108
}
109109
if ($disallowed->getAllowExceptInInstanceOf()) {
110110
if (!$this->isInstanceOf($scope, $disallowed->getAllowExceptInInstanceOf())) {
@@ -196,13 +196,13 @@ private function classOrAncestorMatches(ClassReflection $classReflection, string
196196
* @param Scope $scope
197197
* @param array<Arg>|null $args
198198
* @param array<int|string, Param> $allowConfig
199-
* @param bool $paramsRequired
199+
* @param bool $paramsRequired True to allow only when params match, false to allow unless params match
200200
* @return bool
201201
*/
202202
private function hasAllowedParams(Scope $scope, ?array $args, array $allowConfig, bool $paramsRequired): bool
203203
{
204204
if ($args === null) {
205-
return true;
205+
return !$paramsRequired;
206206
}
207207

208208
$disallowedParams = false;
@@ -232,18 +232,27 @@ private function hasAllowedParams(Scope $scope, ?array $args, array $allowConfig
232232
* @param Scope $scope
233233
* @param array<Arg>|null $args
234234
* @param DisallowedWithParams $disallowed
235-
* @param bool $defaultResult
235+
* @param bool $allowedByDefault What to return when no param condition applies
236236
* @return bool
237237
*/
238-
private function hasAllowedParamsInAllowed(Scope $scope, ?array $args, DisallowedWithParams $disallowed, bool $defaultResult = true): bool
238+
private function hasAllowedParamsInAllowed(Scope $scope, ?array $args, DisallowedWithParams $disallowed, bool $allowedByDefault): bool
239239
{
240+
if ($args === null) {
241+
if ($disallowed->getAllowExceptParamsInAllowed()) {
242+
return true;
243+
}
244+
if ($disallowed->getAllowParamsInAllowed()) {
245+
return false;
246+
}
247+
return $allowedByDefault;
248+
}
240249
if ($disallowed->getAllowExceptParamsInAllowed()) {
241250
return $this->hasAllowedParams($scope, $args, $disallowed->getAllowExceptParamsInAllowed(), false);
242251
}
243252
if ($disallowed->getAllowParamsInAllowed()) {
244253
return $this->hasAllowedParams($scope, $args, $disallowed->getAllowParamsInAllowed(), true);
245254
}
246-
return $defaultResult;
255+
return $allowedByDefault;
247256
}
248257

249258

‎tests/Calls/FunctionFirstClassCallablesAllowExceptInMethodsWithParamsTest.php‎

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
<?php
2+
declare(strict_types = 1);
3+
4+
namespace Spaze\PHPStan\Rules\Disallowed\Calls;
5+
6+
use PHPStan\Rules\Rule;
7+
use PHPStan\ShouldNotHappenException;
8+
use PHPStan\Testing\RuleTestCase;
9+
use PHPUnit\Framework\Attributes\RequiresPhp;
10+
use Spaze\PHPStan\Rules\Disallowed\DisallowedCallFactory;
11+
use Spaze\PHPStan\Rules\Disallowed\RuleErrors\DisallowedFunctionRuleErrors;
12+
13+
/**
14+
* @extends RuleTestCase<FunctionFirstClassCallables>
15+
*/
16+
class FunctionFirstClassCallablesAllowExceptInMethodsWithParamsTest extends RuleTestCase
17+
{
18+
19+
/**
20+
* @throws ShouldNotHappenException
21+
*/
22+
protected function getRule(): Rule
23+
{
24+
$container = self::getContainer();
25+
return new FunctionFirstClassCallables(
26+
$container->getByType(DisallowedFunctionRuleErrors::class),
27+
$container->getByType(DisallowedCallFactory::class),
28+
[
29+
[
30+
'function' => 'crc32()',
31+
'allowExceptInMethods' => [
32+
'Fiction\Pulp\RoyaleExceptFirstClassCallable::methodA()',
33+
],
34+
'allowParamsInAllowed' => [
35+
1 => 'a',
36+
],
37+
],
38+
[
39+
'function' => 'strtolower()',
40+
'allowExceptInMethods' => [
41+
'Fiction\Pulp\RoyaleExceptFirstClassCallable::methodA()',
42+
],
43+
'allowExceptParamsInAllowed' => [
44+
1 => 'a',
45+
],
46+
],
47+
]
48+
);
49+
}
50+
51+
52+
/**
53+
* @requires PHP >= 8.1.0
54+
*/
55+
#[RequiresPhp('>= 8.1.0')]
56+
public function testRule(): void
57+
{
58+
$this->analyse([__DIR__ . '/../src/RoyaleExceptFirstClassCallable.php'], [
59+
[
60+
'Calling crc32() is forbidden.',
61+
11,
62+
],
63+
]);
64+
}
65+
66+
67+
public static function getAdditionalConfigFiles(): array
68+
{
69+
return [
70+
__DIR__ . '/../../extension.neon',
71+
];
72+
}
73+
74+
}

‎tests/Calls/FunctionFirstClassCallablesAllowInMethodsWithParamsTest.php‎

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
<?php
2+
declare(strict_types = 1);
3+
4+
namespace Spaze\PHPStan\Rules\Disallowed\Calls;
5+
6+
use PHPStan\Rules\Rule;
7+
use PHPStan\ShouldNotHappenException;
8+
use PHPStan\Testing\RuleTestCase;
9+
use PHPUnit\Framework\Attributes\RequiresPhp;
10+
use Spaze\PHPStan\Rules\Disallowed\DisallowedCallFactory;
11+
use Spaze\PHPStan\Rules\Disallowed\RuleErrors\DisallowedFunctionRuleErrors;
12+
13+
/**
14+
* @extends RuleTestCase<FunctionFirstClassCallables>
15+
*/
16+
class FunctionFirstClassCallablesAllowInMethodsWithParamsTest extends RuleTestCase
17+
{
18+
19+
/**
20+
* @throws ShouldNotHappenException
21+
*/
22+
protected function getRule(): Rule
23+
{
24+
$container = self::getContainer();
25+
return new FunctionFirstClassCallables(
26+
$container->getByType(DisallowedFunctionRuleErrors::class),
27+
$container->getByType(DisallowedCallFactory::class),
28+
[
29+
[
30+
'function' => 'crc32()',
31+
'allowInMethods' => [
32+
'Fiction\Pulp\RoyaleAllowInFirstClassCallable::methodA()',
33+
],
34+
'allowParamsInAllowed' => [
35+
1 => 'a',
36+
],
37+
],
38+
[
39+
'function' => 'strtolower()',
40+
'allowInMethods' => [
41+
'Fiction\Pulp\RoyaleAllowInFirstClassCallable::methodA()',
42+
],
43+
'allowExceptParamsInAllowed' => [
44+
1 => 'a',
45+
],
46+
],
47+
]
48+
);
49+
}
50+
51+
52+
/**
53+
* @requires PHP >= 8.1.0
54+
*/
55+
#[RequiresPhp('>= 8.1.0')]
56+
public function testRule(): void
57+
{
58+
$this->analyse([__DIR__ . '/../src/RoyaleAllowInFirstClassCallable.php'], [
59+
[
60+
'Calling crc32() is forbidden.',
61+
11,
62+
],
63+
[
64+
'Calling crc32() is forbidden.',
65+
18,
66+
],
67+
[
68+
'Calling strtolower() is forbidden.',
69+
19,
70+
],
71+
]);
72+
}
73+
74+
75+
public static function getAdditionalConfigFiles(): array
76+
{
77+
return [
78+
__DIR__ . '/../../extension.neon',
79+
];
80+
}
81+
82+
}

‎tests/Calls/FunctionFirstClassCallablesParamsAnywhereTest.php‎

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
<?php
2+
declare(strict_types = 1);
3+
4+
namespace Spaze\PHPStan\Rules\Disallowed\Calls;
5+
6+
use PHPStan\Rules\Rule;
7+
use PHPStan\ShouldNotHappenException;
8+
use PHPStan\Testing\RuleTestCase;
9+
use PHPUnit\Framework\Attributes\RequiresPhp;
10+
use Spaze\PHPStan\Rules\Disallowed\DisallowedCallFactory;
11+
use Spaze\PHPStan\Rules\Disallowed\RuleErrors\DisallowedFunctionRuleErrors;
12+
13+
/**
14+
* @extends RuleTestCase<FunctionFirstClassCallables>
15+
*/
16+
class FunctionFirstClassCallablesParamsAnywhereTest extends RuleTestCase
17+
{
18+
19+
/**
20+
* @throws ShouldNotHappenException
21+
*/
22+
protected function getRule(): Rule
23+
{
24+
$container = self::getContainer();
25+
return new FunctionFirstClassCallables(
26+
$container->getByType(DisallowedFunctionRuleErrors::class),
27+
$container->getByType(DisallowedCallFactory::class),
28+
[
29+
[
30+
'function' => 'crc32()',
31+
'allowParamsAnywhere' => [
32+
1 => 'a',
33+
],
34+
],
35+
[
36+
'function' => 'strtolower()',
37+
'allowExceptParamsAnywhere' => [
38+
1 => 'a',
39+
],
40+
],
41+
]
42+
);
43+
}
44+
45+
46+
/**
47+
* @requires PHP >= 8.1.0
48+
*/
49+
#[RequiresPhp('>= 8.1.0')]
50+
public function testRule(): void
51+
{
52+
$this->analyse([__DIR__ . '/../src/FirstClassCallableParamsAnywhere.php'], [
53+
[
54+
'Calling crc32() is forbidden.',
55+
4,
56+
],
57+
]);
58+
}
59+
60+
61+
public static function getAdditionalConfigFiles(): array
62+
{
63+
return [
64+
__DIR__ . '/../../extension.neon',
65+
];
66+
}
67+
68+
}

‎tests/src/FirstClassCallableParamsAnywhere.php‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
<?php
2+
declare(strict_types = 1);
3+
4+
$fn = crc32(...); // disallowed: allowParamsAnywhere, null args can't satisfy param condition
5+
$fn = strtolower(...); // allowed: allowExceptParamsAnywhere, null args don't trigger disallow condition

‎tests/src/RoyaleAllowInFirstClassCallable.php‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
<?php
2+
declare(strict_types = 1);
3+
4+
namespace Fiction\Pulp;
5+
6+
class RoyaleAllowInFirstClassCallable
7+
{
8+
9+
public function methodA(): void
10+
{
11+
$fn = crc32(...); // disallowed: in allowed zone, null args can't satisfy allowParamsInAllowed
12+
$fn = strtolower(...); // allowed: in allowed zone, null args can't trigger allowExceptParamsInAllowed
13+
}
14+
15+
16+
public function methodB(): void
17+
{
18+
$fn = crc32(...); // disallowed: not in allowed zone
19+
$fn = strtolower(...); // disallowed: not in allowed zone
20+
}
21+
22+
}

‎tests/src/RoyaleExceptFirstClassCallable.php‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
<?php
2+
declare(strict_types = 1);
3+
4+
namespace Fiction\Pulp;
5+
6+
class RoyaleExceptFirstClassCallable
7+
{
8+
9+
public function methodA(): void
10+
{
11+
$fn = crc32(...); // disallowed: in except zone, null args can't satisfy allowParamsInAllowed
12+
$fn = strtolower(...); // allowed: in except zone, null args can't trigger allowExceptParamsInAllowed
13+
}
14+
15+
16+
public function methodB(): void
17+
{
18+
$fn = crc32(...); // allowed: not in except zone
19+
$fn = strtolower(...); // allowed: not in except zone
20+
}
21+
22+
}

0 commit comments

Comments
 (0)