Add type checking for class const assignments, fix several other cons…

…t issues.
vimeo · Jan 22, 2022 · 558208e · 558208e
1 parent c877ce0
commit 558208e
Show file tree

Hide file tree

Showing 16 changed files with 241 additions and 5 deletions.
diff --git a/config.xsd b/config.xsd
@@ -255,6 +255,7 @@
             <xs:element name="InvalidCatch" type="ClassIssueHandlerType" minOccurs="0" />
             <xs:element name="InvalidClass" type="ClassIssueHandlerType" minOccurs="0" />
             <xs:element name="InvalidClone" type="IssueHandlerType" minOccurs="0" />
+            <xs:element name="InvalidConstantAssignmentValue" type="ClassConstantIssueHandlerType" minOccurs="0" />
             <xs:element name="InvalidDocblock" type="IssueHandlerType" minOccurs="0" />
             <xs:element name="InvalidDocblockParamName" type="IssueHandlerType" minOccurs="0" />
             <xs:element name="InvalidEnumBackingType" type="ClassIssueHandlerType" minOccurs="0" />
@@ -603,6 +604,24 @@
         <xs:attribute name="errorLevel" type="ErrorLevelType" />
     </xs:complexType>
 
+    <xs:complexType name="ClassConstantIssueHandlerType">
+        <xs:sequence>
+            <xs:element name="errorLevel" minOccurs="0" maxOccurs="unbounded">
+                <xs:complexType>
+                    <xs:choice maxOccurs="unbounded">
+                        <xs:element name="directory" minOccurs="0" maxOccurs="unbounded" type="NameAttributeType" />
+                        <xs:element name="file" minOccurs="0" maxOccurs="unbounded" type="NameAttributeType" />
+                        <xs:element name="referencedConstant" minOccurs="0" maxOccurs="unbounded" type="NameAttributeType" />
+                    </xs:choice>
+
+                    <xs:attribute name="type" type="ErrorLevelType" use="required" />
+                </xs:complexType>
+            </xs:element>
+        </xs:sequence>
+
+        <xs:attribute name="errorLevel" type="ErrorLevelType" />
+    </xs:complexType>
+
     <xs:complexType name="VariableIssueHandlerType">
         <xs:sequence>
             <xs:element name="errorLevel" minOccurs="0" maxOccurs="unbounded">

diff --git a/docs/running_psalm/issues/InvalidConstantAssignmentValue.md b/docs/running_psalm/issues/InvalidConstantAssignmentValue.md
@@ -0,0 +1,12 @@
+# InvalidConstantAssignmentValue
+
+Emitted when attempting to assign a value to a class constant that cannot contain that type.
+
+```php
+<?php
+
+class Foo {
+    /** @var int */
+    public const BAR = "bar";
+}
+```
diff --git a/src/Psalm/Config.php b/src/Psalm/Config.php
@@ -27,6 +27,7 @@
 use Psalm\Internal\Provider\AddRemoveTaints\HtmlFunctionTainter;
 use Psalm\Internal\Scanner\FileScanner;
 use Psalm\Issue\ArgumentIssue;
+use Psalm\Issue\ClassConstantIssue;
 use Psalm\Issue\ClassIssue;
 use Psalm\Issue\CodeIssue;
 use Psalm\Issue\ConfigIssue;
@@ -1583,6 +1584,8 @@ public function getReportingLevelForIssue(CodeIssue $e): string
             $reporting_level = $this->getReportingLevelForFunction($issue_type, $e->function_id);
         } elseif ($e instanceof PropertyIssue) {
             $reporting_level = $this->getReportingLevelForProperty($issue_type, $e->property_id);
+        } elseif ($e instanceof ClassConstantIssue) {
+            $reporting_level = $this->getReportingLevelForClassConstant($issue_type, $e->const_id);
         } elseif ($e instanceof ArgumentIssue && $e->function_id) {
             $reporting_level = $this->getReportingLevelForArgument($issue_type, $e->function_id);
         } elseif ($e instanceof VariableIssue) {
@@ -1794,6 +1797,15 @@ public function getReportingLevelForProperty(string $issue_type, string $propert
         return null;
     }
 
+    public function getReportingLevelForClassConstant(string $issue_type, string $constant_id): ?string
+    {
+        if (isset($this->issue_handlers[$issue_type])) {
+            return $this->issue_handlers[$issue_type]->getReportingLevelForClassConstant($constant_id);
+        }
+
+        return null;
+    }
+
     public function getReportingLevelForVariable(string $issue_type, string $var_name): ?string
     {
         if (isset($this->issue_handlers[$issue_type])) {

diff --git a/src/Psalm/Config/FileFilter.php b/src/Psalm/Config/FileFilter.php
@@ -66,6 +66,11 @@ class FileFilter
      */
     protected $property_ids = [];
 
+    /**
+     * @var array<string>
+     */
+    protected $class_constant_ids = [];
+
     /**
      * @var array<string>
      */
@@ -326,6 +331,13 @@ public static function loadFromArray(
             }
         }
 
+        if (isset($config['referencedConstant']) && is_iterable($config['referencedConstant'])) {
+            /** @var array $referenced_constant */
+            foreach ($config['referencedConstant'] as $referenced_constant) {
+                $filter->class_constant_ids[] = strtolower((string) ($referenced_constant['name'] ?? ''));
+            }
+        }
+
         if (isset($config['referencedVariable']) && is_iterable($config['referencedVariable'])) {
             /** @var array $referenced_variable */
             foreach ($config['referencedVariable'] as $referenced_variable) {
@@ -400,6 +412,14 @@ public static function loadFromXMLElement(
             }
         }
 
+        if ($e->referencedConstant) {
+            $config['referencedConstant'] = [];
+            /** @var SimpleXMLElement $referenced_constant */
+            foreach ($e->referencedConstant as $referenced_constant) {
+                $config['referencedConstant'][]['name'] = strtolower((string)$referenced_constant['name']);
+            }
+        }
+
         if ($e->referencedVariable) {
             $config['referencedVariable'] = [];
 
@@ -533,6 +553,11 @@ public function allowsProperty(string $property_id): bool
         return in_array(strtolower($property_id), $this->property_ids, true);
     }
 
+    public function allowsClassConstant(string $constant_id): bool
+    {
+        return in_array(strtolower($constant_id), $this->class_constant_ids, true);
+    }
+
     public function allowsVariable(string $var_name): bool
     {
         return in_array(strtolower($var_name), $this->var_names, true);

diff --git a/src/Psalm/Config/IssueHandler.php b/src/Psalm/Config/IssueHandler.php
@@ -131,6 +131,17 @@ public function getReportingLevelForProperty(string $property_id): ?string
         return null;
     }
 
+    public function getReportingLevelForClassConstant(string $constant_id): ?string
+    {
+        foreach ($this->custom_levels as $custom_level) {
+            if ($custom_level->allowsClassConstant($constant_id)) {
+                return $custom_level->getErrorLevel();
+            }
+        }
+
+        return null;
+    }
+
     public function getReportingLevelForVariable(string $var_name): ?string
     {
         foreach ($this->custom_levels as $custom_level) {
@@ -155,6 +166,7 @@ public static function getAllIssueTypes(): array
             fn(string $issue_name): bool => $issue_name !== ''
                 && $issue_name !== 'MethodIssue'
                 && $issue_name !== 'PropertyIssue'
+                && $issue_name !== 'ClassConstantIssue'
                 && $issue_name !== 'FunctionIssue'
                 && $issue_name !== 'ArgumentIssue'
                 && $issue_name !== 'VariableIssue'

diff --git a/src/Psalm/Internal/Analyzer/ClassLikeAnalyzer.php b/src/Psalm/Internal/Analyzer/ClassLikeAnalyzer.php
@@ -22,6 +22,7 @@
 use Psalm\StatementsSource;
 use Psalm\Storage\ClassLikeStorage;
 use Psalm\Type;
+use Psalm\Type\Atomic\TKeyedArray;
 use Psalm\Type\Union;
 use UnexpectedValueException;
 
@@ -30,6 +31,7 @@
 use function gettype;
 use function implode;
 use function in_array;
+use function is_array;
 use function is_float;
 use function is_int;
 use function is_string;
@@ -519,10 +521,26 @@ public static function getTypeFromValue($value): Union
      * Gets the Psalm literal type from a particular value
      *
      * @param array|scalar|null $value
+     * @throws InvalidArgumentException
      *
      */
-    public static function getLiteralTypeFromValue($value): Type\Union
+    public static function getLiteralTypeFromValue($value, bool $sealed_array = true): Type\Union
     {
+        if (is_array($value)) {
+            if (empty($value)) {
+                return Type::getEmptyArray();
+            }
+
+            $types = [];
+            /** @var array|scalar|null $val */
+            foreach ($value as $key => $val) {
+                $types[$key] = self::getLiteralTypeFromValue($val, $sealed_array);
+            }
+            $type = new TKeyedArray($types);
+            $type->sealed = $sealed_array;
+            return new Type\Union([$type]);
+        }
+
         if (is_string($value)) {
             return Type::getString($value);
         }
@@ -543,7 +561,11 @@ public static function getLiteralTypeFromValue($value): Type\Union
             return Type::getTrue();
         }
 
-        return Type::getNull();
+        if ($value === null) {
+            return Type::getNull();
+        }
+
+        throw new InvalidArgumentException();
     }
 
     /**

diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/ArrayAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/ArrayAnalyzer.php
@@ -117,6 +117,12 @@ public static function analyze(
             $item_value_type = null;
         }
 
+        if ($item_key_type === null && $item_value_type === null) {
+            $statements_analyzer->node_data->setType($stmt, Type::getEmptyArray());
+
+            return true;
+        }
+
         // if this array looks like an object-like array, let's return that instead
         if ($item_value_type
             && $item_key_type

diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/ClassConstFetchAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/ClassConstFetchAnalyzer.php
@@ -15,11 +15,13 @@
 use Psalm\Internal\Analyzer\StatementsAnalyzer;
 use Psalm\Internal\Analyzer\TraitAnalyzer;
 use Psalm\Internal\FileManipulation\FileManipulationBuffer;
+use Psalm\Internal\Type\Comparator\UnionTypeComparator;
 use Psalm\Issue\CircularReference;
 use Psalm\Issue\DeprecatedClass;
 use Psalm\Issue\DeprecatedConstant;
 use Psalm\Issue\InaccessibleClassConstant;
 use Psalm\Issue\InternalClass;
+use Psalm\Issue\InvalidConstantAssignmentValue;
 use Psalm\Issue\NonStaticSelfCall;
 use Psalm\Issue\ParentNotFound;
 use Psalm\Issue\UndefinedConstant;
@@ -35,6 +37,7 @@
 use Psalm\Type\Union;
 use ReflectionProperty;
 
+use function assert;
 use function explode;
 use function in_array;
 use function strtolower;
@@ -676,6 +679,32 @@ public static function analyzeClassConstAssignment(
     ): void {
         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];
+            if ($assigned_type = $statements_analyzer->node_data->getType($const->value)) {
+                if ($const_storage->type !== null
+                    && $const_storage->stmt_location !== null
+                    && $assigned_type !== $const_storage->type
+                    && !UnionTypeComparator::isContainedBy(
+                        $statements_analyzer->getCodebase(),
+                        $assigned_type,
+                        $const_storage->type
+                    )
+                ) {
+                    IssueBuffer::maybeAdd(
+                        new InvalidConstantAssignmentValue(
+                            "{$context->self}::{$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}"
+                        ),
+                        $const_storage->suppressed_issues,
+                        true
+                    );
+                }
+            }
         }
     }
 }
diff --git a/src/Psalm/Internal/Codebase/ConstantTypeResolver.php b/src/Psalm/Internal/Codebase/ConstantTypeResolver.php
@@ -137,7 +137,9 @@ public static function resolve(
                 }
 
                 if ($left instanceof TKeyedArray && $right instanceof TKeyedArray) {
-                    return new TKeyedArray($left->properties + $right->properties);
+                    $type = new TKeyedArray($left->properties + $right->properties);
+                    $type->sealed = true;
+                    return $type;
                 }
 
                 return new TMixed;

diff --git a/src/Psalm/Internal/PhpVisitor/Reflector/ClassLikeNodeScanner.php b/src/Psalm/Internal/PhpVisitor/Reflector/ClassLikeNodeScanner.php
@@ -1270,8 +1270,10 @@ private function visitClassConstDeclaration(
             );
 
             $type_location = null;
+            $suppressed_issues = [];
             if ($var_comment !== null && $var_comment->type !== null) {
                 $const_type = $var_comment->type;
+                $suppressed_issues = $var_comment->suppressed_issues;
 
                 if ($var_comment->type_start !== null
                     && $var_comment->type_end !== null
@@ -1301,6 +1303,7 @@ private function visitClassConstDeclaration(
                     $const->name
                 )
             );
+            $constant_storage->suppressed_issues = $suppressed_issues;
 
             $constant_storage->type_location = $type_location;
 

diff --git a/src/Psalm/Issue/ClassConstantIssue.php b/src/Psalm/Issue/ClassConstantIssue.php
@@ -0,0 +1,21 @@
+<?php
+namespace Psalm\Issue;
+
+use Psalm\CodeLocation;
+
+abstract class ClassConstantIssue extends CodeIssue
+{
+    /**
+     * @var string
+     */
+    public $const_id;
+
+    public function __construct(
+        string $message,
+        CodeLocation $code_location,
+        string $const_id
+    ) {
+        parent::__construct($message, $code_location);
+        $this->const_id = $const_id;
+    }
+}
diff --git a/src/Psalm/Issue/InvalidConstantAssignmentValue.php b/src/Psalm/Issue/InvalidConstantAssignmentValue.php
@@ -0,0 +1,8 @@
+<?php
+namespace Psalm\Issue;
+
+class InvalidConstantAssignmentValue extends ClassConstantIssue
+{
+    public const ERROR_LEVEL = 6;
+    public const SHORTCODE = 304;
+}
diff --git a/src/Psalm/Storage/ClassConstantStorage.php b/src/Psalm/Storage/ClassConstantStorage.php
@@ -61,6 +61,11 @@ class ClassConstantStorage
      */
     public $attributes = [];
 
+    /**
+     * @var array<int, string>
+     */
+    public $suppressed_issues = [];
+
     /**
      * @var ?string
      */

diff --git a/tests/ArrayAssignmentTest.php b/tests/ArrayAssignmentTest.php
@@ -1514,9 +1514,8 @@ function unpackIterable(Traversable $data): array
                     $y = [];
 
                     $x = [...$x, ...$y];
-
-                    $x ? 1 : 0;
                 ',
+                'assertions' => ['$x' => 'array<never, never>']
             ],
             'unpackEmptyKeepsCorrectKeys' => [
                 'code' => '<?php

diff --git a/tests/Config/ConfigTest.php b/tests/Config/ConfigTest.php
@@ -443,6 +443,11 @@ public function testIssueHandlerWithCustomErrorLevels(): void
                                 <referencedVariable name="a" />
                             </errorLevel>
                         </UndefinedGlobalVariable>
+                        <InvalidConstantAssignmentValue>
+                            <errorLevel type="suppress">
+                                <referencedConstant name="Psalm\Bodger::FOO" />
+                            </errorLevel>
+                        </InvalidConstantAssignmentValue>
                     </issueHandlers>
                 </psalm>'
             )
@@ -589,6 +594,14 @@ public function testIssueHandlerWithCustomErrorLevels(): void
                 'b'
             )
         );
+
+        $this->assertSame(
+            'suppress',
+            $config->getReportingLevelForClassConstant(
+                'InvalidConstantAssignmentValue',
+                'Psalm\Bodger::FOO'
+            )
+        );
     }
 
     public function testIssueHandlerSetDynamically(): void