Skip to content

Commit 97c5364

Browse files
claudeclaude
committed
Include reflected interfaces/extends in dependencies, report errors
- addReflectedInterface and addReflectedExtends now add dependencies (with deduplication), matching the behavior of addInterface/addExtends - Reflection failures are reported as ParsingError through the standard error channel instead of being silently swallowed - FileVisitor collects ParsingErrors, FileParser forwards them - MVC fixture classes added to composer.json classmap for autoloading, so E2E tests resolve reflection correctly https://claude.ai/code/session_01Ko24Jtok9YQagSFair7zQh
1 parent b80c4ba commit 97c5364

File tree

8 files changed

+82
-41
lines changed

8 files changed

+82
-41
lines changed

‎composer.json‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,10 @@
4949
"autoload-dev": {
5050
"psr-4": {
5151
"Arkitect\\Tests\\": "tests/"
52-
}
52+
},
53+
"classmap": [
54+
"tests/E2E/_fixtures/mvc/"
55+
]
5356
},
5457
"config": {
5558
"bin-dir": "bin",

‎src/Analyzer/ClassDescriptionBuilder.php‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -82,10 +82,9 @@ public function addInterface(string $FQCN, int $line): self
8282

8383
/**
8484
* Add an interface discovered via reflection (inherited from parent class or parent interface).
85-
* Unlike addInterface(), this does not add a dependency since the interface is not directly
86-
* referenced in the source file.
85+
* Adds both the interface and a dependency, with deduplication on the interfaces list.
8786
*/
88-
public function addReflectedInterface(string $FQCN): self
87+
public function addReflectedInterface(string $FQCN, int $line): self
8988
{
9089
$fqcn = FullyQualifiedClassName::fromString($FQCN);
9190

@@ -95,6 +94,7 @@ public function addReflectedInterface(string $FQCN): self
9594
}
9695
}
9796

97+
$this->addDependency(new ClassDependency($FQCN, $line));
9898
$this->interfaces[] = $fqcn;
9999

100100
return $this;
@@ -121,10 +121,9 @@ public function addExtends(string $FQCN, int $line): self
121121

122122
/**
123123
* Add a parent class discovered via reflection (ancestor beyond the direct parent).
124-
* Unlike addExtends(), this does not add a dependency since the ancestor is not directly
125-
* referenced in the source file.
124+
* Adds both the extends entry and a dependency, with deduplication on the extends list.
126125
*/
127-
public function addReflectedExtends(string $FQCN): self
126+
public function addReflectedExtends(string $FQCN, int $line): self
128127
{
129128
$fqcn = FullyQualifiedClassName::fromString($FQCN);
130129

@@ -134,6 +133,7 @@ public function addReflectedExtends(string $FQCN): self
134133
}
135134
}
136135

136+
$this->addDependency(new ClassDependency($FQCN, $line));
137137
$this->extends[] = $fqcn;
138138

139139
return $this;

‎src/Analyzer/FileParser.php‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,10 @@ public function parse(string $fileContent, string $filename): void
7474
echo 'Parse Error: ', $e->getMessage();
7575
print_r($e->getTraceAsString());
7676
}
77+
78+
foreach ($this->fileVisitor->getParsingErrors() as $parsingError) {
79+
$this->parsingErrors[] = $parsingError;
80+
}
7781
}
7882

7983
public function getParsingErrors(): array

‎src/Analyzer/FileVisitor.php‎

Lines changed: 38 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
namespace Arkitect\Analyzer;
66

7+
use Arkitect\Rules\ParsingError;
78
use PhpParser\Node;
89
use PhpParser\Node\NullableType;
910
use PhpParser\NodeVisitorAbstract;
@@ -15,13 +16,19 @@ class FileVisitor extends NodeVisitorAbstract
1516
/** @var array<ClassDescription> */
1617
private array $classDescriptions = [];
1718

19+
/** @var array<ParsingError> */
20+
private array $parsingErrors = [];
21+
22+
private ?string $filePath = null;
23+
1824
public function __construct(ClassDescriptionBuilder $classDescriptionBuilder)
1925
{
2026
$this->classDescriptionBuilder = $classDescriptionBuilder;
2127
}
2228

2329
public function setFilePath(?string $filePath): void
2430
{
31+
$this->filePath = $filePath;
2532
$this->classDescriptionBuilder->setFilePath($filePath);
2633
}
2734

@@ -83,9 +90,18 @@ public function getClassDescriptions(): array
8390
return $this->classDescriptions;
8491
}
8592

93+
/**
94+
* @return array<ParsingError>
95+
*/
96+
public function getParsingErrors(): array
97+
{
98+
return $this->parsingErrors;
99+
}
100+
86101
public function clearParsedClassDescriptions(): void
87102
{
88103
$this->classDescriptions = [];
104+
$this->parsingErrors = [];
89105
$this->classDescriptionBuilder->setFilePath(null);
90106
$this->classDescriptionBuilder->clear();
91107
}
@@ -187,7 +203,7 @@ private function handleEnumNode(Node $node): void
187203

188204
// Resolve inherited interfaces from directly implemented interfaces
189205
foreach ($node->implements as $interface) {
190-
$this->addReflectedInterfaceParents($interface->toString());
206+
$this->addReflectedInterfaceParents($interface->toString(), $interface->getLine());
191207
}
192208
}
193209

@@ -305,7 +321,7 @@ private function handleInterfaceNode(Node $node): void
305321

306322
// Resolve ancestor interfaces from directly extended interfaces
307323
foreach ($node->extends as $interface) {
308-
$this->addReflectedExtendedInterfaceParents($interface->toString());
324+
$this->addReflectedExtendedInterfaceParents($interface->toString(), $interface->getLine());
309325
}
310326
}
311327

@@ -418,7 +434,7 @@ private function resolveInheritedInterfacesAndExtends(Node\Stmt\Class_ $node): v
418434
{
419435
// Resolve inherited interfaces from directly implemented interfaces
420436
foreach ($node->implements as $interface) {
421-
$this->addReflectedInterfaceParents($interface->toString());
437+
$this->addReflectedInterfaceParents($interface->toString(), $interface->getLine());
422438
}
423439

424440
// Resolve inherited interfaces and ancestor classes from parent class
@@ -427,58 +443,68 @@ private function resolveInheritedInterfacesAndExtends(Node\Stmt\Class_ $node): v
427443
}
428444

429445
$parentClassName = $node->extends->toString();
446+
$line = $node->extends->getLine();
430447

431448
try {
432449
/** @var class-string $parentClassName */
433450
$reflection = new \ReflectionClass($parentClassName);
434451

435452
foreach ($reflection->getInterfaceNames() as $interfaceName) {
436-
$this->classDescriptionBuilder->addReflectedInterface($interfaceName);
453+
$this->classDescriptionBuilder->addReflectedInterface($interfaceName, $line);
437454
}
438455

439456
$ancestor = $reflection->getParentClass();
440457
while (false !== $ancestor) {
441-
$this->classDescriptionBuilder->addReflectedExtends($ancestor->getName());
458+
$this->classDescriptionBuilder->addReflectedExtends($ancestor->getName(), $line);
442459
$ancestor = $ancestor->getParentClass();
443460
}
444461
} catch (\ReflectionException $e) {
445-
// Parent class not autoloadable, skip reflection-based resolution
462+
$this->parsingErrors[] = ParsingError::create(
463+
$this->filePath ?? '',
464+
"Reflection error: {$e->getMessage()}. Ensure the class is autoloaded."
465+
);
446466
}
447467
}
448468

449469
/**
450470
* Use reflection to discover parent interfaces of a given interface,
451471
* adding them to the interfaces list (for classes and enums).
452472
*/
453-
private function addReflectedInterfaceParents(string $interfaceName): void
473+
private function addReflectedInterfaceParents(string $interfaceName, int $line): void
454474
{
455475
try {
456476
/** @var class-string $interfaceName */
457477
$reflection = new \ReflectionClass($interfaceName);
458478

459479
foreach ($reflection->getInterfaceNames() as $parentInterfaceName) {
460-
$this->classDescriptionBuilder->addReflectedInterface($parentInterfaceName);
480+
$this->classDescriptionBuilder->addReflectedInterface($parentInterfaceName, $line);
461481
}
462482
} catch (\ReflectionException $e) {
463-
// Interface not autoloadable, skip
483+
$this->parsingErrors[] = ParsingError::create(
484+
$this->filePath ?? '',
485+
"Reflection error: {$e->getMessage()}. Ensure the class is autoloaded."
486+
);
464487
}
465488
}
466489

467490
/**
468491
* Use reflection to discover parent interfaces of a given interface,
469492
* adding them to the extends list (for interface definitions).
470493
*/
471-
private function addReflectedExtendedInterfaceParents(string $interfaceName): void
494+
private function addReflectedExtendedInterfaceParents(string $interfaceName, int $line): void
472495
{
473496
try {
474497
/** @var class-string $interfaceName */
475498
$reflection = new \ReflectionClass($interfaceName);
476499

477500
foreach ($reflection->getInterfaceNames() as $parentInterfaceName) {
478-
$this->classDescriptionBuilder->addReflectedExtends($parentInterfaceName);
501+
$this->classDescriptionBuilder->addReflectedExtends($parentInterfaceName, $line);
479502
}
480503
} catch (\ReflectionException $e) {
481-
// Interface not autoloadable, skip
504+
$this->parsingErrors[] = ParsingError::create(
505+
$this->filePath ?? '',
506+
"Reflection error: {$e->getMessage()}. Ensure the class is autoloaded."
507+
);
482508
}
483509
}
484510
}

‎tests/Integration/ImplementsTest.php‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,8 @@ public function test_naming_is_enforced(): void
2626

2727
$runner->run($dir, $rule);
2828

29-
self::assertCount(0, $runner->getParsingErrors());
29+
// App\AnInterface is not autoloadable (vfsStream), so reflection errors are expected
30+
self::assertGreaterThan(0, $runner->getParsingErrors()->count());
3031
self::assertCount(2, $runner->getViolations());
3132

3233
self::assertEquals('App\AClass', $runner->getViolations()->get(0)->getFqcn());

‎tests/Unit/Analyzer/ClassDescriptionBuilderTest.php‎

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -281,17 +281,18 @@ public function test_it_should_filter_internal_classes_with_namespaces(): void
281281
self::assertEquals('App\MyClass', $classDescription->getDependencies()[0]->getFQCN()->toString());
282282
}
283283

284-
public function test_add_reflected_interface_should_add_to_interfaces_without_dependency(): void
284+
public function test_add_reflected_interface_should_add_to_interfaces_and_dependencies(): void
285285
{
286286
$classDescription = (new ClassDescriptionBuilder())
287287
->setFilePath('src/Foo.php')
288288
->setClassName('MyClass')
289-
->addReflectedInterface('Vendor\SomeInterface')
289+
->addReflectedInterface('Vendor\SomeInterface', 10)
290290
->build();
291291

292292
self::assertCount(1, $classDescription->getInterfaces());
293293
self::assertEquals('Vendor\SomeInterface', $classDescription->getInterfaces()[0]->toString());
294-
self::assertCount(0, $classDescription->getDependencies());
294+
self::assertCount(1, $classDescription->getDependencies());
295+
self::assertEquals('Vendor\SomeInterface', $classDescription->getDependencies()[0]->getFQCN()->toString());
295296
}
296297

297298
public function test_add_reflected_interface_should_deduplicate(): void
@@ -300,7 +301,7 @@ public function test_add_reflected_interface_should_deduplicate(): void
300301
->setFilePath('src/Foo.php')
301302
->setClassName('MyClass')
302303
->addInterface('Vendor\SomeInterface', 10)
303-
->addReflectedInterface('Vendor\SomeInterface')
304+
->addReflectedInterface('Vendor\SomeInterface', 10)
304305
->build();
305306

306307
self::assertCount(1, $classDescription->getInterfaces());
@@ -313,25 +314,26 @@ public function test_add_reflected_interface_should_add_new_interfaces(): void
313314
->setFilePath('src/Foo.php')
314315
->setClassName('MyClass')
315316
->addInterface('Vendor\ChildInterface', 10)
316-
->addReflectedInterface('Vendor\ParentInterface')
317+
->addReflectedInterface('Vendor\ParentInterface', 10)
317318
->build();
318319

319320
self::assertCount(2, $classDescription->getInterfaces());
320321
self::assertEquals('Vendor\ChildInterface', $classDescription->getInterfaces()[0]->toString());
321322
self::assertEquals('Vendor\ParentInterface', $classDescription->getInterfaces()[1]->toString());
322323
}
323324

324-
public function test_add_reflected_extends_should_add_to_extends_without_dependency(): void
325+
public function test_add_reflected_extends_should_add_to_extends_and_dependencies(): void
325326
{
326327
$classDescription = (new ClassDescriptionBuilder())
327328
->setFilePath('src/Foo.php')
328329
->setClassName('MyClass')
329-
->addReflectedExtends('Vendor\GrandParentClass')
330+
->addReflectedExtends('Vendor\GrandParentClass', 10)
330331
->build();
331332

332333
self::assertCount(1, $classDescription->getExtends());
333334
self::assertEquals('Vendor\GrandParentClass', $classDescription->getExtends()[0]->toString());
334-
self::assertCount(0, $classDescription->getDependencies());
335+
self::assertCount(1, $classDescription->getDependencies());
336+
self::assertEquals('Vendor\GrandParentClass', $classDescription->getDependencies()[0]->getFQCN()->toString());
335337
}
336338

337339
public function test_add_reflected_extends_should_deduplicate(): void
@@ -340,7 +342,7 @@ public function test_add_reflected_extends_should_deduplicate(): void
340342
->setFilePath('src/Foo.php')
341343
->setClassName('MyClass')
342344
->addExtends('Vendor\ParentClass', 10)
343-
->addReflectedExtends('Vendor\ParentClass')
345+
->addReflectedExtends('Vendor\ParentClass', 10)
344346
->build();
345347

346348
self::assertCount(1, $classDescription->getExtends());
@@ -353,7 +355,7 @@ public function test_add_reflected_extends_should_add_new_ancestors(): void
353355
->setFilePath('src/Foo.php')
354356
->setClassName('MyClass')
355357
->addExtends('Vendor\ParentClass', 10)
356-
->addReflectedExtends('Vendor\GrandParentClass')
358+
->addReflectedExtends('Vendor\GrandParentClass', 10)
357359
->build();
358360

359361
self::assertCount(2, $classDescription->getExtends());

‎tests/Unit/Analyzer/FileParser/CanResolveInheritedInterfacesTest.php‎

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ public function getIterator(): \Traversable
6868
self::assertContains('Traversable', $interfaces);
6969
}
7070

71-
public function test_class_with_non_autoloadable_parent_should_still_parse(): void
71+
public function test_class_with_non_autoloadable_parent_should_report_parsing_error(): void
7272
{
7373
$code = <<< 'EOF'
7474
<?php
@@ -79,18 +79,21 @@ class MyClass extends NonExistent\ParentClass
7979
}
8080
EOF;
8181

82-
$cd = $this->parseCode($code);
82+
$fp = FileParserFactory::forPhpVersion(TargetPhpVersion::PHP_8_2);
83+
$fp->parse($code, 'relativePathName');
8384

84-
self::assertCount(1, $cd);
85+
$parsingErrors = $fp->getParsingErrors();
8586

86-
// Should still have the direct extends, just no inherited interfaces
87-
$extends = array_map(
88-
static fn (FullyQualifiedClassName $fqcn): string => $fqcn->toString(),
89-
$cd[0]->getExtends()
87+
self::assertGreaterThan(0, \count($parsingErrors));
88+
89+
$errorMessages = array_map(
90+
static fn ($error): string => $error->getError(),
91+
$parsingErrors
9092
);
93+
$allErrors = implode(' ', $errorMessages);
9194

92-
self::assertContains('App\NonExistent\ParentClass', $extends);
93-
self::assertCount(0, $cd[0]->getInterfaces());
95+
self::assertStringContainsString('App\NonExistent\ParentClass', $allErrors);
96+
self::assertStringContainsString('autoloaded', $allErrors);
9497
}
9598

9699
public function test_inherited_interfaces_should_not_duplicate_direct_interfaces(): void
@@ -118,7 +121,7 @@ class MyClass extends \ArrayObject implements \Countable
118121
self::assertEquals(1, $countableOccurrences);
119122
}
120123

121-
public function test_inherited_interfaces_should_not_add_dependencies(): void
124+
public function test_inherited_php_core_interfaces_should_be_filtered_from_dependencies(): void
122125
{
123126
$code = <<< 'EOF'
124127
<?php
@@ -133,7 +136,7 @@ class MyClass extends \ArrayObject
133136

134137
self::assertCount(1, $cd);
135138

136-
// Dependencies should only include direct references (ArrayObject), not inherited interfaces
139+
// PHP core interfaces are filtered out by isPhpCoreClass, same as any other core dependency
137140
$dependencyNames = array_map(
138141
static fn ($dep): string => $dep->getFQCN()->toString(),
139142
$cd[0]->getDependencies()

‎tests/Unit/Analyzer/FileParserTest.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ public function test_parse_file(): void
3131

3232
$fileVisitor->setFilePath('foo')->shouldBeCalled();
3333
$fileVisitor->clearParsedClassDescriptions()->shouldBeCalled();
34+
$fileVisitor->getParsingErrors()->willReturn([]);
3435

3536
$fileParser = new FileParser(
3637
$traverser->reveal(),
@@ -61,6 +62,7 @@ public function test_parse_file_with_name_match(): void
6162

6263
$fileVisitor->setFilePath('foo')->shouldBeCalled();
6364
$fileVisitor->clearParsedClassDescriptions()->shouldBeCalled();
65+
$fileVisitor->getParsingErrors()->willReturn([]);
6466

6567
$fileParser = new FileParser(
6668
$traverser->reveal(),

0 commit comments

Comments
 (0)