Enhance documentation for iterators and generators, add ConcreteFileCollection and PluginConfiguration classes, and implement tests for concrete iterable property assignments (#33)

Author: rcalicdan

docs/supported-types/iterators-and-generators.md

@@ -14,6 +14,56 @@ When an iterable or generator is passed into a function accepting `Traversable<K
 
 ---
 
+## Generic Streams vs. Concrete Collection Classes (Zero Proxy Overhead)
+
+A critical architectural design in TypePHP is distinguishing between **abstract generic streams** and **concrete collection classes**:
+
+### 1. Abstract Generic Streams (`iterable<T>`, `Traversable<K, V>`, `Generator<K, V>`)
+When a method or parameter specifies an abstract stream keyword with inner type constraints:
+```php
+/**
+ * Abstract stream contract -> Wrapped in IteratorProxy
+ *
+ * @param Traversable<string, positive-int> $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<int, Animal>`), 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<string, positive-int>` | Any Iterator / Generator | **Wrapped in `IteratorProxy`** | Abstract stream; needs lazy item validation during iteration. |
+| `iterable<User>` | 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<int, Animal>` | `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<K, V>`)
 
 Validate keys and values on any `Traversable` or `ArrayIterator` instance:
@@ -346,3 +396,4 @@ function processCollection(Traversable $collection): void
 
 processCollection(new NestedCollection());
 ```
+```

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
             {

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);
 

tests/Fixtures/Collections/ConcreteFileCollection.php

@@ -0,0 +1,27 @@
+<?php
+
+declare(strict_types=1);
+
+namespace TypePHP\Tests\Fixtures\Collections;
+
+use ArrayIterator;
+use IteratorAggregate;
+use Traversable;
+
+class ConcreteFileCollection implements IteratorAggregate
+{
+    /**
+     * @var array<int, string>
+     */
+    private array $files = [];
+
+    public function add(string $file): void
+    {
+        $this->files[] = $file;
+    }
+
+    public function getIterator(): Traversable
+    {
+        return new ArrayIterator($this->files);
+    }
+}

tests/Fixtures/Collections/PluginConfiguration.php

@@ -0,0 +1,68 @@
+<?php
+
+declare(strict_types=1);
+
+namespace TypePHP\Tests\Fixtures\Collections;
+
+use Traversable;
+
+class PluginConfiguration
+{
+    public ConcreteFileCollection $styleFiles;
+
+    public function __construct(
+        public ?ConcreteFileCollection $promotedFiles = null
+    ) {
+        $this->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<string> $items
+     *
+     * @return list<string>
+     */
+    public function processGenericTraversable(Traversable $items): array
+    {
+        $collected = [];
+        foreach ($items as $item) {
+            $collected[] = $item;
+        }
+
+        return $collected;
+    }
+}

tests/TypeChecking/CallablesAndIterators/ConcreteIterablePropertyAssignmentTest.php

@@ -0,0 +1,91 @@
+<?php
+
+declare(strict_types=1);
+
+use TypePHP\Tests\Fixtures\Collections\ConcreteFileCollection;
+use TypePHP\Tests\Fixtures\Collections\PluginConfiguration;
+
+describe('Concrete Traversable & Iterable Property Assignments (Shopware Bug Reproduction)', function () {
+    describe('Un-annotated & DocBlock-Annotated Methods', function () {
+        test('does not wrap concrete collection in IteratorProxy when setting and getting typed property (un-annotated)', function () {
+            $config = new PluginConfiguration();
+            $files = new ConcreteFileCollection();
+            $files->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<string>', 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']);
+        });
+    });
+});