From ca6d95bad163e4dc7f28bba7803ad8f044b2a88f Mon Sep 17 00:00:00 2001 From: Niki Zakariassen Date: Tue, 7 Feb 2023 10:25:45 +0100 Subject: [PATCH 01/14] First raw implementation --- composer.json | 14 ++ extension.neon | 23 ++- .../Collectors/CastCollector.php | 94 ++++++++++ .../Collectors/ConstructorCollector.php | 153 +++++++++++++++ .../Collectors/FromCollector.php | 102 ++++++++++ src/SpatieLaravelData/Rules/ValidTypeRule.php | 176 ++++++++++++++++++ test/Samples/EmptySpatieLaravelData.php | 9 + test/Samples/SpatieDataCast.php | 15 ++ test/Samples/SpatieLaravelData.php | 54 ++++++ test/Samples/main.php | 20 ++ 10 files changed, 658 insertions(+), 2 deletions(-) create mode 100644 src/SpatieLaravelData/Collectors/CastCollector.php create mode 100644 src/SpatieLaravelData/Collectors/ConstructorCollector.php create mode 100644 src/SpatieLaravelData/Collectors/FromCollector.php create mode 100644 src/SpatieLaravelData/Rules/ValidTypeRule.php create mode 100644 test/Samples/EmptySpatieLaravelData.php create mode 100644 test/Samples/SpatieDataCast.php create mode 100644 test/Samples/SpatieLaravelData.php create mode 100644 test/Samples/main.php diff --git a/composer.json b/composer.json index ec4474d..add3b9b 100644 --- a/composer.json +++ b/composer.json @@ -11,8 +11,22 @@ "email": "niza@cego.dk" } ], + "autoload": { + "psr-4": { + "Cego\\phpstan\\": "src/" + } + }, + "autoload-dev": { + "psr-4": { + "Test\\": "test/" + } + }, "require": { + "php": "^7.4|^8.0", "nunomaduro/larastan": "^2.1", "phpstan/phpstan": "^1.4" + }, + "require-dev": { + "spatie/laravel-data": "^3.0" } } diff --git a/extension.neon b/extension.neon index 11536cc..dcf60bc 100644 --- a/extension.neon +++ b/extension.neon @@ -1,5 +1,24 @@ -includes: - - ./../../nunomaduro/larastan/extension.neon +#includes: +# - ./../../nunomaduro/larastan/extension.neon + +services: + - + class: Cego\phpstan\SpatieLaravelData\Collectors\ConstructorCollector + tags: + - phpstan.collector + + - + class: Cego\phpstan\SpatieLaravelData\Collectors\FromCollector + tags: + - phpstan.collector + + - + class: Cego\phpstan\SpatieLaravelData\Collectors\CastCollector + tags: + - phpstan.collector + +rules: + - Cego\phpstan\SpatieLaravelData\Rules\ValidTypeRule parameters: level: 8 diff --git a/src/SpatieLaravelData/Collectors/CastCollector.php b/src/SpatieLaravelData/Collectors/CastCollector.php new file mode 100644 index 0000000..bb6ee7e --- /dev/null +++ b/src/SpatieLaravelData/Collectors/CastCollector.php @@ -0,0 +1,94 @@ +> + */ +class CastCollector implements Collector +{ + /** + * Returns the node type, this collector operates on + * + * @phpstan-return class-string + */ + public function getNodeType(): string + { + return InClassMethodNode::class; + } + + /** + * Process the nodes and stores value in the collector instance + * + * @phpstan-param StaticCall $node + * @return array|null Collected data + * @throws ShouldNotHappenException + */ + public function processNode(Node $node, Scope $scope): ?array + { + // Skip wrong nodes + if ( ! $node instanceof InClassMethodNode) { + return null; + } + + // Skip wrong methods + if ($this->isNotCastMethod($node)) { + return null; + } + + $variant = ParametersAcceptorSelector::selectSingle($node->getMethodReflection()->getVariants()); + $returnType = $variant->getReturnType(); + + return Str::of($returnType->describe(VerbosityLevel::typeOnly())) + // Get individual union types + ->explode('|') + // Get individual intersection types + ->map(fn (string $type) => Str::of($type)->explode('&')) + // For each intersection type (which might be an intersection of 1 item) + // Only keep cast information for classes / interfaces + ->map(function (Collection $intersectionTypes) { + $classTypes = $intersectionTypes + // We only care about classes / interfaces + ->filter(fn (string $type) => class_exists($type) || interface_exists($type)) + // We do not care for the uncastable class + ->reject(fn (string $type) => is_a($type, Uncastable::class, true)); + + // We only support intersection types of explicit classes / interfaces. + if ($intersectionTypes->count() !== $classTypes->count()) { + return collect(); + } + + return $classTypes; + }) + // Remove any intersection types we have deemed unfit + ->reject(fn (Collection $collection) => $collection->isEmpty()) + // And array the result so it can be serialized by PhpStan + ->toArray(); + } + + /** + * Returns true if the given node is not the cast method of a Cast class + * + * @param InClassMethodNode $node + * + * @return bool + */ + private function isNotCastMethod(InClassMethodNode $node): bool + { + return $node->getMethodReflection()->getName() !== 'cast' + || ! $node->getMethodReflection()->getDeclaringClass()->implementsInterface(Cast::class); + } +} diff --git a/src/SpatieLaravelData/Collectors/ConstructorCollector.php b/src/SpatieLaravelData/Collectors/ConstructorCollector.php new file mode 100644 index 0000000..68de649 --- /dev/null +++ b/src/SpatieLaravelData/Collectors/ConstructorCollector.php @@ -0,0 +1,153 @@ +>> + */ +class ConstructorCollector implements Collector +{ + /** + * Returns the node type, this collector operates on + * + * @phpstan-return class-string + */ + public function getNodeType(): string + { + return InClassMethodNode::class; + } + + /** + * Process the nodes and stores value in the collector instance + * + * @phpstan-param InClassMethodNode $node + * @return array>|null Collected data + */ + public function processNode(Node $node, Scope $scope): ?array + { + if ( ! $node instanceof InClassMethodNode) { + return null; + } + + if ($this->isNotSpatieLaravelDataConstructor($node)) { + return null; + } + + return [ + 'class' => $node->getMethodReflection()->getDeclaringClass()->getName(), + 'properties' => collect($node->getOriginalNode()->getParams()) + ->map(fn (Param $param) => $this->getParameterTypes($param)) + ->all(), + ]; + } + + /** + * Returns a key-value mapping of the parameter name and its allowed types + * + * @param Param $parameter + * + * @return array> + */ + private function getParameterTypes(Param $parameter): array + { + $name = $this->getParameterName($parameter); + $types = $this->parseType($parameter->type); + + return [$name => $types]; + } + + /** + * @param null|Identifier|Name|ComplexType $type + * + * @return array> + */ + private function parseType($type): array + { + // If no type is defined, then return mixed. + if ($type === null) { + return [['mixed']]; + } + + // Simple type (int, string, bool) + if ($type instanceof Identifier) { + return [[$type->name]]; + } + + // Class types + if ($type instanceof Name) { + // We do not support special type checking (self, parent, static) + // since we are unlikely to use this feature, + // and implementing it is currently not straight forward. + if ($type->isSpecialClassName()) { + return [['mixed']]; + } + + return [[$type->toCodeString()]]; + } + + // Complex types + if ($type instanceof Node\ComplexType) { + if ($type instanceof Node\NullableType) { + return [ + ...$this->parseType($type->type), + ['null'], + ]; + } + + if ($type instanceof Node\UnionType) { + return collect($type->types) + ->map(fn ($unionType) => $this->parseType($unionType)) + ->flatten(1) + ->all(); + } + + if ($type instanceof Node\IntersectionType) { + return [ + collect($type->types) + ->map(fn ($intersectionType) => $this->parseType($intersectionType)) + ->flatten(2) + ->all() + ]; + } + } + + return [['mixed']]; + } + + private function getParameterName(Param $parameter): string + { + if ( ! is_string($parameter->var->name)) { + throw new RuntimeException('A constructor property name cannot be an expression'); + } + + return $parameter->var->name; + } + + private function isNotSpatieLaravelDataConstructor(InClassMethodNode $node): bool + { + return $this->isNotConstructor($node) + || $this->isNotSpatieLaravelDataClass($node->getMethodReflection()->getDeclaringClass()); + } + + private function isNotConstructor(InClassMethodNode $node): bool + { + return $node->getMethodReflection()->getName() !== '__construct'; + } + + private function isNotSpatieLaravelDataClass(ClassReflection $class): bool + { + return ! in_array(Data::class, $class->getParentClassesNames(), true); + } +} diff --git a/src/SpatieLaravelData/Collectors/FromCollector.php b/src/SpatieLaravelData/Collectors/FromCollector.php new file mode 100644 index 0000000..368f666 --- /dev/null +++ b/src/SpatieLaravelData/Collectors/FromCollector.php @@ -0,0 +1,102 @@ +> + */ +class FromCollector implements Collector +{ + /** + * Returns the node type, this collector operates on + * + * @phpstan-return class-string + */ + public function getNodeType(): string + { + return StaticCall::class; + } + + /** + * Process the nodes and stores value in the collector instance + * + * @phpstan-param StaticCall $node + * @return array|null Collected data + */ + public function processNode(Node $node, Scope $scope): ?array + { + if ( ! $node instanceof StaticCall) { + return null; + } + + if ($this->isNotSpatieLaravelDataFromCall($node, $scope)) { + return null; + } + + $types = []; + + foreach ($node->args as $arg) { + if ($arg instanceof Node\VariadicPlaceholder) { + continue; + } + + if ( ! $arg->value instanceof Array_) { + continue; + } + + $argData = []; + + foreach ($arg->value->items as $item) { + if ( ! $item->key instanceof Node\Scalar\String_) { + continue; + } + + $type = $scope->getType($item->value); + + if ($type instanceof ConstantScalarType) { + $argData[$item->key->value] = get_debug_type($type->getValue()); + } else { + $argData[$item->key->value] = $scope->getType($item->value)->describe(VerbosityLevel::typeOnly()); + } + } + + $types[] = $argData; + } + + return [ + 'types' => $types, + 'target' => $this->getTargetClass($node, $scope), + 'method' => [ + 'file' => $scope->getFile(), + 'line' => $node->getLine(), + ], + ]; + } + + private function isNotSpatieLaravelDataFromCall(StaticCall $node, Scope $scope): bool + { + if (strtolower($node->name->name) !== 'from') { + return false; + } + + return ! is_a($this->getTargetClass($node, $scope), Data::class, true); + } + + private function getTargetClass(StaticCall $node, Scope $scope) + { + if ($node->class instanceof Node\Expr) { + return $scope->getType($node->class)->getReferencedClasses()[0]; + } + + return $scope->resolveName($node->class); + } +} diff --git a/src/SpatieLaravelData/Rules/ValidTypeRule.php b/src/SpatieLaravelData/Rules/ValidTypeRule.php new file mode 100644 index 0000000..9882233 --- /dev/null +++ b/src/SpatieLaravelData/Rules/ValidTypeRule.php @@ -0,0 +1,176 @@ + + */ + public function getNodeType(): string + { + return CollectedDataNode::class; + } + + /** + * @phpstan-param TNodeType $node + * @return (string|RuleError)[] errors + */ + public function processNode(Node $node, Scope $scope): array + { + if ( ! $node instanceof CollectedDataNode) { + return []; + } + + $fromCollector = $node->get(FromCollector::class); + + $castCollector = collect($node->get(CastCollector::class)) + ->map(fn (array $castTypes) => $this->convertTypeListToString($castTypes[0])) + ->flip() + ->all(); + + $classCollector = collect($node->get(ConstructorCollector::class)) + ->mapWithKeys(fn (array $data) => [$data[0]['class'] => $data[0]['properties']]) // Outer key by class name + ->map(fn (array $data) => collect($data)->mapWithKeys(fn (array $properties) => $properties)->all()) // Inner key by property name + ->all(); + + foreach ($fromCollector as $calls) { + foreach ($calls as $call) { + $errors[] = $this->compareTypes($call, $castCollector, $classCollector[$call['target']]); + } + } + + return Arr::flatten($errors); + } + + private function compareTypes(array $call, array $casts, array $constructor): array + { + $errors = []; + + foreach ($call['types'] as $arrayList) { + foreach ($arrayList as $key => $type) { + // Ignore any additional data, since it does not matter + if (! isset($constructor[$key])) { + continue; + } + + $error = $this->checkType($call, $key, $type, $casts, $constructor[$key]); + + if ($error !== null) { + $errors[] = $error; + } + } + } + + return $errors; + } + + /** + * @param array> $typeList + * + * @return string + */ + private function convertTypeListToString(array $typeList): string + { + return collect($typeList) + // Sort the individual types within each intersection type and implode them. + ->map(fn (array $intersectionList) => ltrim(collect($intersectionList)->sort()->implode('&'), '\\')) + // Then sort the full intersection types, and implode them. + ->sort() + ->implode('|'); + } + + private function checkType(array $call, string $key, string $actualType, array $casts, array $expectedTypes): ?RuleError + { + // Ignore cases, where there exists a cast - since we cannot analyse them in debt + if ($this->expectedTypesMatchesExactlyCast($casts, $expectedTypes)) { + return null; + } + + foreach ($expectedTypes as $typeList) { + if (empty($typeList)) { + return null; + } + + $validType = collect($typeList) + ->reduce(fn (bool $result, string $expectedType) => $result && $this->isTypesMatching($actualType, $expectedType), true); + + if ($validType) { + return null; + } + } + + return RuleErrorBuilder::message(sprintf('Argument $%s for %s::__construct() expects type [%s] but [%s] was given', $key, $call['target'], $this->expectedUnionTypesToString($expectedTypes), $actualType)) + ->line($call['method']['line']) + ->file($call['method']['file']) + ->build(); + } + + /** + * Returns true if a cast exists exactly for the expected types. + * + * @param array $casts + * @param array $expectedTypes + * + * @return bool + */ + private function expectedTypesMatchesExactlyCast(array $casts, array $expectedTypes): bool + { + return isset($casts[$this->convertTypeListToString($expectedTypes)]); + } + + /** + * @param array> $expectedTypes + * + * @return string + */ + private function expectedUnionTypesToString(array $expectedTypes): string + { + if (empty($expectedTypes)) { + return ''; + } + + $typeAsString = ''; + + foreach ($expectedTypes as $expectedType) { + $typeAsString .= '|'. $this->expectedIntersectionTypesToString($expectedType); + } + + return ltrim($typeAsString, '|'); + } + + private function expectedIntersectionTypesToString(array $expectedTypes): string + { + return collect($expectedTypes)->implode('&'); + } + + private function isTypesMatching(string $actualType, string $parentType): bool + { + if ($parentType === 'mixed') { + return true; + } + + if ($actualType === $parentType) { + return true; + } + + // We do not currently support static analysis for + if (class_exists($actualType) && class_exists($parentType)) { + return is_a($actualType, $parentType, true); + } + + return false; + } +} diff --git a/test/Samples/EmptySpatieLaravelData.php b/test/Samples/EmptySpatieLaravelData.php new file mode 100644 index 0000000..fc0b05c --- /dev/null +++ b/test/Samples/EmptySpatieLaravelData.php @@ -0,0 +1,9 @@ +stringProperty = $stringProperty; +// $this->intProperty = $intProperty; +// $this->booleanProperty = $booleanProperty; +// $this->objectProperty = $objectProperty; +// $this->nullableObjectProperty = $nullableObjectProperty; +// $this->undefinedPropertyType = $undefinedPropertyType; + $this->unionType = $unionType; +// $this->intersectionType = $intersectionType; + } + + public function initDefault(): self + { + return self::from([ + 'stringProperty' => 'my string', + 'intProperty' => 123, + 'booleanProperty' => 'cake', + 'objectProperty' => new EmptySpatieLaravelData(), + ], [ + 'nullableObjectProperty' => null, + 'undefinedPropertyType' => 'dsjahiuoas', + 'unionType' => '', + 'intersectionType' => 'sadsa', + ]); + } +} diff --git a/test/Samples/main.php b/test/Samples/main.php new file mode 100644 index 0000000..d1c1463 --- /dev/null +++ b/test/Samples/main.php @@ -0,0 +1,20 @@ + $bla */ +$bla = ""; + +SpatieLaravelData::from([ + 'intProperty' => 12, + 'stringProperty' => 'hohoho', +], [ + 'objectProperty' => new EmptySpatieLaravelData(), + 'nullableObjectProperty' => null, + 'unionType' => (fn () => $bla)(), +]); + From d21569b7cafbaea694cfd15468fa849cfaaf4366 Mon Sep 17 00:00:00 2001 From: Niki Zakariassen Date: Tue, 7 Feb 2023 10:42:45 +0100 Subject: [PATCH 02/14] Fixed bug and added missing extension --- .gitignore | 3 ++- extension.neon | 4 ++-- src/SpatieLaravelData/Collectors/FromCollector.php | 2 +- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/.gitignore b/.gitignore index 98f9d09..cd44a62 100644 --- a/.gitignore +++ b/.gitignore @@ -1,4 +1,5 @@ *.cache /.idea/ /vendor/ -composer.lock \ No newline at end of file +composer.lock +.phpstan diff --git a/extension.neon b/extension.neon index dcf60bc..33b46ee 100644 --- a/extension.neon +++ b/extension.neon @@ -1,5 +1,5 @@ -#includes: -# - ./../../nunomaduro/larastan/extension.neon +includes: + - ./../../nunomaduro/larastan/extension.neon services: - diff --git a/src/SpatieLaravelData/Collectors/FromCollector.php b/src/SpatieLaravelData/Collectors/FromCollector.php index 368f666..0c0d7ea 100644 --- a/src/SpatieLaravelData/Collectors/FromCollector.php +++ b/src/SpatieLaravelData/Collectors/FromCollector.php @@ -85,7 +85,7 @@ public function processNode(Node $node, Scope $scope): ?array private function isNotSpatieLaravelDataFromCall(StaticCall $node, Scope $scope): bool { if (strtolower($node->name->name) !== 'from') { - return false; + return true; } return ! is_a($this->getTargetClass($node, $scope), Data::class, true); From 4508c8016fa056b5b657f3e6649ffe3363757dd0 Mon Sep 17 00:00:00 2001 From: Niki Zakariassen Date: Tue, 7 Feb 2023 11:04:53 +0100 Subject: [PATCH 03/14] Maybe fixed stuff --- src/SpatieLaravelData/Rules/ValidTypeRule.php | 33 ++++++++++++++++--- 1 file changed, 28 insertions(+), 5 deletions(-) diff --git a/src/SpatieLaravelData/Rules/ValidTypeRule.php b/src/SpatieLaravelData/Rules/ValidTypeRule.php index 9882233..23a63bc 100644 --- a/src/SpatieLaravelData/Rules/ValidTypeRule.php +++ b/src/SpatieLaravelData/Rules/ValidTypeRule.php @@ -6,6 +6,7 @@ use PHPStan\Rules\Rule; use PHPStan\Analyser\Scope; use Illuminate\Support\Arr; +use Illuminate\Support\Str; use PHPStan\Rules\RuleError; use Illuminate\Support\Collection; use PHPStan\Node\CollectedDataNode; @@ -99,13 +100,18 @@ private function checkType(array $call, string $key, string $actualType, array $ return null; } + $actualTypeParts = Str::of($actualType) + ->explode('|') + ->map(fn (string $type) => explode('&', $type)) + ->all(); + foreach ($expectedTypes as $typeList) { if (empty($typeList)) { return null; } $validType = collect($typeList) - ->reduce(fn (bool $result, string $expectedType) => $result && $this->isTypesMatching($actualType, $expectedType), true); + ->reduce(fn (bool $result, string $expectedType) => $result && $this->isTypesMatching($actualTypeParts, $expectedType), true); if ($validType) { return null; @@ -156,7 +162,25 @@ private function expectedIntersectionTypesToString(array $expectedTypes): string return collect($expectedTypes)->implode('&'); } - private function isTypesMatching(string $actualType, string $parentType): bool + private function isTypesMatching(array $actualTypeParts, string $parentType): bool + { + if ($parentType === 'mixed') { + return true; + } + + foreach ($actualTypeParts as $intersectionType) { + $typeMatches = collect($intersectionType) + ->reduce(fn (bool $result, string $type) => $result || $this->typeIsSubsetOf($type, $parentType), true); + + if ($typeMatches) { + return true; + } + } + + return false; + } + + private function typeIsSubsetOf(string $actualType, string $parentType): bool { if ($parentType === 'mixed') { return true; @@ -166,9 +190,8 @@ private function isTypesMatching(string $actualType, string $parentType): bool return true; } - // We do not currently support static analysis for - if (class_exists($actualType) && class_exists($parentType)) { - return is_a($actualType, $parentType, true); + if (is_a($actualType, $parentType, true)) { + return true; } return false; From 0c71457ccf37c3236d70187045c802d50593b81c Mon Sep 17 00:00:00 2001 From: Niki Zakariassen Date: Tue, 7 Feb 2023 11:06:39 +0100 Subject: [PATCH 04/14] Fixed bug --- src/SpatieLaravelData/Rules/ValidTypeRule.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/SpatieLaravelData/Rules/ValidTypeRule.php b/src/SpatieLaravelData/Rules/ValidTypeRule.php index 23a63bc..aba8b8c 100644 --- a/src/SpatieLaravelData/Rules/ValidTypeRule.php +++ b/src/SpatieLaravelData/Rules/ValidTypeRule.php @@ -170,7 +170,7 @@ private function isTypesMatching(array $actualTypeParts, string $parentType): bo foreach ($actualTypeParts as $intersectionType) { $typeMatches = collect($intersectionType) - ->reduce(fn (bool $result, string $type) => $result || $this->typeIsSubsetOf($type, $parentType), true); + ->reduce(fn (bool $result, string $type) => $result || $this->typeIsSubsetOf($type, $parentType), false); if ($typeMatches) { return true; From 2e7b18e41091d72bee3f663e31b7e7ac34478858 Mon Sep 17 00:00:00 2001 From: Niki Zakariassen Date: Tue, 7 Feb 2023 11:09:20 +0100 Subject: [PATCH 05/14] Fixed bug --- src/SpatieLaravelData/Rules/ValidTypeRule.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/SpatieLaravelData/Rules/ValidTypeRule.php b/src/SpatieLaravelData/Rules/ValidTypeRule.php index aba8b8c..5d0ab49 100644 --- a/src/SpatieLaravelData/Rules/ValidTypeRule.php +++ b/src/SpatieLaravelData/Rules/ValidTypeRule.php @@ -172,12 +172,12 @@ private function isTypesMatching(array $actualTypeParts, string $parentType): bo $typeMatches = collect($intersectionType) ->reduce(fn (bool $result, string $type) => $result || $this->typeIsSubsetOf($type, $parentType), false); - if ($typeMatches) { - return true; + if (! $typeMatches) { + return false; } } - return false; + return true; } private function typeIsSubsetOf(string $actualType, string $parentType): bool From 0b93d69997a45828b672a65a3c6b798f98a420e4 Mon Sep 17 00:00:00 2001 From: Niki Zakariassen Date: Tue, 7 Feb 2023 11:17:41 +0100 Subject: [PATCH 06/14] Maybe fixed bug --- src/SpatieLaravelData/Rules/ValidTypeRule.php | 38 +++++++------------ 1 file changed, 14 insertions(+), 24 deletions(-) diff --git a/src/SpatieLaravelData/Rules/ValidTypeRule.php b/src/SpatieLaravelData/Rules/ValidTypeRule.php index 5d0ab49..67c88aa 100644 --- a/src/SpatieLaravelData/Rules/ValidTypeRule.php +++ b/src/SpatieLaravelData/Rules/ValidTypeRule.php @@ -95,7 +95,7 @@ private function convertTypeListToString(array $typeList): string private function checkType(array $call, string $key, string $actualType, array $casts, array $expectedTypes): ?RuleError { - // Ignore cases, where there exists a cast - since we cannot analyse them in debt + // Ignore cases, where there exists a cast - since we cannot analyse them in dept if ($this->expectedTypesMatchesExactlyCast($casts, $expectedTypes)) { return null; } @@ -105,16 +105,18 @@ private function checkType(array $call, string $key, string $actualType, array $ ->map(fn (string $type) => explode('&', $type)) ->all(); - foreach ($expectedTypes as $typeList) { - if (empty($typeList)) { - return null; - } + foreach ($actualTypeParts as $actualTypePart) { + foreach ($expectedTypes as $typeList) { + if (empty($typeList)) { + return null; + } - $validType = collect($typeList) - ->reduce(fn (bool $result, string $expectedType) => $result && $this->isTypesMatching($actualTypeParts, $expectedType), true); + $validType = collect($typeList) + ->reduce(fn (bool $result, string $expectedType) => $result && $this->isTypesMatching($actualTypePart, $expectedType), true); - if ($validType) { - return null; + if ($validType) { + return null; + } } } @@ -162,22 +164,10 @@ private function expectedIntersectionTypesToString(array $expectedTypes): string return collect($expectedTypes)->implode('&'); } - private function isTypesMatching(array $actualTypeParts, string $parentType): bool + private function isTypesMatching(array $intersectionType, string $parentType): bool { - if ($parentType === 'mixed') { - return true; - } - - foreach ($actualTypeParts as $intersectionType) { - $typeMatches = collect($intersectionType) - ->reduce(fn (bool $result, string $type) => $result || $this->typeIsSubsetOf($type, $parentType), false); - - if (! $typeMatches) { - return false; - } - } - - return true; + return collect($intersectionType) + ->reduce(fn (bool $result, string $type) => $result || $this->typeIsSubsetOf($type, $parentType), false); } private function typeIsSubsetOf(string $actualType, string $parentType): bool From 2505aabe0ef72881d2f82289da48d445c31244e4 Mon Sep 17 00:00:00 2001 From: Niki Zakariassen Date: Tue, 7 Feb 2023 11:18:10 +0100 Subject: [PATCH 07/14] Rename --- src/SpatieLaravelData/Rules/ValidTypeRule.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/SpatieLaravelData/Rules/ValidTypeRule.php b/src/SpatieLaravelData/Rules/ValidTypeRule.php index 67c88aa..a105e08 100644 --- a/src/SpatieLaravelData/Rules/ValidTypeRule.php +++ b/src/SpatieLaravelData/Rules/ValidTypeRule.php @@ -105,14 +105,14 @@ private function checkType(array $call, string $key, string $actualType, array $ ->map(fn (string $type) => explode('&', $type)) ->all(); - foreach ($actualTypeParts as $actualTypePart) { + foreach ($actualTypeParts as $actualIntersectionType) { foreach ($expectedTypes as $typeList) { if (empty($typeList)) { return null; } $validType = collect($typeList) - ->reduce(fn (bool $result, string $expectedType) => $result && $this->isTypesMatching($actualTypePart, $expectedType), true); + ->reduce(fn (bool $result, string $expectedType) => $result && $this->isTypesMatching($actualIntersectionType, $expectedType), true); if ($validType) { return null; From 989589457788fc5cfee5e0dbf73a54b2d2a29dec Mon Sep 17 00:00:00 2001 From: Niki Zakariassen Date: Tue, 7 Feb 2023 11:24:54 +0100 Subject: [PATCH 08/14] Maybe fixed more bugs --- src/SpatieLaravelData/Rules/ValidTypeRule.php | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/SpatieLaravelData/Rules/ValidTypeRule.php b/src/SpatieLaravelData/Rules/ValidTypeRule.php index a105e08..2a80d6c 100644 --- a/src/SpatieLaravelData/Rules/ValidTypeRule.php +++ b/src/SpatieLaravelData/Rules/ValidTypeRule.php @@ -114,16 +114,20 @@ private function checkType(array $call, string $key, string $actualType, array $ $validType = collect($typeList) ->reduce(fn (bool $result, string $expectedType) => $result && $this->isTypesMatching($actualIntersectionType, $expectedType), true); + // We found a expected type which matches the current union type, we therefor continue to next union type in the outer loop if ($validType) { - return null; + continue 2; } } + + // No valid expected type was found for the union type + return RuleErrorBuilder::message(sprintf('Argument $%s for %s::__construct() expects type [%s] but [%s] was given', $key, $call['target'], $this->expectedUnionTypesToString($expectedTypes), $actualType)) + ->line($call['method']['line']) + ->file($call['method']['file']) + ->build(); } - return RuleErrorBuilder::message(sprintf('Argument $%s for %s::__construct() expects type [%s] but [%s] was given', $key, $call['target'], $this->expectedUnionTypesToString($expectedTypes), $actualType)) - ->line($call['method']['line']) - ->file($call['method']['file']) - ->build(); + return null; } /** From 908b982707a6f6a03769a0d6520dfe2fcff43991 Mon Sep 17 00:00:00 2001 From: Niki Zakariassen Date: Wed, 8 Feb 2023 13:36:29 +0100 Subject: [PATCH 09/14] Cleaned up more of the code and ran the fixer --- .php-cs-fixer.php | 11 ++ composer.json | 10 +- extension.neon | 4 +- .../Collectors/CastCollector.php | 21 +- .../Collectors/ConstructorCollector.php | 62 ++++-- .../Collectors/FromCollector.php | 45 +++-- src/SpatieLaravelData/Data/Call.php | 48 +++++ src/SpatieLaravelData/Data/Constructor.php | 53 +++++ src/SpatieLaravelData/Data/KeyTypePair.php | 45 +++++ src/SpatieLaravelData/Data/Method.php | 44 +++++ src/SpatieLaravelData/Rules/ValidTypeRule.php | 183 ++++++++---------- .../Traits/UnserializesSelf.php | 11 ++ src/TypeSystem/IntersectionType.php | 96 +++++++++ src/TypeSystem/Type.php | 142 ++++++++++++++ src/TypeSystem/TypeSystem.php | 106 ++++++++++ src/TypeSystem/UnionType.php | 128 ++++++++++++ test/Samples/CastedSpatieLaravelData.php | 53 +++++ test/Samples/Casts/BackedEnumCast.php | 16 ++ .../Casts/UncastableSpatieDataCast.php | 24 +++ test/Samples/Enum/BackedEnumExample.php | 8 + .../InvalidComplexSpatieLaravelData.php | 32 +++ .../InvalidScalarSpatieLaravelData.php | 26 +++ test/Samples/SpatieDataCast.php | 15 -- test/Samples/SpatieLaravelData.php | 54 ------ .../Samples/ValidComplexSpatieLaravelData.php | 33 ++++ test/Samples/ValidSpatieLaravelData.php | 41 ++++ test/Samples/main.php | 20 -- test/SpatieLaravelData/ValidTypeRuleTest.php | 88 +++++++++ 28 files changed, 1182 insertions(+), 237 deletions(-) create mode 100644 .php-cs-fixer.php create mode 100644 src/SpatieLaravelData/Data/Call.php create mode 100644 src/SpatieLaravelData/Data/Constructor.php create mode 100644 src/SpatieLaravelData/Data/KeyTypePair.php create mode 100644 src/SpatieLaravelData/Data/Method.php create mode 100644 src/SpatieLaravelData/Traits/UnserializesSelf.php create mode 100644 src/TypeSystem/IntersectionType.php create mode 100644 src/TypeSystem/Type.php create mode 100644 src/TypeSystem/TypeSystem.php create mode 100644 src/TypeSystem/UnionType.php create mode 100644 test/Samples/CastedSpatieLaravelData.php create mode 100644 test/Samples/Casts/BackedEnumCast.php create mode 100644 test/Samples/Casts/UncastableSpatieDataCast.php create mode 100644 test/Samples/Enum/BackedEnumExample.php create mode 100644 test/Samples/InvalidComplexSpatieLaravelData.php create mode 100644 test/Samples/InvalidScalarSpatieLaravelData.php delete mode 100644 test/Samples/SpatieDataCast.php delete mode 100644 test/Samples/SpatieLaravelData.php create mode 100644 test/Samples/ValidComplexSpatieLaravelData.php create mode 100644 test/Samples/ValidSpatieLaravelData.php delete mode 100644 test/Samples/main.php create mode 100644 test/SpatieLaravelData/ValidTypeRuleTest.php diff --git a/.php-cs-fixer.php b/.php-cs-fixer.php new file mode 100644 index 0000000..3258f53 --- /dev/null +++ b/.php-cs-fixer.php @@ -0,0 +1,11 @@ +in(__DIR__ . '/src') + ->in(__DIR__ . '/test'); + +return CegoFixer::applyRules($finder, [ + 'ternary_to_null_coalescing' => true, +]); diff --git a/composer.json b/composer.json index add3b9b..56d4fe8 100644 --- a/composer.json +++ b/composer.json @@ -22,11 +22,13 @@ } }, "require": { - "php": "^7.4|^8.0", - "nunomaduro/larastan": "^2.1", - "phpstan/phpstan": "^1.4" + "php": "^8.1" }, "require-dev": { - "spatie/laravel-data": "^3.0" + "spatie/laravel-data": "^3.0", + "phpunit/phpunit": "^10.0", + "phpstan/phpstan": "^1.4", + "nunomaduro/larastan": "^2.4", + "cego/php-cs-fixer": "^1.0" } } diff --git a/extension.neon b/extension.neon index 33b46ee..dcf60bc 100644 --- a/extension.neon +++ b/extension.neon @@ -1,5 +1,5 @@ -includes: - - ./../../nunomaduro/larastan/extension.neon +#includes: +# - ./../../nunomaduro/larastan/extension.neon services: - diff --git a/src/SpatieLaravelData/Collectors/CastCollector.php b/src/SpatieLaravelData/Collectors/CastCollector.php index bb6ee7e..e9cc4ab 100644 --- a/src/SpatieLaravelData/Collectors/CastCollector.php +++ b/src/SpatieLaravelData/Collectors/CastCollector.php @@ -3,15 +3,16 @@ namespace Cego\phpstan\SpatieLaravelData\Collectors; use PhpParser\Node; -use PHPStan\Analyser\Scope; use Illuminate\Support\Str; +use PHPStan\Analyser\Scope; use PHPStan\Type\VerbosityLevel; use PHPStan\Collectors\Collector; -use Spatie\LaravelData\Casts\Cast; use Illuminate\Support\Collection; +use Spatie\LaravelData\Casts\Cast; use PhpParser\Node\Expr\StaticCall; use PHPStan\Node\InClassMethodNode; use PHPStan\ShouldNotHappenException; +use Cego\phpstan\TypeSystem\UnionType; use Spatie\LaravelData\Casts\Uncastable; use PHPStan\Reflection\ParametersAcceptorSelector; @@ -34,10 +35,12 @@ public function getNodeType(): string * Process the nodes and stores value in the collector instance * * @phpstan-param StaticCall $node - * @return array|null Collected data + * * @throws ShouldNotHappenException + * + * @return string|null Collected data */ - public function processNode(Node $node, Scope $scope): ?array + public function processNode(Node $node, Scope $scope): ?string { // Skip wrong nodes if ( ! $node instanceof InClassMethodNode) { @@ -68,15 +71,15 @@ public function processNode(Node $node, Scope $scope): ?array // We only support intersection types of explicit classes / interfaces. if ($intersectionTypes->count() !== $classTypes->count()) { - return collect(); + return []; } - return $classTypes; + return $classTypes->all(); }) // Remove any intersection types we have deemed unfit - ->reject(fn (Collection $collection) => $collection->isEmpty()) - // And array the result so it can be serialized by PhpStan - ->toArray(); + ->reject(fn (array $collection) => empty($collection)) + ->pipe(UnionType::fromRaw(...)) + ->toString(); } /** diff --git a/src/SpatieLaravelData/Collectors/ConstructorCollector.php b/src/SpatieLaravelData/Collectors/ConstructorCollector.php index 68de649..37120b5 100644 --- a/src/SpatieLaravelData/Collectors/ConstructorCollector.php +++ b/src/SpatieLaravelData/Collectors/ConstructorCollector.php @@ -3,6 +3,7 @@ namespace Cego\phpstan\SpatieLaravelData\Collectors; use PhpParser\Node; +use RuntimeException; use PhpParser\Node\Name; use PhpParser\Node\Param; use PHPStan\Analyser\Scope; @@ -11,8 +12,10 @@ use PhpParser\Node\ComplexType; use PHPStan\Collectors\Collector; use PHPStan\Node\InClassMethodNode; +use Cego\phpstan\TypeSystem\UnionType; use PHPStan\Reflection\ClassReflection; -use Cego\phpstan\Collectors\RuntimeException; +use Cego\phpstan\SpatieLaravelData\Data\Constructor; +use Cego\phpstan\SpatieLaravelData\Data\KeyTypePair; /** * @implements Collector>> @@ -33,9 +36,10 @@ public function getNodeType(): string * Process the nodes and stores value in the collector instance * * @phpstan-param InClassMethodNode $node - * @return array>|null Collected data + * + * @return string|null Collected data */ - public function processNode(Node $node, Scope $scope): ?array + public function processNode(Node $node, Scope $scope): ?string { if ( ! $node instanceof InClassMethodNode) { return null; @@ -45,12 +49,10 @@ public function processNode(Node $node, Scope $scope): ?array return null; } - return [ - 'class' => $node->getMethodReflection()->getDeclaringClass()->getName(), - 'properties' => collect($node->getOriginalNode()->getParams()) - ->map(fn (Param $param) => $this->getParameterTypes($param)) - ->all(), - ]; + return serialize(new Constructor( + $node->getMethodReflection()->getDeclaringClass()->getName(), + collect($node->getOriginalNode()->getParams())->map($this->getParameterTypes(...))->all() + )); } /** @@ -58,14 +60,14 @@ public function processNode(Node $node, Scope $scope): ?array * * @param Param $parameter * - * @return array> + * @return KeyTypePair */ - private function getParameterTypes(Param $parameter): array + private function getParameterTypes(Param $parameter): KeyTypePair { - $name = $this->getParameterName($parameter); - $types = $this->parseType($parameter->type); - - return [$name => $types]; + return new KeyTypePair( + $this->getParameterName($parameter), + UnionType::fromRaw($this->parseType($parameter->type)), + ); } /** @@ -118,7 +120,7 @@ private function parseType($type): array collect($type->types) ->map(fn ($intersectionType) => $this->parseType($intersectionType)) ->flatten(2) - ->all() + ->all(), ]; } } @@ -126,6 +128,13 @@ private function parseType($type): array return [['mixed']]; } + /** + * Returns the name of the given parameter + * + * @param Param $parameter + * + * @return string + */ private function getParameterName(Param $parameter): string { if ( ! is_string($parameter->var->name)) { @@ -135,17 +144,38 @@ private function getParameterName(Param $parameter): string return $parameter->var->name; } + /** + * Returns true if the given node is not a laravel data constructor + * + * @param InClassMethodNode $node + * + * @return bool + */ private function isNotSpatieLaravelDataConstructor(InClassMethodNode $node): bool { return $this->isNotConstructor($node) || $this->isNotSpatieLaravelDataClass($node->getMethodReflection()->getDeclaringClass()); } + /** + * Returns true if the given node is not a constructor class + * + * @param InClassMethodNode $node + * + * @return bool + */ private function isNotConstructor(InClassMethodNode $node): bool { return $node->getMethodReflection()->getName() !== '__construct'; } + /** + * Returns true if the given class is not a laravel data class + * + * @param ClassReflection $class + * + * @return bool + */ private function isNotSpatieLaravelDataClass(ClassReflection $class): bool { return ! in_array(Data::class, $class->getParentClassesNames(), true); diff --git a/src/SpatieLaravelData/Collectors/FromCollector.php b/src/SpatieLaravelData/Collectors/FromCollector.php index 0c0d7ea..6163c53 100644 --- a/src/SpatieLaravelData/Collectors/FromCollector.php +++ b/src/SpatieLaravelData/Collectors/FromCollector.php @@ -10,6 +10,10 @@ use PHPStan\Collectors\Collector; use PhpParser\Node\Expr\StaticCall; use PHPStan\Type\ConstantScalarType; +use Cego\phpstan\TypeSystem\UnionType; +use Cego\phpstan\SpatieLaravelData\Data\Call; +use Cego\phpstan\SpatieLaravelData\Data\Method; +use Cego\phpstan\SpatieLaravelData\Data\KeyTypePair; /** * @implements Collector> @@ -30,9 +34,10 @@ public function getNodeType(): string * Process the nodes and stores value in the collector instance * * @phpstan-param StaticCall $node - * @return array|null Collected data + * + * @return string|null Collected data */ - public function processNode(Node $node, Scope $scope): ?array + public function processNode(Node $node, Scope $scope): ?string { if ( ! $node instanceof StaticCall) { return null; @@ -63,25 +68,33 @@ public function processNode(Node $node, Scope $scope): ?array $type = $scope->getType($item->value); if ($type instanceof ConstantScalarType) { - $argData[$item->key->value] = get_debug_type($type->getValue()); + $argData[] = new KeyTypePair($item->key->value, UnionType::fromString(get_debug_type($type->getValue()))); } else { - $argData[$item->key->value] = $scope->getType($item->value)->describe(VerbosityLevel::typeOnly()); + $argData[] = new KeyTypePair($item->key->value, UnionType::fromString($scope->getType($item->value)->describe(VerbosityLevel::typeOnly()))); } } $types[] = $argData; } - return [ - 'types' => $types, - 'target' => $this->getTargetClass($node, $scope), - 'method' => [ - 'file' => $scope->getFile(), - 'line' => $node->getLine(), - ], - ]; + return serialize(new Call( + $this->getTargetClass($node, $scope), + $types, + new Method( + $scope->getFile(), + $node->getLine(), + ) + )); } + /** + * Returns true if the given node is not a laravel data class static ::From call + * + * @param StaticCall $node + * @param Scope $scope + * + * @return bool + */ private function isNotSpatieLaravelDataFromCall(StaticCall $node, Scope $scope): bool { if (strtolower($node->name->name) !== 'from') { @@ -91,6 +104,14 @@ private function isNotSpatieLaravelDataFromCall(StaticCall $node, Scope $scope): return ! is_a($this->getTargetClass($node, $scope), Data::class, true); } + /** + * Returns the target / result class of the given static call + * + * @param StaticCall $node + * @param Scope $scope + * + * @return string + */ private function getTargetClass(StaticCall $node, Scope $scope) { if ($node->class instanceof Node\Expr) { diff --git a/src/SpatieLaravelData/Data/Call.php b/src/SpatieLaravelData/Data/Call.php new file mode 100644 index 0000000..a7a6f26 --- /dev/null +++ b/src/SpatieLaravelData/Data/Call.php @@ -0,0 +1,48 @@ +> $arrayArguments + * @param Method $method + */ + public function __construct( + public readonly string $target, + public readonly array $arrayArguments, + public readonly Method $method, + ) { + } + + /** + * Returns array containing all the necessary state of the object. + */ + public function __serialize(): array + { + return [ + 'target' => $this->target, + 'method' => serialize($this->method), + 'arrayArguments' => serialize($this->arrayArguments), + ]; + } + + /** + * Restores the object state from the given data array. + * + * @param array $data + */ + public function __unserialize(array $data): void + { + $this->target = $data['target']; + $this->method = Method::unserialize($data['method']); + $this->arrayArguments = unserialize($data['arrayArguments'], ['allowed_classes' => [KeyTypePair::class]]); + } +} diff --git a/src/SpatieLaravelData/Data/Constructor.php b/src/SpatieLaravelData/Data/Constructor.php new file mode 100644 index 0000000..6f6d79f --- /dev/null +++ b/src/SpatieLaravelData/Data/Constructor.php @@ -0,0 +1,53 @@ + $properties + */ + public function __construct( + public readonly string $class, + array $properties + ) { + $this->properties = collect($properties)->keyBy('key')->all(); + } + + /** + * Returns array containing all the necessary state of the object. + * + * @since 7.4 + * @link https://wiki.php.net/rfc/custom_object_serialization + */ + public function __serialize(): array + { + return [ + 'class' => $this->class, + 'properties' => serialize($this->properties), + ]; + } + + /** + * Restores the object state from the given data array. + * + * @param array $data + * + * @since 7.4 + * @link https://wiki.php.net/rfc/custom_object_serialization + */ + public function __unserialize(array $data): void + { + $this->class = $data['class']; + $this->properties = unserialize($data['properties'], ['allowed_classes' => [KeyTypePair::class]]); + } +} diff --git a/src/SpatieLaravelData/Data/KeyTypePair.php b/src/SpatieLaravelData/Data/KeyTypePair.php new file mode 100644 index 0000000..ceab129 --- /dev/null +++ b/src/SpatieLaravelData/Data/KeyTypePair.php @@ -0,0 +1,45 @@ + $this->key, + 'type' => serialize($this->type), + ]; + } + + /** + * Restores the object state from the given data array. + * + * @param array $data + */ + public function __unserialize(array $data): void + { + $this->key = $data['key']; + $this->type = UnionType::unserialize($data['type']); + } +} diff --git a/src/SpatieLaravelData/Data/Method.php b/src/SpatieLaravelData/Data/Method.php new file mode 100644 index 0000000..3320471 --- /dev/null +++ b/src/SpatieLaravelData/Data/Method.php @@ -0,0 +1,44 @@ + $this->file, + 'line' => $this->line, + ]; + } + + /** + * Restores the object state from the given data array. + * + * @param array $data + */ + public function __unserialize(array $data): void + { + $this->file = $data['file']; + $this->line = $data['line']; + } +} diff --git a/src/SpatieLaravelData/Rules/ValidTypeRule.php b/src/SpatieLaravelData/Rules/ValidTypeRule.php index 2a80d6c..c908dae 100644 --- a/src/SpatieLaravelData/Rules/ValidTypeRule.php +++ b/src/SpatieLaravelData/Rules/ValidTypeRule.php @@ -5,19 +5,23 @@ use PhpParser\Node; use PHPStan\Rules\Rule; use PHPStan\Analyser\Scope; -use Illuminate\Support\Arr; -use Illuminate\Support\Str; use PHPStan\Rules\RuleError; -use Illuminate\Support\Collection; use PHPStan\Node\CollectedDataNode; use PHPStan\Rules\RuleErrorBuilder; -use Cego\phpstan\SpatieLaravelData\Collectors\FromCollector; +use PHPStan\ShouldNotHappenException; +use Cego\phpstan\TypeSystem\UnionType; +use Cego\phpstan\TypeSystem\TypeSystem; +use Cego\phpstan\SpatieLaravelData\Data\Call; +use Cego\phpstan\SpatieLaravelData\Data\Constructor; use Cego\phpstan\SpatieLaravelData\Collectors\CastCollector; +use Cego\phpstan\SpatieLaravelData\Collectors\FromCollector; use Cego\phpstan\SpatieLaravelData\Collectors\ConstructorCollector; class ValidTypeRule implements Rule { /** + * Returns the node type this rule should trigger for + * * @phpstan-return class-string */ public function getNodeType(): string @@ -26,7 +30,10 @@ public function getNodeType(): string } /** + * Processes the given node + * * @phpstan-param TNodeType $node + * * @return (string|RuleError)[] errors */ public function processNode(Node $node, Scope $scope): array @@ -35,39 +42,54 @@ public function processNode(Node $node, Scope $scope): array return []; } - $fromCollector = $node->get(FromCollector::class); - $castCollector = collect($node->get(CastCollector::class)) - ->map(fn (array $castTypes) => $this->convertTypeListToString($castTypes[0])) - ->flip() + ->flatten() + ->map(UnionType::fromString(...)) + ->reject(fn (UnionType $unionType) => $unionType->isMixed()) + ->values() ->all(); $classCollector = collect($node->get(ConstructorCollector::class)) - ->mapWithKeys(fn (array $data) => [$data[0]['class'] => $data[0]['properties']]) // Outer key by class name - ->map(fn (array $data) => collect($data)->mapWithKeys(fn (array $properties) => $properties)->all()) // Inner key by property name + ->flatten(1) + ->map(Constructor::unserialize(...)) + ->keyBy('class') ->all(); - foreach ($fromCollector as $calls) { - foreach ($calls as $call) { - $errors[] = $this->compareTypes($call, $castCollector, $classCollector[$call['target']]); - } - } - - return Arr::flatten($errors); + return collect($node->get(FromCollector::class)) + // Flatten from a list of calls pr. file, to just a list of calls. + ->flatten(1) + ->map(Call::unserialize(...)) + // Check each call for errors + ->map(fn (Call $call) => $this->compareTypes($call, $castCollector, $classCollector[$call->target])) + // Flatten from a list of errors pr. call, to just a list of errors. + ->flatten() + // To array, so PhpStan can serialize the data. + ->all(); } - private function compareTypes(array $call, array $casts, array $constructor): array + /** + * Compares the types of the specific call, with the constructor which would be expected + * + * @param Call $call + * @param list $casts + * @param Constructor $constructor + * + * @throws ShouldNotHappenException + * + * @return array + */ + private function compareTypes(Call $call, array $casts, Constructor $constructor): array { $errors = []; - foreach ($call['types'] as $arrayList) { - foreach ($arrayList as $key => $type) { + foreach ($call->arrayArguments as $arrayList) { + foreach ($arrayList as $type) { // Ignore any additional data, since it does not matter - if (! isset($constructor[$key])) { + if ( ! isset($constructor->properties[$type->key])) { continue; } - $error = $this->checkType($call, $key, $type, $casts, $constructor[$key]); + $error = $this->checkType($call, $type->key, $type->type, $casts, $constructor->properties[$type->key]->type); if ($error !== null) { $errors[] = $error; @@ -79,51 +101,29 @@ private function compareTypes(array $call, array $casts, array $constructor): ar } /** - * @param array> $typeList + * Checks the specific type for a single key, with the expected types of that key. * - * @return string + * @param Call $call + * @param string $key + * @param UnionType $actualType + * @param list $casts + * @param UnionType $expectedType + * + * @throws ShouldNotHappenException + * + * @return RuleError|null */ - private function convertTypeListToString(array $typeList): string - { - return collect($typeList) - // Sort the individual types within each intersection type and implode them. - ->map(fn (array $intersectionList) => ltrim(collect($intersectionList)->sort()->implode('&'), '\\')) - // Then sort the full intersection types, and implode them. - ->sort() - ->implode('|'); - } - - private function checkType(array $call, string $key, string $actualType, array $casts, array $expectedTypes): ?RuleError + 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 - if ($this->expectedTypesMatchesExactlyCast($casts, $expectedTypes)) { + if ($this->expectedTypesMatchesExactlyCast($casts, $expectedType)) { return null; } - $actualTypeParts = Str::of($actualType) - ->explode('|') - ->map(fn (string $type) => explode('&', $type)) - ->all(); - - foreach ($actualTypeParts as $actualIntersectionType) { - foreach ($expectedTypes as $typeList) { - if (empty($typeList)) { - return null; - } - - $validType = collect($typeList) - ->reduce(fn (bool $result, string $expectedType) => $result && $this->isTypesMatching($actualIntersectionType, $expectedType), true); - - // We found a expected type which matches the current union type, we therefor continue to next union type in the outer loop - if ($validType) { - continue 2; - } - } - - // No valid expected type was found for the union type - return RuleErrorBuilder::message(sprintf('Argument $%s for %s::__construct() expects type [%s] but [%s] was given', $key, $call['target'], $this->expectedUnionTypesToString($expectedTypes), $actualType)) - ->line($call['method']['line']) - ->file($call['method']['file']) + if ( ! TypeSystem::isSubtypeOf($actualType, $expectedType)) { + return RuleErrorBuilder::message(self::getErrorMessage($key, $call->target, $expectedType, $actualType)) + ->line($call->method->line) + ->file($call->method->file) ->build(); } @@ -131,61 +131,34 @@ private function checkType(array $call, string $key, string $actualType, array $ } /** - * Returns true if a cast exists exactly for the expected types. + * Returns the error message to give the developer on errors * - * @param array $casts - * @param array $expectedTypes + * @param string $property + * @param string $class + * @param string $expectedType + * @param string $actualType * - * @return bool + * @return string */ - private function expectedTypesMatchesExactlyCast(array $casts, array $expectedTypes): bool + public static function getErrorMessage(string $property, string $class, string $expectedType, string $actualType): string { - return isset($casts[$this->convertTypeListToString($expectedTypes)]); + return sprintf('Argument $%s for %s::__construct() expects type [%s] but [%s] was given', $property, $class, $expectedType, $actualType); } /** - * @param array> $expectedTypes + * Returns true if a cast exists exactly for the expected types. * - * @return string + * @param list $casts + * @param UnionType $expectedTypes + * + * @return bool */ - private function expectedUnionTypesToString(array $expectedTypes): string + private function expectedTypesMatchesExactlyCast(array $casts, UnionType $expectedTypes): bool { - if (empty($expectedTypes)) { - return ''; - } - - $typeAsString = ''; - - foreach ($expectedTypes as $expectedType) { - $typeAsString .= '|'. $this->expectedIntersectionTypesToString($expectedType); - } - - return ltrim($typeAsString, '|'); - } - - private function expectedIntersectionTypesToString(array $expectedTypes): string - { - return collect($expectedTypes)->implode('&'); - } - - private function isTypesMatching(array $intersectionType, string $parentType): bool - { - return collect($intersectionType) - ->reduce(fn (bool $result, string $type) => $result || $this->typeIsSubsetOf($type, $parentType), false); - } - - private function typeIsSubsetOf(string $actualType, string $parentType): bool - { - if ($parentType === 'mixed') { - return true; - } - - if ($actualType === $parentType) { - return true; - } - - if (is_a($actualType, $parentType, true)) { - return true; + foreach ($casts as $castType) { + if (TypeSystem::isSubtypeOf($expectedTypes, $castType)) { + return true; + } } return false; diff --git a/src/SpatieLaravelData/Traits/UnserializesSelf.php b/src/SpatieLaravelData/Traits/UnserializesSelf.php new file mode 100644 index 0000000..efb9df7 --- /dev/null +++ b/src/SpatieLaravelData/Traits/UnserializesSelf.php @@ -0,0 +1,11 @@ + [__CLASS__]]); + } +} diff --git a/src/TypeSystem/IntersectionType.php b/src/TypeSystem/IntersectionType.php new file mode 100644 index 0000000..6f988c3 --- /dev/null +++ b/src/TypeSystem/IntersectionType.php @@ -0,0 +1,96 @@ + + */ + public array $types = []; + + /** + * Constructor + * + * @param Type[] $types + */ + public function __construct(array $types) + { + $this->types = $types; + } + + /** + * Constructor from array representation + * + * @param array $intersectionType + * + * @return static + */ + public static function fromRaw(array $intersectionType): self + { + return new self(collect($intersectionType)->mapInto(Type::class)->all()); + } + + /** + * Returns the types composing the intersection type + * + * @return array + */ + public function getTypes(): array + { + return $this->types; + } + + /** + * Returns true if this is an intersection of a single type. + * Meaning it is not actually an intersection type. + * + * @return bool + */ + public function isIntersectionOfOne(): bool + { + return count($this->types) === 1; + } + + /** + * Returns the type in its string representation + * + * @return string + */ + public function toString(): string + { + return $this->__toString(); + } + + /** + * Allows a class to decide how it will react when it is treated like a string. + * + * @return string + */ + public function __toString(): string + { + return collect($this->types) + ->map(fn (Type $type) => $type->toString()) + ->sort() + ->implode('&'); + } + + /** + * 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. + foreach ($this->types as $type) { + if ( ! $type->isMixed()) { + return false; + } + } + + return true; + } +} diff --git a/src/TypeSystem/Type.php b/src/TypeSystem/Type.php new file mode 100644 index 0000000..72f3f96 --- /dev/null +++ b/src/TypeSystem/Type.php @@ -0,0 +1,142 @@ +type), ['void', 'never'], true); + } + + /** + * Returns true if the type is of mixed + * + * @return bool + */ + public function isMixed(): bool + { + return empty($this->type) + || strtolower($this->type) === 'mixed'; + } + + /** + * Returns true if the type is a number + * + * @return bool + */ + public function isNumber(): bool + { + return in_array(strtolower($this->type), ['int', 'float'], true); + } + + /** + * Returns true if the type is a float + * + * @return bool + */ + public function isFloat(): bool + { + return strtolower($this->type) === 'float'; + } + + /** + * Returns true if it is a class type + * + * @return bool + */ + public function isClass(): bool + { + return class_exists($this->type); + } + + /** + * Returns true if the a interface type + * + * @return bool + */ + public function isInterface(): bool + { + return interface_exists($this->type); + } + + /** + * Returns true if the given type exactly matches this type + * + * @param Type $type + * + * @return bool + */ + public function equals(Type $type): bool + { + return strtolower($this->type) === strtolower($type->type); + } + + /** + * Returns true if this type is exactly the given type, or direct subset (for classes and interfaces) + * + * @param Type $type + * + * @return bool + */ + public function isA(Type $type): bool + { + return strtolower($this->type) === strtolower($type->type) + || is_a($this->type, $type->type, true); + } + + /** + * Returns true if the type is class or interface type + * + * @return bool + */ + public function isClassOrInterface(): bool + { + return $this->isClass() || $this->isInterface(); + } + + /** + * Returns the string in its string representation + * + * @return string + */ + public function toString(): string + { + return $this->__toString(); + } + + /** + * Allows a class to decide how it will react when it is treated like a string. + * + * @return string + */ + public function __toString(): string + { + if ($this->isClassOrInterface()) { + return $this->type; + } + + // Handles when empty string == mixed + if ($this->isMixed()) { + return 'mixed'; + } + + return strtolower($this->type); + } +} diff --git a/src/TypeSystem/TypeSystem.php b/src/TypeSystem/TypeSystem.php new file mode 100644 index 0000000..1dcbc92 --- /dev/null +++ b/src/TypeSystem/TypeSystem.php @@ -0,0 +1,106 @@ +getIntersectionTypes() as $intersectionType) { + if ( ! self::isIntersectionTypeSubsetOfUnionType($intersectionType, $parentType)) { + return false; + } + } + + return true; + } + + /** + * Returns true if the given intersection type is a subset of the given parent type + * + * @param IntersectionType $intersectionType + * @param UnionType $parentType + * + * @return bool + */ + private static function isIntersectionTypeSubsetOfUnionType(IntersectionType $intersectionType, UnionType $parentType): bool + { + // For an intersection type to be a subset of a union type, then at least one of the underlying types + // for the intersection type has to be a subset of the type options for the union type + foreach ($intersectionType->getTypes() as $type) { + if (self::isTypeSubsetOfUnionType($type, $parentType)) { + return true; + } + } + + return false; + } + + /** + * Returns true if the given type, is a subset of the given union type + * + * @param Type $type + * @param UnionType $parentUnionType + * + * @return bool + */ + private static function isTypeSubsetOfUnionType(Type $type, UnionType $parentUnionType): bool + { + // For a specific type to be a subset of a union type, then the type + // has to be a subset of only one of the types of the underlying intersection types. + foreach ($parentUnionType->getIntersectionTypes() as $parentIntersectionType) { + foreach ($parentIntersectionType->getTypes() as $parentType) { + if (self::isTypeSubsetOfType($type, $parentType)) { + return true; + } + } + } + + return false; + } + + /** + * Returns true if the given type is a subset of the given parent type + * + * @param Type $type + * @param Type $parentType + * + * @return bool + */ + private static function isTypeSubsetOfType(Type $type, Type $parentType): bool + { + // A non-real type cannot exist as a variable, and therefor is never a subtype. + if ($type->isNotReal()) { + return false; + } + + // Everything is a subset of mixed + if ($parentType->isMixed()) { + return true; + } + + // All numbers are a subset of float + if ($type->isNumber() && $parentType->isFloat()) { + return true; + } + + // If a type equals or is a instance of the parent type, then its a subset. + if ($type->isA($parentType)) { + return true; + } + + // Otherwise it is not. + return false; + } +} diff --git a/src/TypeSystem/UnionType.php b/src/TypeSystem/UnionType.php new file mode 100644 index 0000000..2ed42e3 --- /dev/null +++ b/src/TypeSystem/UnionType.php @@ -0,0 +1,128 @@ + + */ + private array $intersectionTypes; + + /** + * @param IntersectionType[] $intersectionTypes + */ + public function __construct(array $intersectionTypes) + { + $this->intersectionTypes = $intersectionTypes; + } + + public static function fromRaw(array|Arrayable $unionType): self + { + if ($unionType instanceof Arrayable) { + $unionType = $unionType->toArray(); + } + + return new self(collect($unionType)->map(IntersectionType::fromRaw(...))->all()); + } + + public static function fromString(string $type): self + { + return self::fromRaw( + Str::of($type) + ->explode('|') + ->map(fn (string $type) => explode('&', $type)) + ->all() + ); + } + + public function isMixed(): bool + { + // A union type is mixed, if just one of the types are considered mixed + foreach ($this->intersectionTypes as $type) { + if ($type->isMixed()) { + return true; + } + } + + return false; + } + + /** + * @return array + */ + public function getIntersectionTypes(): array + { + return $this->intersectionTypes; + } + + public function isUnionOfOne(): bool + { + return count($this->intersectionTypes) === 1; + } + + public function toString(): string + { + return $this->__toString(); + } + + /** + * Magic method {@see https://www.php.net/manual/en/language.oop5.magic.php#object.tostring} + * allows a class to decide how it will react when it is treated like a string. + * + * @return string Returns string representation of the object that + * implements this interface (and/or "__toString" magic method). + */ + public function __toString(): string + { + $type = collect($this->intersectionTypes) + ->map(function (IntersectionType $intersectionType) { + if ($intersectionType->isIntersectionOfOne()) { + return $intersectionType->toString(); + } + + return sprintf('(%s)', $intersectionType->toString()); + }) + ->sort() + ->implode('|'); + + // No need to add parentheses from intersection, unless there are more types. + if ($this->isUnionOfOne()) { + return trim($type, '()'); + } + + return $type; + } + + /** + * Returns array containing all the necessary state of the object. + * + * @since 7.4 + * @link https://wiki.php.net/rfc/custom_object_serialization + */ + public function __serialize(): array + { + return [ + 'type' => $this->toString(), + ]; + } + + /** + * Restores the object state from the given data array. + * + * @param array $data + * + * @since 7.4 + * @link https://wiki.php.net/rfc/custom_object_serialization + */ + public function __unserialize(array $data): void + { + $this->intersectionTypes = self::fromString($data['type'])->intersectionTypes; + } +} diff --git a/test/Samples/CastedSpatieLaravelData.php b/test/Samples/CastedSpatieLaravelData.php new file mode 100644 index 0000000..02e6745 --- /dev/null +++ b/test/Samples/CastedSpatieLaravelData.php @@ -0,0 +1,53 @@ + '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 + ], + [ + '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: + ], + ); + } +} diff --git a/test/Samples/Casts/BackedEnumCast.php b/test/Samples/Casts/BackedEnumCast.php new file mode 100644 index 0000000..069fdf3 --- /dev/null +++ b/test/Samples/Casts/BackedEnumCast.php @@ -0,0 +1,16 @@ + 123, + 'nullableTypeStringProperty' => 123.45, + ], + [ + 'nullableMarkStringProperty' => [], + 'nullableTypeStringProperty' => [], + 'intersectionProperty' => (object) [], + ], + ); + } +} diff --git a/test/Samples/InvalidScalarSpatieLaravelData.php b/test/Samples/InvalidScalarSpatieLaravelData.php new file mode 100644 index 0000000..bc4b3cc --- /dev/null +++ b/test/Samples/InvalidScalarSpatieLaravelData.php @@ -0,0 +1,26 @@ + null, + 'intProperty' => 123.45, + 'booleanProperty' => [], + 'floatProperty' => '12.5', + ]); + } +} diff --git a/test/Samples/SpatieDataCast.php b/test/Samples/SpatieDataCast.php deleted file mode 100644 index 3a8b882..0000000 --- a/test/Samples/SpatieDataCast.php +++ /dev/null @@ -1,15 +0,0 @@ -stringProperty = $stringProperty; -// $this->intProperty = $intProperty; -// $this->booleanProperty = $booleanProperty; -// $this->objectProperty = $objectProperty; -// $this->nullableObjectProperty = $nullableObjectProperty; -// $this->undefinedPropertyType = $undefinedPropertyType; - $this->unionType = $unionType; -// $this->intersectionType = $intersectionType; - } - - public function initDefault(): self - { - return self::from([ - 'stringProperty' => 'my string', - 'intProperty' => 123, - 'booleanProperty' => 'cake', - 'objectProperty' => new EmptySpatieLaravelData(), - ], [ - 'nullableObjectProperty' => null, - 'undefinedPropertyType' => 'dsjahiuoas', - 'unionType' => '', - 'intersectionType' => 'sadsa', - ]); - } -} diff --git a/test/Samples/ValidComplexSpatieLaravelData.php b/test/Samples/ValidComplexSpatieLaravelData.php new file mode 100644 index 0000000..e4591bd --- /dev/null +++ b/test/Samples/ValidComplexSpatieLaravelData.php @@ -0,0 +1,33 @@ + null, + 'nullableTypeStringProperty' => null, + ], + [ + 'nullableMarkStringProperty' => null, + 'nullableTypeStringProperty' => null, + 'intersectionProperty' => new UncastableSpatieDataCast(), + ], + ); + } +} diff --git a/test/Samples/ValidSpatieLaravelData.php b/test/Samples/ValidSpatieLaravelData.php new file mode 100644 index 0000000..3d0f2d2 --- /dev/null +++ b/test/Samples/ValidSpatieLaravelData.php @@ -0,0 +1,41 @@ + 'my string', + 'intProperty' => 123, + 'floatProperty' => 123.45, + 'float2Property' => 123, // int is smaller than float, so is always compatible. + 'booleanProperty' => true, + 'objectProperty' => new EmptySpatieLaravelData(), + ], [ + 'nullableObjectProperty' => null, + 'mixedPropertyType' => 'dsjahiuoas', + 'unionType' => new UncastableSpatieDataCast(), + 'intersectionType' => new EmptySpatieLaravelData(), + ]); + } +} diff --git a/test/Samples/main.php b/test/Samples/main.php deleted file mode 100644 index d1c1463..0000000 --- a/test/Samples/main.php +++ /dev/null @@ -1,20 +0,0 @@ - $bla */ -$bla = ""; - -SpatieLaravelData::from([ - 'intProperty' => 12, - 'stringProperty' => 'hohoho', -], [ - 'objectProperty' => new EmptySpatieLaravelData(), - 'nullableObjectProperty' => null, - 'unionType' => (fn () => $bla)(), -]); - diff --git a/test/SpatieLaravelData/ValidTypeRuleTest.php b/test/SpatieLaravelData/ValidTypeRuleTest.php new file mode 100644 index 0000000..60a68e3 --- /dev/null +++ b/test/SpatieLaravelData/ValidTypeRuleTest.php @@ -0,0 +1,88 @@ +analyse([__DIR__ . '/../Samples/ValidSpatieLaravelData.php'], []); + } + + /** @test */ + public function it_returns_no_errors_for_valid_complex_data(): void + { + $this->analyse([__DIR__ . '/../Samples/ValidComplexSpatieLaravelData.php'], []); + } + + /** @test */ + public function it_returns_errors_for_invalid_scalar_data(): void + { + $this->analyse([__DIR__ . '/../Samples/InvalidScalarSpatieLaravelData.php'], [ + $this->expectError(21, 'stringProperty', InvalidScalarSpatieLaravelData::class, 'string', 'null'), + $this->expectError(21, 'intProperty', InvalidScalarSpatieLaravelData::class, 'int', 'float'), + $this->expectError(21, 'booleanProperty', InvalidScalarSpatieLaravelData::class, 'bool', 'array'), + $this->expectError(21, 'floatProperty', InvalidScalarSpatieLaravelData::class, 'float', 'string'), + ]); + } + + /** @test */ + public function it_returns_errors_for_invalid_complex_data(): void + { + $this->analyse([__DIR__ . '/../Samples/InvalidComplexSpatieLaravelData.php'], [ + $this->expectError(21, 'nullableMarkStringProperty', InvalidComplexSpatieLaravelData::class, 'null|string', 'int'), + $this->expectError(21, 'nullableTypeStringProperty', InvalidComplexSpatieLaravelData::class, 'null|string', 'float'), + $this->expectError(21, 'nullableMarkStringProperty', InvalidComplexSpatieLaravelData::class, 'null|string', 'array'), + $this->expectError(21, 'nullableTypeStringProperty', InvalidComplexSpatieLaravelData::class, 'null|string', 'array'), + $this->expectError(21, 'intersectionProperty', InvalidComplexSpatieLaravelData::class, '\Spatie\LaravelData\Casts\Cast&\Stringable', 'stdClass'), + ]); + } + + /** @test */ + public function it_ignores_potential_problems_for_objects_with_casts(): void + { + $this->analyse([ + __DIR__ . '/../Samples/CastedSpatieLaravelData.php', + __DIR__ . '/../Samples/Casts/BackedEnumCast.php', + __DIR__ . '/../Samples/Casts/UncastableSpatieDataCast.php', + ], []); + } + + private function expectError(int $line, string $property, string $class, string $expectedType, string $actualType): array + { + return [ + ValidTypeRule::getErrorMessage($property, $class, $expectedType, $actualType), + $line, + ]; + } +} From 4b26e3df10cdca503f8f455a6fc782fbd5096934 Mon Sep 17 00:00:00 2001 From: Niki Zakariassen Date: Wed, 8 Feb 2023 13:36:50 +0100 Subject: [PATCH 10/14] Added missing include --- extension.neon | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/extension.neon b/extension.neon index dcf60bc..33b46ee 100644 --- a/extension.neon +++ b/extension.neon @@ -1,5 +1,5 @@ -#includes: -# - ./../../nunomaduro/larastan/extension.neon +includes: + - ./../../nunomaduro/larastan/extension.neon services: - From 76ecff2ff01e505cc01a5687e251b67d4cbdacfb Mon Sep 17 00:00:00 2001 From: Niki Zakariassen Date: Wed, 8 Feb 2023 13:46:54 +0100 Subject: [PATCH 11/14] Fixed composer --- composer.json | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/composer.json b/composer.json index 56d4fe8..9936c87 100644 --- a/composer.json +++ b/composer.json @@ -22,13 +22,13 @@ } }, "require": { - "php": "^8.1" + "php": "^8.1", + "phpstan/phpstan": "^1.4", + "nunomaduro/larastan": "^2.4", + "spatie/laravel-data": "^3.0" }, "require-dev": { - "spatie/laravel-data": "^3.0", "phpunit/phpunit": "^10.0", - "phpstan/phpstan": "^1.4", - "nunomaduro/larastan": "^2.4", "cego/php-cs-fixer": "^1.0" } } From cd2ae901cb40ac46037f584884552312ea01d167 Mon Sep 17 00:00:00 2001 From: Niki Zakariassen Date: Wed, 8 Feb 2023 14:14:05 +0100 Subject: [PATCH 12/14] Added fix for bug with class generics --- src/TypeSystem/Type.php | 13 ++++++++--- test/Samples/EmptySpatieLaravelData.php | 3 +++ test/Samples/GenericsSpatieLaravelData.php | 23 ++++++++++++++++++++ test/SpatieLaravelData/ValidTypeRuleTest.php | 8 +++++++ 4 files changed, 44 insertions(+), 3 deletions(-) create mode 100644 test/Samples/GenericsSpatieLaravelData.php diff --git a/src/TypeSystem/Type.php b/src/TypeSystem/Type.php index 72f3f96..8b40393 100644 --- a/src/TypeSystem/Type.php +++ b/src/TypeSystem/Type.php @@ -3,16 +3,24 @@ namespace Cego\phpstan\TypeSystem; use Stringable; +use Illuminate\Support\Str; class Type implements Stringable { + public readonly string $type; + /** * Constructor * * @param string $type */ - public function __construct(public readonly string $type) + public function __construct(string $type) { + if (empty($type)) { + $this->type = 'mixed'; + } else { + $this->type = Str::of($type)->replaceMatches('/<[^<>]*>/', '')->toString(); + } } /** @@ -32,8 +40,7 @@ public function isNotReal(): bool */ public function isMixed(): bool { - return empty($this->type) - || strtolower($this->type) === 'mixed'; + return strtolower($this->type) === 'mixed'; } /** diff --git a/test/Samples/EmptySpatieLaravelData.php b/test/Samples/EmptySpatieLaravelData.php index fc0b05c..e11db62 100644 --- a/test/Samples/EmptySpatieLaravelData.php +++ b/test/Samples/EmptySpatieLaravelData.php @@ -4,6 +4,9 @@ use Spatie\LaravelData\Data; +/** + * @template key + */ class EmptySpatieLaravelData extends Data { } diff --git a/test/Samples/GenericsSpatieLaravelData.php b/test/Samples/GenericsSpatieLaravelData.php new file mode 100644 index 0000000..2846d71 --- /dev/null +++ b/test/Samples/GenericsSpatieLaravelData.php @@ -0,0 +1,23 @@ + $var */ + $var = ''; + + return self::from([ + 'genericProperty' => $var, + ]); + } +} diff --git a/test/SpatieLaravelData/ValidTypeRuleTest.php b/test/SpatieLaravelData/ValidTypeRuleTest.php index 60a68e3..1c04568 100644 --- a/test/SpatieLaravelData/ValidTypeRuleTest.php +++ b/test/SpatieLaravelData/ValidTypeRuleTest.php @@ -78,6 +78,14 @@ public function it_ignores_potential_problems_for_objects_with_casts(): void ], []); } + /** @test */ + public function it_does_not_care_about_generics(): void + { + $this->analyse([ + __DIR__ . '/../Samples/GenericsSpatieLaravelData.php', + ], []); + } + private function expectError(int $line, string $property, string $class, string $expectedType, string $actualType): array { return [ From b94858f123251ab7e2e5fc438124067287ed9eda Mon Sep 17 00:00:00 2001 From: Niki Zakariassen Date: Wed, 8 Feb 2023 14:17:56 +0100 Subject: [PATCH 13/14] Added tip for other developers, in case of bugs. --- src/SpatieLaravelData/Rules/ValidTypeRule.php | 1 + test/SpatieLaravelData/ValidTypeRuleTest.php | 19 ++++++++++--------- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/src/SpatieLaravelData/Rules/ValidTypeRule.php b/src/SpatieLaravelData/Rules/ValidTypeRule.php index c908dae..b8fd0ba 100644 --- a/src/SpatieLaravelData/Rules/ValidTypeRule.php +++ b/src/SpatieLaravelData/Rules/ValidTypeRule.php @@ -124,6 +124,7 @@ private function checkType(Call $call, string $key, UnionType $actualType, array 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(); } diff --git a/test/SpatieLaravelData/ValidTypeRuleTest.php b/test/SpatieLaravelData/ValidTypeRuleTest.php index 1c04568..c5efaa5 100644 --- a/test/SpatieLaravelData/ValidTypeRuleTest.php +++ b/test/SpatieLaravelData/ValidTypeRuleTest.php @@ -49,10 +49,10 @@ public function it_returns_no_errors_for_valid_complex_data(): void public function it_returns_errors_for_invalid_scalar_data(): void { $this->analyse([__DIR__ . '/../Samples/InvalidScalarSpatieLaravelData.php'], [ - $this->expectError(21, 'stringProperty', InvalidScalarSpatieLaravelData::class, 'string', 'null'), - $this->expectError(21, 'intProperty', InvalidScalarSpatieLaravelData::class, 'int', 'float'), - $this->expectError(21, 'booleanProperty', InvalidScalarSpatieLaravelData::class, 'bool', 'array'), - $this->expectError(21, 'floatProperty', InvalidScalarSpatieLaravelData::class, 'float', 'string'), + $this->expectError(19, 'stringProperty', InvalidScalarSpatieLaravelData::class, 'string', 'null'), + $this->expectError(19, 'intProperty', InvalidScalarSpatieLaravelData::class, 'int', 'float'), + $this->expectError(19, 'booleanProperty', InvalidScalarSpatieLaravelData::class, 'bool', 'array'), + $this->expectError(19, 'floatProperty', InvalidScalarSpatieLaravelData::class, 'float', 'string'), ]); } @@ -60,11 +60,11 @@ public function it_returns_errors_for_invalid_scalar_data(): void public function it_returns_errors_for_invalid_complex_data(): void { $this->analyse([__DIR__ . '/../Samples/InvalidComplexSpatieLaravelData.php'], [ - $this->expectError(21, 'nullableMarkStringProperty', InvalidComplexSpatieLaravelData::class, 'null|string', 'int'), - $this->expectError(21, 'nullableTypeStringProperty', InvalidComplexSpatieLaravelData::class, 'null|string', 'float'), - $this->expectError(21, 'nullableMarkStringProperty', InvalidComplexSpatieLaravelData::class, 'null|string', 'array'), - $this->expectError(21, 'nullableTypeStringProperty', InvalidComplexSpatieLaravelData::class, 'null|string', 'array'), - $this->expectError(21, 'intersectionProperty', InvalidComplexSpatieLaravelData::class, '\Spatie\LaravelData\Casts\Cast&\Stringable', 'stdClass'), + $this->expectError(20, 'nullableMarkStringProperty', InvalidComplexSpatieLaravelData::class, 'null|string', 'int'), + $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'), ]); } @@ -91,6 +91,7 @@ private function expectError(int $line, string $property, string $class, string return [ ValidTypeRule::getErrorMessage($property, $class, $expectedType, $actualType), $line, + 'This is a custom CEGO rule, if you found a bug fix it in the cego/phpstan project', ]; } } From dcaed5c1aed49bf743ab74640e84d6476d2784bf Mon Sep 17 00:00:00 2001 From: Niki Zakariassen Date: Wed, 8 Feb 2023 14:44:43 +0100 Subject: [PATCH 14/14] Updated regex --- src/TypeSystem/Type.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/TypeSystem/Type.php b/src/TypeSystem/Type.php index 8b40393..7bef4a1 100644 --- a/src/TypeSystem/Type.php +++ b/src/TypeSystem/Type.php @@ -19,7 +19,7 @@ public function __construct(string $type) if (empty($type)) { $this->type = 'mixed'; } else { - $this->type = Str::of($type)->replaceMatches('/<[^<>]*>/', '')->toString(); + $this->type = Str::of($type)->replaceMatches('/<.*>/', '')->toString(); } }