Skip to content

Add stringable access check to ClassConstantRule #3910

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.

Sign up for GitHub

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

Merged
merged 6 commits into from
Jul 13, 2025
Merged

Add stringable access check to ClassConstantRule #3910

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
8e8bcd5
Add stringable access check to ClassConstantRule
zonuexe Mar 29, 2025
4e59d4f
Add class name to message
zonuexe Jul 11, 2025
f9d4a82
fixup! Add class name to message
zonuexe Jul 12, 2025
808ed2e
fixup! Add stringable access check to ClassConstantRule
zonuexe Jul 12, 2025
b0539eb
fixup! Add stringable access check to ClassConstantRule
zonuexe Jul 12, 2025
399e22d
fixup! fixup! Add stringable access check to ClassConstantRule
zonuexe Jul 13, 2025
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
1 change: 1 addition & 0 deletions conf/bleedingEdge.neon
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
parameters:
featureToggles:
bleedingEdge: true
checkNonStringableDynamicAccess: true
checkParameterCastableToNumberFunctions: true
skipCheckGenericClasses!: []
stricterFunctionMap: true
Expand Down
3 changes: 3 additions & 0 deletions conf/config.level0.neon
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ conditionalTags:
phpstan.rules.rule: %featureToggles.newStaticInAbstractClassStaticMethod%

services:
-
class: PHPStan\Rules\Classes\ClassConstantRule
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this added here? It's not needed at all, the rule has #[RegisteredRule(level: 0)] attribute.


-
class: PHPStan\Rules\Classes\NewStaticInAbstractClassStaticMethodRule

Expand Down
1 change: 1 addition & 0 deletions conf/config.neon
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ parameters:
tooWideThrowType: true
featureToggles:
bleedingEdge: false
checkNonStringableDynamicAccess: false
checkParameterCastableToNumberFunctions: false
skipCheckGenericClasses: []
stricterFunctionMap: false
Expand Down
1 change: 1 addition & 0 deletions conf/parametersSchema.neon
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ parametersSchema:
])
featureToggles: structure([
bleedingEdge: bool(),
checkNonStringableDynamicAccess: bool(),
checkParameterCastableToNumberFunctions: bool(),
skipCheckGenericClasses: listOf(string()),
stricterFunctionMap: bool()
Expand Down
23 changes: 23 additions & 0 deletions src/Rules/Classes/ClassConstantRule.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
use PhpParser\Node;
use PhpParser\Node\Expr\BinaryOp\Identical;
use PhpParser\Node\Expr\ClassConstFetch;
use PhpParser\Node\Name;
use PhpParser\Node\Scalar\String_;
use PHPStan\Analyser\NullsafeOperatorHelper;
use PHPStan\Analyser\Scope;
Expand Down Expand Up @@ -42,6 +43,7 @@ public function __construct(
private RuleLevelHelper $ruleLevelHelper,
private ClassNameCheck $classCheck,
private PhpVersion $phpVersion,
private bool $checkNonStringableDynamicAccess = true,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand that we shouldn't use the default value, but I was getting the following error, probably due to a missing DI configuration, so I temporarily added = true to test.

% make phpstan
php bin/phpstan clear-result-cache -q && php -d memory_limit=448M bin/phpstan

In Resolver.php line 677:

  Service 'rules.26' (type of PHPStan\Rules\Classes\ClassConstantRule): Parameter $checkNonStringableDynamicAccess in ClassConstantRule::__construct() has no class type or default value, so its value must be
  specified.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My suggestion above will help you get rid of this error.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Do not make this optional.
  2. Put #[AutowiredParameter(ref: '%featureToggles.checkNonStringableDynamicAccess%')] above the parameter.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After you composer dump it should work.

)
{
}
Expand All @@ -63,6 +65,27 @@ public function processNode(Node $node, Scope $scope): array
$name = $constantString->getValue();
$constantNameScopes[$name] = $scope->filterByTruthyValue(new Identical($node->name, new String_($name)));
}

if ($this->checkNonStringableDynamicAccess) {
$typeResult = $this->ruleLevelHelper->findTypeToCheck(
$scope,
$node->name,
'',
static fn (Type $type) => $type->isString()->yes(),
);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After this call there should be:

		$type = $typeResult->getType();
		if ($type instanceof ErrorType) {
			return [];
		}

Some rules return "unknown class errors" but that's irrelevant here.

Copy link
Contributor Author

@zonuexe zonuexe Jul 11, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dynamic fetching of constants doesn't do implicit casting, so it seems appropriate to simply check isString()->yes() without toString().

I was wrong in the early stages of this PR.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd rename the variable to $nameTypeResult.


$type = $typeResult->getType();

if (!$type->isString()->yes()) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (!$type->isString()->yes()) {
if (!$type instanceof ErrorType && !$type->isString()->yes()) {

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All calls to findTypeToCheck follow this pattern. ErrorType is returned if unknown classes were encountered there.

$className = $node->class instanceof Name
? $scope->resolveName($node->class)
: $scope->getType($node->class)->describe(VerbosityLevel::typeOnly());

$errors[] = RuleErrorBuilder::message(sprintf('Cannot fetch constant from %s with a non-stringable type %s.', $className, $nameType->describe(VerbosityLevel::precise())))
->identifier('classConstant.fetchInvalidExpression')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error message and the identifier should reflect we're trying to access a class constant by non-stringable name. That's not obvious.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also it might be nice to mention the class name here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error message is wrong regarding the isString/toString point you made above. This is no longer about "stringables", but about "strings". Also the identifier should reflect it's about "name".

->build();
}
}
}

foreach ($constantNameScopes as $constantName => $constantScope) {
Expand Down
72 changes: 71 additions & 1 deletion tests/PHPStan/Rules/Classes/ClassConstantRuleTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,15 @@ protected function getRule(): Rule
$reflectionProvider = self::createReflectionProvider();
return new ClassConstantRule(
$reflectionProvider,
new RuleLevelHelper($reflectionProvider, true, false, true, false, false, false, true),
new RuleLevelHelper($reflectionProvider, true, false, true, true, true, false, true),
new ClassNameCheck(
new ClassCaseSensitivityCheck($reflectionProvider, true),
new ClassForbiddenNameCheck(self::getContainer()),
$reflectionProvider,
self::getContainer(),
),
new PhpVersion($this->phpVersion),
true,
);
}

Expand All @@ -59,6 +60,10 @@ public function testClassConstant(): void
'Access to undefined constant ClassConstantNamespace\Foo::DOLOR.',
10,
],
[
'Cannot access constant LOREM on mixed.',
11,
],
[
'Access to undefined constant ClassConstantNamespace\Foo::DOLOR.',
16,
Expand Down Expand Up @@ -439,6 +444,14 @@ public function testDynamicAccess(): void
$this->phpVersion = PHP_VERSION_ID;

$this->analyse([__DIR__ . '/data/dynamic-constant-access.php'], [
[
'Access to undefined constant ClassConstantDynamicAccess\Foo::FOO.',
17,
],
[
'Cannot fetch constant from ClassConstantDynamicAccess\Foo with a non-stringable type object.',
19,
],
[
'Access to undefined constant ClassConstantDynamicAccess\Foo::FOO.',
20,
Expand Down Expand Up @@ -474,4 +487,61 @@ public function testDynamicAccess(): void
]);
}

#[RequiresPhp('>= 8.3')]
public function testStringableDynamicAccess(): void
{
$this->phpVersion = PHP_VERSION_ID;

$this->analyse([__DIR__ . '/data/dynamic-constant-stringable-access.php'], [
[
'Cannot fetch constant from ClassConstantDynamicStringableAccess\Foo with a non-stringable type mixed.',
14,
],
[
'Cannot fetch constant from ClassConstantDynamicStringableAccess\Foo with a non-stringable type string|null.',
15,
],
[
'Cannot fetch constant from ClassConstantDynamicStringableAccess\Foo with a non-stringable type Stringable|null.',
16,
],
[
'Cannot fetch constant from ClassConstantDynamicStringableAccess\Foo with a non-stringable type int.',
17,
],
[
'Cannot fetch constant from ClassConstantDynamicStringableAccess\Foo with a non-stringable type int|null.',
18,
],
[
'Cannot fetch constant from ClassConstantDynamicStringableAccess\Foo with a non-stringable type DateTime|string.',
19,
],
[
'Cannot fetch constant from ClassConstantDynamicStringableAccess\Foo with a non-stringable type 1111.',
20,
],
[
'Cannot fetch constant from ClassConstantDynamicStringableAccess\Foo with a non-stringable type Stringable.',
22,
],
[
'Cannot fetch constant from ClassConstantDynamicStringableAccess\Foo with a non-stringable type mixed.',
32,
],
[
'Cannot fetch constant from ClassConstantDynamicStringableAccess\Bar with a non-stringable type mixed.',
33,
],
[
'Cannot fetch constant from DateTime|DateTimeImmutable with a non-stringable type mixed.',
38,
],
[
'Cannot fetch constant from object with a non-stringable type mixed.',
39,
],
]);
}

}
3 changes: 1 addition & 2 deletions tests/PHPStan/Rules/Classes/data/dynamic-constant-access.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ public function test(string $string, object $obj): void
{
$bar = 'FOO';

echo self::{$foo};
echo self::{$bar};
echo self::{$string};
echo self::{$obj};
echo self::{$this->name};
Expand Down Expand Up @@ -44,5 +44,4 @@ public function testScope(): void
echo self::{$name};
}


}
42 changes: 42 additions & 0 deletions tests/PHPStan/Rules/Classes/data/dynamic-constant-stringable-access.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
<?php // lint >= 8.3

namespace ClassConstantDynamicStringableAccess;

use Stringable;
use DateTime;
use DateTimeImmutable;

abstract class Foo
{

public function test(mixed $mixed, ?string $nullableStr, ?Stringable $nullableStringable, int $int, ?int $nullableInt, DateTime|string $datetimeOrStr, Stringable $stringable): void
{
echo self::{$mixed};
echo self::{$nullableStr};
echo self::{$nullableStringable};
echo self::{$int};
echo self::{$nullableInt};
echo self::{$datetimeOrStr};
echo self::{1111};
echo self::{(string)$stringable};
echo self::{$stringable}; // Uncast Stringable objects will cause a runtime error
}

}

final class Bar extends Foo
{

public function test(mixed $mixed, ?string $nullableStr, ?Stringable $nullableStringable, int $int, ?int $nullableInt, DateTime|string $datetimeOrStr, Stringable $stringable): void
{
echo parent::{$mixed};
echo self::{$mixed};
}

public function testClassDynamic(DateTime|DateTimeImmutable $datetime, object $obj, mixed $mixed): void
{
echo $datetime::{$mixed};
echo $obj::{$mixed};
}

}
Loading