From 1a55bef9de026763beeeed1fc8f2bc679bda00ba Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Mon, 6 Nov 2023 14:08:41 +0100 Subject: [PATCH] PHP 8.3 - check class constant PHPDoc type and native type against assigned value --- conf/config.level2.neon | 1 + src/Reflection/ClassConstantReflection.php | 5 + .../ValueAssignedToClassConstantRule.php | 133 ++++++++++++++++++ ...ncompatibleClassConstantPhpDocTypeRule.php | 37 ++--- .../ValueAssignedToClassConstantRuleTest.php | 81 +++++++++++ .../{PhpDoc => Constants}/data/bug-5655.php | 0 .../{PhpDoc => Constants}/data/bug-7273.php | 0 .../{PhpDoc => Constants}/data/bug-7273b.php | 0 .../data/bug-7352-with-sub-namespace.php | 0 .../{PhpDoc => Constants}/data/bug-7352.php | 0 ...assigned-to-class-constant-native-type.php | 23 +++ .../data/value-assigned-to-class-constant.php | 49 +++++++ ...patibleClassConstantPhpDocTypeRuleTest.php | 53 +++---- ...ible-class-constant-phpdoc-native-type.php | 19 +++ .../incompatible-class-constant-phpdoc.php | 38 ----- 15 files changed, 347 insertions(+), 92 deletions(-) create mode 100644 src/Rules/Constants/ValueAssignedToClassConstantRule.php create mode 100644 tests/PHPStan/Rules/Constants/ValueAssignedToClassConstantRuleTest.php rename tests/PHPStan/Rules/{PhpDoc => Constants}/data/bug-5655.php (100%) rename tests/PHPStan/Rules/{PhpDoc => Constants}/data/bug-7273.php (100%) rename tests/PHPStan/Rules/{PhpDoc => Constants}/data/bug-7273b.php (100%) rename tests/PHPStan/Rules/{PhpDoc => Constants}/data/bug-7352-with-sub-namespace.php (100%) rename tests/PHPStan/Rules/{PhpDoc => Constants}/data/bug-7352.php (100%) create mode 100644 tests/PHPStan/Rules/Constants/data/value-assigned-to-class-constant-native-type.php create mode 100644 tests/PHPStan/Rules/Constants/data/value-assigned-to-class-constant.php create mode 100644 tests/PHPStan/Rules/PhpDoc/data/incompatible-class-constant-phpdoc-native-type.php diff --git a/conf/config.level2.neon b/conf/config.level2.neon index 1d30ec755f..56d536f22e 100644 --- a/conf/config.level2.neon +++ b/conf/config.level2.neon @@ -13,6 +13,7 @@ rules: - PHPStan\Rules\Cast\PrintRule - PHPStan\Rules\Classes\AccessPrivateConstantThroughStaticRule - PHPStan\Rules\Comparison\UsageOfVoidMatchExpressionRule + - PHPStan\Rules\Constants\ValueAssignedToClassConstantRule - PHPStan\Rules\Functions\IncompatibleDefaultParameterTypeRule - PHPStan\Rules\Generics\ClassAncestorsRule - PHPStan\Rules\Generics\ClassTemplateTypeRule diff --git a/src/Reflection/ClassConstantReflection.php b/src/Reflection/ClassConstantReflection.php index 1ee5b65882..614370424f 100644 --- a/src/Reflection/ClassConstantReflection.php +++ b/src/Reflection/ClassConstantReflection.php @@ -64,6 +64,11 @@ public function hasPhpDocType(): bool return $this->phpDocType !== null; } + public function getPhpDocType(): ?Type + { + return $this->phpDocType; + } + public function hasNativeType(): bool { return $this->nativeType !== null; diff --git a/src/Rules/Constants/ValueAssignedToClassConstantRule.php b/src/Rules/Constants/ValueAssignedToClassConstantRule.php new file mode 100644 index 0000000000..9f342bc947 --- /dev/null +++ b/src/Rules/Constants/ValueAssignedToClassConstantRule.php @@ -0,0 +1,133 @@ + + */ +class ValueAssignedToClassConstantRule implements Rule +{ + + public function getNodeType(): string + { + return Node\Stmt\ClassConst::class; + } + + public function processNode(Node $node, Scope $scope): array + { + if (!$scope->isInClass()) { + throw new ShouldNotHappenException(); + } + + $nativeType = null; + if ($node->type !== null) { + $nativeType = ParserNodeTypeToPHPStanType::resolve($node->type, $scope->getClassReflection()); + } + + $errors = []; + foreach ($node->consts as $const) { + $constantName = $const->name->toString(); + $errors = array_merge($errors, $this->processSingleConstant( + $scope->getClassReflection(), + $constantName, + $scope->getType($const->value), + $nativeType, + )); + } + + return $errors; + } + + /** + * @return RuleError[] + */ + private function processSingleConstant(ClassReflection $classReflection, string $constantName, Type $valueExprType, ?Type $nativeType): array + { + $constantReflection = $classReflection->getConstant($constantName); + if (!$constantReflection instanceof ClassConstantReflection) { + return []; + } + + $phpDocType = $constantReflection->getPhpDocType(); + if ($phpDocType === null) { + if ($nativeType === null) { + return []; + } + + $isSuperType = $nativeType->isSuperTypeOf($valueExprType); + if ($isSuperType->yes()) { + return []; + } + + return [ + RuleErrorBuilder::message(sprintf( + 'Constant %s::%s (%s) does not accept value %s.', + $constantReflection->getDeclaringClass()->getDisplayName(), + $constantName, + $nativeType->describe(VerbosityLevel::typeOnly()), + $valueExprType->describe(VerbosityLevel::value()), + ))->nonIgnorable()->build(), + ]; + } elseif ($nativeType === null) { + $isSuperType = $phpDocType->isSuperTypeOf($valueExprType); + $verbosity = VerbosityLevel::getRecommendedLevelByType($phpDocType, $valueExprType); + if ($isSuperType->no()) { + return [ + RuleErrorBuilder::message(sprintf( + 'PHPDoc tag @var for constant %s::%s with type %s is incompatible with value %s.', + $constantReflection->getDeclaringClass()->getDisplayName(), + $constantName, + $phpDocType->describe($verbosity), + $valueExprType->describe(VerbosityLevel::value()), + ))->build(), + ]; + + } elseif ($isSuperType->maybe()) { + return [ + RuleErrorBuilder::message(sprintf( + 'PHPDoc tag @var for constant %s::%s with type %s is not subtype of value %s.', + $constantReflection->getDeclaringClass()->getDisplayName(), + $constantName, + $phpDocType->describe($verbosity), + $valueExprType->describe(VerbosityLevel::value()), + ))->build(), + ]; + } + + return []; + } + + $type = $constantReflection->getValueType(); + $isSuperType = $type->isSuperTypeOf($valueExprType); + if ($isSuperType->yes()) { + return []; + } + + $verbosity = VerbosityLevel::getRecommendedLevelByType($type, $valueExprType); + + return [ + RuleErrorBuilder::message(sprintf( + 'Constant %s::%s (%s) does not accept value %s.', + $constantReflection->getDeclaringClass()->getDisplayName(), + $constantName, + $type->describe(VerbosityLevel::typeOnly()), + $valueExprType->describe($verbosity), + ))->build(), + ]; + } + +} diff --git a/src/Rules/PhpDoc/IncompatibleClassConstantPhpDocTypeRule.php b/src/Rules/PhpDoc/IncompatibleClassConstantPhpDocTypeRule.php index f25f1be2cb..611ea3507e 100644 --- a/src/Rules/PhpDoc/IncompatibleClassConstantPhpDocTypeRule.php +++ b/src/Rules/PhpDoc/IncompatibleClassConstantPhpDocTypeRule.php @@ -7,13 +7,13 @@ use PHPStan\Internal\SprintfHelper; use PHPStan\Reflection\ClassConstantReflection; use PHPStan\Reflection\ClassReflection; -use PHPStan\Reflection\InitializerExprContext; -use PHPStan\Reflection\InitializerExprTypeResolver; use PHPStan\Rules\Generics\GenericObjectTypeCheck; use PHPStan\Rules\Rule; use PHPStan\Rules\RuleError; use PHPStan\Rules\RuleErrorBuilder; use PHPStan\ShouldNotHappenException; +use PHPStan\Type\ParserNodeTypeToPHPStanType; +use PHPStan\Type\Type; use PHPStan\Type\VerbosityLevel; use function array_merge; use function sprintf; @@ -27,7 +27,6 @@ class IncompatibleClassConstantPhpDocTypeRule implements Rule public function __construct( private GenericObjectTypeCheck $genericObjectTypeCheck, private UnresolvableTypeHelper $unresolvableTypeHelper, - private InitializerExprTypeResolver $initializerExprTypeResolver, ) { } @@ -43,10 +42,15 @@ public function processNode(Node $node, Scope $scope): array throw new ShouldNotHappenException(); } + $nativeType = null; + if ($node->type !== null) { + $nativeType = ParserNodeTypeToPHPStanType::resolve($node->type, $scope->getClassReflection()); + } + $errors = []; foreach ($node->consts as $const) { $constantName = $const->name->toString(); - $errors = array_merge($errors, $this->processSingleConstant($scope->getClassReflection(), $constantName)); + $errors = array_merge($errors, $this->processSingleConstant($scope->getClassReflection(), $nativeType, $constantName)); } return $errors; @@ -55,19 +59,18 @@ public function processNode(Node $node, Scope $scope): array /** * @return RuleError[] */ - private function processSingleConstant(ClassReflection $classReflection, string $constantName): array + private function processSingleConstant(ClassReflection $classReflection, ?Type $nativeType, string $constantName): array { $constantReflection = $classReflection->getConstant($constantName); if (!$constantReflection instanceof ClassConstantReflection) { return []; } - if (!$constantReflection->hasPhpDocType()) { + $phpDocType = $constantReflection->getPhpDocType(); + if ($phpDocType === null) { return []; } - $phpDocType = $constantReflection->getValueType(); - $errors = []; if ( $this->unresolvableTypeHelper->containsUnresolvableType($phpDocType) @@ -77,26 +80,24 @@ private function processSingleConstant(ClassReflection $classReflection, string $constantReflection->getDeclaringClass()->getName(), $constantName, ))->build(); - } else { - $nativeType = $this->initializerExprTypeResolver->getType($constantReflection->getValueExpr(), InitializerExprContext::fromClassReflection($constantReflection->getDeclaringClass())); - $isSuperType = $phpDocType->isSuperTypeOf($nativeType); - $verbosity = VerbosityLevel::getRecommendedLevelByType($phpDocType, $nativeType); + } elseif ($nativeType !== null) { + $isSuperType = $nativeType->isSuperTypeOf($phpDocType); if ($isSuperType->no()) { $errors[] = RuleErrorBuilder::message(sprintf( - 'PHPDoc tag @var for constant %s::%s with type %s is incompatible with value %s.', + 'PHPDoc tag @var for constant %s::%s with type %s is incompatible with native type %s.', $constantReflection->getDeclaringClass()->getDisplayName(), $constantName, - $phpDocType->describe($verbosity), - $nativeType->describe(VerbosityLevel::value()), + $phpDocType->describe(VerbosityLevel::typeOnly()), + $nativeType->describe(VerbosityLevel::typeOnly()), ))->build(); } elseif ($isSuperType->maybe()) { $errors[] = RuleErrorBuilder::message(sprintf( - 'PHPDoc tag @var for constant %s::%s with type %s is not subtype of value %s.', + 'PHPDoc tag @var for constant %s::%s with type %s is not subtype of native type %s.', $constantReflection->getDeclaringClass()->getDisplayName(), $constantName, - $phpDocType->describe($verbosity), - $nativeType->describe(VerbosityLevel::value()), + $phpDocType->describe(VerbosityLevel::typeOnly()), + $nativeType->describe(VerbosityLevel::typeOnly()), ))->build(); } } diff --git a/tests/PHPStan/Rules/Constants/ValueAssignedToClassConstantRuleTest.php b/tests/PHPStan/Rules/Constants/ValueAssignedToClassConstantRuleTest.php new file mode 100644 index 0000000000..cbb29f090f --- /dev/null +++ b/tests/PHPStan/Rules/Constants/ValueAssignedToClassConstantRuleTest.php @@ -0,0 +1,81 @@ + + */ +class ValueAssignedToClassConstantRuleTest extends RuleTestCase +{ + + protected function getRule(): TRule + { + return new ValueAssignedToClassConstantRule(); + } + + public function testRule(): void + { + $this->analyse([__DIR__ . '/data/value-assigned-to-class-constant.php'], [ + [ + 'PHPDoc tag @var for constant ValueAssignedToClassConstant\Foo::BAZ with type string is incompatible with value 1.', + 14, + ], + [ + 'PHPDoc tag @var for constant ValueAssignedToClassConstant\Foo::DOLOR with type ValueAssignedToClassConstant\Foo is incompatible with value 1.', + 23, + ], + [ + 'PHPDoc tag @var for constant ValueAssignedToClassConstant\Bar::BAZ with type string is incompatible with value 2.', + 32, + ], + ]); + } + + public function testBug7352(): void + { + $this->analyse([__DIR__ . '/data/bug-7352.php'], []); + } + + public function testBug7352WithSubNamespace(): void + { + $this->analyse([__DIR__ . '/data/bug-7352-with-sub-namespace.php'], []); + } + + public function testBug7273(): void + { + $this->analyse([__DIR__ . '/data/bug-7273.php'], []); + } + + public function testBug7273b(): void + { + $this->analyse([__DIR__ . '/data/bug-7273b.php'], []); + } + + public function testBug5655(): void + { + $this->analyse([__DIR__ . '/data/bug-5655.php'], []); + } + + public function testNativeType(): void + { + if (PHP_VERSION_ID < 80300) { + $this->markTestSkipped('Test requires PHP 8.3.'); + } + + $this->analyse([__DIR__ . '/data/value-assigned-to-class-constant-native-type.php'], [ + [ + 'Constant ValueAssignedToClassConstantNativeType\Foo::BAR (int) does not accept value \'bar\'.', + 10, + ], + [ + 'Constant ValueAssignedToClassConstantNativeType\Bar::BAR (int<1, max>) does not accept value 0.', + 21, + ], + ]); + } + +} diff --git a/tests/PHPStan/Rules/PhpDoc/data/bug-5655.php b/tests/PHPStan/Rules/Constants/data/bug-5655.php similarity index 100% rename from tests/PHPStan/Rules/PhpDoc/data/bug-5655.php rename to tests/PHPStan/Rules/Constants/data/bug-5655.php diff --git a/tests/PHPStan/Rules/PhpDoc/data/bug-7273.php b/tests/PHPStan/Rules/Constants/data/bug-7273.php similarity index 100% rename from tests/PHPStan/Rules/PhpDoc/data/bug-7273.php rename to tests/PHPStan/Rules/Constants/data/bug-7273.php diff --git a/tests/PHPStan/Rules/PhpDoc/data/bug-7273b.php b/tests/PHPStan/Rules/Constants/data/bug-7273b.php similarity index 100% rename from tests/PHPStan/Rules/PhpDoc/data/bug-7273b.php rename to tests/PHPStan/Rules/Constants/data/bug-7273b.php diff --git a/tests/PHPStan/Rules/PhpDoc/data/bug-7352-with-sub-namespace.php b/tests/PHPStan/Rules/Constants/data/bug-7352-with-sub-namespace.php similarity index 100% rename from tests/PHPStan/Rules/PhpDoc/data/bug-7352-with-sub-namespace.php rename to tests/PHPStan/Rules/Constants/data/bug-7352-with-sub-namespace.php diff --git a/tests/PHPStan/Rules/PhpDoc/data/bug-7352.php b/tests/PHPStan/Rules/Constants/data/bug-7352.php similarity index 100% rename from tests/PHPStan/Rules/PhpDoc/data/bug-7352.php rename to tests/PHPStan/Rules/Constants/data/bug-7352.php diff --git a/tests/PHPStan/Rules/Constants/data/value-assigned-to-class-constant-native-type.php b/tests/PHPStan/Rules/Constants/data/value-assigned-to-class-constant-native-type.php new file mode 100644 index 0000000000..93746f52bd --- /dev/null +++ b/tests/PHPStan/Rules/Constants/data/value-assigned-to-class-constant-native-type.php @@ -0,0 +1,23 @@ += 8.3 + +namespace ValueAssignedToClassConstantNativeType; + +class Foo +{ + + public const int FOO = 1; + + public const int BAR = 'bar'; + +} + +class Bar +{ + + /** @var int<1, max> */ + public const int FOO = 1; + + /** @var int<1, max> */ + public const int BAR = 0; + +} diff --git a/tests/PHPStan/Rules/Constants/data/value-assigned-to-class-constant.php b/tests/PHPStan/Rules/Constants/data/value-assigned-to-class-constant.php new file mode 100644 index 0000000000..bdcf81838c --- /dev/null +++ b/tests/PHPStan/Rules/Constants/data/value-assigned-to-class-constant.php @@ -0,0 +1,49 @@ + */ + const DOLOR = 1; + +} + +class Bar extends Foo +{ + + const BAR = 2; + + const BAZ = 2; + +} + +class Baz +{ + + /** @var string */ + private const BAZ = 'foo'; + +} + +class Lorem extends Baz +{ + + private const BAZ = 1; + +} diff --git a/tests/PHPStan/Rules/PhpDoc/IncompatibleClassConstantPhpDocTypeRuleTest.php b/tests/PHPStan/Rules/PhpDoc/IncompatibleClassConstantPhpDocTypeRuleTest.php index 042333cc92..a66be60a2d 100644 --- a/tests/PHPStan/Rules/PhpDoc/IncompatibleClassConstantPhpDocTypeRuleTest.php +++ b/tests/PHPStan/Rules/PhpDoc/IncompatibleClassConstantPhpDocTypeRuleTest.php @@ -2,10 +2,10 @@ namespace PHPStan\Rules\PhpDoc; -use PHPStan\Reflection\InitializerExprTypeResolver; use PHPStan\Rules\Generics\GenericObjectTypeCheck; use PHPStan\Rules\Rule; use PHPStan\Testing\RuleTestCase; +use const PHP_VERSION_ID; /** * @extends RuleTestCase @@ -15,7 +15,7 @@ class IncompatibleClassConstantPhpDocTypeRuleTest extends RuleTestCase protected function getRule(): Rule { - return new IncompatibleClassConstantPhpDocTypeRule(new GenericObjectTypeCheck(), new UnresolvableTypeHelper(), self::getContainer()->getByType(InitializerExprTypeResolver::class)); + return new IncompatibleClassConstantPhpDocTypeRule(new GenericObjectTypeCheck(), new UnresolvableTypeHelper()); } public function testRule(): void @@ -25,48 +25,29 @@ public function testRule(): void 'PHPDoc tag @var for constant IncompatibleClassConstantPhpDoc\Foo::FOO contains unresolvable type.', 9, ], - [ - 'PHPDoc tag @var for constant IncompatibleClassConstantPhpDoc\Foo::BAZ with type string is incompatible with value 1.', - 17, - ], - [ - 'PHPDoc tag @var for constant IncompatibleClassConstantPhpDoc\Foo::DOLOR with type IncompatibleClassConstantPhpDoc\Foo is incompatible with value 1.', - 26, - ], [ 'PHPDoc tag @var for constant IncompatibleClassConstantPhpDoc\Foo::DOLOR contains generic type IncompatibleClassConstantPhpDoc\Foo but class IncompatibleClassConstantPhpDoc\Foo is not generic.', - 26, - ], - [ - 'PHPDoc tag @var for constant IncompatibleClassConstantPhpDoc\Bar::BAZ with type string is incompatible with value 2.', - 35, + 12, ], ]); } - public function testBug7352(): void - { - $this->analyse([__DIR__ . '/data/bug-7352.php'], []); - } - - public function testBug7352WithSubNamespace(): void - { - $this->analyse([__DIR__ . '/data/bug-7352-with-sub-namespace.php'], []); - } - - public function testBug7273(): void - { - $this->analyse([__DIR__ . '/data/bug-7273.php'], []); - } - - public function testBug7273b(): void + public function testNativeType(): void { - $this->analyse([__DIR__ . '/data/bug-7273b.php'], []); - } + if (PHP_VERSION_ID < 80300) { + $this->markTestSkipped('Test requires PHP 8.3.'); + } - public function testBug5655(): void - { - $this->analyse([__DIR__ . '/data/bug-5655.php'], []); + $this->analyse([__DIR__ . '/data/incompatible-class-constant-phpdoc-native-type.php'], [ + [ + 'PHPDoc tag @var for constant IncompatibleClassConstantPhpDocNativeType\Foo::BAZ with type string is incompatible with native type int.', + 14, + ], + [ + 'PHPDoc tag @var for constant IncompatibleClassConstantPhpDocNativeType\Foo::LOREM with type int|string is not subtype of native type int.', + 17, + ], + ]); } } diff --git a/tests/PHPStan/Rules/PhpDoc/data/incompatible-class-constant-phpdoc-native-type.php b/tests/PHPStan/Rules/PhpDoc/data/incompatible-class-constant-phpdoc-native-type.php new file mode 100644 index 0000000000..91fada5626 --- /dev/null +++ b/tests/PHPStan/Rules/PhpDoc/data/incompatible-class-constant-phpdoc-native-type.php @@ -0,0 +1,19 @@ += 8.3 + +namespace IncompatibleClassConstantPhpDocNativeType; + +class Foo +{ + + public const int FOO = 1; + + /** @var positive-int */ + public const int BAR = 1; + + /** @var non-empty-string */ + public const int BAZ = 1; + + /** @var int|string */ + public const int LOREM = 1; + +} diff --git a/tests/PHPStan/Rules/PhpDoc/data/incompatible-class-constant-phpdoc.php b/tests/PHPStan/Rules/PhpDoc/data/incompatible-class-constant-phpdoc.php index c4f163e773..d2b9714425 100644 --- a/tests/PHPStan/Rules/PhpDoc/data/incompatible-class-constant-phpdoc.php +++ b/tests/PHPStan/Rules/PhpDoc/data/incompatible-class-constant-phpdoc.php @@ -8,45 +8,7 @@ class Foo /** @var self&\stdClass */ const FOO = 1; - /** @var int */ - const BAR = 1; - - const NO_TYPE = 'string'; - - /** @var string */ - const BAZ = 1; - - /** @var string|int */ - const LOREM = 1; - - /** @var int */ - const IPSUM = self::LOREM; // resolved to 1, I'd prefer string|int - /** @var self */ const DOLOR = 1; } - -class Bar extends Foo -{ - - const BAR = 2; - - const BAZ = 2; - -} - -class Baz -{ - - /** @var string */ - private const BAZ = 'foo'; - -} - -class Lorem extends Baz -{ - - private const BAZ = 1; - -}