Skip to content

Commit 1181775

Browse files
Fixing parser error when encountering complex variables. (#26)
Fixes #18 In PhpParser, the $node->name property of a Node\Expr\Variable can be either: - a string (for simple variables like $foo) - or an Expr node (usually another Variable or an expression, for complex variables like ${$bar} or variable variables) This is because PHP allows variable variables and complex expressions as variable names, so the parser represents them as objects rather than plain strings. Always check the type before using it as an array key. When `$node->name is an object (not a string)`, it represents complex or variable variables (like `${$foo}`) in PHP. These cases are rare in typical code and are challenging to statically analyze. For metrics like variable counting, they are usually not relevant or are intentionally skipped, since you cannot reliably determine the variable name at parse time.
1 parent e31325b commit 1181775

File tree

2 files changed

+53
-4
lines changed

2 files changed

+53
-4
lines changed

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)