Skip to content

Commit 59a8deb

Browse files
Fixing parser error when encountering complex variables.
#18
1 parent e9ec3d3 commit 59a8deb

2 files changed

Lines changed: 53 additions & 4 deletions

File tree

src/PhpParser/CognitiveMetricsVisitor.php

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -194,7 +194,7 @@ private function gatherMetrics(Node $node): void
194194
{
195195
match (true) {
196196
$node instanceof Node\Stmt\Return_ => $this->incrementReturnCount(),
197-
$node instanceof Node\Expr\Variable => $this->trackVariable($node),
197+
$node instanceof Node\Expr\Variable => $this->countVariablesNotAlreadyTrackedAsArguments($node),
198198
$node instanceof Node\Expr\PropertyFetch => $this->trackPropertyFetch($node),
199199
$node instanceof Node\Stmt\If_ => $this->trackIfStatement(),
200200
$node instanceof Node\Stmt\Else_,
@@ -208,10 +208,30 @@ private function incrementReturnCount(): void
208208
$this->currentReturnCount++;
209209
}
210210

211-
private function trackVariable(Node\Expr\Variable $node): void
211+
/**
212+
* Count variables that are not already tracked as method arguments.
213+
*
214+
* Important note about variable handling in this method:
215+
*
216+
* In PhpParser, the $node->name property of a Node\Expr\Variable can be either:
217+
*
218+
* - a string (for simple variables like $foo)
219+
* - or an Expr node (usually another Variable or an expression, for complex variables like ${$bar} or variable
220+
* variables)
221+
*
222+
* This is because PHP allows variable variables and complex expressions as variable names, so the parser represents
223+
* them as objects rather than plain strings. Always check the type before using it as an array key.
224+
*
225+
* When $node->name is an object (not a string), it represents complex or variable variables (like ${$foo})
226+
* in PHP. These cases are rare in typical code and are challenging to statically analyze. For metrics like
227+
* variable counting, they are usually not relevant or are intentionally skipped, since you cannot reliably
228+
* determine the variable name at parse time.
229+
*
230+
* @link https://github.com/Phauthentic/cognitive-code-analysis/issues
231+
*/
232+
private function countVariablesNotAlreadyTrackedAsArguments(Node\Expr\Variable $node): void
212233
{
213-
// Only count variables not already tracked as arguments
214-
if (!isset($this->methodArguments[$node->name])) {
234+
if (is_string($node->name) && !isset($this->methodArguments[$node->name])) {
215235
$this->currentVariables[$node->name] = true;
216236
}
217237
}

tests/Unit/PhpParser/CognitiveMetricsVisitorTest.php

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,4 +64,33 @@ public function testMethodMetricsCalculation(): void
6464
$this->assertEquals(1, $metrics['ifNestingLevel']);
6565
$this->assertEquals(2, $metrics['elseCount']);
6666
}
67+
68+
public function testCountVariablesNotAlreadyTrackedAsArguments(): void
69+
{
70+
$code = <<<'CODE'
71+
<?php
72+
class Example {
73+
public function foo($arg1) {
74+
$a = 1;
75+
$b = 2;
76+
$arg1 = 3;
77+
${$dynamic} = 4;
78+
}
79+
}
80+
CODE;
81+
82+
$parser = (new ParserFactory())->createForNewestSupportedVersion();
83+
$ast = $parser->parse($code);
84+
85+
$visitor = new CognitiveMetricsVisitor();
86+
$traverser = new NodeTraverser();
87+
$traverser->addVisitor($visitor);
88+
$traverser->traverse($ast);
89+
90+
$metrics = $visitor->getMethodMetrics();
91+
$method = '\Example::foo';
92+
93+
$this->assertArrayHasKey($method, $metrics);
94+
$this->assertEquals(3, $metrics[$method]['variableCount']);
95+
}
6796
}

0 commit comments

Comments
 (0)