Detect callables in constructors

Follow-up to #281 Ref #275
spaze · Jan 9, 2025 · 70a6a0f · 70a6a0f
1 parent 56f9401
commit 70a6a0f
Show file tree

Hide file tree

Showing 6 changed files with 265 additions and 10 deletions.
diff --git a/src/Calls/NewCalls.php b/src/Calls/NewCalls.php
@@ -13,6 +13,7 @@
 use PHPStan\ShouldNotHappenException;
 use Spaze\PHPStan\Rules\Disallowed\DisallowedCall;
 use Spaze\PHPStan\Rules\Disallowed\DisallowedCallFactory;
+use Spaze\PHPStan\Rules\Disallowed\RuleErrors\DisallowedCallableParameterRuleErrors;
 use Spaze\PHPStan\Rules\Disallowed\RuleErrors\DisallowedCallsRuleErrors;
 use Spaze\PHPStan\Rules\Disallowed\RuleErrors\ErrorIdentifiers;
 
@@ -24,25 +25,33 @@
  */
 class NewCalls implements Rule
 {
-	private const CONSTRUCT = '::__construct';
+	public const CONSTRUCT = '__construct';
 
 	private DisallowedCallsRuleErrors $disallowedCallsRuleErrors;
 
+	private DisallowedCallableParameterRuleErrors $disallowedCallableParameterRuleErrors;
+
 	/** @var list<DisallowedCall> */
 	private array $disallowedCalls;
 
 
 	/**
 	 * @param DisallowedCallsRuleErrors $disallowedCallsRuleErrors
+	 * @param DisallowedCallableParameterRuleErrors $disallowedCallableParameterRuleErrors
 	 * @param DisallowedCallFactory $disallowedCallFactory
 	 * @param array $forbiddenCalls
 	 * @phpstan-param ForbiddenCallsConfig $forbiddenCalls
 	 * @noinspection PhpUndefinedClassInspection ForbiddenCallsConfig is a type alias defined in PHPStan config
 	 * @throws ShouldNotHappenException
 	 */
-	public function __construct(DisallowedCallsRuleErrors $disallowedCallsRuleErrors, DisallowedCallFactory $disallowedCallFactory, array $forbiddenCalls)
-	{
+	public function __construct(
+		DisallowedCallsRuleErrors $disallowedCallsRuleErrors,
+		DisallowedCallableParameterRuleErrors $disallowedCallableParameterRuleErrors,
+		DisallowedCallFactory $disallowedCallFactory,
+		array $forbiddenCalls
+	) {
 		$this->disallowedCallsRuleErrors = $disallowedCallsRuleErrors;
+		$this->disallowedCallableParameterRuleErrors = $disallowedCallableParameterRuleErrors;
 		$this->disallowedCalls = $disallowedCallFactory->createFromConfig($forbiddenCalls);
 	}
 
@@ -89,11 +98,11 @@ public function processNode(Node $node, Scope $scope): array
 			$definedIn = $reflection ? $reflection->getFileName() : null;
 
 			foreach ($names as $name) {
-				$name .= self::CONSTRUCT;
-				$errors = array_merge(
-					$errors,
-					$this->disallowedCallsRuleErrors->get($node, $scope, $name, $type->getClassName() . self::CONSTRUCT, $definedIn, $this->disallowedCalls, ErrorIdentifiers::DISALLOWED_NEW)
-				);
+				$ruleErrors = $this->disallowedCallsRuleErrors->get($node, $scope, $name . '::' . self::CONSTRUCT, $type->getClassName() . '::' . self::CONSTRUCT, $definedIn, $this->disallowedCalls, ErrorIdentifiers::DISALLOWED_NEW);
+				$paramErrors = $this->disallowedCallableParameterRuleErrors->getForConstructor(new Name($name), $node, $scope);
+				if ($errors || $ruleErrors || $paramErrors) {
+					$errors = array_merge($errors, $ruleErrors, $paramErrors);
+				}
 			}
 		}
 

diff --git a/src/RuleErrors/DisallowedCallableParameterRuleErrors.php b/src/RuleErrors/DisallowedCallableParameterRuleErrors.php
@@ -7,6 +7,7 @@
 use PhpParser\Node\Expr\CallLike;
 use PhpParser\Node\Expr\FuncCall;
 use PhpParser\Node\Expr\MethodCall;
+use PhpParser\Node\Expr\New_;
 use PhpParser\Node\Expr\StaticCall;
 use PhpParser\Node\Name;
 use PHPStan\Analyser\ArgumentsNormalizer;
@@ -18,6 +19,7 @@
 use PHPStan\Rules\IdentifierRuleError;
 use PHPStan\ShouldNotHappenException;
 use PHPStan\Type\TypeCombinator;
+use Spaze\PHPStan\Rules\Disallowed\Calls\NewCalls;
 use Spaze\PHPStan\Rules\Disallowed\DisallowedCall;
 use Spaze\PHPStan\Rules\Disallowed\DisallowedCallFactory;
 use Spaze\PHPStan\Rules\Disallowed\PHPStan1Compatibility;
@@ -106,6 +108,34 @@ public function getForFunction(FuncCall $node, Scope $scope): array
 	 * @throws ShouldNotHappenException
 	 */
 	public function getForMethod($class, CallLike $node, Scope $scope): array
+	{
+		$names = $this->typeResolver->getNamesFromCall($node, $scope);
+		return $this->getForMethods($class, $names, $node, $scope);
+	}
+
+
+	/**
+	 * @param Name|Expr $class
+	 * @param New_ $node
+	 * @param Scope $scope
+	 * @return list<IdentifierRuleError>
+	 * @throws ShouldNotHappenException
+	 */
+	public function getForConstructor($class, New_ $node, Scope $scope): array
+	{
+		return $this->getForMethods($class, [new Name(NewCalls::CONSTRUCT)], $node, $scope);
+	}
+
+
+	/**
+	 * @param Name|Expr $class
+	 * @param list<Name> $names
+	 * @param CallLike $node
+	 * @param Scope $scope
+	 * @return list<IdentifierRuleError>
+	 * @throws ShouldNotHappenException
+	 */
+	public function getForMethods($class, array $names, CallLike $node, Scope $scope): array
 	{
 		$ruleErrors = [];
 		$classType = $this->typeResolver->getType($class, $scope);
@@ -117,7 +147,7 @@ public function getForMethod($class, CallLike $node, Scope $scope): array
 				continue;
 			}
 			$classReflection = $this->reflectionProvider->getClass($className);
-			foreach ($this->typeResolver->getNamesFromCall($node, $scope) as $name) {
+			foreach ($names as $name) {
 				if (!$classReflection->hasMethod($name->toString())) {
 					continue;
 				}
@@ -134,7 +164,7 @@ public function getForMethod($class, CallLike $node, Scope $scope): array
 
 	/**
 	 * @param Scope $scope
-	 * @param FuncCall|MethodCall|StaticCall $node
+	 * @param CallLike $node
 	 * @param ExtendedMethodReflection|FunctionReflection $reflection
 	 * @return list<IdentifierRuleError>
 	 * @throws ShouldNotHappenException

diff --git a/tests/Calls/NewCallsCallableParametersTest.php b/tests/Calls/NewCallsCallableParametersTest.php
@@ -0,0 +1,184 @@
+<?php
+declare(strict_types = 1);
+
+namespace Spaze\PHPStan\Rules\Disallowed\Calls;
+
+use PHPStan\Reflection\ReflectionProvider;
+use PHPStan\Rules\Rule;
+use PHPStan\ShouldNotHappenException;
+use PHPStan\Testing\RuleTestCase;
+use Spaze\PHPStan\Rules\Disallowed\DisallowedCallFactory;
+use Spaze\PHPStan\Rules\Disallowed\RuleErrors\DisallowedCallableParameterRuleErrors;
+use Spaze\PHPStan\Rules\Disallowed\RuleErrors\DisallowedCallsRuleErrors;
+use Spaze\PHPStan\Rules\Disallowed\RuleErrors\DisallowedFunctionRuleErrors;
+use Spaze\PHPStan\Rules\Disallowed\RuleErrors\DisallowedMethodRuleErrors;
+use Spaze\PHPStan\Rules\Disallowed\Type\TypeResolver;
+
+class NewCallsCallableParametersTest extends RuleTestCase
+{
+
+	/**
+	 * @throws ShouldNotHappenException
+	 */
+	protected function getRule(): Rule
+	{
+		$container = self::getContainer();
+		$disallowedCallableParameterRuleErrors = new DisallowedCallableParameterRuleErrors(
+			$container->getByType(TypeResolver::class),
+			$container->getByType(DisallowedFunctionRuleErrors::class),
+			$container->getByType(DisallowedMethodRuleErrors::class),
+			$container->getByType(DisallowedCallFactory::class),
+			$container->getByType(ReflectionProvider::class),
+			[
+				[
+					'function' => 'var_dump()',
+					'allowIn' => [
+						__DIR__ . '/../src/disallowed-allow/*.php',
+						__DIR__ . '/../src/*-allow/*.*',
+					],
+				],
+			],
+			[
+				[
+					'method' => 'Callbacks::call()',
+					'allowIn' => [
+						__DIR__ . '/../src/disallowed-allow/*.php',
+						__DIR__ . '/../src/*-allow/*.*',
+					],
+				],
+				[
+					'method' => 'CallbacksInterface::interfaceCall()',
+					'allowIn' => [
+						__DIR__ . '/../src/disallowed-allow/*.php',
+						__DIR__ . '/../src/*-allow/*.*',
+					],
+				],
+				[
+					'method' => 'CallbacksTrait::traitCall()',
+					'allowIn' => [
+						__DIR__ . '/../src/disallowed-allow/*.php',
+						__DIR__ . '/../src/*-allow/*.*',
+					],
+				],
+			],
+			[
+				[
+					'method' => 'Callbacks::staticCall()',
+					'allowIn' => [
+						__DIR__ . '/../src/disallowed-allow/*.php',
+						__DIR__ . '/../src/*-allow/*.*',
+					],
+				],
+				[
+					'method' => 'CallbacksInterface::interfaceStaticCall()',
+					'allowIn' => [
+						__DIR__ . '/../src/disallowed-allow/*.php',
+						__DIR__ . '/../src/*-allow/*.*',
+					],
+				],
+				[
+					'method' => 'CallbacksTrait::traitStaticCall()',
+					'allowIn' => [
+						__DIR__ . '/../src/disallowed-allow/*.php',
+						__DIR__ . '/../src/*-allow/*.*',
+					],
+				],
+			],
+		);
+		return new NewCalls(
+			$container->getByType(DisallowedCallsRuleErrors::class),
+			$disallowedCallableParameterRuleErrors,
+			$container->getByType(DisallowedCallFactory::class),
+			[],
+		);
+	}
+
+
+	public function testRule(): void
+	{
+		// Based on the configuration above, in this file:
+		$this->analyse([__DIR__ . '/../src/disallowed/callableParameters.php'], [
+			[
+				// expect this error message:
+				'Calling var_dump() is forbidden.',
+				// on this line:
+				176,
+			],
+			[
+				'Calling var_dump() is forbidden.',
+				177,
+			],
+			[
+				'Calling var_dump() is forbidden.',
+				178,
+			],
+			[
+				'Calling Callbacks::call() is forbidden.',
+				179,
+			],
+			[
+				'Calling Callbacks::call() is forbidden.',
+				180,
+			],
+			[
+				'Calling Callbacks::call() is forbidden.',
+				181,
+			],
+			[
+				'Calling Callbacks::call() (as Callbacks2::call()) is forbidden.',
+				182,
+			],
+			[
+				'Calling Callbacks::call() (as Callbacks2::call()) is forbidden.',
+				183,
+			],
+			[
+				'Calling Callbacks::call() (as Callbacks2::call()) is forbidden.',
+				184,
+			],
+			[
+				'Calling Callbacks::staticCall() is forbidden.',
+				185,
+			],
+			[
+				'Calling Callbacks::staticCall() is forbidden.',
+				186,
+			],
+			[
+				'Calling Callbacks::staticCall() is forbidden.',
+				187,
+			],
+			[
+				'Calling Callbacks::staticCall() is forbidden.',
+				188,
+			],
+			[
+				'Calling Callbacks::staticCall() (as Callbacks2::staticCall()) is forbidden.',
+				189,
+			],
+			[
+				'Calling Callbacks::staticCall() (as Callbacks2::staticCall()) is forbidden.',
+				190,
+			],
+			[
+				'Calling Callbacks::staticCall() (as Callbacks2::staticCall()) is forbidden.',
+				191,
+			],
+			[
+				'Calling Callbacks::staticCall() (as Callbacks2::staticCall()) is forbidden.',
+				192,
+			],
+		]);
+		// Based on the configuration above, no errors in this file:
+		$this->analyse([__DIR__ . '/../src/disallowed-allow/callableParameters.php'], []);
+	}
+
+
+	public static function getAdditionalConfigFiles(): array
+	{
+		return [
+			__DIR__ . '/../../extension.neon',
+		];
+	}
+
+}
diff --git a/tests/Calls/NewCallsDefinedInTest.php b/tests/Calls/NewCallsDefinedInTest.php
@@ -7,6 +7,7 @@
 use PHPStan\ShouldNotHappenException;
 use PHPStan\Testing\RuleTestCase;
 use Spaze\PHPStan\Rules\Disallowed\DisallowedCallFactory;
+use Spaze\PHPStan\Rules\Disallowed\RuleErrors\DisallowedCallableParameterRuleErrors;
 use Spaze\PHPStan\Rules\Disallowed\RuleErrors\DisallowedCallsRuleErrors;
 
 class NewCallsDefinedInTest extends RuleTestCase
@@ -20,6 +21,7 @@ protected function getRule(): Rule
 		$container = self::getContainer();
 		return new NewCalls(
 			$container->getByType(DisallowedCallsRuleErrors::class),
+			$container->getByType(DisallowedCallableParameterRuleErrors::class),
 			$container->getByType(DisallowedCallFactory::class),
 			[
 				[

diff --git a/tests/Calls/NewCallsTest.php b/tests/Calls/NewCallsTest.php
@@ -7,6 +7,7 @@
 use PHPStan\ShouldNotHappenException;
 use PHPStan\Testing\RuleTestCase;
 use Spaze\PHPStan\Rules\Disallowed\DisallowedCallFactory;
+use Spaze\PHPStan\Rules\Disallowed\RuleErrors\DisallowedCallableParameterRuleErrors;
 use Spaze\PHPStan\Rules\Disallowed\RuleErrors\DisallowedCallsRuleErrors;
 
 class NewCallsTest extends RuleTestCase
@@ -20,6 +21,7 @@ protected function getRule(): Rule
 		$container = self::getContainer();
 		return new NewCalls(
 			$container->getByType(DisallowedCallsRuleErrors::class),
+			$container->getByType(DisallowedCallableParameterRuleErrors::class),
 			$container->getByType(DisallowedCallFactory::class),
 			[
 				[

diff --git a/tests/src/disallowed/callableParameters.php b/tests/src/disallowed/callableParameters.php
@@ -166,3 +166,31 @@ public static function interfaceStaticCall(): void { echo __METHOD__; }
 CallableParams::staticCall([$callbacksPlusPlus, 'interfaceStaticCall']);
 CallableParams::staticCall([$callbacksPlusPlus, 'traitCall']);
 CallableParams::staticCall([CallbacksPlusPlus::class, 'traitStaticCall']);
+
+class ConstructorCallable
+{
+	public function __construct(?callable $callback, ?string $string = null) {}
+}
+
+// disallowed callable params in constructors
+new ConstructorCallable('var_dump');
+new ConstructorCallable($varDump);
+new ConstructorCallable(string: null, callback: $varDump);
+new ConstructorCallable([$callbacks, 'call']);
+new ConstructorCallable([$callbacks, $call]);
+new ConstructorCallable([new Callbacks, $call]);
+new ConstructorCallable([$callbacks2, 'call']);
+new ConstructorCallable([$callbacks2, $call]);
+new ConstructorCallable([new Callbacks2, $call]);
+new ConstructorCallable(['Callbacks', 'staticCall']);
+new ConstructorCallable(['Callbacks', $staticCall]);
+new ConstructorCallable([Callbacks::class, 'staticCall']);
+new ConstructorCallable([Callbacks::class, $staticCall]);
+new ConstructorCallable(['Callbacks2', 'staticCall']);
+new ConstructorCallable(['Callbacks2', $staticCall]);
+new ConstructorCallable([Callbacks2::class, 'staticCall']);
+new ConstructorCallable([Callbacks2::class, $staticCall]);
+
+// not a callable param
+new ConstructorCallable(null, 'var_dump');
+new ConstructorCallable(string: $varDump, callback: null);