Arrays in param configs not supported at the moment, throw ShouldNotH…

…appenException Ref #165
spaze · Mar 19, 2023 · 4f9d638 · 4f9d638
1 parent cbdf5c9
commit 4f9d638
Show file tree

Hide file tree

Showing 7 changed files with 158 additions and 61 deletions.
diff --git a/README.md b/README.md
@@ -306,7 +306,9 @@ The function or method names support [fnmatch()](https://www.php.net/function.fn
 
 ### Allow with specified parameters only
 
-You can also narrow down the allowed items when called with some parameters (applies only to disallowed method, static & function calls, for obvious reasons). For example, you want to disallow calling `print_r()` but want to allow `print_r(..., true)`.
+You can also narrow down the allowed items when called with some parameters (applies only to disallowed method, static & function calls, for obvious reasons). _Please note that for now, only scalar values are supported in the configuration, not arrays._
+
+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:
 
 ```neon
@@ -368,7 +370,9 @@ Such configuration only makes sense when both the parameters of `log()` are opti
 
 ### Allow calls except when a param has a specified value
 
-Sometimes, it's handy to disallow a function or a method call only when a parameter matches but allow it otherwise. For example the `hash()` function, it's fine using it with algorithm families like SHA-2 & SHA-3 (not for passwords though) but you'd like PHPStan to report when it's used with MD5 like `hash('md5', ...)`.
+Sometimes, it's handy to disallow a function or a method call only when a parameter matches a configured value but allow it otherwise. _Please note that currently only scalar values are supported, not arrays._
+
+For example the `hash()` function, it's fine using it with algorithm families like SHA-2 & SHA-3 (not for passwords though) but you'd like PHPStan to report when it's used with MD5 like `hash('md5', ...)`.
 You can use `allowExceptParams` (or `disallowParams`), `allowExceptCaseInsensitiveParams` (or `disallowCaseInsensitiveParams`), `allowExceptParamsInAllowed` (or `disallowParamsInAllowed`) config options to disallow only some calls:
 
 ```neon

diff --git a/src/DisallowedCallFactory.php b/src/DisallowedCallFactory.php
@@ -4,6 +4,7 @@
 namespace Spaze\PHPStan\Rules\Disallowed;
 
 use PHPStan\ShouldNotHappenException;
+use Spaze\PHPStan\Rules\Disallowed\Exceptions\UnsupportedParamTypeInConfigException;
 use Spaze\PHPStan\Rules\Disallowed\Params\DisallowedCallParamValue;
 use Spaze\PHPStan\Rules\Disallowed\Params\DisallowedCallParamValueAny;
 use Spaze\PHPStan\Rules\Disallowed\Params\DisallowedCallParamValueCaseInsensitiveExcept;
@@ -31,62 +32,68 @@ public function createFromConfig(array $config): array
 			if (!$calls) {
 				throw new ShouldNotHappenException("Either 'method' or 'function' must be set in configuration items");
 			}
-			foreach ((array)$calls as $call) {
-				$allowInCalls = $allowExceptInCalls = $allowParamsInAllowed = $allowParamsAnywhere = $allowExceptParamsInAllowed = $allowExceptParams = [];
-				foreach ($disallowed['allowInFunctions'] ?? $disallowed['allowInMethods'] ?? [] as $allowedCall) {
-					$allowInCalls[] = $this->normalizeCall($allowedCall);
+			$calls = (array)$calls;
+			try {
+				foreach ($calls as $call) {
+					$allowInCalls = $allowExceptInCalls = $allowParamsInAllowed = $allowParamsAnywhere = $allowExceptParamsInAllowed = $allowExceptParams = [];
+					foreach ($disallowed['allowInFunctions'] ?? $disallowed['allowInMethods'] ?? [] as $allowedCall) {
+						$allowInCalls[] = $this->normalizeCall($allowedCall);
+					}
+					foreach ($disallowed['allowExceptInFunctions'] ?? $disallowed['allowExceptInMethods'] ?? $disallowed['disallowInFunctions'] ?? $disallowed['disallowInMethods'] ?? [] as $disallowedCall) {
+						$allowExceptInCalls[] = $this->normalizeCall($disallowedCall);
+					}
+					foreach ($disallowed['allowParamsInAllowed'] ?? [] as $param => $value) {
+						$allowParamsInAllowed[$param] = $this->paramFactory(DisallowedCallParamValueSpecific::class, $param, $value);
+					}
+					foreach ($disallowed['allowParamsInAllowedAnyValue'] ?? [] as $param => $value) {
+						$allowParamsInAllowed[$param] = $this->paramFactory(DisallowedCallParamValueAny::class, $param, $value);
+					}
+					foreach ($disallowed['allowParamFlagsInAllowed'] ?? [] as $param => $value) {
+						$allowParamsInAllowed[$param] = $this->paramFactory(DisallowedCallParamValueFlagSpecific::class, $param, $value);
+					}
+					foreach ($disallowed['allowParamsAnywhere'] ?? [] as $param => $value) {
+						$allowParamsAnywhere[$param] = $this->paramFactory(DisallowedCallParamValueSpecific::class, $param, $value);
+					}
+					foreach ($disallowed['allowParamsAnywhereAnyValue'] ?? [] as $param => $value) {
+						$allowParamsAnywhere[$param] = $this->paramFactory(DisallowedCallParamValueAny::class, $param, $value);
+					}
+					foreach ($disallowed['allowParamFlagsAnywhere'] ?? [] as $param => $value) {
+						$allowParamsAnywhere[$param] = $this->paramFactory(DisallowedCallParamValueFlagSpecific::class, $param, $value);
+					}
+					foreach ($disallowed['allowExceptParamsInAllowed'] ?? $disallowed['disallowParamsInAllowed'] ?? [] as $param => $value) {
+						$allowExceptParamsInAllowed[$param] = $this->paramFactory(DisallowedCallParamValueExcept::class, $param, $value);
+					}
+					foreach ($disallowed['allowExceptParamFlagsInAllowed'] ?? $disallowed['disallowParamFlagsInAllowed'] ?? [] as $param => $value) {
+						$allowExceptParamsInAllowed[$param] = $this->paramFactory(DisallowedCallParamValueFlagExcept::class, $param, $value);
+					}
+					foreach ($disallowed['allowExceptParams'] ?? $disallowed['disallowParams'] ?? [] as $param => $value) {
+						$allowExceptParams[$param] = $this->paramFactory(DisallowedCallParamValueExcept::class, $param, $value);
+					}
+					foreach ($disallowed['allowExceptParamFlags'] ?? $disallowed['disallowParamFlags'] ?? [] as $param => $value) {
+						$allowExceptParams[$param] = $this->paramFactory(DisallowedCallParamValueFlagExcept::class, $param, $value);
+					}
+					foreach ($disallowed['allowExceptCaseInsensitiveParams'] ?? $disallowed['disallowCaseInsensitiveParams'] ?? [] as $param => $value) {
+						$allowExceptParams[$param] = $this->paramFactory(DisallowedCallParamValueCaseInsensitiveExcept::class, $param, $value);
+					}
+					$disallowedCall = new DisallowedCall(
+						$this->normalizeCall($call),
+						$disallowed['message'] ?? null,
+						$disallowed['allowIn'] ?? [],
+						$disallowed['allowExceptIn'] ?? $disallowed['disallowIn'] ?? [],
+						$allowInCalls,
+						$allowExceptInCalls,
+						$allowParamsInAllowed,
+						$allowParamsAnywhere,
+						$allowExceptParamsInAllowed,
+						$allowExceptParams,
+						$disallowed['errorIdentifier'] ?? null,
+						$disallowed['errorTip'] ?? null
+					);
+					$disallowedCalls[$disallowedCall->getKey()] = $disallowedCall;
 				}
-				foreach ($disallowed['allowExceptInFunctions'] ?? $disallowed['allowExceptInMethods'] ?? $disallowed['disallowInFunctions'] ?? $disallowed['disallowInMethods'] ?? [] as $disallowedCall) {
-					$allowExceptInCalls[] = $this->normalizeCall($disallowedCall);
-				}
-				foreach ($disallowed['allowParamsInAllowed'] ?? [] as $param => $value) {
-					$allowParamsInAllowed[$param] = $this->paramFactory(DisallowedCallParamValueSpecific::class, $param, $value);
-				}
-				foreach ($disallowed['allowParamsInAllowedAnyValue'] ?? [] as $param => $value) {
-					$allowParamsInAllowed[$param] = $this->paramFactory(DisallowedCallParamValueAny::class, $param, $value);
-				}
-				foreach ($disallowed['allowParamFlagsInAllowed'] ?? [] as $param => $value) {
-					$allowParamsInAllowed[$param] = $this->paramFactory(DisallowedCallParamValueFlagSpecific::class, $param, $value);
-				}
-				foreach ($disallowed['allowParamsAnywhere'] ?? [] as $param => $value) {
-					$allowParamsAnywhere[$param] = $this->paramFactory(DisallowedCallParamValueSpecific::class, $param, $value);
-				}
-				foreach ($disallowed['allowParamsAnywhereAnyValue'] ?? [] as $param => $value) {
-					$allowParamsAnywhere[$param] = $this->paramFactory(DisallowedCallParamValueAny::class, $param, $value);
-				}
-				foreach ($disallowed['allowParamFlagsAnywhere'] ?? [] as $param => $value) {
-					$allowParamsAnywhere[$param] = $this->paramFactory(DisallowedCallParamValueFlagSpecific::class, $param, $value);
-				}
-				foreach ($disallowed['allowExceptParamsInAllowed'] ?? $disallowed['disallowParamsInAllowed'] ?? [] as $param => $value) {
-					$allowExceptParamsInAllowed[$param] = $this->paramFactory(DisallowedCallParamValueExcept::class, $param, $value);
-				}
-				foreach ($disallowed['allowExceptParamFlagsInAllowed'] ?? $disallowed['disallowParamFlagsInAllowed'] ?? [] as $param => $value) {
-					$allowExceptParamsInAllowed[$param] = $this->paramFactory(DisallowedCallParamValueFlagExcept::class, $param, $value);
-				}
-				foreach ($disallowed['allowExceptParams'] ?? $disallowed['disallowParams'] ?? [] as $param => $value) {
-					$allowExceptParams[$param] = $this->paramFactory(DisallowedCallParamValueExcept::class, $param, $value);
-				}
-				foreach ($disallowed['allowExceptParamFlags'] ?? $disallowed['disallowParamFlags'] ?? [] as $param => $value) {
-					$allowExceptParams[$param] = $this->paramFactory(DisallowedCallParamValueFlagExcept::class, $param, $value);
-				}
-				foreach ($disallowed['allowExceptCaseInsensitiveParams'] ?? $disallowed['disallowCaseInsensitiveParams'] ?? [] as $param => $value) {
-					$allowExceptParams[$param] = $this->paramFactory(DisallowedCallParamValueCaseInsensitiveExcept::class, $param, $value);
-				}
-				$disallowedCall = new DisallowedCall(
-					$this->normalizeCall($call),
-					$disallowed['message'] ?? null,
-					$disallowed['allowIn'] ?? [],
-					$disallowed['allowExceptIn'] ?? $disallowed['disallowIn'] ?? [],
-					$allowInCalls,
-					$allowExceptInCalls,
-					$allowParamsInAllowed,
-					$allowParamsAnywhere,
-					$allowExceptParamsInAllowed,
-					$allowExceptParams,
-					$disallowed['errorIdentifier'] ?? null,
-					$disallowed['errorTip'] ?? null
-				);
-				$disallowedCalls[$disallowedCall->getKey()] = $disallowedCall;
+			} catch (UnsupportedParamTypeInConfigException $e) {
+				$message = count($calls) === 1 ? $calls[0] : '{' . implode(',', $calls) . '}';
+				throw new ShouldNotHappenException("{$message}: {$e->getMessage()}");
 			}
 		}
 		return array_values($disallowedCalls);
@@ -106,6 +113,7 @@ private function normalizeCall(string $call): string
 	 * @param int|string $key
 	 * @param int|bool|string|null|array{position:int, value?:int|bool|string, name?:string} $value
 	 * @return T
+	 * @throws UnsupportedParamTypeInConfigException
 	 */
 	private function paramFactory(string $class, $key, $value): DisallowedCallParamValue
 	{
@@ -116,15 +124,15 @@ private function paramFactory(string $class, $key, $value): DisallowedCallParamV
 				$paramValue = $value['value'] ?? null;
 			} elseif ($class === DisallowedCallParamValueAny::class) {
 				if (is_numeric($value)) {
-					$paramPosition = $value;
+					$paramPosition = (int)$value;
 					$paramName = null;
 				} else {
 					$paramPosition = null;
-					$paramName = $value;
+					$paramName = (string)$value;
 				}
 				$paramValue = null;
 			} else {
-				$paramPosition = $key;
+				$paramPosition = (int)$key;
 				$paramName = null;
 				$paramValue = $value;
 			}
@@ -134,6 +142,9 @@ private function paramFactory(string $class, $key, $value): DisallowedCallParamV
 			$paramValue = $value;
 		}
 
+		if (!is_int($paramValue) && !is_bool($paramValue) && !is_string($paramValue) && !is_null($paramValue)) {
+			throw new UnsupportedParamTypeInConfigException($paramPosition, $paramName, gettype($paramValue));
+		}
 		return new $class($paramPosition, $paramName, $paramValue);
 	}
 

diff --git a/src/Exceptions/UnsupportedParamTypeInConfigException.php b/src/Exceptions/UnsupportedParamTypeInConfigException.php
@@ -0,0 +1,23 @@
+<?php
+declare(strict_types = 1);
+
+namespace Spaze\PHPStan\Rules\Disallowed\Exceptions;
+
+use Exception;
+use Throwable;
+
+class UnsupportedParamTypeInConfigException extends Exception
+{
+
+	public function __construct(?int $position, ?string $name, string $type, int $code = 0, ?Throwable $previous = null)
+	{
+		$message = sprintf(
+			'Parameter%s%s has an unsupported type %s specified in configuration',
+			$position ? " #{$position}" : '',
+			$name ? " \${$name}" : '',
+			$type
+		);
+		parent::__construct($message, $code, $previous);
+	}
+
+}
diff --git a/tests/Calls/FunctionCallsUnsupportedParamConfigTest.php b/tests/Calls/FunctionCallsUnsupportedParamConfigTest.php
@@ -0,0 +1,51 @@
+<?php
+declare(strict_types = 1);
+
+namespace Spaze\PHPStan\Rules\Disallowed\Calls;
+
+use PHPStan\File\FileHelper;
+use PHPStan\ShouldNotHappenException;
+use PHPStan\Testing\PHPStanTestCase;
+use Spaze\PHPStan\Rules\Disallowed\AllowedPath;
+use Spaze\PHPStan\Rules\Disallowed\DisallowedCallFactory;
+use Spaze\PHPStan\Rules\Disallowed\RuleErrors\DisallowedRuleErrors;
+
+class FunctionCallsUnsupportedParamConfigTest extends PHPStanTestCase
+{
+
+	/**
+	 * @throws ShouldNotHappenException
+	 */
+	public function testUnsupportedArrayInParamConfig(): void
+	{
+		$this->expectException(ShouldNotHappenException::class);
+		$this->expectExceptionMessage('{foo(),bar()}: Parameter #2 $definitelyNotScalar has an unsupported type array specified in configuration');
+		new FunctionCalls(
+			new DisallowedRuleErrors(new AllowedPath(new FileHelper(__DIR__))),
+			new DisallowedCallFactory(),
+			[
+				[
+					'function' => [
+						'foo()',
+						'bar()',
+					],
+					'disallowParams' => [
+						1 => [
+							'position' => 1,
+							'name' => 'key',
+							'value' => 'scalar',
+						],
+						2 => [
+							'position' => 2,
+							'name' => 'definitelyNotScalar',
+							'value' => [
+								'key' => 'unsupported',
+							],
+						],
+					],
+				],
+			]
+		);
+	}
+
+}
diff --git a/tests/libs/Functions.php b/tests/libs/Functions.php
@@ -44,6 +44,6 @@ function mocky(string $className): void
 /**
  * @param array|string|null $key
  */
-function config($key = null)
+function config($key = null, $default = null)
 {
 }
diff --git a/tests/src/disallowed-allow/functionCalls.php b/tests/src/disallowed-allow/functionCalls.php
@@ -89,3 +89,7 @@
 \Foo\Bar\Waldo\config(['key' => 'string']);
 // allowed by path
 \Foo\Bar\Waldo\config('string-key');
+// not disallowed array param, unsupported type in config
+\Foo\Bar\Waldo\config('foo', ['key' => 'allow']);
+// allowed by path
+\Foo\Bar\Waldo\config('foo', ['key' => 'disallow']);
diff --git a/tests/src/disallowed/functionCalls.php b/tests/src/disallowed/functionCalls.php
@@ -89,3 +89,7 @@
 \Foo\Bar\Waldo\config(['key' => 'string']);
 // disallowed param
 \Foo\Bar\Waldo\config('string-key');
+// not disallowed array param, unsupported type in config
+\Foo\Bar\Waldo\config('foo', ['key' => 'allow']);
+// disallowed array param, unsupported type in config
+\Foo\Bar\Waldo\config('foo', ['key' => 'disallow']);