diff --git a/config.xsd b/config.xsd index 2211a48e638..ff3dd6a15dd 100644 --- a/config.xsd +++ b/config.xsd @@ -249,6 +249,7 @@ + @@ -343,6 +344,7 @@ + diff --git a/docs/running_psalm/issues.md b/docs/running_psalm/issues.md index a06d6349e7e..0d1d1b128dd 100644 --- a/docs/running_psalm/issues.md +++ b/docs/running_psalm/issues.md @@ -61,6 +61,7 @@ - [InvalidCast](issues/InvalidCast.md) - [InvalidCatch](issues/InvalidCatch.md) - [InvalidClass](issues/InvalidClass.md) + - [InvalidClassConstantType](issues/InvalidClassConstantType.md) - [InvalidClone](issues/InvalidClone.md) - [InvalidConstantAssignmentValue](issues/InvalidConstantAssignmentValue.md) - [InvalidDocblock](issues/InvalidDocblock.md) @@ -155,6 +156,7 @@ - [NullPropertyAssignment](issues/NullPropertyAssignment.md) - [NullPropertyFetch](issues/NullPropertyFetch.md) - [NullReference](issues/NullReference.md) + - [OverriddenFinalConstant](issues/OverriddenFinalConstant.md) - [OverriddenInterfaceConstant](issues/OverriddenInterfaceConstant.md) - [OverriddenMethodAccess](issues/OverriddenMethodAccess.md) - [OverriddenPropertyAccess](issues/OverriddenPropertyAccess.md) diff --git a/docs/running_psalm/issues/InvalidClassConstantType.md b/docs/running_psalm/issues/InvalidClassConstantType.md new file mode 100644 index 00000000000..01e770175c0 --- /dev/null +++ b/docs/running_psalm/issues/InvalidClassConstantType.md @@ -0,0 +1,28 @@ +# InvalidClassConstantType + +Emitted when a constant type in a child does not satisfy the type in the parent. + +```php + */ + public const CONSTANT = 3; + + public static function bar(): array + { + return str_split("foobar", static::CONSTANT); + } +} + +class Bar extends Foo +{ + /** @var int */ + public const CONSTANT = -1; +} + +Bar::bar(); // Error: str_split argument 2 must be greater than 0 +``` + +This issue will always show up when overriding a constant that doesn't have a docblock type. Psalm will infer the most specific type for the constant that it can, you have to add a type annotation to tell it what type constraint you wish to be applied. Otherwise Psalm has no way of telling if you mean for the constant to be a literal `1`, `int<1, max>`, `int`, `numeric`, etc. diff --git a/docs/running_psalm/issues/OverriddenFinalConstant.md b/docs/running_psalm/issues/OverriddenFinalConstant.md new file mode 100644 index 00000000000..fa04c1fdb44 --- /dev/null +++ b/docs/running_psalm/issues/OverriddenFinalConstant.md @@ -0,0 +1,19 @@ +# OverriddenFinalConstant + +Emitted when a constant declared as final is overridden in a child class or interface. + +```php +constants as $const_name => $const_storage) { - // Check covariance [$parent_classlike_storage, $parent_const_storage] = self::getOverriddenConstant( $class_storage, $const_storage, $const_name, $codebase ); + + $type_location = $const_storage->location ?? $const_storage->stmt_location; + if ($type_location === null) { + continue; + } + if ($parent_const_storage !== null) { assert($parent_classlike_storage !== null); - $location = $const_storage->type_location ?? $const_storage->stmt_location; - if ($location !== null - && $const_storage->type !== null + + // Check covariance + if ($const_storage->type !== null && $parent_const_storage->type !== null && !UnionTypeComparator::isContainedBy( $codebase, @@ -740,19 +748,65 @@ public static function analyze( $parent_const_storage->type ) ) { + if (UnionTypeComparator::isContainedBy( + $codebase, + $parent_const_storage->type, + $const_storage->type, + )) { + // Contravariant + IssueBuffer::maybeAdd( + new LessSpecificClassConstantType( + "The type \"{$const_storage->type->getId()}\" for {$class_storage->name}::" + . "{$const_name} is more general than the type " + . "\"{$parent_const_storage->type->getId()}\" inherited from " + . "{$parent_classlike_storage->name}::{$const_name}", + $type_location, + "{$class_storage->name}::{$const_name}" + ), + $const_storage->suppressed_issues + ); + } else { + // Completely different + IssueBuffer::maybeAdd( + new InvalidClassConstantType( + "The type \"{$const_storage->type->getId()}\" for {$class_storage->name}::" + . "{$const_name} does not satisfy the type " + . "\"{$parent_const_storage->type->getId()}\" inherited from " + . "{$parent_classlike_storage->name}::{$const_name}", + $type_location, + "{$class_storage->name}::{$const_name}" + ), + $const_storage->suppressed_issues + ); + } + } + + // Check overridden final + if ($parent_const_storage->final) { IssueBuffer::maybeAdd( - new LessSpecificClassConstantType( - "The type '{$const_storage->type->getId()}' for {$class_storage->name}::" - . "{$const_name} is more general than the type " - . "'{$parent_const_storage->type->getId()}' inherited from " - . "{$parent_classlike_storage->name}::{$const_name}", - $location, + new OverriddenFinalConstant( + "{$const_name} cannot be overridden because it is marked as final in " + . $parent_classlike_storage->name, + $type_location, "{$class_storage->name}::{$const_name}" ), $const_storage->suppressed_issues ); } } + + if ($const_storage->stmt_location !== null) { + // Check final in PHP < 8.1 + if ($codebase->analysis_php_version_id < 8_01_00 && $const_storage->final) { + IssueBuffer::maybeAdd( + new ParseError( + "Class constants cannot be marked final before PHP 8.1", + $const_storage->stmt_location, + ), + $const_storage->suppressed_issues + ); + } + } } } diff --git a/src/Psalm/Internal/PhpVisitor/Reflector/ClassLikeNodeScanner.php b/src/Psalm/Internal/PhpVisitor/Reflector/ClassLikeNodeScanner.php index 0cb67909e86..ae25c5cadeb 100644 --- a/src/Psalm/Internal/PhpVisitor/Reflector/ClassLikeNodeScanner.php +++ b/src/Psalm/Internal/PhpVisitor/Reflector/ClassLikeNodeScanner.php @@ -989,7 +989,7 @@ private function extendTemplatedType( $extended_type_parameters = []; $storage->template_type_extends_count[$atomic_type->value] = count($atomic_type->type_params); - + foreach ($atomic_type->type_params as $type_param) { $extended_type_parameters[] = $type_param; } @@ -1340,6 +1340,7 @@ private function visitClassConstDeclaration( } $constant_storage->description = $description; + $constant_storage->final = $stmt->isFinal(); foreach ($stmt->attrGroups as $attr_group) { foreach ($attr_group->attrs as $attr) { diff --git a/src/Psalm/Issue/InvalidClassConstantType.php b/src/Psalm/Issue/InvalidClassConstantType.php new file mode 100644 index 00000000000..ee49503056b --- /dev/null +++ b/src/Psalm/Issue/InvalidClassConstantType.php @@ -0,0 +1,9 @@ + */ diff --git a/tests/ConstantTest.php b/tests/ConstantTest.php index c5de2a910a9..1c62914a355 100644 --- a/tests/ConstantTest.php +++ b/tests/ConstantTest.php @@ -1879,10 +1879,58 @@ class Baz implements Foo, Bar public const BAR=""; } ', - 'error_message' => "LessSpecificClassConstantType", + 'error_message' => "InvalidClassConstantType", 'ignored_issues' => [], 'php_version' => '8.1', ], + 'overrideFinalClassConstFromExtendedClass' => [ + 'code' => ' "OverriddenFinalConstant", + 'ignored_issues' => [], + 'php_version' => '8.1', + ], + 'overrideFinalClassConstFromImplementedInterface' => [ + 'code' => ' "OverriddenFinalConstant", + 'ignored_issues' => [], + 'php_version' => '8.1', + ], + 'finalConstantIsIllegalBefore8.1' => [ + 'code' => ' 'ParseError - src' . DIRECTORY_SEPARATOR . 'somefile.php:5:44', + 'ignored_issues' => [], + 'php_version' => '8.0', + ], ]; } } diff --git a/tests/DocumentationTest.php b/tests/DocumentationTest.php index 69fe0293c33..23484bc70cc 100644 --- a/tests/DocumentationTest.php +++ b/tests/DocumentationTest.php @@ -268,22 +268,12 @@ public function providerInvalidCodeParse(): array $php_version = '8.0'; $ignored_issues = []; switch ($issue_name) { - case 'MissingThrowsDocblock': - continue 2; - - case 'UncaughtThrowInGlobalScope': - continue 2; - case 'InvalidStringClass': - continue 2; - + case 'MissingThrowsDocblock': case 'PluginClass': - continue 2; - case 'RedundantIdentityWithTrue': - continue 2; - case 'TraitMethodSignatureMismatch': + case 'UncaughtThrowInGlobalScope': continue 2; /** @todo reinstate this test when the issue is restored */ @@ -320,12 +310,13 @@ public function providerInvalidCodeParse(): array break; case 'AmbiguousConstantInheritance': - case 'InvalidEnumBackingType': - case 'InvalidEnumCaseValue': + case 'DeprecatedConstant': case 'DuplicateEnumCase': case 'DuplicateEnumCaseValue': + case 'InvalidEnumBackingType': + case 'InvalidEnumCaseValue': case 'NoEnumProperties': - case 'DeprecatedConstant': + case 'OverriddenFinalConstant': $php_version = '8.1'; break; }