Skip to content

Commit

Permalink
More dynamic calls (#278)
Browse files Browse the repository at this point in the history
Detect
- `${'variable'}('foo')` dynamic function calls
- `$object->$method()` and `$object->${'method'}()`
- `Class::$method()` and `Class::${'method'}()`

Ref #275
Follow-up to #276
  • Loading branch information
spaze authored Dec 7, 2024
2 parents 02d0741 + e4e3341 commit 9e5b799
Show file tree
Hide file tree
Showing 21 changed files with 105 additions and 8 deletions.
11 changes: 9 additions & 2 deletions src/Calls/FunctionCalls.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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.
Expand All @@ -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
Expand All @@ -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;
}


Expand All @@ -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 [];
}
Expand Down
14 changes: 11 additions & 3 deletions src/RuleErrors/DisallowedMethodRuleErrors.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
14 changes: 14 additions & 0 deletions src/Type/TypeResolver.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
}

}
2 changes: 2 additions & 0 deletions tests/Calls/FunctionCallsAllowInFunctionsTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand All @@ -24,6 +25,7 @@ protected function getRule(): Rule
$container->getByType(DisallowedCallFactory::class),
$this->createReflectionProvider(),
$container->getByType(Normalizer::class),
$container->getByType(TypeResolver::class),
[
[
'function' => 'md*()',
Expand Down
2 changes: 2 additions & 0 deletions tests/Calls/FunctionCallsAllowInMethodsTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand All @@ -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()',
Expand Down
2 changes: 2 additions & 0 deletions tests/Calls/FunctionCallsDefinedInTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand All @@ -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*()',
Expand Down
2 changes: 2 additions & 0 deletions tests/Calls/FunctionCallsInMultipleNamespacesTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand All @@ -24,6 +25,7 @@ protected function getRule(): Rule
$container->getByType(DisallowedCallFactory::class),
$this->createReflectionProvider(),
$container->getByType(Normalizer::class),
$container->getByType(TypeResolver::class),
[
[
'function' => '__()',
Expand Down
2 changes: 2 additions & 0 deletions tests/Calls/FunctionCallsNamedParamsTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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()',
Expand Down
2 changes: 2 additions & 0 deletions tests/Calls/FunctionCallsParamsMessagesTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand All @@ -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()',
Expand Down
12 changes: 11 additions & 1 deletion tests/Calls/FunctionCallsTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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()',
Expand Down Expand Up @@ -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'], [
[
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand All @@ -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()',
Expand Down
2 changes: 2 additions & 0 deletions tests/Calls/FunctionCallsTypeStringParamsTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand All @@ -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()',
Expand Down
2 changes: 2 additions & 0 deletions tests/Calls/FunctionCallsUnsupportedParamConfigTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand All @@ -25,6 +26,7 @@ public function testUnsupportedArrayInParamConfig(): void
$container->getByType(DisallowedCallFactory::class),
$this->createReflectionProvider(),
$container->getByType(Normalizer::class),
$container->getByType(TypeResolver::class),
[
[
'function' => [
Expand Down
8 changes: 8 additions & 0 deletions tests/Calls/MethodCallsTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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'], [
[
Expand Down
8 changes: 8 additions & 0 deletions tests/Calls/StaticCallsTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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'], [
[
Expand Down
5 changes: 4 additions & 1 deletion tests/src/disallowed-allow/functionCalls.php
Original file line number Diff line number Diff line change
Expand Up @@ -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');
5 changes: 5 additions & 0 deletions tests/src/disallowed-allow/methodCalls.php
Original file line number Diff line number Diff line change
Expand Up @@ -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');
4 changes: 4 additions & 0 deletions tests/src/disallowed-allow/staticCalls.php
Original file line number Diff line number Diff line change
Expand Up @@ -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'}();
5 changes: 4 additions & 1 deletion tests/src/disallowed/functionCalls.php
Original file line number Diff line number Diff line change
Expand Up @@ -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');
5 changes: 5 additions & 0 deletions tests/src/disallowed/methodCalls.php
Original file line number Diff line number Diff line change
Expand Up @@ -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');
4 changes: 4 additions & 0 deletions tests/src/disallowed/staticCalls.php
Original file line number Diff line number Diff line change
Expand Up @@ -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'}();

0 comments on commit 9e5b799

Please sign in to comment.