From f7837aedd9c553e2411f5dd304fc3964121a4a2d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Florian=20Kr=C3=A4mer?= Date: Wed, 24 Sep 2025 16:46:00 +0200 Subject: [PATCH 1/3] Improved Error Message --- src/Business/Cognitive/CognitiveMetrics.php | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/src/Business/Cognitive/CognitiveMetrics.php b/src/Business/Cognitive/CognitiveMetrics.php index 615e83c..c75cfaf 100644 --- a/src/Business/Cognitive/CognitiveMetrics.php +++ b/src/Business/Cognitive/CognitiveMetrics.php @@ -96,7 +96,20 @@ private function setRequiredMetricProperties(array $metrics): void { $missingKeys = array_diff_key($this->metrics, $metrics); if (!empty($missingKeys)) { - throw new InvalidArgumentException('Missing required keys: ' . implode(', ', $missingKeys)); + $class = $metrics['class'] ?? 'Unknown'; + $method = $metrics['method'] ?? 'Unknown'; + $file = $metrics['file'] ?? 'Unknown'; + + $errorMessage = sprintf( + 'Missing required keys for %s::%s in file %s: %s. Available keys: %s', + $class, + $method, + $file, + implode(', ', $missingKeys), + implode(', ', array_keys($metrics)) + ); + + throw new InvalidArgumentException($errorMessage); } // Not pretty to set each but more efficient than using a loop and $this->metrics From a468e86e21618582ed3352ea5d30cfe8e7dcebed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Florian=20Kr=C3=A4mer?= Date: Wed, 24 Sep 2025 17:20:21 +0200 Subject: [PATCH 2/3] Adding tests for anonymous classes --- .../CognitiveMetricsCollectorTest.php | 181 ++++++++++++++++++ tests/Unit/Business/Cognitive/ParserTest.php | 172 +++++++++++++++++ .../PhpParser/CognitiveMetricsVisitorTest.php | 141 ++++++++++++++ 3 files changed, 494 insertions(+) diff --git a/tests/Unit/Business/Cognitive/CognitiveMetricsCollectorTest.php b/tests/Unit/Business/Cognitive/CognitiveMetricsCollectorTest.php index 3919b5a..3de9bee 100644 --- a/tests/Unit/Business/Cognitive/CognitiveMetricsCollectorTest.php +++ b/tests/Unit/Business/Cognitive/CognitiveMetricsCollectorTest.php @@ -146,4 +146,185 @@ public function testCollectedMetrics(): void $this->assertSame(3, $metrics->getVariableCount()); $this->assertSame(2, $metrics->getPropertyCallCount()); } + + /** + * Test that the collector handles files with anonymous classes without errors. + * This test ensures that the full pipeline (Parser -> Collector -> CognitiveMetrics) + * works correctly with anonymous classes and doesn't cause "Missing required keys" errors. + */ + #[Test] + public function testCollectWithAnonymousClasses(): void + { + // Create a temporary test file with anonymous classes + $testFile = sys_get_temp_dir() . '/test_anonymous_classes.php'; + $testCode = <<<'CODE' + value = $value; + } + + public function process() { + if ($this->value > 0) { + return $this->value; + } + return 0; + } + }; + + $anotherSubscriber = new class(456) extends AnotherBaseClass { + public function __construct(private int $value) { + $this->value = $value; + } + + public function handle() { + return $this->value * 2; + } + }; + + return $subscriber->process(); + } + + public function normalMethod() { + $var = 1; + return $var; + } + } + CODE; + + file_put_contents($testFile, $testCode); + + try { + // This should not throw any exceptions + $metricsCollection = $this->metricsCollector->collect($testFile, $this->configService->getConfig()); + + $this->assertInstanceOf(CognitiveMetricsCollection::class, $metricsCollection); + $this->assertCount(2, $metricsCollection); + + // Verify that we can get the metrics for both methods + $methodWithAnonymous = $metricsCollection->getClassWithMethod('\\TestNamespace\\TestClass', 'methodWithAnonymousClass'); + $normalMethod = $metricsCollection->getClassWithMethod('\\TestNamespace\\TestClass', 'normalMethod'); + + $this->assertNotNull($methodWithAnonymous); + $this->assertNotNull($normalMethod); + + // Verify that the metrics have all required properties + $this->assertGreaterThan(0, $methodWithAnonymous->getLineCount()); + $this->assertEquals(0, $methodWithAnonymous->getArgCount()); + $this->assertGreaterThan(0, $methodWithAnonymous->getReturnCount()); + + $this->assertGreaterThan(0, $normalMethod->getLineCount()); + $this->assertEquals(0, $normalMethod->getArgCount()); + $this->assertEquals(1, $normalMethod->getReturnCount()); + $this->assertEquals(1, $normalMethod->getVariableCount()); + } finally { + // Clean up the temporary file + if (file_exists($testFile)) { + unlink($testFile); + } + } + } + + /** + * Test that the collector handles complex anonymous class scenarios. + * This test ensures that the fix works in various anonymous class scenarios. + */ + #[Test] + public function testCollectWithComplexAnonymousClassScenarios(): void + { + // Create a temporary test file with complex anonymous class scenarios + $testFile = sys_get_temp_dir() . '/test_complex_anonymous_classes.php'; + $testCode = <<<'CODE' + value = $value; + } + + public function getValue() { + return $this->value; + } + }; + + // Anonymous class extending another class + $obj2 = new class extends BaseClass { + public function __construct() { + parent::__construct(); + } + + public function process() { + if (true) { + return 'processed'; + } + return 'not processed'; + } + }; + + // Anonymous class implementing interface + $obj3 = new class implements SomeInterface { + public function __construct() { + $this->initialize(); + } + + public function initialize() { + $var = 1; + return $var; + } + + public function execute() { + return 'executed'; + } + }; + + return $obj1->getValue(); + } + + public function simpleMethod() { + return 'simple'; + } + } + CODE; + + file_put_contents($testFile, $testCode); + + try { + // This should not throw any exceptions + $metricsCollection = $this->metricsCollector->collect($testFile, $this->configService->getConfig()); + + $this->assertInstanceOf(CognitiveMetricsCollection::class, $metricsCollection); + $this->assertCount(2, $metricsCollection); + + // Verify that we can get the metrics for both methods + $createAnonymousObjects = $metricsCollection->getClassWithMethod('\\TestNamespace\\MainClass', 'createAnonymousObjects'); + $simpleMethod = $metricsCollection->getClassWithMethod('\\TestNamespace\\MainClass', 'simpleMethod'); + + $this->assertNotNull($createAnonymousObjects); + $this->assertNotNull($simpleMethod); + + // Verify that the metrics have all required properties + $this->assertGreaterThan(0, $createAnonymousObjects->getLineCount()); + $this->assertEquals(0, $createAnonymousObjects->getArgCount()); + $this->assertGreaterThan(0, $createAnonymousObjects->getReturnCount()); + + $this->assertGreaterThan(0, $simpleMethod->getLineCount()); + $this->assertEquals(0, $simpleMethod->getArgCount()); + $this->assertEquals(1, $simpleMethod->getReturnCount()); + } finally { + // Clean up the temporary file + if (file_exists($testFile)) { + unlink($testFile); + } + } + } } diff --git a/tests/Unit/Business/Cognitive/ParserTest.php b/tests/Unit/Business/Cognitive/ParserTest.php index 7319246..f1225e3 100644 --- a/tests/Unit/Business/Cognitive/ParserTest.php +++ b/tests/Unit/Business/Cognitive/ParserTest.php @@ -267,4 +267,176 @@ class NormalClass { $this->assertContains('\\MyNamespace\\IgnoredClass2', $ignoredClasses); $this->assertNotContains('\\MyNamespace\\NormalClass', $ignoredClasses); } + + /** + * Test that anonymous classes are properly handled and don't cause missing keys errors. + * This test reproduces the issue that was fixed where anonymous classes would cause + * "Missing required keys" errors in CognitiveMetrics constructor. + */ + public function testHandlesAnonymousClassesCorrectly(): void + { + $code = <<<'CODE' + value = $value; + } + + public function process() { + if ($this->value > 0) { + return $this->value; + } + return 0; + } + }; + + $anotherSubscriber = new class(456) extends AnotherBaseClass { + public function __construct(private int $value) { + $this->value = $value; + } + + public function handle() { + return $this->value * 2; + } + }; + + return $subscriber->process(); + } + + public function normalMethod() { + $var = 1; + return $var; + } + } + CODE; + + // This should not throw any exceptions + $metrics = $this->parser->parse($code); + + // Only the methods from the named class should have metrics + // Anonymous class methods should be ignored + $this->assertCount(2, $metrics); + $this->assertArrayHasKey('\\MyNamespace\\TestClass::testMethod', $metrics); + $this->assertArrayHasKey('\\MyNamespace\\TestClass::normalMethod', $metrics); + + // Verify that anonymous class methods are NOT present + $this->assertArrayNotHasKey('\\MyNamespace\\TestClass::__construct', $metrics); + $this->assertArrayNotHasKey('\\MyNamespace\\TestClass::process', $metrics); + $this->assertArrayNotHasKey('\\MyNamespace\\TestClass::handle', $metrics); + + // Verify that the metrics have all required keys + foreach ($metrics as $methodKey => $methodMetrics) { + $this->assertArrayHasKey('lineCount', $methodMetrics, "Method {$methodKey} missing lineCount"); + $this->assertArrayHasKey('argCount', $methodMetrics, "Method {$methodKey} missing argCount"); + $this->assertArrayHasKey('returnCount', $methodMetrics, "Method {$methodKey} missing returnCount"); + $this->assertArrayHasKey('variableCount', $methodMetrics, "Method {$methodKey} missing variableCount"); + $this->assertArrayHasKey('propertyCallCount', $methodMetrics, "Method {$methodKey} missing propertyCallCount"); + $this->assertArrayHasKey('ifCount', $methodMetrics, "Method {$methodKey} missing ifCount"); + $this->assertArrayHasKey('ifNestingLevel', $methodMetrics, "Method {$methodKey} missing ifNestingLevel"); + $this->assertArrayHasKey('elseCount', $methodMetrics, "Method {$methodKey} missing elseCount"); + } + + // Verify specific metrics for testMethod + $testMethodMetrics = $metrics['\\MyNamespace\\TestClass::testMethod']; + $this->assertGreaterThan(0, $testMethodMetrics['lineCount']); + $this->assertEquals(0, $testMethodMetrics['argCount']); + $this->assertGreaterThan(0, $testMethodMetrics['returnCount']); + + // Verify specific metrics for normalMethod + $normalMethodMetrics = $metrics['\\MyNamespace\\TestClass::normalMethod']; + $this->assertGreaterThan(0, $normalMethodMetrics['lineCount']); + $this->assertEquals(0, $normalMethodMetrics['argCount']); + $this->assertEquals(1, $normalMethodMetrics['returnCount']); + $this->assertEquals(1, $normalMethodMetrics['variableCount']); + } + + /** + * Test that the parser handles complex anonymous class scenarios without errors. + * This test ensures that the fix for anonymous classes works in various scenarios. + */ + public function testHandlesComplexAnonymousClassScenarios(): void + { + $code = <<<'CODE' + value = $value; + } + + public function getValue() { + return $this->value; + } + }; + + // Anonymous class extending another class + $obj2 = new class extends BaseClass { + public function __construct() { + parent::__construct(); + } + + public function process() { + if (true) { + return 'processed'; + } + return 'not processed'; + } + }; + + // Anonymous class implementing interface + $obj3 = new class implements SomeInterface { + public function __construct() { + $this->initialize(); + } + + public function initialize() { + $var = 1; + return $var; + } + + public function execute() { + return 'executed'; + } + }; + + return $obj1->getValue(); + } + + public function simpleMethod() { + return 'simple'; + } + } + CODE; + + // This should not throw any exceptions + $metrics = $this->parser->parse($code); + + // Only methods from the named class should be present + $this->assertCount(2, $metrics); + $this->assertArrayHasKey('\\TestNamespace\\MainClass::createAnonymousObjects', $metrics); + $this->assertArrayHasKey('\\TestNamespace\\MainClass::simpleMethod', $metrics); + + // Verify all metrics have required keys + foreach ($metrics as $methodKey => $methodMetrics) { + $requiredKeys = [ + 'lineCount', 'argCount', 'returnCount', 'variableCount', + 'propertyCallCount', 'ifCount', 'ifNestingLevel', 'elseCount' + ]; + + foreach ($requiredKeys as $key) { + $this->assertArrayHasKey($key, $methodMetrics, "Method {$methodKey} missing required key: {$key}"); + $this->assertIsInt($methodMetrics[$key], "Method {$methodKey} key {$key} should be an integer"); + } + } + } } diff --git a/tests/Unit/PhpParser/CognitiveMetricsVisitorTest.php b/tests/Unit/PhpParser/CognitiveMetricsVisitorTest.php index c3df012..9d79139 100644 --- a/tests/Unit/PhpParser/CognitiveMetricsVisitorTest.php +++ b/tests/Unit/PhpParser/CognitiveMetricsVisitorTest.php @@ -96,4 +96,145 @@ public function foo($arg1) { $this->assertArrayHasKey($method, $metrics); $this->assertEquals(3, $metrics[$method]['variableCount']); } + + /** + * Test that anonymous classes are properly skipped and don't cause issues. + * This test ensures that the CognitiveMetricsVisitor correctly handles + * anonymous classes without creating incomplete metrics. + */ + public function testSkipsAnonymousClasses(): void + { + $code = <<<'CODE' + value = $value; + } + + public function getValue() { + return $this->value; + } + }; + + return $anonymous->getValue(); + } + + public function normalMethod() { + $var = 1; + return $var; + } + } + CODE; + + $parser = (new ParserFactory())->createForHostVersion(); + $statements = $parser->parse($code); + + $traverser = new NodeTraverser(); + $metricsVisitor = new CognitiveMetricsVisitor(); + $traverser->addVisitor($metricsVisitor); + + $traverser->traverse($statements); + + $methodMetrics = $metricsVisitor->getMethodMetrics(); + + // Only methods from the named class should be present + $this->assertCount(2, $methodMetrics); + $this->assertArrayHasKey('\\TestNamespace\\TestClass::methodWithAnonymousClass', $methodMetrics); + $this->assertArrayHasKey('\\TestNamespace\\TestClass::normalMethod', $methodMetrics); + + // Verify that all metrics have the required keys + foreach ($methodMetrics as $methodKey => $metrics) { + $requiredKeys = [ + 'lineCount', 'argCount', 'returnCount', 'variableCount', + 'propertyCallCount', 'ifCount', 'ifNestingLevel', 'elseCount' + ]; + + foreach ($requiredKeys as $key) { + $this->assertArrayHasKey($key, $metrics, "Method {$methodKey} missing required key: {$key}"); + $this->assertIsInt($metrics[$key], "Method {$methodKey} key {$key} should be an integer"); + } + } + + // Verify specific metrics + $methodWithAnonymousMetrics = $methodMetrics['\\TestNamespace\\TestClass::methodWithAnonymousClass']; + $this->assertGreaterThan(0, $methodWithAnonymousMetrics['lineCount']); + $this->assertEquals(0, $methodWithAnonymousMetrics['argCount']); + $this->assertGreaterThan(0, $methodWithAnonymousMetrics['returnCount']); + + $normalMethodMetrics = $methodMetrics['\\TestNamespace\\TestClass::normalMethod']; + $this->assertGreaterThan(0, $normalMethodMetrics['lineCount']); + $this->assertEquals(0, $normalMethodMetrics['argCount']); + $this->assertEquals(1, $normalMethodMetrics['returnCount']); + $this->assertEquals(1, $normalMethodMetrics['variableCount']); + } + + /** + * Test that the visitor handles multiple anonymous classes correctly. + * This test ensures that the fix works when there are multiple anonymous + * classes in the same method or file. + */ + public function testHandlesMultipleAnonymousClasses(): void + { + $code = <<<'CODE' + value = $value; + } + }; + + $obj2 = new class(2) { + public function __construct(private int $value) { + $this->value = $value; + } + }; + + $obj3 = new class(3) { + public function __construct(private int $value) { + $this->value = $value; + } + }; + + return $obj1->value + $obj2->value + $obj3->value; + } + } + CODE; + + $parser = (new ParserFactory())->createForHostVersion(); + $statements = $parser->parse($code); + + $traverser = new NodeTraverser(); + $metricsVisitor = new CognitiveMetricsVisitor(); + $traverser->addVisitor($metricsVisitor); + + $traverser->traverse($statements); + + $methodMetrics = $metricsVisitor->getMethodMetrics(); + + // Only the method from the named class should be present + $this->assertCount(1, $methodMetrics); + $this->assertArrayHasKey('\\TestNamespace\\TestClass::methodWithMultipleAnonymousClasses', $methodMetrics); + + // Verify that the metrics have all required keys + $metrics = $methodMetrics['\\TestNamespace\\TestClass::methodWithMultipleAnonymousClasses']; + $requiredKeys = [ + 'lineCount', 'argCount', 'returnCount', 'variableCount', + 'propertyCallCount', 'ifCount', 'ifNestingLevel', 'elseCount' + ]; + + foreach ($requiredKeys as $key) { + $this->assertArrayHasKey($key, $metrics, "Missing required key: {$key}"); + $this->assertIsInt($metrics[$key], "Key {$key} should be an integer"); + } + } } From 60b55f8f9c1edc9d63c5d78efea0e32552beb4a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Florian=20Kr=C3=A4mer?= Date: Wed, 24 Sep 2025 17:21:13 +0200 Subject: [PATCH 3/3] Fix analysis breaking for anonymous classes --- src/Business/Cognitive/CognitiveMetrics.php | 4 ++-- .../Cognitive/CognitiveMetricsCollector.php | 2 ++ src/Business/Cognitive/Parser.php | 10 ++++++++-- src/PhpParser/CognitiveMetricsVisitor.php | 15 +++++++++++++-- src/PhpParser/HalsteadMetricsVisitor.php | 14 ++++++++++++-- 5 files changed, 37 insertions(+), 8 deletions(-) diff --git a/src/Business/Cognitive/CognitiveMetrics.php b/src/Business/Cognitive/CognitiveMetrics.php index c75cfaf..d10bd33 100644 --- a/src/Business/Cognitive/CognitiveMetrics.php +++ b/src/Business/Cognitive/CognitiveMetrics.php @@ -99,7 +99,7 @@ private function setRequiredMetricProperties(array $metrics): void $class = $metrics['class'] ?? 'Unknown'; $method = $metrics['method'] ?? 'Unknown'; $file = $metrics['file'] ?? 'Unknown'; - + $errorMessage = sprintf( 'Missing required keys for %s::%s in file %s: %s. Available keys: %s', $class, @@ -108,7 +108,7 @@ private function setRequiredMetricProperties(array $metrics): void implode(', ', $missingKeys), implode(', ', array_keys($metrics)) ); - + throw new InvalidArgumentException($errorMessage); } diff --git a/src/Business/Cognitive/CognitiveMetricsCollector.php b/src/Business/Cognitive/CognitiveMetricsCollector.php index c3f6737..845f7df 100644 --- a/src/Business/Cognitive/CognitiveMetricsCollector.php +++ b/src/Business/Cognitive/CognitiveMetricsCollector.php @@ -128,8 +128,10 @@ private function processMethodMetrics( continue; } + [$class, $method] = explode('::', $classAndMethod); + $metricsArray = array_merge($metrics, [ 'class' => $class, 'method' => $method, diff --git a/src/Business/Cognitive/Parser.php b/src/Business/Cognitive/Parser.php index 0791c49..5873d7e 100644 --- a/src/Business/Cognitive/Parser.php +++ b/src/Business/Cognitive/Parser.php @@ -128,7 +128,10 @@ private function getHalsteadMetricsVisitor(array $methodMetrics): array if (str_ends_with($method, '::')) { continue; } - $methodMetrics[$method]['halstead'] = $metrics; + // Only add Halstead metrics to methods that were processed by CognitiveMetricsVisitor + if (isset($methodMetrics[$method])) { + $methodMetrics[$method]['halstead'] = $metrics; + } } return $methodMetrics; @@ -150,7 +153,10 @@ private function getCyclomaticComplexityVisitor(array $methodMetrics): array if (str_ends_with($method, '::')) { continue; } - $methodMetrics[$method]['cyclomatic_complexity'] = $complexity; + // Only add cyclomatic complexity to methods that were processed by CognitiveMetricsVisitor + if (isset($methodMetrics[$method])) { + $methodMetrics[$method]['cyclomatic_complexity'] = $complexity; + } } return $methodMetrics; diff --git a/src/PhpParser/CognitiveMetricsVisitor.php b/src/PhpParser/CognitiveMetricsVisitor.php index d8a8ac6..b8ac571 100644 --- a/src/PhpParser/CognitiveMetricsVisitor.php +++ b/src/PhpParser/CognitiveMetricsVisitor.php @@ -5,6 +5,8 @@ namespace Phauthentic\CognitiveCodeAnalysis\PhpParser; use PhpParser\Node; +use PhpParser\NodeTraverser; +use PhpParser\NodeVisitor; use PhpParser\NodeVisitorAbstract; /** @@ -217,6 +219,7 @@ private function setCurrentClassOnEnterNode(Node $node): bool } if ($node->name === null) { + // Skip anonymous classes - they don't have a proper class name return false; } @@ -240,11 +243,12 @@ private function normalizeFqcn(string $fqcn): string return str_starts_with($fqcn, '\\') ? $fqcn : '\\' . $fqcn; } - public function enterNode(Node $node): void + public function enterNode(Node $node): int|Node|null { $this->setCurrentNamespaceOnEnterNode($node); if (!$this->setCurrentClassOnEnterNode($node)) { - return; + // Skip the entire subtree for anonymous classes or ignored classes + return NodeVisitor::DONT_TRAVERSE_CHILDREN; } $this->classMethodOnEnterNode($node); @@ -252,6 +256,8 @@ public function enterNode(Node $node): void if ($this->currentMethod) { $this->gatherMetrics($node); } + + return null; } private function gatherMetrics(Node $node): void @@ -389,6 +395,11 @@ private function checkNameSpaceOnLeaveNode(Node $node): void private function checkClassOnLeaveNode(Node $node): void { if ($this->isClassOrTraitNode($node)) { + if (!empty($this->currentMethod)) { + // Don't clear the class context if we're still processing a method + return; + } + $this->currentClassName = ''; } } diff --git a/src/PhpParser/HalsteadMetricsVisitor.php b/src/PhpParser/HalsteadMetricsVisitor.php index 2900655..2828170 100644 --- a/src/PhpParser/HalsteadMetricsVisitor.php +++ b/src/PhpParser/HalsteadMetricsVisitor.php @@ -65,7 +65,11 @@ public function enterNode(Node $node) $this->setCurrentClassName($node); // Check if this class should be ignored - if ($this->annotationVisitor !== null && $this->annotationVisitor->isClassIgnored($this->currentClassName)) { + if ( + $this->currentClassName !== null + && $this->annotationVisitor !== null + && $this->annotationVisitor->isClassIgnored($this->currentClassName) + ) { $this->currentClassName = null; // Clear the class name if ignored } }, @@ -113,7 +117,13 @@ private function setCurrentNamespace(Namespace_ $node): void private function setCurrentClassName(Node $node): void { - $className = $node->name ? $node->name->toString() : ''; + // Skip anonymous classes - they don't have a proper class name + if ($node->name === null) { + $this->currentClassName = null; + return; + } + + $className = $node->name->toString(); // Always build FQCN as "namespace\class" (even if namespace is empty) $fqcn = ($this->currentNamespace !== '' ? $this->currentNamespace . '\\' : '') . $className; $this->currentClassName = $this->normalizeFqcn($fqcn);