From f5872405e5e829c58080b81119d1ef302d7025a8 Mon Sep 17 00:00:00 2001 From: Artyom Osepyan Date: Wed, 6 May 2026 12:43:37 +0300 Subject: [PATCH 1/5] fix: remarks --- src/Contracts/InsertNodeContract.php | 10 --- src/Contracts/InsertNodesContract.php | 11 ++++ src/Visitors/AddImports.php | 13 +--- src/Visitors/AddTraits.php | 7 +-- src/Visitors/BaseNodeVisitorAbstract.php | 19 +++--- src/Visitors/InsertNodesAbstractVisitor.php | 63 ++++--------------- src/Visitors/PropertyVisitors/SetProperty.php | 18 +++--- 7 files changed, 45 insertions(+), 96 deletions(-) delete mode 100644 src/Contracts/InsertNodeContract.php create mode 100644 src/Contracts/InsertNodesContract.php diff --git a/src/Contracts/InsertNodeContract.php b/src/Contracts/InsertNodeContract.php deleted file mode 100644 index 70a38ad..0000000 --- a/src/Contracts/InsertNodeContract.php +++ /dev/null @@ -1,10 +0,0 @@ -filter() - ->unique(); + ->unique() + ->map(fn ($import) => new Use_([new UseItem(new Name($import))])); parent::__construct( nodesToInsert: $nodesToInsert, @@ -24,11 +25,6 @@ public function __construct(array $imports) ); } - public function leaveNode(Node $node): Node - { - return $node; - } - public function afterTraverse(array $nodes): ?array { $targetNamespace = array_find($nodes, fn ($node) => $node instanceof Namespace_); @@ -50,9 +46,4 @@ protected function getChildNodes(Node $node): array { return $node->uses; } - - protected function getInsertableNode(string $name): Node - { - return new Use_([new UseItem(new Name($name))]); - } } diff --git a/src/Visitors/AddTraits.php b/src/Visitors/AddTraits.php index c743372..aa44377 100644 --- a/src/Visitors/AddTraits.php +++ b/src/Visitors/AddTraits.php @@ -22,7 +22,7 @@ public function __construct(array $traits) $nodesToInsert = collect($traits) ->filter() ->unique() - ->map(fn ($trait) => class_basename($trait)); + ->map(fn ($trait) => new TraitUse([new Name(class_basename($trait))])); parent::__construct( nodesToInsert: $nodesToInsert, @@ -35,9 +35,4 @@ protected function getChildNodes(Node $node): array { return $node->traits; } - - protected function getInsertableNode(string $name): Node - { - return new TraitUse([new Name($name)]); - } } diff --git a/src/Visitors/BaseNodeVisitorAbstract.php b/src/Visitors/BaseNodeVisitorAbstract.php index 3adf8fe..08ce6cf 100644 --- a/src/Visitors/BaseNodeVisitorAbstract.php +++ b/src/Visitors/BaseNodeVisitorAbstract.php @@ -7,7 +7,7 @@ use PhpParser\Node\Stmt\Enum_; use PhpParser\Node\Stmt\Trait_; use PhpParser\NodeVisitorAbstract; -use RonasIT\Larabuilder\Contracts\InsertNodeContract; +use RonasIT\Larabuilder\Contracts\InsertNodesContract; use RonasIT\Larabuilder\Contracts\UpdateNodeContract; use RonasIT\Larabuilder\Exceptions\InvalidStructureTypeException; use RonasIT\Larabuilder\Support\NodeInserter; @@ -71,24 +71,23 @@ protected function modify(Node $node): Node $this->updatableNotFoundHook(); - return ($this instanceof InsertNodeContract) - ? $this->insertNode($node) - : $node; + if ($this instanceof InsertNodesContract) { + $this->insertNodes($node->stmts); + } + + return $node; } protected function updatableNotFoundHook(): void { } - /** @param Class_|Trait_|Enum_ $node */ - private function insertNode(Node $node): Node + protected function insertNodes(array &$nodes): void { $this->nodeInserter ??= new NodeInserter(); - $newNode = $this->getInsertableNode(); + $newNodes = $this->getInsertableNodes($nodes); - $this->nodeInserter->insertNodes($node->stmts, [$newNode], true); - - return $node; + $this->nodeInserter->insertNodes($nodes, $newNodes, true); } } diff --git a/src/Visitors/InsertNodesAbstractVisitor.php b/src/Visitors/InsertNodesAbstractVisitor.php index ac129ea..7ddda6f 100644 --- a/src/Visitors/InsertNodesAbstractVisitor.php +++ b/src/Visitors/InsertNodesAbstractVisitor.php @@ -4,76 +4,37 @@ use Illuminate\Support\Collection; use PhpParser\Node; -use PhpParser\Node\Stmt\Class_; -use PhpParser\Node\Stmt\Enum_; -use PhpParser\Node\Stmt\Trait_; -use PhpParser\Node\Stmt\TraitUse; -use PhpParser\Node\Stmt\Use_; -use RonasIT\Larabuilder\Support\NodeInserter; +use RonasIT\Larabuilder\Contracts\InsertNodesContract; -abstract class InsertNodesAbstractVisitor extends BaseNodeVisitorAbstract +abstract class InsertNodesAbstractVisitor extends BaseNodeVisitorAbstract implements InsertNodesContract { - abstract protected function getInsertableNode(string $name): Node; - abstract protected function getChildNodes(Node $node): array; - protected NodeInserter $nodeInserter; - public function __construct( protected Collection $nodesToInsert, protected string $targetNodeClass, ) { - $this->nodeInserter = new NodeInserter(); } - /** @param Class_|Enum_|Trait_ $node */ - protected function modify(Node $node): Node + public function getInsertableNodes(array $nodes): array { - $this->insertNodes($node->stmts); - - return $node; - } - - protected function insertNodes(array &$nodes): void - { - $newNodes = $this->getNodesToAdd($nodes); - - if (!empty($newNodes)) { - $nodes = $this->addNodes($nodes, $newNodes); - } - } - - protected function getNodesToAdd(array $nodes): Collection - { - $existingNodes = []; + $existingNames = []; foreach ($nodes as $node) { if (!($node instanceof $this->targetNodeClass)) { continue; } - /** @var TraitUse|Use_ $node */ - $childNodes = $this->getChildNodes($node); - - foreach ($childNodes as $childNode) { - if ($this->nodesToInsert->contains($childNode->name)) { - $existingNodes[] = $childNode->name; - } + foreach ($this->getChildNodes($node) as $childNode) { + $existingNames[] = (string) $childNode->name; } } - return $this - ->nodesToInsert - ->diff($existingNodes) - ->values(); - } - - protected function addNodes(array $nodes, Collection $newNodes): array - { - $insertableNodes = $newNodes->map(fn ($node) => $this->getInsertableNode($node))->all(); - - $this->nodeInserter->insertNodes($nodes, $insertableNodes); - - return $nodes; + return $this->nodesToInsert->filter( + fn (Node $newNode) => !in_array( + (string) $this->getChildNodes($newNode)[0]->name, + $existingNames, + ), + )->values()->all(); } } diff --git a/src/Visitors/PropertyVisitors/SetProperty.php b/src/Visitors/PropertyVisitors/SetProperty.php index b8c29ee..6e12f4f 100644 --- a/src/Visitors/PropertyVisitors/SetProperty.php +++ b/src/Visitors/PropertyVisitors/SetProperty.php @@ -6,13 +6,13 @@ use PhpParser\Node\Identifier; use PhpParser\Node\PropertyItem; use PhpParser\Node\Stmt\Property; -use RonasIT\Larabuilder\Contracts\InsertNodeContract; +use RonasIT\Larabuilder\Contracts\InsertNodesContract; use RonasIT\Larabuilder\Contracts\UpdateNodeContract; use RonasIT\Larabuilder\Enums\AccessModifierEnum; use RonasIT\Larabuilder\Support\ParentNodeLinker; use RonasIT\Larabuilder\Support\ValueNodeFactory; -class SetProperty extends AbstractPropertyVisitor implements InsertNodeContract, UpdateNodeContract +class SetProperty extends AbstractPropertyVisitor implements InsertNodesContract, UpdateNodeContract { protected PropertyItem $propertyItem; protected Identifier $typeIdentifier; @@ -51,12 +51,14 @@ public function updateNode(Node $node): void } } - public function getInsertableNode(): Node + public function getInsertableNodes(array $nodes): array { - return new Property( - flags: ($this->accessModifier ?? AccessModifierEnum::Public)->value, - props: [$this->propertyItem], - type: $this->typeIdentifier, - ); + return [ + new Property( + flags: ($this->accessModifier ?? AccessModifierEnum::Public)->value, + props: [$this->propertyItem], + type: $this->typeIdentifier, + ), + ]; } } From 5ff20d74a3d2de4ee536b2bb8df523183caa8134 Mon Sep 17 00:00:00 2001 From: Artyom Osepyan Date: Wed, 6 May 2026 15:58:21 +0300 Subject: [PATCH 2/5] refactor: replace abstract visitor hierarchy with contract-based polymorphic dispatch --- ARCHITECTURE.md | 14 +++--- src/Contracts/InsertNodeContract.php | 10 +++++ src/Contracts/InsertNodesContract.php | 5 ++- src/Visitors/AddImports.php | 29 ++++++------ src/Visitors/AddTraits.php | 23 +++++----- src/Visitors/BaseNodeVisitorAbstract.php | 45 ++++++++++++++++--- src/Visitors/InsertNodesAbstractVisitor.php | 40 ----------------- src/Visitors/PropertyVisitors/SetProperty.php | 18 ++++---- 8 files changed, 95 insertions(+), 89 deletions(-) create mode 100644 src/Contracts/InsertNodeContract.php delete mode 100644 src/Visitors/InsertNodesAbstractVisitor.php diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 5e04652..be65532 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -43,11 +43,10 @@ For modifying classes, traits, enums, etc. The base class handles: #### Contracts - **`UpdateNodeContract`** — The visitor can update an existing node. Requires `shouldUpdateNode(Node): bool` and `updateNode(Node): void`. The base class iterates over child statements and calls `updateNode()` on the first match. -- **`InsertNodeContract`** — The visitor can insert a new node. Requires `getInsertableNode(): Node`. The base class handles positioning (via `NodeInserter`) and empty line insertion. +- **`InsertNodeContract`** — The visitor inserts a single node. Requires `getInsertableNode(): Node`. The base class handles positioning (via `NodeInserter`) and empty line insertion. +- **`InsertNodesContract`** — The visitor inserts multiple nodes with duplicate filtering. Requires `getInsertableNodes(): array` (returns all candidate nodes) and `getSubNodes(Node): array` (returns child nodes used for duplicate detection). The base class calls `filterExistingNodes()` to exclude already-present nodes before inserting. -A visitor may implement both contracts. In that case, update is attempted first — insertion happens only if no existing node matched. - -**Bulk insertion visitors** extend `InsertNodesAbstractVisitor` (which extends `BaseNodeVisitorAbstract`) and handle inserting multiple nodes with built-in duplicate filtering. +A visitor may implement `UpdateNodeContract` together with `InsertNodeContract` or `InsertNodesContract`. In that case, update is attempted first — insertion happens only if no existing node matched. ### App bootstrap visitors (`AbstractAppBootstrapVisitor`) @@ -72,8 +71,11 @@ Located in `src/Support/`: ## Creating a New Visitor -1. Extend `BaseNodeVisitorAbstract` (or `InsertNodesAbstractVisitor` for bulk insertions). +1. Extend `BaseNodeVisitorAbstract`. 2. Set `$allowedParentNodesTypes` to the node types your visitor targets. -3. Implement `InsertNodeContract`, `UpdateNodeContract`, or both. +3. Implement the appropriate contract(s): + - `InsertNodeContract` — for inserting a single node, implement `getInsertableNode(): Node` + - `InsertNodesContract` — for inserting multiple nodes with dedup, implement `getInsertableNodes(): array` and `getSubNodes(Node): array` + - `UpdateNodeContract` — for updating an existing node, implement `shouldUpdateNode(Node): bool` and `updateNode(Node): void` 4. Add a corresponding fluent method in `PHPFileBuilder` that creates and registers the visitor. 5. Add fixture files and a test case. diff --git a/src/Contracts/InsertNodeContract.php b/src/Contracts/InsertNodeContract.php new file mode 100644 index 0000000..70a38ad --- /dev/null +++ b/src/Contracts/InsertNodeContract.php @@ -0,0 +1,10 @@ +filter() - ->unique() - ->map(fn ($import) => new Use_([new UseItem(new Name($import))])); - - parent::__construct( - nodesToInsert: $nodesToInsert, - targetNodeClass: Use_::class, - ); + public function __construct( + protected array $imports, + ) { } public function afterTraverse(array $nodes): ?array { $targetNamespace = array_find($nodes, fn ($node) => $node instanceof Namespace_); - if (!is_null($targetNamespace)) { + if ($targetNamespace !== null) { /** @var Namespace_ $targetNamespace */ $targetNodes = &$targetNamespace->stmts; } else { @@ -41,8 +34,16 @@ public function afterTraverse(array $nodes): ?array return $nodes; } + public function getInsertableNodes(): array + { + return array_map( + fn ($import) => new Use_([new UseItem(new Name($import))]), + array_unique(array_filter($this->imports)), + ); + } + /** @param Use_ $node */ - protected function getChildNodes(Node $node): array + public function getSubNodes(Node $node): array { return $node->uses; } diff --git a/src/Visitors/AddTraits.php b/src/Visitors/AddTraits.php index aa44377..2ed69d3 100644 --- a/src/Visitors/AddTraits.php +++ b/src/Visitors/AddTraits.php @@ -8,8 +8,9 @@ use PhpParser\Node\Stmt\Enum_; use PhpParser\Node\Stmt\Trait_; use PhpParser\Node\Stmt\TraitUse; +use RonasIT\Larabuilder\Contracts\InsertNodesContract; -class AddTraits extends InsertNodesAbstractVisitor +class AddTraits extends BaseNodeVisitorAbstract implements InsertNodesContract { protected array $allowedParentNodesTypes = [ Class_::class, @@ -17,21 +18,21 @@ class AddTraits extends InsertNodesAbstractVisitor Enum_::class, ]; - public function __construct(array $traits) - { - $nodesToInsert = collect($traits) - ->filter() - ->unique() - ->map(fn ($trait) => new TraitUse([new Name(class_basename($trait))])); + public function __construct( + protected array $traits, + ) { + } - parent::__construct( - nodesToInsert: $nodesToInsert, - targetNodeClass: TraitUse::class, + public function getInsertableNodes(): array + { + return array_map( + fn ($trait) => new TraitUse([new Name(class_basename($trait))]), + array_unique(array_filter($this->traits)), ); } /** @param TraitUse $node */ - protected function getChildNodes(Node $node): array + public function getSubNodes(Node $node): array { return $node->traits; } diff --git a/src/Visitors/BaseNodeVisitorAbstract.php b/src/Visitors/BaseNodeVisitorAbstract.php index 08ce6cf..a68cd7c 100644 --- a/src/Visitors/BaseNodeVisitorAbstract.php +++ b/src/Visitors/BaseNodeVisitorAbstract.php @@ -7,6 +7,7 @@ use PhpParser\Node\Stmt\Enum_; use PhpParser\Node\Stmt\Trait_; use PhpParser\NodeVisitorAbstract; +use RonasIT\Larabuilder\Contracts\InsertNodeContract; use RonasIT\Larabuilder\Contracts\InsertNodesContract; use RonasIT\Larabuilder\Contracts\UpdateNodeContract; use RonasIT\Larabuilder\Exceptions\InvalidStructureTypeException; @@ -71,9 +72,7 @@ protected function modify(Node $node): Node $this->updatableNotFoundHook(); - if ($this instanceof InsertNodesContract) { - $this->insertNodes($node->stmts); - } + $this->insertNodes($node->stmts); return $node; } @@ -84,10 +83,44 @@ protected function updatableNotFoundHook(): void protected function insertNodes(array &$nodes): void { - $this->nodeInserter ??= new NodeInserter(); + $newNodes = match (true) { + $this instanceof InsertNodesContract => $this->filterExistingNodes($nodes), + $this instanceof InsertNodeContract => [$this->getInsertableNode()], + default => null, + }; + + if (!empty($newNodes)) { + $this->nodeInserter ??= new NodeInserter(); + + $this->nodeInserter->insertNodes($nodes, $newNodes, true); + } + } + + private function filterExistingNodes(array $nodes): array + { + $insertableNodes = $this->getInsertableNodes(); - $newNodes = $this->getInsertableNodes($nodes); + if (empty($insertableNodes)) { + return []; + } + + $targetNodeClass = get_class($insertableNodes[0]); + + $existingNames = []; + + foreach ($nodes as $node) { + if (!($node instanceof $targetNodeClass)) { + continue; + } + + foreach ($this->getSubNodes($node) as $childNode) { + $existingNames[] = (string) $childNode->name; + } + } - $this->nodeInserter->insertNodes($nodes, $newNodes, true); + return array_values(array_filter( + $insertableNodes, + fn (Node $newNode) => !in_array((string) $this->getSubNodes($newNode)[0]->name, $existingNames), + )); } } diff --git a/src/Visitors/InsertNodesAbstractVisitor.php b/src/Visitors/InsertNodesAbstractVisitor.php deleted file mode 100644 index 7ddda6f..0000000 --- a/src/Visitors/InsertNodesAbstractVisitor.php +++ /dev/null @@ -1,40 +0,0 @@ -targetNodeClass)) { - continue; - } - - foreach ($this->getChildNodes($node) as $childNode) { - $existingNames[] = (string) $childNode->name; - } - } - - return $this->nodesToInsert->filter( - fn (Node $newNode) => !in_array( - (string) $this->getChildNodes($newNode)[0]->name, - $existingNames, - ), - )->values()->all(); - } -} diff --git a/src/Visitors/PropertyVisitors/SetProperty.php b/src/Visitors/PropertyVisitors/SetProperty.php index 6e12f4f..b8c29ee 100644 --- a/src/Visitors/PropertyVisitors/SetProperty.php +++ b/src/Visitors/PropertyVisitors/SetProperty.php @@ -6,13 +6,13 @@ use PhpParser\Node\Identifier; use PhpParser\Node\PropertyItem; use PhpParser\Node\Stmt\Property; -use RonasIT\Larabuilder\Contracts\InsertNodesContract; +use RonasIT\Larabuilder\Contracts\InsertNodeContract; use RonasIT\Larabuilder\Contracts\UpdateNodeContract; use RonasIT\Larabuilder\Enums\AccessModifierEnum; use RonasIT\Larabuilder\Support\ParentNodeLinker; use RonasIT\Larabuilder\Support\ValueNodeFactory; -class SetProperty extends AbstractPropertyVisitor implements InsertNodesContract, UpdateNodeContract +class SetProperty extends AbstractPropertyVisitor implements InsertNodeContract, UpdateNodeContract { protected PropertyItem $propertyItem; protected Identifier $typeIdentifier; @@ -51,14 +51,12 @@ public function updateNode(Node $node): void } } - public function getInsertableNodes(array $nodes): array + public function getInsertableNode(): Node { - return [ - new Property( - flags: ($this->accessModifier ?? AccessModifierEnum::Public)->value, - props: [$this->propertyItem], - type: $this->typeIdentifier, - ), - ]; + return new Property( + flags: ($this->accessModifier ?? AccessModifierEnum::Public)->value, + props: [$this->propertyItem], + type: $this->typeIdentifier, + ); } } From 0520d5e81847c25421547680a88125595a25a5ea Mon Sep 17 00:00:00 2001 From: Artyom Osepyan Date: Wed, 6 May 2026 16:36:26 +0300 Subject: [PATCH 3/5] fix: remarks --- src/Visitors/AddImports.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Visitors/AddImports.php b/src/Visitors/AddImports.php index 155f6ff..935b6e9 100644 --- a/src/Visitors/AddImports.php +++ b/src/Visitors/AddImports.php @@ -22,7 +22,7 @@ public function afterTraverse(array $nodes): ?array { $targetNamespace = array_find($nodes, fn ($node) => $node instanceof Namespace_); - if ($targetNamespace !== null) { + if (!is_null($targetNamespace)) { /** @var Namespace_ $targetNamespace */ $targetNodes = &$targetNamespace->stmts; } else { From c2bed30d62f40356fed5972b12e931d923aaa02d Mon Sep 17 00:00:00 2001 From: Artyom Osepyan Date: Thu, 7 May 2026 17:11:48 +0300 Subject: [PATCH 4/5] fix: remarks --- src/Visitors/AbstractNodeVisitor.php | 19 ++++++------------- src/Visitors/AddImports.php | 2 +- src/Visitors/AddTraits.php | 2 +- 3 files changed, 8 insertions(+), 15 deletions(-) diff --git a/src/Visitors/AbstractNodeVisitor.php b/src/Visitors/AbstractNodeVisitor.php index 4a8b9cf..21de695 100644 --- a/src/Visitors/AbstractNodeVisitor.php +++ b/src/Visitors/AbstractNodeVisitor.php @@ -85,9 +85,6 @@ protected function updatableNodeNotFoundHook(): void { } -<<<<<<< HEAD:src/Visitors/BaseNodeVisitorAbstract.php - protected function insertNodes(array &$nodes): void -======= protected function linkParents(Node $parent): void { foreach ($parent->getSubNodeNames() as $name) { @@ -100,9 +97,7 @@ protected function linkParents(Node $parent): void } } - /** @param Class_|Trait_|Enum_ $node */ - private function insertNode(Node $node): Node ->>>>>>> origin/master:src/Visitors/AbstractNodeVisitor.php + protected function insertNodes(array &$nodes): void { $newNodes = match (true) { $this instanceof InsertNodesContract => $this->filterExistingNodes($nodes), @@ -113,15 +108,13 @@ private function insertNode(Node $node): Node if (!empty($newNodes)) { $this->nodeInserter ??= new NodeInserter(); -<<<<<<< HEAD:src/Visitors/BaseNodeVisitorAbstract.php - $this->nodeInserter->insertNodes($nodes, $newNodes, true); + foreach ($newNodes as $newNode) { + $this->linkParents($newNode); + } + + $this->nodeInserter->insertNodes($nodes, $newNodes); } } -======= - $this->linkParents($newNode); - - $this->nodeInserter->insertNodes($node->stmts, [$newNode]); ->>>>>>> origin/master:src/Visitors/AbstractNodeVisitor.php private function filterExistingNodes(array $nodes): array { diff --git a/src/Visitors/AddImports.php b/src/Visitors/AddImports.php index ac07678..4c1d6c7 100644 --- a/src/Visitors/AddImports.php +++ b/src/Visitors/AddImports.php @@ -9,7 +9,7 @@ use PhpParser\Node\UseItem; use RonasIT\Larabuilder\Contracts\InsertNodesContract; -class AddImports extends AbstractInsertNodesVisitor implements InsertNodesContract +class AddImports extends AbstractNodeVisitor implements InsertNodesContract { protected array $allowedParentNodesTypes = self::ANY_TYPE; diff --git a/src/Visitors/AddTraits.php b/src/Visitors/AddTraits.php index 7a9f7e5..3723187 100644 --- a/src/Visitors/AddTraits.php +++ b/src/Visitors/AddTraits.php @@ -10,7 +10,7 @@ use PhpParser\Node\Stmt\TraitUse; use RonasIT\Larabuilder\Contracts\InsertNodesContract; -class AddTraits extends AbstractInsertNodesVisitor implements InsertNodesContract +class AddTraits extends AbstractNodeVisitor implements InsertNodesContract { protected array $allowedParentNodesTypes = [ Class_::class, From d13ce0838918548ba581b1d81c333d54077f34b6 Mon Sep 17 00:00:00 2001 From: Artyom Osepyan Date: Thu, 7 May 2026 17:14:17 +0300 Subject: [PATCH 5/5] fix: remarks --- ARCHITECTURE.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 2fabaa1..06da59a 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -70,8 +70,7 @@ Located in `src/Support/`: ## Creating a New Visitor -1. Extend `AbstractNodeVisitor` (or `AbstractInsertNodesVisitor` for bulk insertions). -1. Extend `BaseNodeVisitorAbstract`. +1. Extend `AbstractNodeVisitor`. 2. Set `$allowedParentNodesTypes` to the node types your visitor targets. 3. Implement the appropriate contract(s): - `InsertNodeContract` — for inserting a single node, implement `getInsertableNode(): Node`