From f3f41e581366191fd6b7ce0b523ed9bc79629908 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=A0pa=C4=8Dek?= Date: Sat, 7 Dec 2024 04:15:50 +0100 Subject: [PATCH 1/2] Support ${'variable'}('foo') dynamic function calls --- src/Calls/FunctionCalls.php | 11 +++++++++-- src/Type/TypeResolver.php | 14 ++++++++++++++ tests/Calls/FunctionCallsAllowInFunctionsTest.php | 2 ++ tests/Calls/FunctionCallsAllowInMethodsTest.php | 2 ++ tests/Calls/FunctionCallsDefinedInTest.php | 2 ++ .../FunctionCallsInMultipleNamespacesTest.php | 2 ++ tests/Calls/FunctionCallsNamedParamsTest.php | 2 ++ tests/Calls/FunctionCallsParamsMessagesTest.php | 2 ++ tests/Calls/FunctionCallsTest.php | 12 +++++++++++- ...CallsTypeStringParamsInvalidFlagsConfigTest.php | 2 ++ tests/Calls/FunctionCallsTypeStringParamsTest.php | 2 ++ .../FunctionCallsUnsupportedParamConfigTest.php | 2 ++ tests/src/disallowed-allow/functionCalls.php | 5 ++++- tests/src/disallowed/functionCalls.php | 5 ++++- 14 files changed, 60 insertions(+), 5 deletions(-) diff --git a/src/Calls/FunctionCalls.php b/src/Calls/FunctionCalls.php index 91a8658..494d651 100644 --- a/src/Calls/FunctionCalls.php +++ b/src/Calls/FunctionCalls.php @@ -5,6 +5,7 @@ use PhpParser\Node; use PhpParser\Node\Expr\FuncCall; +use PhpParser\Node\Expr\Variable; use PhpParser\Node\Name; use PhpParser\Node\Scalar\String_; use PHPStan\Analyser\Scope; @@ -17,6 +18,7 @@ use Spaze\PHPStan\Rules\Disallowed\Normalizer\Normalizer; use Spaze\PHPStan\Rules\Disallowed\RuleErrors\DisallowedCallsRuleErrors; use Spaze\PHPStan\Rules\Disallowed\RuleErrors\ErrorIdentifiers; +use Spaze\PHPStan\Rules\Disallowed\Type\TypeResolver; /** * Reports on dynamically calling a disallowed function. @@ -36,12 +38,15 @@ class FunctionCalls implements Rule private Normalizer $normalizer; + private TypeResolver $typeResolver; + /** * @param DisallowedCallsRuleErrors $disallowedCallsRuleErrors * @param DisallowedCallFactory $disallowedCallFactory * @param ReflectionProvider $reflectionProvider * @param Normalizer $normalizer + * @param TypeResolver $typeResolver * @param array $forbiddenCalls * @phpstan-param ForbiddenCallsConfig $forbiddenCalls * @noinspection PhpUndefinedClassInspection ForbiddenCallsConfig is a type alias defined in PHPStan config @@ -52,12 +57,14 @@ public function __construct( DisallowedCallFactory $disallowedCallFactory, ReflectionProvider $reflectionProvider, Normalizer $normalizer, + TypeResolver $typeResolver, array $forbiddenCalls ) { $this->disallowedCallsRuleErrors = $disallowedCallsRuleErrors; $this->disallowedCalls = $disallowedCallFactory->createFromConfig($forbiddenCalls); $this->reflectionProvider = $reflectionProvider; $this->normalizer = $normalizer; + $this->typeResolver = $typeResolver; } @@ -83,8 +90,8 @@ public function processNode(Node $node, Scope $scope): array $names = [$namespacedName, $node->name]; } elseif ($node->name instanceof String_) { $names = [new Name($this->normalizer->normalizeNamespace($node->name->value))]; - } elseif ($node->name instanceof Node\Expr\Variable && is_string($node->name->name)) { - $value = $scope->getVariableType($node->name->name)->getConstantScalarValues()[0]; + } elseif ($node->name instanceof Variable) { + $value = $this->typeResolver->getVariableStringValue($node->name, $scope); if (!is_string($value)) { return []; } diff --git a/src/Type/TypeResolver.php b/src/Type/TypeResolver.php index dace673..6d0fe17 100644 --- a/src/Type/TypeResolver.php +++ b/src/Type/TypeResolver.php @@ -4,7 +4,9 @@ namespace Spaze\PHPStan\Rules\Disallowed\Type; use PhpParser\Node\Expr; +use PhpParser\Node\Expr\Variable; use PhpParser\Node\Name; +use PhpParser\Node\Scalar\String_; use PHPStan\Analyser\Scope; use PHPStan\Type\ObjectType; use PHPStan\Type\Type; @@ -22,4 +24,16 @@ public function getType($class, Scope $scope): Type return $class instanceof Name ? new ObjectType($scope->resolveName($class)) : $scope->getType($class); } + + public function getVariableStringValue(Variable $variable, Scope $scope): ?string + { + $variableNameNode = $variable->name; + $variableName = $variableNameNode instanceof String_ ? $variableNameNode->value : $variableNameNode; + if (!is_string($variableName)) { + return null; + } + $value = $scope->getVariableType($variableName)->getConstantScalarValues()[0]; + return is_string($value) ? $value : null; + } + } diff --git a/tests/Calls/FunctionCallsAllowInFunctionsTest.php b/tests/Calls/FunctionCallsAllowInFunctionsTest.php index 01eb490..0e67521 100644 --- a/tests/Calls/FunctionCallsAllowInFunctionsTest.php +++ b/tests/Calls/FunctionCallsAllowInFunctionsTest.php @@ -9,6 +9,7 @@ use Spaze\PHPStan\Rules\Disallowed\DisallowedCallFactory; use Spaze\PHPStan\Rules\Disallowed\Normalizer\Normalizer; use Spaze\PHPStan\Rules\Disallowed\RuleErrors\DisallowedCallsRuleErrors; +use Spaze\PHPStan\Rules\Disallowed\Type\TypeResolver; class FunctionCallsAllowInFunctionsTest extends RuleTestCase { @@ -24,6 +25,7 @@ protected function getRule(): Rule $container->getByType(DisallowedCallFactory::class), $this->createReflectionProvider(), $container->getByType(Normalizer::class), + $container->getByType(TypeResolver::class), [ [ 'function' => 'md*()', diff --git a/tests/Calls/FunctionCallsAllowInMethodsTest.php b/tests/Calls/FunctionCallsAllowInMethodsTest.php index 4b497ac..61d5dd8 100644 --- a/tests/Calls/FunctionCallsAllowInMethodsTest.php +++ b/tests/Calls/FunctionCallsAllowInMethodsTest.php @@ -9,6 +9,7 @@ use Spaze\PHPStan\Rules\Disallowed\DisallowedCallFactory; use Spaze\PHPStan\Rules\Disallowed\Normalizer\Normalizer; use Spaze\PHPStan\Rules\Disallowed\RuleErrors\DisallowedCallsRuleErrors; +use Spaze\PHPStan\Rules\Disallowed\Type\TypeResolver; class FunctionCallsAllowInMethodsTest extends RuleTestCase { @@ -24,6 +25,7 @@ protected function getRule(): Rule $container->getByType(DisallowedCallFactory::class), $this->createReflectionProvider(), $container->getByType(Normalizer::class), + $container->getByType(TypeResolver::class), [ [ 'function' => 'md5_file()', diff --git a/tests/Calls/FunctionCallsDefinedInTest.php b/tests/Calls/FunctionCallsDefinedInTest.php index 820aff3..2a5b45b 100644 --- a/tests/Calls/FunctionCallsDefinedInTest.php +++ b/tests/Calls/FunctionCallsDefinedInTest.php @@ -9,6 +9,7 @@ use Spaze\PHPStan\Rules\Disallowed\DisallowedCallFactory; use Spaze\PHPStan\Rules\Disallowed\Normalizer\Normalizer; use Spaze\PHPStan\Rules\Disallowed\RuleErrors\DisallowedCallsRuleErrors; +use Spaze\PHPStan\Rules\Disallowed\Type\TypeResolver; class FunctionCallsDefinedInTest extends RuleTestCase { @@ -24,6 +25,7 @@ protected function getRule(): Rule $container->getByType(DisallowedCallFactory::class), $this->createReflectionProvider(), $container->getByType(Normalizer::class), + $container->getByType(TypeResolver::class), [ [ 'function' => '\\Foo\\Bar\\Waldo\\f*()', diff --git a/tests/Calls/FunctionCallsInMultipleNamespacesTest.php b/tests/Calls/FunctionCallsInMultipleNamespacesTest.php index f67056e..b09c73b 100644 --- a/tests/Calls/FunctionCallsInMultipleNamespacesTest.php +++ b/tests/Calls/FunctionCallsInMultipleNamespacesTest.php @@ -9,6 +9,7 @@ use Spaze\PHPStan\Rules\Disallowed\DisallowedCallFactory; use Spaze\PHPStan\Rules\Disallowed\Normalizer\Normalizer; use Spaze\PHPStan\Rules\Disallowed\RuleErrors\DisallowedCallsRuleErrors; +use Spaze\PHPStan\Rules\Disallowed\Type\TypeResolver; class FunctionCallsInMultipleNamespacesTest extends RuleTestCase { @@ -24,6 +25,7 @@ protected function getRule(): Rule $container->getByType(DisallowedCallFactory::class), $this->createReflectionProvider(), $container->getByType(Normalizer::class), + $container->getByType(TypeResolver::class), [ [ 'function' => '__()', diff --git a/tests/Calls/FunctionCallsNamedParamsTest.php b/tests/Calls/FunctionCallsNamedParamsTest.php index f05ff54..3a36c65 100644 --- a/tests/Calls/FunctionCallsNamedParamsTest.php +++ b/tests/Calls/FunctionCallsNamedParamsTest.php @@ -10,6 +10,7 @@ use Spaze\PHPStan\Rules\Disallowed\DisallowedCallFactory; use Spaze\PHPStan\Rules\Disallowed\Normalizer\Normalizer; use Spaze\PHPStan\Rules\Disallowed\RuleErrors\DisallowedCallsRuleErrors; +use Spaze\PHPStan\Rules\Disallowed\Type\TypeResolver; /** * @requires PHP >= 8.0 @@ -29,6 +30,7 @@ protected function getRule(): Rule $container->getByType(DisallowedCallFactory::class), $this->createReflectionProvider(), $container->getByType(Normalizer::class), + $container->getByType(TypeResolver::class), [ [ 'function' => 'Foo\Bar\Waldo\foo()', diff --git a/tests/Calls/FunctionCallsParamsMessagesTest.php b/tests/Calls/FunctionCallsParamsMessagesTest.php index a41a729..8912088 100644 --- a/tests/Calls/FunctionCallsParamsMessagesTest.php +++ b/tests/Calls/FunctionCallsParamsMessagesTest.php @@ -9,6 +9,7 @@ use Spaze\PHPStan\Rules\Disallowed\DisallowedCallFactory; use Spaze\PHPStan\Rules\Disallowed\Normalizer\Normalizer; use Spaze\PHPStan\Rules\Disallowed\RuleErrors\DisallowedCallsRuleErrors; +use Spaze\PHPStan\Rules\Disallowed\Type\TypeResolver; class FunctionCallsParamsMessagesTest extends RuleTestCase { @@ -24,6 +25,7 @@ protected function getRule(): Rule $container->getByType(DisallowedCallFactory::class), $this->createReflectionProvider(), $container->getByType(Normalizer::class), + $container->getByType(TypeResolver::class), [ [ 'function' => '\Foo\Bar\Waldo\config()', diff --git a/tests/Calls/FunctionCallsTest.php b/tests/Calls/FunctionCallsTest.php index 8c1f65d..c123c64 100644 --- a/tests/Calls/FunctionCallsTest.php +++ b/tests/Calls/FunctionCallsTest.php @@ -9,6 +9,7 @@ use Spaze\PHPStan\Rules\Disallowed\DisallowedCallFactory; use Spaze\PHPStan\Rules\Disallowed\Normalizer\Normalizer; use Spaze\PHPStan\Rules\Disallowed\RuleErrors\DisallowedCallsRuleErrors; +use Spaze\PHPStan\Rules\Disallowed\Type\TypeResolver; use Waldo\Quux\Blade; class FunctionCallsTest extends RuleTestCase @@ -25,6 +26,7 @@ protected function getRule(): Rule $container->getByType(DisallowedCallFactory::class), $this->createReflectionProvider(), $container->getByType(Normalizer::class), + $container->getByType(TypeResolver::class), [ [ 'function' => '\var_dump()', @@ -335,9 +337,17 @@ public function testRule(): void 114, ], [ - 'Calling Foo\Bar\Waldo() is forbidden, whoa, a namespace. [Foo\Bar\Waldo() matches Foo\Bar\waldo()]', + 'Calling Foo\Bar\waldo() is forbidden, whoa, a namespace.', 115, ], + [ + 'Calling Foo\Bar\waldo() is forbidden, whoa, a namespace.', + 117, + ], + [ + 'Calling Foo\Bar\Waldo() is forbidden, whoa, a namespace. [Foo\Bar\Waldo() matches Foo\Bar\waldo()]', + 118, + ], ]); $this->analyse([__DIR__ . '/../src/disallowed-allow/functionCalls.php'], [ [ diff --git a/tests/Calls/FunctionCallsTypeStringParamsInvalidFlagsConfigTest.php b/tests/Calls/FunctionCallsTypeStringParamsInvalidFlagsConfigTest.php index ea18bee..0572f22 100644 --- a/tests/Calls/FunctionCallsTypeStringParamsInvalidFlagsConfigTest.php +++ b/tests/Calls/FunctionCallsTypeStringParamsInvalidFlagsConfigTest.php @@ -8,6 +8,7 @@ use Spaze\PHPStan\Rules\Disallowed\DisallowedCallFactory; use Spaze\PHPStan\Rules\Disallowed\Normalizer\Normalizer; use Spaze\PHPStan\Rules\Disallowed\RuleErrors\DisallowedCallsRuleErrors; +use Spaze\PHPStan\Rules\Disallowed\Type\TypeResolver; class FunctionCallsTypeStringParamsInvalidFlagsConfigTest extends PHPStanTestCase { @@ -25,6 +26,7 @@ public function testException(): void $container->getByType(DisallowedCallFactory::class), $this->createReflectionProvider(), $container->getByType(Normalizer::class), + $container->getByType(TypeResolver::class), [ [ 'function' => '\Foo\Bar\Waldo\intParam1()', diff --git a/tests/Calls/FunctionCallsTypeStringParamsTest.php b/tests/Calls/FunctionCallsTypeStringParamsTest.php index dacae0a..8263b38 100644 --- a/tests/Calls/FunctionCallsTypeStringParamsTest.php +++ b/tests/Calls/FunctionCallsTypeStringParamsTest.php @@ -9,6 +9,7 @@ use Spaze\PHPStan\Rules\Disallowed\DisallowedCallFactory; use Spaze\PHPStan\Rules\Disallowed\Normalizer\Normalizer; use Spaze\PHPStan\Rules\Disallowed\RuleErrors\DisallowedCallsRuleErrors; +use Spaze\PHPStan\Rules\Disallowed\Type\TypeResolver; class FunctionCallsTypeStringParamsTest extends RuleTestCase { @@ -24,6 +25,7 @@ protected function getRule(): Rule $container->getByType(DisallowedCallFactory::class), $this->createReflectionProvider(), $container->getByType(Normalizer::class), + $container->getByType(TypeResolver::class), [ [ 'function' => '\Foo\Bar\Waldo\config()', diff --git a/tests/Calls/FunctionCallsUnsupportedParamConfigTest.php b/tests/Calls/FunctionCallsUnsupportedParamConfigTest.php index 6ded666..e87ec27 100644 --- a/tests/Calls/FunctionCallsUnsupportedParamConfigTest.php +++ b/tests/Calls/FunctionCallsUnsupportedParamConfigTest.php @@ -8,6 +8,7 @@ use Spaze\PHPStan\Rules\Disallowed\DisallowedCallFactory; use Spaze\PHPStan\Rules\Disallowed\Normalizer\Normalizer; use Spaze\PHPStan\Rules\Disallowed\RuleErrors\DisallowedCallsRuleErrors; +use Spaze\PHPStan\Rules\Disallowed\Type\TypeResolver; class FunctionCallsUnsupportedParamConfigTest extends PHPStanTestCase { @@ -25,6 +26,7 @@ public function testUnsupportedArrayInParamConfig(): void $container->getByType(DisallowedCallFactory::class), $this->createReflectionProvider(), $container->getByType(Normalizer::class), + $container->getByType(TypeResolver::class), [ [ 'function' => [ diff --git a/tests/src/disallowed-allow/functionCalls.php b/tests/src/disallowed-allow/functionCalls.php index fcda1ce..293e72a 100644 --- a/tests/src/disallowed-allow/functionCalls.php +++ b/tests/src/disallowed-allow/functionCalls.php @@ -108,8 +108,11 @@ $sneaky = 'Foo\Bar\waldo'; $sneaky('foo'); -('Foo\Bar\waldo')('foo'); +${'sneaky'}('foo'); $sneaky = '\Foo\Bar\waldo'; $sneaky('foo'); +${'sneaky'}('foo'); + +('Foo\Bar\waldo')('foo'); ('\Foo\Bar\Waldo')('foo'); diff --git a/tests/src/disallowed/functionCalls.php b/tests/src/disallowed/functionCalls.php index c68d3c3..048c2a6 100644 --- a/tests/src/disallowed/functionCalls.php +++ b/tests/src/disallowed/functionCalls.php @@ -108,8 +108,11 @@ $sneaky = 'Foo\Bar\waldo'; $sneaky('foo'); -('Foo\Bar\waldo')('foo'); +${'sneaky'}('foo'); $sneaky = '\Foo\Bar\waldo'; $sneaky('foo'); +${'sneaky'}('foo'); + +('Foo\Bar\waldo')('foo'); ('\Foo\Bar\Waldo')('foo'); From e4e3341e4ef0aea7ece9b0e660d81535ebc5d9b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=A0pa=C4=8Dek?= Date: Sat, 7 Dec 2024 04:37:12 +0100 Subject: [PATCH 2/2] Detect $object->$method() and $object->${'method'}(), also static calls --- src/RuleErrors/DisallowedMethodRuleErrors.php | 14 +++++++++++--- tests/Calls/MethodCallsTest.php | 8 ++++++++ tests/Calls/StaticCallsTest.php | 8 ++++++++ tests/src/disallowed-allow/methodCalls.php | 5 +++++ tests/src/disallowed-allow/staticCalls.php | 4 ++++ tests/src/disallowed/methodCalls.php | 5 +++++ tests/src/disallowed/staticCalls.php | 4 ++++ 7 files changed, 45 insertions(+), 3 deletions(-) diff --git a/src/RuleErrors/DisallowedMethodRuleErrors.php b/src/RuleErrors/DisallowedMethodRuleErrors.php index 2df5b86..a7d8e06 100644 --- a/src/RuleErrors/DisallowedMethodRuleErrors.php +++ b/src/RuleErrors/DisallowedMethodRuleErrors.php @@ -7,6 +7,7 @@ use PhpParser\Node\Expr\CallLike; use PhpParser\Node\Expr\MethodCall; use PhpParser\Node\Expr\StaticCall; +use PhpParser\Node\Expr\Variable; use PhpParser\Node\Identifier; use PhpParser\Node\Name; use PHPStan\Analyser\Scope; @@ -49,13 +50,20 @@ public function __construct( */ public function get($class, CallLike $node, Scope $scope, array $disallowedCalls): array { - if (!isset($node->name) || !($node->name instanceof Identifier)) { + if ($node->name instanceof Identifier) { + $methodName = $node->name->name; + } elseif ($node->name instanceof Variable) { + $methodName = $this->typeResolver->getVariableStringValue($node->name, $scope); + if (!is_string($methodName)) { + return []; + } + } else { return []; } $calledOnType = $this->typeResolver->getType($class, $scope); - if ($calledOnType->canCallMethods()->yes() && $calledOnType->hasMethod($node->name->name)->yes()) { - $method = $calledOnType->getMethod($node->name->name, $scope); + if ($calledOnType->canCallMethods()->yes() && $calledOnType->hasMethod($methodName)->yes()) { + $method = $calledOnType->getMethod($methodName, $scope); $declaringClass = $method->getDeclaringClass(); $classes = $calledOnType->getObjectClassReflections(); $classNames = array_map(fn($class): string => $class->isAnonymous() ? 'class@anonymous' : $class->getName(), $classes); diff --git a/tests/Calls/MethodCallsTest.php b/tests/Calls/MethodCallsTest.php index d5f8d74..48fb4f6 100644 --- a/tests/Calls/MethodCallsTest.php +++ b/tests/Calls/MethodCallsTest.php @@ -221,6 +221,14 @@ public function testRule(): void 'Calling Inheritance\Base::x() (as class@anonymous::x()) is forbidden, Base::x*() methods are dangerous. [Inheritance\Base::x() matches Inheritance\Base::x*()]', 91, ], + [ + "Calling Waldo\Quux\Blade::runner() is forbidden, I've seen tests you people wouldn't believe. [Waldo\Quux\Blade::runner() matches Waldo\Quux\Blade::run*()]", + 95, + ], + [ + "Calling Waldo\Quux\Blade::runner() is forbidden, I've seen tests you people wouldn't believe. [Waldo\Quux\Blade::runner() matches Waldo\Quux\Blade::run*()]", + 96, + ], ]); $this->analyse([__DIR__ . '/../src/disallowed-allow/methodCalls.php'], [ [ diff --git a/tests/Calls/StaticCallsTest.php b/tests/Calls/StaticCallsTest.php index 6098859..57a870f 100644 --- a/tests/Calls/StaticCallsTest.php +++ b/tests/Calls/StaticCallsTest.php @@ -185,6 +185,14 @@ public function testRule(): void 'Calling Inheritance\Base::woofer() (as class@anonymous::woofer()) is forbidden, method Base::woofer() is dangerous. [Inheritance\Base::woofer() matches Inheritance\Base::w*f*r()]', 57, ], + [ + 'Calling Fiction\Pulp\Royale::withCheese() is forbidden, a Quarter Pounder with Cheese?', + 60, + ], + [ + 'Calling Fiction\Pulp\Royale::withCheese() is forbidden, a Quarter Pounder with Cheese?', + 61, + ], ]); $this->analyse([__DIR__ . '/../src/disallowed-allow/staticCalls.php'], [ [ diff --git a/tests/src/disallowed-allow/methodCalls.php b/tests/src/disallowed-allow/methodCalls.php index 05f87d9..b69ee86 100644 --- a/tests/src/disallowed-allow/methodCalls.php +++ b/tests/src/disallowed-allow/methodCalls.php @@ -89,3 +89,8 @@ public static function y(): void // allowed by path $foo = new class extends Inheritance\Base {}; $foo->x(); + +// dynamic method name allowed by path and only with these params +$method = 'runner'; +$blade->$method(42, true, '909'); +$blade->${'method'}(42, true, '909'); diff --git a/tests/src/disallowed-allow/staticCalls.php b/tests/src/disallowed-allow/staticCalls.php index 54a8179..3d4a3b2 100644 --- a/tests/src/disallowed-allow/staticCalls.php +++ b/tests/src/disallowed-allow/staticCalls.php @@ -55,3 +55,7 @@ public static function y(): void // allowed by path $foo = new class extends Inheritance\Base {}; $foo::woofer(); + +$method = 'withCheese'; +Pulp\Royale::$method(); +Pulp\Royale::${'method'}(); diff --git a/tests/src/disallowed/methodCalls.php b/tests/src/disallowed/methodCalls.php index ef3af71..bc81020 100644 --- a/tests/src/disallowed/methodCalls.php +++ b/tests/src/disallowed/methodCalls.php @@ -89,3 +89,8 @@ public static function y(): void // disallowed parent method $foo = new class extends Inheritance\Base {}; $foo->x(); + +// disallowed dynamic method name +$method = 'runner'; +$blade->$method(42, true, '909'); +$blade->${'method'}(42, true, '909'); diff --git a/tests/src/disallowed/staticCalls.php b/tests/src/disallowed/staticCalls.php index a52666c..400b5c5 100644 --- a/tests/src/disallowed/staticCalls.php +++ b/tests/src/disallowed/staticCalls.php @@ -55,3 +55,7 @@ public static function y(): void // disallowed parent method $foo = new class extends Inheritance\Base {}; $foo::woofer(); + +$method = 'withCheese'; +Pulp\Royale::$method(); +Pulp\Royale::${'method'}();