diff --git a/src/SpatieLaravelData/Rules/ValidTypeRule.php b/src/SpatieLaravelData/Rules/ValidTypeRule.php index b8fd0ba..7be593d 100644 --- a/src/SpatieLaravelData/Rules/ValidTypeRule.php +++ b/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 * diff --git a/src/TypeSystem/IntersectionType.php b/src/TypeSystem/IntersectionType.php index 6f988c3..4c8bc8e 100644 --- a/src/TypeSystem/IntersectionType.php +++ b/src/TypeSystem/IntersectionType.php @@ -77,6 +77,23 @@ 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" * @@ -84,13 +101,13 @@ public function __toString(): string */ 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; } } diff --git a/src/TypeSystem/Type.php b/src/TypeSystem/Type.php index 7bef4a1..3a6712f 100644 --- a/src/TypeSystem/Type.php +++ b/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 diff --git a/src/TypeSystem/UnionType.php b/src/TypeSystem/UnionType.php index 2ed42e3..e1aaf7e 100644 --- a/src/TypeSystem/UnionType.php +++ b/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 diff --git a/test/Samples/CastedSpatieLaravelData.php b/test/Samples/CastedSpatieLaravelData.php index 02e6745..7fa311e 100644 --- a/test/Samples/CastedSpatieLaravelData.php +++ b/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 diff --git a/test/Samples/InvalidNullableCastedSpatieLaravelData.php b/test/Samples/InvalidNullableCastedSpatieLaravelData.php new file mode 100644 index 0000000..ee371b2 --- /dev/null +++ b/test/Samples/InvalidNullableCastedSpatieLaravelData.php @@ -0,0 +1,23 @@ + null, // We should know for certain that null is not accepted here + ], + ); + } +} diff --git a/test/SpatieLaravelData/ValidTypeRuleTest.php b/test/SpatieLaravelData/ValidTypeRuleTest.php index c5efaa5..a5e7fe6 100644 --- a/test/SpatieLaravelData/ValidTypeRuleTest.php +++ b/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 {