Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix for handling non-stringable types in dynamic access #3885

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 26 additions & 3 deletions src/Rules/Classes/ClassConstantRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
use PHPStan\Reflection\ReflectionProvider;
use PHPStan\Rules\ClassNameCheck;
use PHPStan\Rules\ClassNameNodePair;
use PHPStan\Rules\IdentifierRuleError;
use PHPStan\Rules\Rule;
use PHPStan\Rules\RuleErrorBuilder;
use PHPStan\Rules\RuleLevelHelper;
Expand All @@ -20,6 +21,7 @@
use PHPStan\Type\Type;
use PHPStan\Type\TypeCombinator;
use PHPStan\Type\VerbosityLevel;
use function array_map;
use function array_merge;
use function in_array;
use function sprintf;
Expand Down Expand Up @@ -47,11 +49,32 @@ public function getNodeType(): string

public function processNode(Node $node, Scope $scope): array
{
if (!$node->name instanceof Node\Identifier) {
return [];
$errors = [];
if ($node->name instanceof Node\Identifier) {
$constantNames = [$node->name->name];
} else {
$fetchType = $scope->getType($node->name);
$constantNames = array_map(static fn ($type): string => $type->getValue(), $fetchType->getConstantStrings());
$fetchStringType = $fetchType->toString();
if (!$fetchStringType->isString()->yes()) {
$errors[] = RuleErrorBuilder::message(sprintf('Cannot fetch class constant with a non-stringable type %s.', $fetchType->describe(VerbosityLevel::typeOnly())))
->identifier('classConstant.fetchInvalidExpression')
->build();
}
}

foreach ($constantNames as $constantName) {
$errors = array_merge($errors, $this->processSingleClassConstFetch($scope, $node, $constantName));
}
$constantName = $node->name->name;

return $errors;
}

/**
* @return list<IdentifierRuleError>
*/
private function processSingleClassConstFetch(Scope $scope, ClassConstFetch $node, string $constantName): array
{
$class = $node->class;
$messages = [];
if ($class instanceof Node\Name) {
Expand Down
31 changes: 28 additions & 3 deletions src/Rules/Methods/CallMethodsRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,13 @@
use PHPStan\Internal\SprintfHelper;
use PHPStan\Reflection\ParametersAcceptorSelector;
use PHPStan\Rules\FunctionCallParametersCheck;
use PHPStan\Rules\IdentifierRuleError;
use PHPStan\Rules\Rule;
use PHPStan\Rules\RuleErrorBuilder;
use PHPStan\Type\VerbosityLevel;
use function array_map;
use function array_merge;
use function sprintf;

/**
* @implements Rule<Node\Expr\MethodCall>
Expand All @@ -31,12 +36,32 @@ public function getNodeType(): string

public function processNode(Node $node, Scope $scope): array
{
if (!$node->name instanceof Node\Identifier) {
return [];
$errors = [];
if ($node->name instanceof Node\Identifier) {
$methodNames = [$node->name->name];
} else {
$callType = $scope->getType($node->name);
$methodNames = array_map(static fn ($type): string => $type->getValue(), $callType->getConstantStrings());
$callStringType = $callType->toString();
if (!$callStringType->isString()->yes()) {
$errors[] = RuleErrorBuilder::message(sprintf('Cannot call method name with a non-stringable type %s.', $callType->describe(VerbosityLevel::typeOnly())))
->identifier('method.callNameInvalidExpression')
->build();
}
}

$methodName = $node->name->name;
foreach ($methodNames as $methodName) {
$errors = array_merge($errors, $this->processSingleMethodCall($scope, $node, $methodName));
}

return $errors;
}

/**
* @return list<IdentifierRuleError>
*/
private function processSingleMethodCall(Scope $scope, MethodCall $node, string $methodName): array
{
[$errors, $methodReflection] = $this->methodCallCheck->check($scope, $methodName, $node->var);
if ($methodReflection === null) {
return $errors;
Expand Down
31 changes: 28 additions & 3 deletions src/Rules/Methods/CallStaticMethodsRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,11 @@
use PHPStan\Internal\SprintfHelper;
use PHPStan\Reflection\ParametersAcceptorSelector;
use PHPStan\Rules\FunctionCallParametersCheck;
use PHPStan\Rules\IdentifierRuleError;
use PHPStan\Rules\Rule;
use PHPStan\Rules\RuleErrorBuilder;
use PHPStan\Type\VerbosityLevel;
use function array_map;
use function array_merge;
use function sprintf;

Expand All @@ -32,11 +36,32 @@ public function getNodeType(): string

public function processNode(Node $node, Scope $scope): array
{
if (!$node->name instanceof Node\Identifier) {
return [];
$errors = [];
if ($node->name instanceof Node\Identifier) {
$methodNames = [$node->name->name];
} else {
$callType = $scope->getType($node->name);
$methodNames = array_map(static fn ($type): string => $type->getValue(), $callType->getConstantStrings());
$callStringType = $callType->toString();
if (!$callStringType->isString()->yes()) {
$errors[] = RuleErrorBuilder::message(sprintf('Cannot call static method name with a non-stringable type %s.', $callType->describe(VerbosityLevel::typeOnly())))
->identifier('staticMethod.callNameInvalidExpression')
->build();
}
}
$methodName = $node->name->name;

foreach ($methodNames as $methodName) {
$errors = array_merge($errors, $this->processSingleMethodCall($scope, $node, $methodName));
}

return $errors;
}

/**
* @return list<IdentifierRuleError>
*/
private function processSingleMethodCall(Scope $scope, StaticCall $node, string $methodName): array
{
[$errors, $method] = $this->methodCallCheck->check($scope, $methodName, $node->class);
if ($method === null) {
return $errors;
Expand Down
11 changes: 9 additions & 2 deletions src/Rules/Properties/AccessPropertiesCheck.php
Original file line number Diff line number Diff line change
Expand Up @@ -41,13 +41,20 @@ public function __construct(
*/
public function check(PropertyFetch $node, Scope $scope, bool $write): array
{
$errors = [];
if ($node->name instanceof Identifier) {
$names = [$node->name->name];
} else {
$names = array_map(static fn (ConstantStringType $type): string => $type->getValue(), $scope->getType($node->name)->getConstantStrings());
$fetchType = $scope->getType($node->name);
$names = array_map(static fn (ConstantStringType $type): string => $type->getValue(), $fetchType->getConstantStrings());
$fetchStringType = $fetchType->toString();
if (!$fetchStringType->isString()->yes()) {
$errors[] = RuleErrorBuilder::message(sprintf('Cannot access property with a non-stringable type %s.', $fetchType->describe(VerbosityLevel::typeOnly())))
->identifier('property.fetchInvalidExpression')
->build();
}
}

$errors = [];
foreach ($names as $name) {
$errors = array_merge($errors, $this->processSingleProperty($scope, $node, $name, $write));
}
Expand Down
11 changes: 9 additions & 2 deletions src/Rules/Properties/AccessStaticPropertiesRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -51,13 +51,20 @@ public function getNodeType(): string

public function processNode(Node $node, Scope $scope): array
{
$errors = [];
if ($node->name instanceof Node\VarLikeIdentifier) {
$names = [$node->name->name];
} else {
$names = array_map(static fn (ConstantStringType $type): string => $type->getValue(), $scope->getType($node->name)->getConstantStrings());
$fetchType = $scope->getType($node->name);
$names = array_map(static fn (ConstantStringType $type): string => $type->getValue(), $fetchType->getConstantStrings());
$fetchStringType = $fetchType->toString();
if (!$fetchStringType->isString()->yes()) {
$errors[] = RuleErrorBuilder::message(sprintf('Cannot access static property with a non-stringable type %s.', $fetchType->describe(VerbosityLevel::typeOnly())))
->identifier('property.fetchInvalidExpression')
->build();
}
}

$errors = [];
foreach ($names as $name) {
$errors = array_merge($errors, $this->processSingleProperty($scope, $node, $name));
}
Expand Down
41 changes: 34 additions & 7 deletions src/Rules/Variables/DefinedVariableRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,13 @@
use PhpParser\Node;
use PhpParser\Node\Expr\Variable;
use PHPStan\Analyser\Scope;
use PHPStan\Rules\IdentifierRuleError;
use PHPStan\Rules\Rule;
use PHPStan\Rules\RuleErrorBuilder;
use PHPStan\Type\Constant\ConstantStringType;
use PHPStan\Type\VerbosityLevel;
use function array_map;
use function array_merge;
use function in_array;
use function is_string;
use function sprintf;
Expand All @@ -31,11 +36,33 @@ public function getNodeType(): string

public function processNode(Node $node, Scope $scope): array
{
if (!is_string($node->name)) {
return [];
$errors = [];
if (is_string($node->name)) {
$variableNames = [$node->name];
} else {
$fetchType = $scope->getType($node->name);
$variableNames = array_map(static fn (ConstantStringType $type): string => $type->getValue(), $fetchType->getConstantStrings());
$fetchStringType = $fetchType->toString();
if (! $fetchStringType->isString()->yes()) {
$errors[] = RuleErrorBuilder::message(sprintf('Cannot access variable with a non-stringable type %s.', $fetchType->describe(VerbosityLevel::typeOnly())))
->identifier('variable.fetchInvalidExpression')
->build();
}
}

foreach ($variableNames as $name) {
$errors = array_merge($errors, $this->processSingleVariable($scope, $node, $name));
}

if ($this->cliArgumentsVariablesRegistered && in_array($node->name, [
return $errors;
}

/**
* @return list<IdentifierRuleError>
*/
private function processSingleVariable(Scope $scope, Variable $node, string $variableName): array
{
if ($this->cliArgumentsVariablesRegistered && in_array($variableName, [
'argc',
'argv',
], true)) {
Expand All @@ -49,18 +76,18 @@ public function processNode(Node $node, Scope $scope): array
return [];
}

if ($scope->hasVariableType($node->name)->no()) {
if ($scope->hasVariableType($variableName)->no()) {
return [
RuleErrorBuilder::message(sprintf('Undefined variable: $%s', $node->name))
RuleErrorBuilder::message(sprintf('Undefined variable: $%s', $variableName))
->identifier('variable.undefined')
->build(),
];
} elseif (
$this->checkMaybeUndefinedVariables
&& !$scope->hasVariableType($node->name)->yes()
&& !$scope->hasVariableType($variableName)->yes()
) {
return [
RuleErrorBuilder::message(sprintf('Variable $%s might not be defined.', $node->name))
RuleErrorBuilder::message(sprintf('Variable $%s might not be defined.', $variableName))
->identifier('variable.undefined')
->build(),
];
Expand Down
24 changes: 24 additions & 0 deletions tests/PHPStan/Rules/Classes/ClassConstantRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -435,4 +435,28 @@ public function testClassConstantAccessedOnTrait(): void
]);
}

public function testBug9475(): void
{
if (PHP_VERSION_ID < 80200) {
$this->markTestSkipped('Test requires PHP 8.2.');
}

$this->phpVersion = PHP_VERSION_ID;

$this->analyse([__DIR__ . '/data/bug-9475.php'], [
[
'Cannot fetch class constant with a non-stringable type $this(Bug9475\Classes).',
12,
],
[
'Cannot fetch class constant with a non-stringable type object.',
13,
],
[
'Cannot fetch class constant with a non-stringable type array.',
14,
],
]);
}

}
20 changes: 20 additions & 0 deletions tests/PHPStan/Rules/Classes/data/bug-9475.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
<?php // lint >=8.3

namespace Bug9475;

use Stringable;

final class Classes
{

public function testStaticMethods(string $name, Stringable $stringable, object $object, array $array): void
{
echo 'Hello, ' . self::{$this};
echo 'Hello, ' . self::{$object};
echo 'Hello, ' . self::{$array};

echo 'Hello, ' . self::{$name}; // valid
echo 'Hello, ' . self::{$stringable}; // valid
}

}
26 changes: 26 additions & 0 deletions tests/PHPStan/Rules/Methods/CallMethodsRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -3552,4 +3552,30 @@ public function testBug6828(): void
$this->analyse([__DIR__ . '/data/bug-6828.php'], []);
}

public function testBug9475(): void
{
$this->checkThisOnly = false;
$this->checkNullables = true;
$this->checkUnionTypes = true;
$this->checkExplicitMixed = true;
$this->analyse([__DIR__ . '/data/bug-9475.php'], [
[
'Cannot call method name with a non-stringable type $this(Bug9475\Methods).',
12,
],
[
'Cannot call method name with a non-stringable type $this(Bug9475\Methods).',
13,
],
[
'Cannot call method name with a non-stringable type object.',
14,
],
[
'Cannot call method name with a non-stringable type array.',
15,
],
]);
}

}
25 changes: 25 additions & 0 deletions tests/PHPStan/Rules/Methods/CallStaticMethodsRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -862,4 +862,29 @@ public function testBug12015(): void
$this->analyse([__DIR__ . '/data/bug-12015.php'], []);
}

public function testBug9475(): void
{
$this->checkThisOnly = false;
$this->checkExplicitMixed = true;
$this->checkImplicitMixed = true;
$this->analyse([__DIR__ . '/data/bug-9475.php'], [
[
'Cannot call static method name with a non-stringable type $this(Bug9475\Methods).',
23,
],
[
'Cannot call static method name with a non-stringable type $this(Bug9475\Methods).',
24,
],
[
'Cannot call static method name with a non-stringable type object.',
25,
],
[
'Cannot call static method name with a non-stringable type array.',
26,
],
]);
}

}
Loading
Loading