Skip to content

Commit

Permalink
Can add more rules for the same call to have different messages for v…
Browse files Browse the repository at this point in the history
…arious params (#232)

The rules have to be sorted from the most specific to the most general to have different messages for different params, see `FunctionCallsParamsMessagesTest`.

This makes sense mostly|only when you want to show a different message when various params have been disallowed. For which the new config options `allowExceptParamsAnyValue` and `disallowParamsAnyValue` (which is an alias for the former) have been added.

Close #155
  • Loading branch information
spaze committed Dec 15, 2023
2 parents 5d722f7 + a6056d8 commit ee8ec5f
Show file tree
Hide file tree
Showing 8 changed files with 208 additions and 6 deletions.
16 changes: 16 additions & 0 deletions docs/allow-with-parameters.md
Expand Up @@ -96,6 +96,22 @@ parameters:
```
will disallow `foo('bar', 'baz')` but not `foo('bar', 'BAZ')`.

If you don't care about the value but would like to disallow a call based just on the parameter presence, you can use `allowExceptParamsAnyValue` (or `disallowParamsAnyValue`):
```neon
parameters:
disallowedFunctionCalls:
-
function: 'waldo()'
disallowParamsAnyValue:
-
position: 1
name: 'message'
-
position: 2
name: 'alert'
```
This configuration will disallow calls like `waldo('foo', 'bar')` or `waldo('*', '*')`, but `waldo('foo')` or `waldo()` will be still allowed.

It's also possible to disallow functions and methods previously allowed by path (using `allowIn`) or by function/method name (`allowInMethods`) when they're called with specified parameters, and allow when called with any other parameter. This is done using the `allowExceptParamsInAllowed` config option.

Take this example configuration:
Expand Down
8 changes: 8 additions & 0 deletions extension.neon
Expand Up @@ -67,6 +67,8 @@ parametersSchema:
?disallowParamsInAllowed: arrayOf(anyOf(int(), string(), bool(), structure([position: int(), ?value: anyOf(int(), string(), bool()), ?name: string()])), anyOf(int(), string())),
?allowExceptParams: arrayOf(anyOf(int(), string(), bool(), structure([position: int(), ?value: anyOf(int(), string(), bool()), ?name: string()])), anyOf(int(), string())),
?disallowParams: arrayOf(anyOf(int(), string(), bool(), structure([position: int(), ?value: anyOf(int(), string(), bool()), ?name: string()])), anyOf(int(), string())),
?allowExceptParamsAnyValue: arrayOf(anyOf(int(), structure([position: int(), ?name: string()])), anyOf(int(), string())),
?disallowParamsAnyValue: arrayOf(anyOf(int(), structure([position: int(), ?name: string()])), anyOf(int(), string())),
?allowExceptParamFlags: arrayOf(anyOf(int(), structure([position: int(), ?value: int(), ?name: string()])), anyOf(int(), string())),
?disallowParamFlags: arrayOf(anyOf(int(), structure([position: int(), ?value: int(), ?name: string()])), anyOf(int(), string())),
?allowExceptCaseInsensitiveParams: arrayOf(anyOf(int(), string(), bool(), structure([position: int(), ?value: anyOf(int(), string(), bool()), ?name: string()])), anyOf(int(), string())),
Expand Down Expand Up @@ -103,6 +105,8 @@ parametersSchema:
?disallowParamsInAllowed: arrayOf(anyOf(int(), string(), bool(), structure([position: int(), ?value: anyOf(int(), string(), bool()), ?name: string()])), anyOf(int(), string())),
?allowExceptParams: arrayOf(anyOf(int(), string(), bool(), structure([position: int(), ?value: anyOf(int(), string(), bool()), ?name: string()])), anyOf(int(), string())),
?disallowParams: arrayOf(anyOf(int(), string(), bool(), structure([position: int(), ?value: anyOf(int(), string(), bool()), ?name: string()])), anyOf(int(), string())),
?allowExceptParamsAnyValue: arrayOf(anyOf(int(), structure([position: int(), ?name: string()])), anyOf(int(), string())),
?disallowParamsAnyValue: arrayOf(anyOf(int(), structure([position: int(), ?name: string()])), anyOf(int(), string())),
?allowExceptParamFlags: arrayOf(anyOf(int(), structure([position: int(), ?value: int(), ?name: string()])), anyOf(int(), string())),
?disallowParamFlags: arrayOf(anyOf(int(), structure([position: int(), ?value: int(), ?name: string()])), anyOf(int(), string())),
?allowExceptCaseInsensitiveParams: arrayOf(anyOf(int(), string(), bool(), structure([position: int(), ?value: anyOf(int(), string(), bool()), ?name: string()])), anyOf(int(), string())),
Expand Down Expand Up @@ -139,6 +143,8 @@ parametersSchema:
?disallowParamsInAllowed: arrayOf(anyOf(int(), string(), bool(), structure([position: int(), ?value: anyOf(int(), string(), bool()), ?name: string()])), anyOf(int(), string())),
?allowExceptParams: arrayOf(anyOf(int(), string(), bool(), structure([position: int(), ?value: anyOf(int(), string(), bool()), ?name: string()])), anyOf(int(), string())),
?disallowParams: arrayOf(anyOf(int(), string(), bool(), structure([position: int(), ?value: anyOf(int(), string(), bool()), ?name: string()])), anyOf(int(), string())),
?allowExceptParamsAnyValue: arrayOf(anyOf(int(), structure([position: int(), ?name: string()])), anyOf(int(), string())),
?disallowParamsAnyValue: arrayOf(anyOf(int(), structure([position: int(), ?name: string()])), anyOf(int(), string())),
?allowExceptParamFlags: arrayOf(anyOf(int(), structure([position: int(), ?value: int(), ?name: string()])), anyOf(int(), string())),
?disallowParamFlags: arrayOf(anyOf(int(), structure([position: int(), ?value: int(), ?name: string()])), anyOf(int(), string())),
?allowExceptCaseInsensitiveParams: arrayOf(anyOf(int(), string(), bool(), structure([position: int(), ?value: anyOf(int(), string(), bool()), ?name: string()])), anyOf(int(), string())),
Expand Down Expand Up @@ -196,6 +202,8 @@ parametersSchema:
?disallowParamsInAllowed: arrayOf(anyOf(int(), string(), bool(), structure([position: int(), ?value: anyOf(int(), string(), bool()), ?name: string()])), anyOf(int(), string())),
?allowExceptParams: arrayOf(anyOf(int(), string(), bool(), structure([position: int(), ?value: anyOf(int(), string(), bool()), ?name: string()])), anyOf(int(), string())),
?disallowParams: arrayOf(anyOf(int(), string(), bool(), structure([position: int(), ?value: anyOf(int(), string(), bool()), ?name: string()])), anyOf(int(), string())),
?allowExceptParamsAnyValue: arrayOf(anyOf(int(), structure([position: int(), ?name: string()])), anyOf(int(), string())),
?disallowParamsAnyValue: arrayOf(anyOf(int(), structure([position: int(), ?name: string()])), anyOf(int(), string())),
?allowExceptParamFlags: arrayOf(anyOf(int(), structure([position: int(), ?value: int(), ?name: string()])), anyOf(int(), string())),
?disallowParamFlags: arrayOf(anyOf(int(), structure([position: int(), ?value: int(), ?name: string()])), anyOf(int(), string())),
?allowExceptCaseInsensitiveParams: arrayOf(anyOf(int(), string(), bool(), structure([position: int(), ?value: anyOf(int(), string(), bool()), ?name: string()])), anyOf(int(), string())),
Expand Down
2 changes: 1 addition & 1 deletion phpstan.neon
Expand Up @@ -6,7 +6,7 @@ parameters:
CallParamConfig: 'array<int|string, int|bool|string|array{position:int, value?:int|bool|string, name?:string}>'
CallParamAnyValueConfig: 'array<int|string, int|array{position:int, value?:int|bool|string, name?:string}>'
CallParamFlagAnyValueConfig: 'array<int|string, int|array{position:int, value?:int, name?:string}>'
AllowDirectives: 'allowIn?:list<string>, allowExceptIn?:list<string>, disallowIn?:list<string>, allowInFunctions?:list<string>, allowInMethods?:list<string>, allowExceptInFunctions?:list<string>, allowExceptInMethods?:list<string>, disallowInFunctions?:list<string>, disallowInMethods?:list<string>, allowParamsInAllowed?:CallParamConfig, allowParamsInAllowedAnyValue?:CallParamAnyValueConfig, allowParamFlagsInAllowed?:CallParamFlagAnyValueConfig, allowParamsAnywhere?:CallParamConfig, allowParamsAnywhereAnyValue?:CallParamAnyValueConfig, allowParamFlagsAnywhere?:CallParamFlagAnyValueConfig, allowExceptParamsInAllowed?:CallParamConfig, allowExceptParamFlagsInAllowed?:CallParamFlagAnyValueConfig, disallowParamFlagsInAllowed?:CallParamFlagAnyValueConfig, disallowParamsInAllowed?:CallParamConfig, allowExceptParams?:CallParamConfig, disallowParams?:CallParamConfig, allowExceptParamFlags?:CallParamFlagAnyValueConfig, disallowParamFlags?:CallParamFlagAnyValueConfig, allowExceptCaseInsensitiveParams?:CallParamConfig, disallowCaseInsensitiveParams?:CallParamConfig'
AllowDirectives: 'allowIn?:list<string>, allowExceptIn?:list<string>, disallowIn?:list<string>, allowInFunctions?:list<string>, allowInMethods?:list<string>, allowExceptInFunctions?:list<string>, allowExceptInMethods?:list<string>, disallowInFunctions?:list<string>, disallowInMethods?:list<string>, allowParamsInAllowed?:CallParamConfig, allowParamsInAllowedAnyValue?:CallParamAnyValueConfig, allowParamFlagsInAllowed?:CallParamFlagAnyValueConfig, allowParamsAnywhere?:CallParamConfig, allowParamsAnywhereAnyValue?:CallParamAnyValueConfig, allowParamFlagsAnywhere?:CallParamFlagAnyValueConfig, allowExceptParamsInAllowed?:CallParamConfig, allowExceptParamFlagsInAllowed?:CallParamFlagAnyValueConfig, disallowParamFlagsInAllowed?:CallParamFlagAnyValueConfig, disallowParamsInAllowed?:CallParamConfig, allowExceptParams?:CallParamConfig, disallowParams?:CallParamConfig, allowExceptParamsAnyValue?:CallParamAnyValueConfig, disallowParamsAnyValue?:CallParamAnyValueConfig, allowExceptParamFlags?:CallParamFlagAnyValueConfig, disallowParamFlags?:CallParamFlagAnyValueConfig, allowExceptCaseInsensitiveParams?:CallParamConfig, disallowCaseInsensitiveParams?:CallParamConfig'
ForbiddenCallsConfig: 'array<array{function?:string|list<string>, method?:string|list<string>, exclude?:string|list<string>, definedIn?:string|list<string>, message?:string, %typeAliases.AllowDirectives%, errorIdentifier?:string, errorTip?:string}>'
DisallowedAttributesConfig: 'array<array{attribute:string|list<string>, exclude?:string|list<string>, message?:string, %typeAliases.AllowDirectives%, errorIdentifier?:string, errorTip?:string}>'
AllowDirectivesConfig: 'array{%typeAliases.AllowDirectives%}'
Expand Down
13 changes: 8 additions & 5 deletions src/Allowed/Allowed.php
Expand Up @@ -19,6 +19,7 @@
use Spaze\PHPStan\Rules\Disallowed\Params\ParamValueAny;
use Spaze\PHPStan\Rules\Disallowed\Params\ParamValueCaseInsensitiveExcept;
use Spaze\PHPStan\Rules\Disallowed\Params\ParamValueExcept;
use Spaze\PHPStan\Rules\Disallowed\Params\ParamValueExceptAny;
use Spaze\PHPStan\Rules\Disallowed\Params\ParamValueFlagExcept;
use Spaze\PHPStan\Rules\Disallowed\Params\ParamValueFlagSpecific;
use Spaze\PHPStan\Rules\Disallowed\Params\ParamValueSpecific;
Expand Down Expand Up @@ -111,6 +112,7 @@ private function hasAllowedParams(Scope $scope, ?array $args, array $allowConfig
return true;
}

$disallowedParams = false;
foreach ($allowConfig as $param) {
$type = $this->getArgType($args, $scope, $param);
if ($type === null) {
Expand All @@ -123,15 +125,13 @@ private function hasAllowedParams(Scope $scope, ?array $args, array $allowConfig
}
foreach ($types as $type) {
try {
if (!$param->matches($type)) {
return false;
}
$disallowedParams = $disallowedParams || !$param->matches($type);
} catch (UnsupportedParamTypeException $e) {
return !$paramsRequired;
}
}
}
return true;
return !$disallowedParams;
}


Expand Down Expand Up @@ -216,6 +216,9 @@ public function getConfig(array $allowed): AllowedConfig
foreach ($allowed['allowExceptParams'] ?? $allowed['disallowParams'] ?? [] as $param => $value) {
$allowExceptParams[$param] = $this->paramFactory(ParamValueExcept::class, $param, $value);
}
foreach ($allowed['allowExceptParamsAnyValue'] ?? $allowed['disallowParamsAnyValue'] ?? [] as $param => $value) {
$allowExceptParams[$param] = $this->paramFactory(ParamValueExceptAny::class, $param, $value);
}
foreach ($allowed['allowExceptParamFlags'] ?? $allowed['disallowParamFlags'] ?? [] as $param => $value) {
$allowExceptParams[$param] = $this->paramFactory(ParamValueFlagExcept::class, $param, $value);
}
Expand Down Expand Up @@ -250,7 +253,7 @@ private function paramFactory(string $class, $key, $value): ParamValue
$paramPosition = $value['position'];
$paramName = $value['name'] ?? null;
$paramValue = $value['value'] ?? null;
} elseif ($class === ParamValueAny::class) {
} elseif (in_array($class, [ParamValueAny::class, ParamValueExceptAny::class], true)) {
if (is_numeric($value)) {
$paramPosition = (int)$value;
$paramName = null;
Expand Down
19 changes: 19 additions & 0 deletions src/Params/ParamValueExceptAny.php
@@ -0,0 +1,19 @@
<?php
declare(strict_types = 1);

namespace Spaze\PHPStan\Rules\Disallowed\Params;

use PHPStan\Type\Type;

/**
* @extends ParamValue<int|bool|string|null>
*/
final class ParamValueExceptAny extends ParamValue
{

public function matches(Type $type): bool
{
return false;
}

}
136 changes: 136 additions & 0 deletions tests/Calls/FunctionCallsParamsMessagesTest.php
@@ -0,0 +1,136 @@
<?php
declare(strict_types = 1);

namespace Spaze\PHPStan\Rules\Disallowed\Calls;

use PHPStan\Rules\Rule;
use PHPStan\ShouldNotHappenException;
use PHPStan\Testing\RuleTestCase;
use Spaze\PHPStan\Rules\Disallowed\DisallowedCallFactory;
use Spaze\PHPStan\Rules\Disallowed\RuleErrors\DisallowedCallsRuleErrors;

class FunctionCallsParamsMessagesTest extends RuleTestCase
{

/**
* @throws ShouldNotHappenException
*/
protected function getRule(): Rule
{
$container = self::getContainer();
return new FunctionCalls(
$container->getByType(DisallowedCallsRuleErrors::class),
$container->getByType(DisallowedCallFactory::class),
$this->createReflectionProvider(),
[
[
'function' => '\Foo\Bar\Waldo\config()',
'message' => 'foo & bar',
'allowIn' => [
__DIR__ . '/../src/disallowed-allow/*.php',
__DIR__ . '/../src/*-allow/*.*',
],
'disallowParamsAnyValue' => [
1,
2,
],
],
[
'function' => '\Foo\Bar\Waldo\config()',
'message' => 'foo',
'allowIn' => [
__DIR__ . '/../src/disallowed-allow/*.php',
__DIR__ . '/../src/*-allow/*.*',
],
'disallowParamsAnyValue' => [
1,
],
],
[
'function' => '\Foo\Bar\Waldo\config()',
'message' => 'nothing',
'allowIn' => [
__DIR__ . '/../src/disallowed-allow/*.php',
__DIR__ . '/../src/*-allow/*.*',
],
],
[
'function' => '\Foo\Bar\Waldo\bar()',
'message' => 'foo & bar',
'allowIn' => [
__DIR__ . '/../src/disallowed-allow/*.php',
__DIR__ . '/../src/*-allow/*.*',
],
'disallowParams' => [
1 => 'foo',
2 => 'bar',
],
],
[
'function' => '\Foo\Bar\Waldo\bar()',
'message' => 'foo',
'allowIn' => [
__DIR__ . '/../src/disallowed-allow/*.php',
__DIR__ . '/../src/*-allow/*.*',
],
'disallowParams' => [
1 => 'foo',
],
],
[
'function' => '\Foo\Bar\Waldo\bar()',
'message' => 'nothing',
'allowIn' => [
__DIR__ . '/../src/disallowed-allow/*.php',
__DIR__ . '/../src/*-allow/*.*',
],
],
]
);
}


public function testRule(): void
{
// Based on the configuration above, in this file:
$this->analyse([__DIR__ . '/../src/disallowed/functionCallsParamsMessages.php'], [
[
// expect this error message:
'Calling Foo\Bar\Waldo\config() is forbidden, nothing.',
// on this line:
5,
],
[
'Calling Foo\Bar\Waldo\config() is forbidden, foo.',
6,
],
[
'Calling Foo\Bar\Waldo\config() is forbidden, foo & bar.',
7,
],
[
'Calling Foo\Bar\Waldo\bar() is forbidden, nothing.',
8,
],
[
'Calling Foo\Bar\Waldo\bar() is forbidden, foo.',
9,
],
[
'Calling Foo\Bar\Waldo\bar() is forbidden, foo & bar.',
10,
],
]);
// Based on the configuration above, no errors in this file:
$this->analyse([__DIR__ . '/../src/disallowed-allow/functionCallsParamsMessages.php'], []);
}


public static function getAdditionalConfigFiles(): array
{
return [
__DIR__ . '/../../extension.neon',
];
}

}
10 changes: 10 additions & 0 deletions tests/src/disallowed-allow/functionCallsParamsMessages.php
@@ -0,0 +1,10 @@
<?php
declare(strict_types = 1);

// allowed by path
\Foo\Bar\Waldo\config();
\Foo\Bar\Waldo\config('foo');
\Foo\Bar\Waldo\config('foo', 'bar');
\Foo\Bar\Waldo\bar();
\Foo\Bar\Waldo\bar('foo');
\Foo\Bar\Waldo\bar('foo', 'bar');
10 changes: 10 additions & 0 deletions tests/src/disallowed/functionCallsParamsMessages.php
@@ -0,0 +1,10 @@
<?php
declare(strict_types = 1);

// disallowed with different messages
\Foo\Bar\Waldo\config();
\Foo\Bar\Waldo\config('foo');
\Foo\Bar\Waldo\config('foo', 'bar');
\Foo\Bar\Waldo\bar();
\Foo\Bar\Waldo\bar('foo');
\Foo\Bar\Waldo\bar('foo', 'bar');

0 comments on commit ee8ec5f

Please sign in to comment.