Can allow calls with specified params only, either anywhere, or in al…

…lowedIn paths

Fix #1

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.

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));
+		}
 	}
 
 }

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'),
 				];

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'),
 					];

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'),
 				];

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'], []);
 	}

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'], []);
 	}
 
 }

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'], []);
 	}

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
 	{
 	}
 

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,10 +18,19 @@
 
 Pulp\Royale::withCheese();
 Pulp\Royale::leBigMac();
+Pulp\Royale::withBadCheese();
+Pulp\Royale::withoutCheese(1, 2, 3);
+Pulp\Royale::withoutCheese(1, 2, 4);
 
 
 use Waldo\Quux;
 
 $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);

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,10 +17,19 @@
 
 Pulp\Royale::withCheese();
 Pulp\Royale::leBigMac();
+Pulp\Royale::withBadCheese();
+Pulp\Royale::withoutCheese(1, 2, 3);
+Pulp\Royale::withoutCheese(1, 2, 4);
 
 
 use Waldo\Quux;
 
 $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);