diff --git a/docs/supported-types/iterators-and-generators.md b/docs/supported-types/iterators-and-generators.md index 9096e68..6e4ec29 100644 --- a/docs/supported-types/iterators-and-generators.md +++ b/docs/supported-types/iterators-and-generators.md @@ -14,6 +14,56 @@ When an iterable or generator is passed into a function accepting `Traversable`, `Traversable`, `Generator`) +When a method or parameter specifies an abstract stream keyword with inner type constraints: +```php +/** + * Abstract stream contract -> Wrapped in IteratorProxy + * + * @param Traversable $scores + */ +public function processScores(Traversable $scores): void +``` +Because `Traversable` is a general interface with no methods of its own, TypePHP wraps it in a lazy `IteratorProxy` to validate yielded items and keys during iteration. + +### 2. Concrete Collection Classes (`FileCollection`, `ArrayCollection`, `OrderList`) +When a method parameter, property, or return value is a concrete class (even if that class implements `\IteratorAggregate` or `\Traversable`): +```php +class StorefrontConfig +{ + // Native PHP class type hint + protected FileCollection $styleFiles; + + public function setStyleFiles(FileCollection $styleFiles): void + { + $this->styleFiles = $styleFiles; // Preserved as raw FileCollection! + } +} +``` + +TypePHP leaves concrete collections **100% unwrapped as raw PHP objects**: + +* **Preserves Native PHP Nominal Type Hints:** Prevents fatal PHP engine errors (e.g. `Cannot assign IteratorProxy to property ...::$styleFiles of type FileCollection`). +* **Preserves Custom Domain Methods & State:** Custom business methods (like `$files->getPublicUrls()` or `$files->filterByExtension()`) and private properties remain directly accessible. +* **Direct `\WeakMap` Enforcement:** For generic concrete classes (like `ArrayCollection`), TypePHP tracks generic template bindings directly in `\WeakMap` memory, running validation rules inside `$collection->add()` and `$collection->set()` without needing any wrapper proxy! +* **Zero Allocation Overhead:** Eliminates proxy object allocations and garbage collection pressure, allowing concrete collections to run at native C-level speed. + +### Summary: When TypePHP Wraps vs. Leaves Unwrapped + +| Type Annotation | Object Passed | Action Taken | Why | +| :--- | :--- | :---: | :--- | +| `Traversable` | Any Iterator / Generator | **Wrapped in `IteratorProxy`** | Abstract stream; needs lazy item validation during iteration. | +| `iterable` | Any Array / Traversable | **Wrapped in `IteratorProxy`** | Abstract stream; validates items on-the-fly. | +| `FileCollection` | `new FileCollection()` | **Unwrapped (Raw Object)** | Concrete class; preserves native PHP type hints and domain methods. | +| `ArrayCollection` | `new ArrayCollection()` | **Unwrapped (Raw Object)** | Generic state is tracked directly via `\WeakMap` inside `add()`/`set()`. | +| Un-annotated (`mixed $items`) | Any Iterator | **Unwrapped (Raw Object)** | No type contracts to enforce; zero overhead. | + +--- + ## Traversable & Iterator Contracts (`Traversable`) Validate keys and values on any `Traversable` or `ArrayIterator` instance: @@ -346,3 +396,4 @@ function processCollection(Traversable $collection): void processCollection(new NestedCollection()); ``` +``` \ No newline at end of file diff --git a/src/Internal/Visitor/FunctionContractInjector.php b/src/Internal/Visitor/FunctionContractInjector.php index d8b800a..6ba4fa6 100644 --- a/src/Internal/Visitor/FunctionContractInjector.php +++ b/src/Internal/Visitor/FunctionContractInjector.php @@ -74,7 +74,7 @@ private static function isGenerator(Node\Stmt\Function_|Node\Stmt\ClassMethod $n return false; } - $visitor = new class () extends NodeVisitorAbstract { + $visitor = new class() extends NodeVisitorAbstract { public bool $isGen = false; public function enterNode(Node $n): ?int @@ -182,7 +182,7 @@ private static function buildParamInjections( } } - if ($isClassMethod || str_contains($docText, 'iterable') || str_contains($docText, 'Traversable') || str_contains($docText, 'Generator') || str_contains($docText, 'Iterator')) { + if (str_contains($docText, 'iterable') || str_contains($docText, 'Traversable') || str_contains($docText, 'Generator') || str_contains($docText, 'Iterator')) { foreach ($node->params as $param) { if ($param->var instanceof Node\Expr\Variable && \is_string($param->var->name)) { $paramName = $param->var->name; @@ -217,10 +217,8 @@ private static function buildParamInjections( private static function wrapGeneratorReturns(array $stmts, Node\Expr $thisArg): array { $traverser = new NodeTraverser(); - $traverser->addVisitor(new class ($thisArg) extends NodeVisitorAbstract { - public function __construct(private Node\Expr $thisArg) - { - } + $traverser->addVisitor(new class($thisArg) extends NodeVisitorAbstract { + public function __construct(private Node\Expr $thisArg) {} public function enterNode(Node $n): int|Node|null { @@ -344,12 +342,11 @@ public function enterNode(Node $n): int|Node|null private static function wrapNonGeneratorReturns(array $stmts, Node\Expr $thisArg, bool $isNativeVoid): array { $traverser = new NodeTraverser(); - $traverser->addVisitor(new class ($thisArg, $isNativeVoid) extends NodeVisitorAbstract { + $traverser->addVisitor(new class($thisArg, $isNativeVoid) extends NodeVisitorAbstract { public function __construct( private Node\Expr $thisArg, private bool $isNativeVoid - ) { - } + ) {} public function enterNode(Node $n): int|array|null { diff --git a/src/Wrapper/IterableWrapper.php b/src/Wrapper/IterableWrapper.php index 8ae2fa0..3f23541 100644 --- a/src/Wrapper/IterableWrapper.php +++ b/src/Wrapper/IterableWrapper.php @@ -37,33 +37,43 @@ public static function wrap(string $function, string $paramName, mixed $iterable $contract = ContractParser::parse($function); $typeNode = ($paramName === 'return') ? ($contract['return'] ?? null) : ($contract['types'][$paramName] ?? null); + + // If no docblock contract exists, never wrap! + if ($typeNode === null) { + return $iterable; + } + $aliases = $contract['aliases'] ?? []; $templates = $contract['templates'] ?? []; $thisObj = \is_object($thisOrClass) ? $thisOrClass : null; $boundTemplates = TemplateManager::getBoundTemplates($function, $thisObj, $templates); - if ($typeNode !== null && (\count($boundTemplates) > 0 || \count($templates) > 0)) { + if (\count($boundTemplates) > 0 || \count($templates) > 0) { $typeNode = TemplateSubstitutor::substitute($typeNode, $boundTemplates, $templates); $typeNode = SpecialTypeResolver::resolve($typeNode, $function, $thisObj); } - if ($typeNode !== null) { - $baseName = ''; - if ($typeNode instanceof IdentifierTypeNode) { - $baseName = strtolower(ltrim($typeNode->name, '\\')); - } elseif ($typeNode instanceof GenericTypeNode) { - $baseName = strtolower(ltrim($typeNode->type->name, '\\')); - } + // Only wrap generic iterable keywords, never concrete classes like FileCollection + $baseName = ''; + if ($typeNode instanceof IdentifierTypeNode) { + $baseName = strtolower(ltrim($typeNode->name, '\\')); + } elseif ($typeNode instanceof GenericTypeNode) { + $baseName = strtolower(ltrim($typeNode->type->name, '\\')); + } - $standardIterables = ['iterable', 'traversable', 'iterator', 'generator', 'iteratoraggregate', 'array']; - if ($baseName !== '' && ! \in_array($baseName, $standardIterables, true)) { - return $iterable; - } + $standardIterables = ['iterable', 'traversable', 'iterator', 'generator']; + if (! \in_array($baseName, $standardIterables, true)) { + return $iterable; } [$keyTypeNode, $itemTypeNode] = self::extractKeyAndItemTypeNodes($typeNode, $aliases); + // If there are no inner type constraints to validate, never wrap! + if ($keyTypeNode === null && $itemTypeNode === null) { + return $iterable; + } + $prefix = ($paramName === 'return') ? "$function(): Return iterator" : "$function(): Iterator \$$paramName"; $typeCheckCallback = self::createValidationCallback($registry, $keyTypeNode, $itemTypeNode, $prefix); diff --git a/tests/Fixtures/Collections/ConcreteFileCollection.php b/tests/Fixtures/Collections/ConcreteFileCollection.php new file mode 100644 index 0000000..b383f79 --- /dev/null +++ b/tests/Fixtures/Collections/ConcreteFileCollection.php @@ -0,0 +1,27 @@ + + */ + private array $files = []; + + public function add(string $file): void + { + $this->files[] = $file; + } + + public function getIterator(): Traversable + { + return new ArrayIterator($this->files); + } +} \ No newline at end of file diff --git a/tests/Fixtures/Collections/PluginConfiguration.php b/tests/Fixtures/Collections/PluginConfiguration.php new file mode 100644 index 0000000..b86e200 --- /dev/null +++ b/tests/Fixtures/Collections/PluginConfiguration.php @@ -0,0 +1,68 @@ +styleFiles = $promotedFiles ?? new ConcreteFileCollection(); + } + + public function getStyleFiles(): ConcreteFileCollection + { + return $this->styleFiles; + } + + public function setStyleFiles(ConcreteFileCollection $styleFiles): void + { + $this->styleFiles = $styleFiles; + } + + /** + * @param ConcreteFileCollection $styleFiles + */ + public function setStyleFilesWithDocblock(ConcreteFileCollection $styleFiles): void + { + $this->styleFiles = $styleFiles; + } + + /** + * @return ConcreteFileCollection + */ + public function getStyleFilesWithDocblock(): ConcreteFileCollection + { + return $this->styleFiles; + } + + public function setNullableFiles(?ConcreteFileCollection $files = null): void + { + if ($files !== null) { + $this->styleFiles = $files; + } + } + + /** + * Legitimate generic Traversable parameter that SHOULD be wrapped to validate strings + * + * @param Traversable $items + * + * @return list + */ + public function processGenericTraversable(Traversable $items): array + { + $collected = []; + foreach ($items as $item) { + $collected[] = $item; + } + + return $collected; + } +} \ No newline at end of file diff --git a/tests/TypeChecking/CallablesAndIterators/ConcreteIterablePropertyAssignmentTest.php b/tests/TypeChecking/CallablesAndIterators/ConcreteIterablePropertyAssignmentTest.php new file mode 100644 index 0000000..bf1135b --- /dev/null +++ b/tests/TypeChecking/CallablesAndIterators/ConcreteIterablePropertyAssignmentTest.php @@ -0,0 +1,91 @@ +add('style.css'); + + $config->setStyleFiles($files); + + expect($config->getStyleFiles())->toBeInstanceOf(ConcreteFileCollection::class) + ->and($config->getStyleFiles())->toBe($files); + }); + + test('does not wrap concrete collection when method has explicit @param ConcreteFileCollection docblock', function () { + $config = new PluginConfiguration(); + $files = new ConcreteFileCollection(); + $files->add('custom.css'); + + $config->setStyleFilesWithDocblock($files); + + expect($config->styleFiles)->toBeInstanceOf(ConcreteFileCollection::class) + ->and($config->styleFiles)->toBe($files); + }); + + test('does not wrap concrete collection when method has explicit @return ConcreteFileCollection docblock', function () { + $config = new PluginConfiguration(); + $files = new ConcreteFileCollection(); + $files->add('bundle.css'); + $config->setStyleFiles($files); + + $returned = $config->getStyleFilesWithDocblock(); + + expect($returned)->toBeInstanceOf(ConcreteFileCollection::class) + ->and($returned)->toBe($files); + }); + }); + + describe('Constructor Promotion & Nullable Properties', function () { + test('preserves concrete collection type in constructor property promotion without wrapping in IteratorProxy', function () { + $files = new ConcreteFileCollection(); + $files->add('promoted.css'); + + $config = new PluginConfiguration($files); + + expect($config->promotedFiles)->toBeInstanceOf(ConcreteFileCollection::class) + ->and($config->promotedFiles)->toBe($files); + }); + + test('handles nullable concrete collection parameter cleanly', function () { + $config = new PluginConfiguration(); + + $config->setNullableFiles(null); + expect($config->styleFiles)->toBeInstanceOf(ConcreteFileCollection::class); + + $files = new ConcreteFileCollection(); + $files->add('new.css'); + $config->setNullableFiles($files); + + expect($config->styleFiles)->toBe($files); + }); + }); + + describe('Inline @var Assignments', function () { + test('preserves concrete collection instance in inline @var variable assignment', function () { + /** @var ConcreteFileCollection $localFiles */ + $localFiles = new ConcreteFileCollection(); + $localFiles->add('local.css'); + + expect($localFiles)->toBeInstanceOf(ConcreteFileCollection::class); + }); + }); + + describe('Legitimate Generic Traversable Contracts', function () { + test('correctly validates items when method explicitly declares @param Traversable', function () { + $config = new PluginConfiguration(); + $collection = new ConcreteFileCollection(); + $collection->add('valid_a.css'); + $collection->add('valid_b.css'); + + $result = $config->processGenericTraversable($collection); + expect($result)->toBe(['valid_a.css', 'valid_b.css']); + }); + }); +}); \ No newline at end of file