diff --git a/config.xsd b/config.xsd index 92a91a81b2e..a272922a501 100644 --- a/config.xsd +++ b/config.xsd @@ -218,9 +218,9 @@ - + @@ -313,6 +313,7 @@ + @@ -358,10 +359,10 @@ - - + + @@ -372,11 +373,13 @@ + + @@ -425,8 +428,8 @@ - + @@ -484,8 +487,8 @@ - + diff --git a/docs/running_psalm/issues/AmbiguousConstantInheritance.md b/docs/running_psalm/issues/AmbiguousConstantInheritance.md new file mode 100644 index 00000000000..7fd02204151 --- /dev/null +++ b/docs/running_psalm/issues/AmbiguousConstantInheritance.md @@ -0,0 +1,42 @@ +# AmbiguousConstantInheritance + +Emitted when a constant is inherited from multiple sources. + +```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/OverriddenInterfaceConstant.md b/docs/running_psalm/issues/OverriddenInterfaceConstant.md new file mode 100644 index 00000000000..2e36de280de --- /dev/null +++ b/docs/running_psalm/issues/OverriddenInterfaceConstant.md @@ -0,0 +1,17 @@ +# OverriddenInterfaceConstant + +Emitted when a constant declared on an interface is overridden by a child (illegal in PHP < 8.1). + +```php +analyze($member_stmts, $class_context, $global_context, true); + ClassConstAnalyzer::analyze($storage, $this->getCodebase()); + $config = Config::getInstance(); if ($class instanceof PhpParser\Node\Stmt\Class_) { diff --git a/src/Psalm/Internal/Analyzer/InterfaceAnalyzer.php b/src/Psalm/Internal/Analyzer/InterfaceAnalyzer.php index 2b17ceff514..213f2976842 100644 --- a/src/Psalm/Internal/Analyzer/InterfaceAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/InterfaceAnalyzer.php @@ -6,12 +6,17 @@ use PhpParser; use Psalm\CodeLocation; use Psalm\Context; +use Psalm\FileManipulation; +use Psalm\Internal\Analyzer\Statements\Expression\ClassConstAnalyzer; +use Psalm\Internal\FileManipulation\FileManipulationBuffer; use Psalm\Internal\Provider\NodeDataProvider; use Psalm\Issue\ParseError; use Psalm\Issue\UndefinedInterface; use Psalm\IssueBuffer; use UnexpectedValueException; +use function strtolower; + /** * @internal */ @@ -31,6 +36,8 @@ public function analyze(): void throw new LogicException('Something went badly wrong'); } + $interface_context = new Context($this->fq_class_name); + $project_analyzer = $this->file_analyzer->project_analyzer; $codebase = $project_analyzer->getCodebase(); $config = $project_analyzer->getConfig(); @@ -105,6 +112,7 @@ public function analyze(): void ); } + $member_stmts = []; foreach ($this->class->stmts as $stmt) { if ($stmt instanceof PhpParser\Node\Stmt\ClassMethod) { $method_analyzer = new MethodAnalyzer($stmt, $this); @@ -140,7 +148,35 @@ public function analyze(): void ); return; + } elseif ($stmt instanceof PhpParser\Node\Stmt\ClassConst) { + $member_stmts[] = $stmt; + + foreach ($stmt->consts as $const) { + $const_id = strtolower($this->fq_class_name) . '::' . $const->name; + + foreach ($codebase->class_constants_to_rename as $original_const_id => $new_const_name) { + if ($const_id === $original_const_id) { + $file_manipulations = [ + new FileManipulation( + (int) $const->name->getAttribute('startFilePos'), + (int) $const->name->getAttribute('endFilePos') + 1, + $new_const_name + ) + ]; + + FileManipulationBuffer::add( + $this->getFilePath(), + $file_manipulations + ); + } + } + } } } + + $statements_analyzer = new StatementsAnalyzer($this, new NodeDataProvider()); + $statements_analyzer->analyze($member_stmts, $interface_context, null, true); + + ClassConstAnalyzer::analyze($this->storage, $this->getCodebase()); } } diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/ClassConstFetchAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/ClassConstAnalyzer.php similarity index 78% rename from src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/ClassConstFetchAnalyzer.php rename to src/Psalm/Internal/Analyzer/Statements/Expression/ClassConstAnalyzer.php index f87f6f2226b..c50e9d9481d 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/ClassConstFetchAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/ClassConstAnalyzer.php @@ -1,9 +1,10 @@ class and * analyse the ::class int $stmt->name separately */ - public static function analyze( + public static function analyzeFetch( StatementsAnalyzer $statements_analyzer, PhpParser\Node\Expr\ClassConstFetch $stmt, Context $context @@ -651,17 +657,19 @@ public static function analyze( return true; } - public static function analyzeClassConstAssignment( + public static function analyzeAssignment( StatementsAnalyzer $statements_analyzer, PhpParser\Node\Stmt\ClassConst $stmt, Context $context ): void { + assert($context->self !== null); + $class_storage = $statements_analyzer->getCodebase()->classlike_storage_provider->get($context->self); + foreach ($stmt->consts as $const) { ExpressionAnalyzer::analyze($statements_analyzer, $const->value, $context); - - assert($context->self !== null); - $class_storage = $statements_analyzer->getCodebase()->classlike_storage_provider->get($context->self); $const_storage = $class_storage->constants[$const->name->name]; + + // Check assigned type matches docblock type if ($assigned_type = $statements_analyzer->node_data->getType($const->value)) { if ($const_storage->type !== null && $const_storage->stmt_location !== null @@ -674,10 +682,10 @@ public static function analyzeClassConstAssignment( ) { IssueBuffer::maybeAdd( new InvalidConstantAssignmentValue( - "{$context->self}::{$const->name->name} with declared type {$const_storage->type->getId()} " - . "cannot be assigned type {$assigned_type->getId()}", + "{$class_storage->name}::{$const->name->name} with declared type " + . "{$const_storage->type->getId()} cannot be assigned type {$assigned_type->getId()}", $const_storage->stmt_location, - "{$context->self}::{$const->name->name}" + "{$class_storage->name}::{$const->name->name}" ), $const_storage->suppressed_issues, true @@ -686,4 +694,143 @@ public static function analyzeClassConstAssignment( } } } + + public static function analyze( + ClassLikeStorage $class_storage, + Codebase $codebase + ): void { + foreach ($class_storage->constants as $const_name => $const_storage) { + // Check covariance + /** @psalm-suppress PossiblyNullArrayAccess https://github.com/vimeo/psalm/issues/7151 */ + [$parent_classlike_storage, $parent_const_storage] = self::getOverriddenConstant( + $class_storage, + $const_storage, + $const_name, + $codebase + ); + /** @psalm-suppress RedundantConditionGivenDocblockType https://github.com/vimeo/psalm/issues/7151 */ + if ($parent_const_storage !== null) { + $location = $const_storage->type_location ?? $const_storage->stmt_location; + if ($location !== null + && $const_storage->type !== null + && $parent_const_storage->type !== null + && !UnionTypeComparator::isContainedBy( + $codebase, + $const_storage->type, + $parent_const_storage->type + ) + ) { + 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, + "{$class_storage->name}::{$const_name}" + ), + $const_storage->suppressed_issues + ); + } + } + } + } + + /** + * Get the const storage from the parent or interface that this class is overriding. + * + * @return array{ClassLikeStorage, ClassConstantStorage}|null + */ + private static function getOverriddenConstant( + ClassLikeStorage $class_storage, + ClassConstantStorage $const_storage, + string $const_name, + Codebase $codebase + ): ?array { + $parent_classlike_storage = $interface_const_storage = $parent_const_storage = null; + $interface_overrides = []; + foreach ($class_storage->class_implements ?: $class_storage->direct_interface_parents as $interface) { + $interface_storage = $codebase->classlike_storage_provider->get($interface); + $parent_const_storage = $interface_storage->constants[$const_name] ?? null; + if ($parent_const_storage !== null) { + if ($const_storage->location + && $const_storage !== $parent_const_storage + && ( + $codebase->php_major_version < 8 + || ($codebase->php_major_version === 8 && $codebase->php_minor_version < 1) + ) + ) { + $interface_overrides[strtolower($interface)] = new OverriddenInterfaceConstant( + "{$class_storage->name}::{$const_name} cannot override constant from $interface", + $const_storage->location, + "{$class_storage->name}::{$const_name}" + ); + } + if ($interface_const_storage !== null && $const_storage->location !== null) { + assert($parent_classlike_storage !== null); + if (!isset($parent_classlike_storage->parent_interfaces[strtolower($interface)]) + && !isset($interface_storage->parent_interfaces[strtolower($parent_classlike_storage->name)]) + ) { + IssueBuffer::maybeAdd( + new AmbiguousConstantInheritance( + "Ambiguous inheritance of {$class_storage->name}::{$const_name} from $interface and " + . $parent_classlike_storage->name, + $const_storage->location, + "{$class_storage->name}::{$const_name}" + ), + $const_storage->suppressed_issues + ); + } + } + $interface_const_storage = $parent_const_storage; + $parent_classlike_storage = $interface_storage; + } + } + + foreach ($class_storage->parent_classes as $parent_class) { + $parent_class_storage = $codebase->classlike_storage_provider->get($parent_class); + $parent_const_storage = $parent_class_storage->constants[$const_name] ?? null; + if ($parent_const_storage !== null) { + if ($const_storage->location !== null && $interface_const_storage !== null) { + assert($parent_classlike_storage !== null); + if (!isset($parent_class_storage->class_implements[strtolower($parent_classlike_storage->name)])) { + IssueBuffer::maybeAdd( + new AmbiguousConstantInheritance( + "Ambiguous inheritance of {$class_storage->name}::{$const_name} from " + . "$parent_classlike_storage->name and $parent_class", + $const_storage->location, + "{$class_storage->name}::{$const_name}" + ), + $const_storage->suppressed_issues + ); + } + } + foreach ($interface_overrides as $interface_lc => $_) { + // If the parent is the one with the const that's overriding the interface const, and the parent + // doesn't implement the interface, it's just an AmbiguousConstantInheritance, not an + // OverriddenInterfaceConstant + if (!isset($parent_class_storage->class_implements[$interface_lc]) + && $parent_const_storage === $const_storage + ) { + unset($interface_overrides[$interface_lc]); + } + } + $parent_classlike_storage = $parent_class_storage; + break; + } + } + + foreach ($interface_overrides as $_ => $issue) { + IssueBuffer::maybeAdd( + $issue, + $const_storage->suppressed_issues + ); + } + + if ($parent_classlike_storage !== null) { + assert($parent_const_storage !== null); + return [$parent_classlike_storage, $parent_const_storage]; + } + return null; + } } diff --git a/src/Psalm/Internal/Analyzer/Statements/ExpressionAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/ExpressionAnalyzer.php index 46350fb848c..947e50b12fa 100644 --- a/src/Psalm/Internal/Analyzer/Statements/ExpressionAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/ExpressionAnalyzer.php @@ -206,7 +206,7 @@ private static function handleExpression( } if ($stmt instanceof PhpParser\Node\Expr\ClassConstFetch) { - return Expression\Fetch\ClassConstFetchAnalyzer::analyze($statements_analyzer, $stmt, $context); + return Expression\ClassConstAnalyzer::analyzeFetch($statements_analyzer, $stmt, $context); } if ($stmt instanceof PhpParser\Node\Expr\PropertyFetch) { diff --git a/src/Psalm/Internal/Analyzer/StatementsAnalyzer.php b/src/Psalm/Internal/Analyzer/StatementsAnalyzer.php index 39badf3bcec..46eed8ad3ec 100644 --- a/src/Psalm/Internal/Analyzer/StatementsAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/StatementsAnalyzer.php @@ -19,7 +19,7 @@ use Psalm\Internal\Analyzer\Statements\Block\WhileAnalyzer; use Psalm\Internal\Analyzer\Statements\Expression\Assignment\InstancePropertyAssignmentAnalyzer; use Psalm\Internal\Analyzer\Statements\Expression\AssignmentAnalyzer; -use Psalm\Internal\Analyzer\Statements\Expression\Fetch\ClassConstFetchAnalyzer; +use Psalm\Internal\Analyzer\Statements\Expression\ClassConstAnalyzer; use Psalm\Internal\Analyzer\Statements\Expression\Fetch\ConstFetchAnalyzer; use Psalm\Internal\Analyzer\Statements\Expression\Fetch\VariableFetchAnalyzer; use Psalm\Internal\Analyzer\Statements\Expression\SimpleTypeInferer; @@ -575,7 +575,7 @@ private static function analyzeStatement( } elseif ($stmt instanceof PhpParser\Node\Stmt\Property) { InstancePropertyAssignmentAnalyzer::analyzeStatement($statements_analyzer, $stmt, $context); } elseif ($stmt instanceof PhpParser\Node\Stmt\ClassConst) { - ClassConstFetchAnalyzer::analyzeClassConstAssignment($statements_analyzer, $stmt, $context); + ClassConstAnalyzer::analyzeAssignment($statements_analyzer, $stmt, $context); } elseif ($stmt instanceof PhpParser\Node\Stmt\Class_) { try { $class_analyzer = new ClassAnalyzer( diff --git a/src/Psalm/Issue/AmbiguousConstantInheritance.php b/src/Psalm/Issue/AmbiguousConstantInheritance.php new file mode 100644 index 00000000000..9064237cbb9 --- /dev/null +++ b/src/Psalm/Issue/AmbiguousConstantInheritance.php @@ -0,0 +1,8 @@ + */ public const SHORTCODE = 205; /** diff --git a/src/Psalm/Type/Atomic.php b/src/Psalm/Type/Atomic.php index 7e39f434a58..263330e824c 100644 --- a/src/Psalm/Type/Atomic.php +++ b/src/Psalm/Type/Atomic.php @@ -60,6 +60,7 @@ abstract class Atomic implements TypeNode { + /** @var non-empty-lowercase-string */ public const KEY = 'atomic'; /** diff --git a/src/Psalm/Type/Atomic/TKeyedArray.php b/src/Psalm/Type/Atomic/TKeyedArray.php index 432f921f383..48c57d8e582 100644 --- a/src/Psalm/Type/Atomic/TKeyedArray.php +++ b/src/Psalm/Type/Atomic/TKeyedArray.php @@ -63,6 +63,7 @@ class TKeyedArray extends Atomic */ public $is_list = false; + /** @var non-empty-lowercase-string */ public const KEY = 'array'; /** @@ -99,7 +100,6 @@ function ($name, Union $type): string { sort($property_strings); } - /** @psalm-suppress MixedOperand */ return static::KEY . '{' . implode(', ', $property_strings) . '}'; } @@ -125,7 +125,6 @@ function ($name, Union $type): string { sort($property_strings); } - /** @psalm-suppress MixedOperand */ return static::KEY . '{' . implode(', ', $property_strings) . '}' @@ -156,7 +155,6 @@ public function toNamespacedString( ); } - /** @psalm-suppress MixedOperand */ return static::KEY . '{' . implode( ', ', diff --git a/src/Psalm/Type/Atomic/TList.php b/src/Psalm/Type/Atomic/TList.php index f6e240056e9..575cfceb004 100644 --- a/src/Psalm/Type/Atomic/TList.php +++ b/src/Psalm/Type/Atomic/TList.php @@ -25,6 +25,7 @@ class TList extends Atomic */ public $type_param; + /** @var non-empty-lowercase-string */ public const KEY = 'list'; /** @@ -37,13 +38,11 @@ public function __construct(Union $type_param) public function __toString(): string { - /** @psalm-suppress MixedOperand */ return static::KEY . '<' . $this->type_param . '>'; } public function getId(bool $nested = false): string { - /** @psalm-suppress MixedOperand */ return static::KEY . '<' . $this->type_param->getId() . '>'; } @@ -72,7 +71,6 @@ public function toNamespacedString( ); } - /** @psalm-suppress MixedOperand */ return static::KEY . '<' . $this->type_param->toNamespacedString( diff --git a/src/Psalm/Type/Atomic/TNonEmptyList.php b/src/Psalm/Type/Atomic/TNonEmptyList.php index aeaa8278b42..dd0815afcb5 100644 --- a/src/Psalm/Type/Atomic/TNonEmptyList.php +++ b/src/Psalm/Type/Atomic/TNonEmptyList.php @@ -11,6 +11,7 @@ class TNonEmptyList extends TList */ public $count; + /** @var non-empty-lowercase-string */ public const KEY = 'non-empty-list'; public function getAssertionString(bool $exact = false): string diff --git a/tests/ConstantTest.php b/tests/ConstantTest.php index ee80b061b3f..dc157c8a5f9 100644 --- a/tests/ConstantTest.php +++ b/tests/ConstantTest.php @@ -427,6 +427,7 @@ function foo(string $s) : void { 'resolveClassConstToCurrentClass' => [ ' [ ' [ ' */ protected const A = 1; public static function test(): void { @@ -971,7 +976,9 @@ class B extends A { 'referenceClassConstantWithSelf' => [ ' */ public const KEYS = []; + /** @var array */ public const VALUES = []; } @@ -1289,6 +1296,57 @@ function foo(array $arg): void {} foo([...A::ARR]); ', ], + 'classConstCovariant' => [ + ' [ + ' [ + ' 'InvalidReturnStatement', + [], + false, + '8.1', ], 'outOfScopeDefinedConstant' => [ ' "InvalidConstantAssignmentValue", ], + 'classConstContravariant' => [ + ' "LessSpecificClassConstantType", + ], + 'classConstAmbiguousInherit' => [ + ' 'AmbiguousConstantInheritance', + ], + 'overrideClassConstFromInterface' => [ + ' 'OverriddenInterfaceConstant', + ], + 'overrideClassConstFromInterfaceWithInterface' => [ + ' 'OverriddenInterfaceConstant', + ], ]; } }