Merge pull request #12 from cego/niza/do-not-ignore-exact-null-types-for-casts

Added check for nullable, even for casted values

Author: nizacegodk

src/SpatieLaravelData/Rules/ValidTypeRule.php

@@ -115,22 +115,46 @@ private function compareTypes(Call $call, array $casts, Constructor $constructor
      */
     private function checkType(Call $call, string $key, UnionType $actualType, array $casts, UnionType $expectedType): ?RuleError
     {
-        // Ignore cases, where there exists a cast - since we cannot analyse them in dept
+        // Casters cannot cast nullable values, so quick test for type error
+        // is simply to check if the expected type accepts null values or not.
+        if ($actualType->isNullable() && $expectedType->isNotNullable()) {
+            return $this->buildError($call, $key, $expectedType, $actualType);
+        }
+
+        // Otherwise, ignore cases where there exists a cast - since we cannot analyse them in dept.
         if ($this->expectedTypesMatchesExactlyCast($casts, $expectedType)) {
             return null;
         }
 
+        // Run full type inspection and return any errors found.
         if ( ! TypeSystem::isSubtypeOf($actualType, $expectedType)) {
-            return RuleErrorBuilder::message(self::getErrorMessage($key, $call->target, $expectedType, $actualType))
-                ->line($call->method->line)
-                ->file($call->method->file)
-                ->tip('This is a custom CEGO rule, if you found a bug fix it in the cego/phpstan project')
-                ->build();
+            return $this->buildError($call, $key, $expectedType, $actualType);
         }
 
         return null;
     }
 
+    /**
+     * Builds a RuleError instance
+     *
+     * @param Call $call
+     * @param string $key
+     * @param string $expectedType
+     * @param string $actualType
+     *
+     * @throws ShouldNotHappenException
+     *
+     * @return RuleError
+     */
+    private function buildError(Call $call, string $key, string $expectedType, string $actualType): RuleError
+    {
+        return RuleErrorBuilder::message(self::getErrorMessage($key, $call->target, $expectedType, $actualType))
+            ->line($call->method->line)
+            ->file($call->method->file)
+            ->tip('This is a custom CEGO rule, if you found a bug fix it in the cego/phpstan project')
+            ->build();
+    }
+
     /**
      * Returns the error message to give the developer on errors
      *

src/TypeSystem/IntersectionType.php

@@ -77,20 +77,37 @@ public function __toString(): string
             ->implode('&');
     }
 
+    /**
+     * Returns true if the type is considered to accept "null"
+     *
+     * @return bool
+     */
+    public function isNullable(): bool
+    {
+        // An intersection type is mixed, if any of the types are considered nullable.
+        foreach ($this->types as $type) {
+            if ($type->isNull()) {
+                return true;
+            }
+        }
+
+        return false;
+    }
+
     /**
      * Returns true if the type is considered to accept "mixed"
      *
      * @return bool
      */
     public function isMixed(): bool
     {
-        // An intersection type is mixed, if all the types are considered mixed.
+        // An intersection type is mixed, if any of the types are considered mixed.
         foreach ($this->types as $type) {
-            if ( ! $type->isMixed()) {
-                return false;
+            if ($type->isMixed()) {
+                return true;
             }
         }
 
-        return true;
+        return false;
     }
 }

src/TypeSystem/Type.php

@@ -33,6 +33,16 @@ public function isNotReal(): bool
         return in_array(strtolower($this->type), ['void', 'never'], true);
     }
 
+    /**
+     * Returns true if the type is null
+     *
+     * @return bool
+     */
+    public function isNull(): bool
+    {
+        return strtolower($this->type) === 'null';
+    }
+
     /**
      * Returns true if the type is of mixed
      *
@@ -136,7 +146,7 @@ public function toString(): string
     public function __toString(): string
     {
         if ($this->isClassOrInterface()) {
-            return $this->type;
+            return ltrim($this->type, '\\');
         }
 
         // Handles when empty string == mixed

src/TypeSystem/UnionType.php

@@ -42,6 +42,33 @@ public static function fromString(string $type): self
         );
     }
 
+    /**
+     * Returns true if the type is nullable
+     *
+     * @return bool
+     */
+    public function isNullable(): bool
+    {
+        // A union type is nullable, if just one of the types are nullable
+        foreach ($this->intersectionTypes as $type) {
+            if ($type->isNullable()) {
+                return true;
+            }
+        }
+
+        return false;
+    }
+
+    /**
+     * Returns true if the type is not nullable
+     *
+     * @return bool
+     */
+    public function isNotNullable(): bool
+    {
+        return ! $this->isNullable();
+    }
+
     public function isMixed(): bool
     {
         // A union type is mixed, if just one of the types are considered mixed

test/Samples/CastedSpatieLaravelData.php

@@ -14,33 +14,6 @@ public function __construct(
 
     public function initDefault(): self
     {
-        self::from(
-            [
-                'castedProperty' => 'hello world', // A cast exists, so we don't actually know if this is legal or not
-            ],
-            [
-                'castedProperty' => 123,          // A cast exists, so we don't actually know if this is legal or not | Maybe the cast converts the int into a string :shrug:
-            ],
-        );
-
-        self::from(
-            [
-                'castedProperty' => 'hello world', // A cast exists, so we don't actually know if this is legal or not
-            ],
-            [
-                'castedProperty' => 123,          // A cast exists, so we don't actually know if this is legal or not | Maybe the cast converts the int into a string :shrug:
-            ],
-        );
-
-        self::from(
-            [
-                'castedProperty' => 'hello world', // A cast exists, so we don't actually know if this is legal or not
-            ],
-            [
-                'castedProperty' => 123,          // A cast exists, so we don't actually know if this is legal or not | Maybe the cast converts the int into a string :shrug:
-            ],
-        );
-
         return self::from(
             [
                 'castedProperty' => 'hello world', // A cast exists, so we don't actually know if this is legal or not

test/Samples/InvalidNullableCastedSpatieLaravelData.php

@@ -0,0 +1,23 @@
+<?php
+
+namespace Test\Samples;
+
+use Spatie\LaravelData\Data;
+use Test\Samples\Enum\BackedEnumExample;
+
+class InvalidNullableCastedSpatieLaravelData extends Data
+{
+    public function __construct(
+        public readonly BackedEnumExample $castedProperty,
+    ) {
+    }
+
+    public function initDefault(): self
+    {
+        self::from(
+            [
+                'castedProperty' => null, // We should know for certain that null is not accepted here
+            ],
+        );
+    }
+}

test/SpatieLaravelData/ValidTypeRuleTest.php

@@ -4,9 +4,11 @@
 
 use PHPStan\Rules\Rule;
 use PHPStan\Testing\RuleTestCase;
+use Test\Samples\Enum\BackedEnumExample;
 use Test\Samples\InvalidScalarSpatieLaravelData;
 use Test\Samples\InvalidComplexSpatieLaravelData;
 use Cego\phpstan\SpatieLaravelData\Rules\ValidTypeRule;
+use Test\Samples\InvalidNullableCastedSpatieLaravelData;
 use Cego\phpstan\SpatieLaravelData\Collectors\CastCollector;
 use Cego\phpstan\SpatieLaravelData\Collectors\FromCollector;
 use Cego\phpstan\SpatieLaravelData\Collectors\ConstructorCollector;
@@ -64,7 +66,7 @@ public function it_returns_errors_for_invalid_complex_data(): void
             $this->expectError(20, 'nullableTypeStringProperty', InvalidComplexSpatieLaravelData::class, 'null|string', 'float'),
             $this->expectError(20, 'nullableMarkStringProperty', InvalidComplexSpatieLaravelData::class, 'null|string', 'array'),
             $this->expectError(20, 'nullableTypeStringProperty', InvalidComplexSpatieLaravelData::class, 'null|string', 'array'),
-            $this->expectError(20, 'intersectionProperty', InvalidComplexSpatieLaravelData::class, '\Spatie\LaravelData\Casts\Cast&\Stringable', 'stdClass'),
+            $this->expectError(20, 'intersectionProperty', InvalidComplexSpatieLaravelData::class, 'Spatie\LaravelData\Casts\Cast&Stringable', 'stdClass'),
         ]);
     }
 
@@ -78,6 +80,18 @@ public function it_ignores_potential_problems_for_objects_with_casts(): void
         ], []);
     }
 
+    /** @test */
+    public function it_does_not_ignore_null_check_for_casted_types(): void
+    {
+        $this->analyse([
+            __DIR__ . '/../Samples/InvalidNullableCastedSpatieLaravelData.php',
+            __DIR__ . '/../Samples/Casts/BackedEnumCast.php',
+            __DIR__ . '/../Samples/Casts/UncastableSpatieDataCast.php',
+        ], [
+            $this->expectError(17, 'castedProperty', InvalidNullableCastedSpatieLaravelData::class, BackedEnumExample::class, 'null'),
+        ]);
+    }
+
     /** @test */
     public function it_does_not_care_about_generics(): void
     {