diff --git a/README.md b/README.md index 446f1f3..99dbdb9 100644 --- a/README.md +++ b/README.md @@ -85,7 +85,7 @@ The `message` key is optional. Don't forget to add the `DisallowedHelper` servic ------ -------------------------------------------------------- ``` -## Ignore some calls +## Allow some previously disallowed calls Sometimes, the method or the function needs to be called once in your code, for example in a custom wrapper. You can use PHPStan's [`ignoreErrors` feature](https://github.com/phpstan/phpstan#ignore-error-messages-with-regular-expressions) to ignore that one call: @@ -123,6 +123,30 @@ services: The paths in `allowIn` are relative to the config file location and support [fnmatch()](https://www.php.net/function.fnmatch) patterns. +You can also narrow down the allowed items when called with some parameters. For example, you want to disallow calling `print_r()` but want to allow `print_r(..., true)`. +This can be done with optional `allowParamsInAllowed` or `allowParamsAnywhere` configuration keys: + +``` +arguments: + forbiddenCalls: + - + method: 'Tracy\ILogger::log()' + message: 'use our own logger instead' + allowIn: + - optional/path/to/*.tests.php + - another/file.php + allowParamsInAllowed: + 1: 'foo' + 2: true + allowParamsAnywhere: + 2: true +``` + +When using `allowParamsInAllowed`, calls will be allowed only when they are in one of the `allowIn` paths, and are called with all parameters listed in `allowParamsInAllowed`. +With `allowParamsAnywhere`, calls are allowed when called with all parameters listed no matter in which file. In the example above, the `log()` method will be disallowed unless called as: +- `log(..., true)` anywhere +- `log('foo', true)` in `another/file.php` or `optional/path/to/log.tests.php` + ## Running tests If you want to contribute (awesome, thanks!), you should add/run tests for your contributions. diff --git a/src/DisallowedHelper.php b/src/DisallowedHelper.php index 95a2818..c22f9f3 100644 --- a/src/DisallowedHelper.php +++ b/src/DisallowedHelper.php @@ -3,6 +3,8 @@ namespace Spaze\PHPStan\Rules\Disallowed; +use PhpParser\Node\Arg; +use PhpParser\Node\Scalar; use PHPStan\File\FileHelper; class DisallowedHelper @@ -20,17 +22,62 @@ public function __construct(FileHelper $fileHelper) /** * @param string $file + * @param Arg[] $args * @param string[] $config * @return boolean */ - public function isAllowed(string $file, array $config): bool + public function isAllowed(string $file, array $args, array $config): bool { foreach (($config['allowIn'] ?? []) as $allowedPath) { - if (fnmatch($this->fileHelper->absolutizePath($allowedPath), $file)) { + if (fnmatch($this->fileHelper->absolutizePath($allowedPath), $file) && $this->hasAllowedParams($config, $args, 'allowParamsInAllowed', true)) { return true; } } - return false; + return $this->hasAllowedParams($config, $args, 'allowParamsAnywhere', false); + } + + + /** + * @param string[] $config + * @param Arg[] $args + * @param string $configKey + * @param boolean $default + * @return boolean + */ + private function hasAllowedParams(array $config, array $args, string $configKey, bool $default): bool + { + if (isset($config[$configKey]) && is_array($config[$configKey])) { + $disallowed = false; + foreach ($config[$configKey] as $param => $value) { + if (isset($args[$param - 1])) { + $arg = $args[$param - 1]; + $disallowed = $disallowed || $this->isDisallowedParam($value, $arg); + } else { + $disallowed = true; + } + } + if (count($config[$configKey]) > 0) { + return !$disallowed; + } + } + return $default; + } + + + /** + * @param mixed $value + * @param Arg $arg + * @return boolean + */ + private function isDisallowedParam($value, Arg $arg): bool + { + // 2nd param in print_r(..., true) is returned as Node\Expr\ConstFetch by the parser and I can't find a way to + // get it as a bool, only as a string. So to support booleans in the .neon config file we have to convert to a string manually. + if (is_bool($value)) { + return $this->isDisallowedParam($value ? 'true' : 'false', $arg); + } else { + return ($value !== ($arg->value instanceof Scalar ? $arg->value->value : (string)$arg->value->name)); + } } } diff --git a/src/FunctionCalls.php b/src/FunctionCalls.php index 24e459e..73a06f3 100644 --- a/src/FunctionCalls.php +++ b/src/FunctionCalls.php @@ -22,6 +22,11 @@ * allowIn: * - optional/path/to/*.tests.php * - another/file.php + * allowParamsInAllowed: + * 1: 'foo' + * 2: true + * allowParamsAnywhere: + * 2: true * - * function: 'Foo\Bar\baz()' * message: 'waldo instead' @@ -65,7 +70,7 @@ public function processNode(Node $node, Scope $scope): array $name = $node->name . '()'; foreach ($this->forbiddenCalls as $forbiddenCall) { - if ($name === $forbiddenCall['function'] && !$this->disallowedHelper->isAllowed($scope->getFile(), $forbiddenCall)) { + if ($name === $forbiddenCall['function'] && !$this->disallowedHelper->isAllowed($scope->getFile(), $node->args, $forbiddenCall)) { return [ sprintf('Calling %s is forbidden, %s', $name, $forbiddenCall['message'] ?? 'because reasons'), ]; diff --git a/src/MethodCalls.php b/src/MethodCalls.php index 3dfb8a4..ac567c5 100644 --- a/src/MethodCalls.php +++ b/src/MethodCalls.php @@ -27,6 +27,11 @@ * allowIn: * - optional/path/to/*.tests.php * - another/file.php + * allowParamsInAllowed: + * 1: 'foo' + * 2: true + * allowParamsAnywhere: + * 2: true * - * method: 'Foo\Bar::baz()' * message: 'waldo instead' @@ -86,7 +91,7 @@ static function (Type $type) use ($name): bool { foreach ($typeResult->getReferencedClasses() as $referencedClass) { $fullyQualified = current($typeResult->getReferencedClasses()) . "::{$name}()"; foreach ($this->forbiddenCalls as $forbiddenCall) { - if ($fullyQualified === $forbiddenCall['method'] && !$this->disallowedHelper->isAllowed($scope->getFile(), $forbiddenCall)) { + if ($fullyQualified === $forbiddenCall['method'] && !$this->disallowedHelper->isAllowed($scope->getFile(), $node->args, $forbiddenCall)) { return [ sprintf('Calling %s is forbidden, %s', $fullyQualified, $forbiddenCall['message'] ?? 'because reasons'), ]; diff --git a/src/StaticCalls.php b/src/StaticCalls.php index c07d8f2..14befdc 100644 --- a/src/StaticCalls.php +++ b/src/StaticCalls.php @@ -24,6 +24,11 @@ * allowIn: * - optional/path/to/*.tests.php * - another/file.php + * allowParamsInAllowed: + * 1: 'foo' + * 2: true + * allowParamsAnywhere: + * 2: true * - * method: 'Foo\Bar::baz()' * message: 'waldo instead' @@ -69,7 +74,7 @@ public function processNode(Node $node, Scope $scope): array $name = $node->name->name; $fullyQualified = "{$node->class}::{$name}()"; foreach ($this->forbiddenCalls as $forbiddenCall) { - if ($fullyQualified === $forbiddenCall['method'] && !$this->disallowedHelper->isAllowed($scope->getFile(), $forbiddenCall)) { + if ($fullyQualified === $forbiddenCall['method'] && !$this->disallowedHelper->isAllowed($scope->getFile(), $node->args, $forbiddenCall)) { return [ sprintf('Calling %s is forbidden, %s', $fullyQualified, $forbiddenCall['message'] ?? 'because reasons'), ]; diff --git a/tests/FunctionCallsTest.php b/tests/FunctionCallsTest.php index 7ef9643..20eaebd 100644 --- a/tests/FunctionCallsTest.php +++ b/tests/FunctionCallsTest.php @@ -30,6 +30,9 @@ protected function getRule(): Rule 'data/*-allowed.php', 'data/*-allowed.*', ], + 'allowParamsAnywhere' => [ + 2 => true, + ] ], [ 'function' => 'printf()', @@ -74,6 +77,10 @@ public function testRule(): void 'Calling Foo\Bar\waldo() is forbidden, whoa, a namespace', 11, ], + [ + 'Calling print_r() is forbidden, nope', + 35, + ], ]); $this->analyse([__DIR__ . '/data/disallowed-calls-allowed.php'], []); } diff --git a/tests/MethodCallsTest.php b/tests/MethodCallsTest.php index a0dd550..b44d268 100644 --- a/tests/MethodCallsTest.php +++ b/tests/MethodCallsTest.php @@ -23,6 +23,11 @@ protected function getRule(): Rule 'data/*-allowed.php', 'data/*-allowed.*', ], + 'allowParamsInAllowed' => [ + 1 => 42, + 2 => true, + 3 => '909', + ], ], ] ); @@ -34,10 +39,27 @@ public function testRule(): void $this->analyse([__DIR__ . '/data/disallowed-calls.php'], [ [ "Calling Waldo\Quux\Blade::runner() is forbidden, I've seen tests you people wouldn't believe", - 25, + 28, + ], + [ + "Calling Waldo\Quux\Blade::runner() is forbidden, I've seen tests you people wouldn't believe", + 30, + ], + [ + "Calling Waldo\Quux\Blade::runner() is forbidden, I've seen tests you people wouldn't believe", + 31, + ], + ]); + $this->analyse([__DIR__ . '/data/disallowed-calls-allowed.php'], [ + [ + "Calling Waldo\Quux\Blade::runner() is forbidden, I've seen tests you people wouldn't believe", + 29, + ], + [ + "Calling Waldo\Quux\Blade::runner() is forbidden, I've seen tests you people wouldn't believe", + 32, ], ]); - $this->analyse([__DIR__ . '/data/disallowed-calls-allowed.php'], []); } } diff --git a/tests/StaticCallsTest.php b/tests/StaticCallsTest.php index 3529ef6..4a996b5 100644 --- a/tests/StaticCallsTest.php +++ b/tests/StaticCallsTest.php @@ -22,6 +22,34 @@ protected function getRule(): Rule 'data/*-allowed.php', 'data/*-allowed.*', ], + 'allowParamsInAllowed' => [], + ], + [ + 'method' => 'Fiction\Pulp\Royale::withBadCheese()', + 'message' => 'a Quarter Pounder with Cheese?', + 'allowIn' => [ + 'data/*-allowed.php', + 'data/*-allowed.*', + ], + 'allowParamsInAllowed' => '', + ], + [ + 'method' => 'Fiction\Pulp\Royale::withoutCheese()', + 'message' => 'a Quarter Pounder without Cheese?', + 'allowIn' => [ + 'data/*-allowed.php', + 'data/*-allowed.*', + ], + 'allowParamsInAllowed' => [ + 1 => 1, + 2 => 2, + 3 => 3, + ], + 'allowParamsAnywhere' => [ + 1 => 1, + 2 => 2, + 3 => 4, + ], ], ] ); @@ -39,6 +67,14 @@ public function testRule(): void 'Calling Fiction\Pulp\Royale::withCheese() is forbidden, a Quarter Pounder with Cheese?', 18, ], + [ + 'Calling Fiction\Pulp\Royale::withBadCheese() is forbidden, a Quarter Pounder with Cheese?', + 20, + ], + [ + 'Calling Fiction\Pulp\Royale::withoutCheese() is forbidden, a Quarter Pounder without Cheese?', + 21, + ], ]); $this->analyse([__DIR__ . '/data/disallowed-calls-allowed.php'], []); } diff --git a/tests/data/Blade.php b/tests/data/Blade.php index 66b0b19..c0fd412 100644 --- a/tests/data/Blade.php +++ b/tests/data/Blade.php @@ -6,7 +6,7 @@ class Blade { - public function runner(): void + public function runner(int $everything = 0, bool $yes = false, string $roland = '303'): void { } diff --git a/tests/data/disallowed-calls-allowed.php b/tests/data/disallowed-calls-allowed.php index eb1fda6..90d86ac 100644 --- a/tests/data/disallowed-calls-allowed.php +++ b/tests/data/disallowed-calls-allowed.php @@ -4,7 +4,7 @@ use function Foo\Bar\waldo; -var_dump('foo'); +var_dump('foo', true); print_r('bar'); \printf('foobar'); var_export('not disallowed'); @@ -18,6 +18,9 @@ Pulp\Royale::withCheese(); Pulp\Royale::leBigMac(); +Pulp\Royale::withBadCheese(); +Pulp\Royale::withoutCheese(1, 2, 3); +Pulp\Royale::withoutCheese(1, 2, 4); use Waldo\Quux; @@ -25,3 +28,9 @@ $blade = new Quux\Blade(); $blade->runner(); $blade->server(); +$blade->runner(42, true, '909'); +$blade->runner(42, true, '808'); + +print_r('bar bar', true); +print_r('bar bar baz', true, 303); +print_r('bar bar was', false); diff --git a/tests/data/disallowed-calls.php b/tests/data/disallowed-calls.php index 25b6e27..5250dcc 100644 --- a/tests/data/disallowed-calls.php +++ b/tests/data/disallowed-calls.php @@ -3,7 +3,7 @@ use function Foo\Bar\waldo; -var_dump('foo'); +var_dump('foo', true); print_r('bar'); \printf('foobar'); var_export('not disallowed'); @@ -17,6 +17,9 @@ Pulp\Royale::withCheese(); Pulp\Royale::leBigMac(); +Pulp\Royale::withBadCheese(); +Pulp\Royale::withoutCheese(1, 2, 3); +Pulp\Royale::withoutCheese(1, 2, 4); use Waldo\Quux; @@ -24,3 +27,9 @@ $blade = new Quux\Blade(); $blade->runner(); $blade->server(); +$blade->runner(42, true, '909'); +$blade->runner(42, true, '808'); + +print_r('bar bar', true); +print_r('bar bar baz', true, 303); +print_r('bar bar was', false);