From 1b4195a80ca2f28e168e45fadf3881a6eb08c0ea Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 2 Jul 2026 09:21:18 +0000 Subject: [PATCH] Simplify FileVisitor and dedup property hook param dependencies Readability refactor with one behavioral fix: - Remove handlePropertyHookNode: the traverser already visits hook params as Node\Param, so the dedicated handler registered every set hook param dependency twice. The existing hook param test now asserts the exact dependency list to lock this in. - Merge the four copy-paste expression handlers (new, static call, class const fetch, instanceof) into handleClassReferenceExpression. - Collapse leaveNode's four identical branches into one ClassLike check. - Fold the anonymous class branch into handleClassNode and inline the addParamDependency indirection. - Group enterNode handlers by intent (declarations, type declarations, expressions, attributes/docblocks). No dependency detection behavior changes other than the dedup. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01VUHWv3r1BJyZj4H1WzM3v9 --- src/Analyzer/FileVisitor.php | 274 ++++++------------ .../FileParser/CanParsePropertyHooksTest.php | 2 +- 2 files changed, 85 insertions(+), 191 deletions(-) diff --git a/src/Analyzer/FileVisitor.php b/src/Analyzer/FileVisitor.php index 9af23317..01a9365f 100644 --- a/src/Analyzer/FileVisitor.php +++ b/src/Analyzer/FileVisitor.php @@ -5,7 +5,6 @@ namespace Arkitect\Analyzer; use PhpParser\Node; -use PhpParser\Node\NullableType; use PhpParser\NodeVisitorAbstract; class FileVisitor extends NodeVisitorAbstract @@ -27,61 +26,28 @@ public function setFilePath(?string $filePath): void public function enterNode(Node $node): void { + // class-like declarations: class, anonymous class, enum, interface, trait, use MyTrait; $this->handleClassNode($node); - - // handles anonymous class definition like new class() {} - $this->handleAnonClassNode($node); - - // handles enum definition $this->handleEnumNode($node); - - // handles interface definition like interface MyInterface {} $this->handleInterfaceNode($node); - - // handles trait definition like trait MyTrait {} $this->handleTraitNode($node); - - // handles trait usage like use MyTrait; $this->handleTraitUseNode($node); - // handles code like $constantValue = StaticClass::constant; - $this->handleStaticClassConstantNode($node); - - // handles code like $static = StaticClass::foo(); - $this->handleStaticClassCallsNode($node); - - // handles code lik $a instanceof MyClass - $this->handleInstanceOf($node); - - // handles code like $a = new MyClass(); - $this->handleNewExpression($node); - - // handles code like public MyClass $myClass; + // dependencies from type declarations: public MyClass $a;, myMethod(MyClass $a): MyClass, + // const MyClass FOO = null;, catch (MyException $e) $this->handleTypedProperty($node); - - // handles docblock like /** @var MyClass $myClass */ - $this->handleDocComment($node); - - // handles code like public function myMethod(MyClass $myClass) {} $this->handleParamDependency($node); - - // handles return types like public function myMethod(): MyClass {}, function (): MyClass {}, fn (): MyClass => ... $this->handleReturnTypeDependency($node); + $this->handleClassConstDependency($node); + $this->handleCatchDependency($node); - // handles attribute definition like #[MyAttribute] - $this->handleAttributeNode($node); - - // handles property hooks like public string $name { get => ...; set { ... } } - $this->handlePropertyHookNode($node); + // dependencies from expressions: new MyClass(), MyClass::foo(), MyClass::CONST, $a instanceof MyClass + $this->handleClassReferenceExpression($node); - // handles throws types like @throws MyClass + // attributes like #[MyAttribute], docblocks like /** @var MyClass $a */ and @throws MyClass + $this->handleAttributeNode($node); + $this->handleDocComment($node); $this->handleThrowsTags($node); - - // handles catch types like catch (MyException $e) - $this->handleCatchDependency($node); - - // handles typed class constants like const MyClass FOO = null; - $this->handleClassConstDependency($node); } public function getClassDescriptions(): array @@ -98,25 +64,16 @@ public function clearParsedClassDescriptions(): void public function leaveNode(Node $node): void { - if ($node instanceof Node\Stmt\Class_ && !$node->isAnonymous()) { - $this->classDescriptions[] = $this->classDescriptionBuilder->build(); - $this->classDescriptionBuilder->clear(); - } - - if ($node instanceof Node\Stmt\Enum_) { - $this->classDescriptions[] = $this->classDescriptionBuilder->build(); - $this->classDescriptionBuilder->clear(); + if (!$node instanceof Node\Stmt\ClassLike) { + return; } - if ($node instanceof Node\Stmt\Interface_) { - $this->classDescriptions[] = $this->classDescriptionBuilder->build(); - $this->classDescriptionBuilder->clear(); + if ($node instanceof Node\Stmt\Class_ && $node->isAnonymous()) { + return; } - if ($node instanceof Node\Stmt\Trait_) { - $this->classDescriptions[] = $this->classDescriptionBuilder->build(); - $this->classDescriptionBuilder->clear(); - } + $this->classDescriptions[] = $this->classDescriptionBuilder->build(); + $this->classDescriptionBuilder->clear(); } private function handleClassNode(Node $node): void @@ -126,6 +83,18 @@ private function handleClassNode(Node $node): void } if ($node->isAnonymous()) { + // an anonymous class is not a class description of its own: + // what it extends and implements are dependencies of the class defining it + foreach ($node->implements as $interface) { + $this->classDescriptionBuilder + ->addDependency(new ClassDependency($interface->toString(), $interface->getLine())); + } + + if (null !== $node->extends) { + $this->classDescriptionBuilder + ->addDependency(new ClassDependency($node->extends->toString(), $node->getLine())); + } + return; } @@ -150,27 +119,6 @@ private function handleClassNode(Node $node): void $this->classDescriptionBuilder->setAbstract($node->isAbstract()); } - private function handleAnonClassNode(Node $node): void - { - if (!$node instanceof Node\Stmt\Class_) { - return; - } - - if (!$node->isAnonymous()) { - return; - } - - foreach ($node->implements as $interface) { - $this->classDescriptionBuilder - ->addDependency(new ClassDependency($interface->toString(), $interface->getLine())); - } - - if (null !== $node->extends) { - $this->classDescriptionBuilder - ->addDependency(new ClassDependency($node->extends->toString(), $node->getLine())); - } - } - private function handleEnumNode(Node $node): void { if (!$node instanceof Node\Stmt\Enum_) { @@ -190,60 +138,49 @@ private function handleEnumNode(Node $node): void } } - private function handleStaticClassConstantNode(Node $node): void + private function handleInterfaceNode(Node $node): void { - if (!$node instanceof Node\Expr\ClassConstFetch) { + if (!$node instanceof Node\Stmt\Interface_) { return; } - if (!$node->class instanceof Node\Name\FullyQualified) { + if (null === $node->namespacedName) { return; } - $this->classDescriptionBuilder - ->addDependency(new ClassDependency($node->class->toString(), $node->getLine())); - } - - private function handleStaticClassCallsNode(Node $node): void - { - if (!$node instanceof Node\Expr\StaticCall) { - return; - } + $this->classDescriptionBuilder->setClassName($node->namespacedName->toCodeString()); + $this->classDescriptionBuilder->setInterface(true); - if (!$node->class instanceof Node\Name\FullyQualified) { - return; + foreach ($node->extends as $interface) { + $this->classDescriptionBuilder + ->addExtends($interface->toString(), $interface->getLine()); } - - $this->classDescriptionBuilder - ->addDependency(new ClassDependency($node->class->toString(), $node->getLine())); } - private function handleInstanceOf(Node $node): void + private function handleTraitNode(Node $node): void { - if (!$node instanceof Node\Expr\Instanceof_) { + if (!$node instanceof Node\Stmt\Trait_) { return; } - if (!$node->class instanceof Node\Name\FullyQualified) { + if (null === $node->namespacedName) { return; } - $this->classDescriptionBuilder - ->addDependency(new ClassDependency($node->class->toString(), $node->getLine())); + $this->classDescriptionBuilder->setClassName($node->namespacedName->toCodeString()); + $this->classDescriptionBuilder->setTrait(true); } - private function handleNewExpression(Node $node): void + private function handleTraitUseNode(Node $node): void { - if (!$node instanceof Node\Expr\New_) { + if (!$node instanceof Node\Stmt\TraitUse) { return; } - if (!$node->class instanceof Node\Name\FullyQualified) { - return; + foreach ($node->traits as $trait) { + $this->classDescriptionBuilder + ->addTrait($trait->toString(), $trait->getLine()); } - - $this->classDescriptionBuilder - ->addDependency(new ClassDependency($node->class->toString(), $node->getLine())); } private function handleTypedProperty(Node $node): void @@ -258,79 +195,69 @@ private function handleTypedProperty(Node $node): void } } - private function handleDocComment(Node $node): void + private function handleParamDependency(Node $node): void { - $docComment = $node->getDocComment(); - - if (null === $docComment) { + if (!$node instanceof Node\Param) { return; } - $this->classDescriptionBuilder->addDocBlock($docComment->getText()); - } - - private function handleParamDependency(Node $node): void - { - if ($node instanceof Node\Param) { - $this->addParamDependency($node); + foreach ($this->extractFullyQualifiedTypes($node->type) as $type) { + $this->classDescriptionBuilder + ->addDependency(new ClassDependency($type->toString(), $node->getLine())); } } - private function handleInterfaceNode(Node $node): void + private function handleReturnTypeDependency(Node $node): void { - if (!$node instanceof Node\Stmt\Interface_) { - return; - } - - if (null === $node->namespacedName) { + if (!$node instanceof Node\FunctionLike) { return; } - $this->classDescriptionBuilder->setClassName($node->namespacedName->toCodeString()); - $this->classDescriptionBuilder->setInterface(true); - - foreach ($node->extends as $interface) { + foreach ($this->extractFullyQualifiedTypes($node->getReturnType()) as $returnType) { $this->classDescriptionBuilder - ->addExtends($interface->toString(), $interface->getLine()); + ->addDependency(new ClassDependency($returnType->toString(), $returnType->getLine())); } } - private function handleTraitNode(Node $node): void + private function handleClassConstDependency(Node $node): void { - if (!$node instanceof Node\Stmt\Trait_) { + if (!$node instanceof Node\Stmt\ClassConst) { return; } - if (null === $node->namespacedName) { - return; + foreach ($this->extractFullyQualifiedTypes($node->type) as $type) { + $this->classDescriptionBuilder + ->addDependency(new ClassDependency($type->toString(), $node->getLine())); } - - $this->classDescriptionBuilder->setClassName($node->namespacedName->toCodeString()); - $this->classDescriptionBuilder->setTrait(true); } - private function handleTraitUseNode(Node $node): void + private function handleCatchDependency(Node $node): void { - if (!$node instanceof Node\Stmt\TraitUse) { + if (!$node instanceof Node\Stmt\Catch_) { return; } - foreach ($node->traits as $trait) { + foreach ($node->types as $type) { $this->classDescriptionBuilder - ->addTrait($trait->toString(), $trait->getLine()); + ->addDependency(new ClassDependency($type->toString(), $node->getLine())); } } - private function handleReturnTypeDependency(Node $node): void + private function handleClassReferenceExpression(Node $node): void { - if (!$node instanceof Node\FunctionLike) { + if (!$node instanceof Node\Expr\New_ + && !$node instanceof Node\Expr\StaticCall + && !$node instanceof Node\Expr\ClassConstFetch + && !$node instanceof Node\Expr\Instanceof_) { return; } - foreach ($this->extractFullyQualifiedTypes($node->getReturnType()) as $returnType) { - $this->classDescriptionBuilder - ->addDependency(new ClassDependency($returnType->toString(), $returnType->getLine())); + if (!$node->class instanceof Node\Name\FullyQualified) { + return; } + + $this->classDescriptionBuilder + ->addDependency(new ClassDependency($node->class->toString(), $node->getLine())); } private function handleAttributeNode(Node $node): void @@ -349,48 +276,27 @@ private function handleAttributeNode(Node $node): void ->addAttribute($node->name->toString(), $node->getLine()); } - private function handleThrowsTags(Node $node): void + private function handleDocComment(Node $node): void { - if (!$node->hasAttribute(DocblockTypesResolver::THROWS_TYPES_ATTRIBUTE)) { - return; - } - - /** @var Node\Name\FullyQualified $throw */ - foreach ($node->getAttribute(DocblockTypesResolver::THROWS_TYPES_ATTRIBUTE) as $throw) { - $this->classDescriptionBuilder - ->addDependency(new ClassDependency($throw->toString(), $throw->getLine())); - } - } + $docComment = $node->getDocComment(); - private function handleClassConstDependency(Node $node): void - { - if (!$node instanceof Node\Stmt\ClassConst) { + if (null === $docComment) { return; } - foreach ($this->extractFullyQualifiedTypes($node->type) as $type) { - $this->classDescriptionBuilder - ->addDependency(new ClassDependency($type->toString(), $node->getLine())); - } + $this->classDescriptionBuilder->addDocBlock($docComment->getText()); } - private function handleCatchDependency(Node $node): void + private function handleThrowsTags(Node $node): void { - if (!$node instanceof Node\Stmt\Catch_) { + if (!$node->hasAttribute(DocblockTypesResolver::THROWS_TYPES_ATTRIBUTE)) { return; } - foreach ($node->types as $type) { - $this->classDescriptionBuilder - ->addDependency(new ClassDependency($type->toString(), $node->getLine())); - } - } - - private function addParamDependency(Node\Param $node): void - { - foreach ($this->extractFullyQualifiedTypes($node->type) as $type) { + /** @var Node\Name\FullyQualified $throw */ + foreach ($node->getAttribute(DocblockTypesResolver::THROWS_TYPES_ATTRIBUTE) as $throw) { $this->classDescriptionBuilder - ->addDependency(new ClassDependency($type->toString(), $node->getLine())); + ->addDependency(new ClassDependency($throw->toString(), $throw->getLine())); } } @@ -402,7 +308,7 @@ private function addParamDependency(Node\Param $node): void */ private function extractFullyQualifiedTypes(?Node $type): array { - if ($type instanceof NullableType) { + if ($type instanceof Node\NullableType) { return $this->extractFullyQualifiedTypes($type->type); } @@ -421,16 +327,4 @@ private function extractFullyQualifiedTypes(?Node $type): array return []; } - - private function handlePropertyHookNode(Node $node): void - { - if (!$node instanceof Node\PropertyHook) { - return; - } - - // Handle parameters in set hooks (e.g., set(MyClass $value) { ... }) - foreach ($node->params as $param) { - $this->addParamDependency($param); - } - } } diff --git a/tests/Unit/Analyzer/FileParser/CanParsePropertyHooksTest.php b/tests/Unit/Analyzer/FileParser/CanParsePropertyHooksTest.php index 4068c181..b87a7060 100644 --- a/tests/Unit/Analyzer/FileParser/CanParsePropertyHooksTest.php +++ b/tests/Unit/Analyzer/FileParser/CanParsePropertyHooksTest.php @@ -109,6 +109,6 @@ class User { $dependencies = $cd[0]->getDependencies(); $dependencyNames = array_map(static fn ($dep) => $dep->getFQCN()->toString(), $dependencies); - self::assertContains('App\ValueObjects\Name', $dependencyNames); + self::assertEquals(['App\ValueObjects\Name'], $dependencyNames); } }