Add php-parser v5 compatibility (^4.18 || ^5)#77
Conversation
Agent-Logs-Url: https://github.com/voku/Simple-PHP-Code-Parser/sessions/36a711fc-c894-41e1-bbe1-415f9b714722 Co-authored-by: voku <264695+voku@users.noreply.github.com>
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces support for nikic/php-parser v5 by updating the composer constraints and implementing compatibility checks for deprecated or removed methods like getLine() and getParts(). While the transition to createForNewestSupportedVersion() and the line number handling are correct, there are critical issues in the fallback logic for Name nodes. Specifically, the code attempts to access the 'name' property which was removed in v5; these instances should be updated to use the 'parts' property to ensure compatibility across both versions.
| $value = method_exists($node->value->name, 'getParts') | ||
| ? implode('\\', $node->value->name->getParts()) | ||
| : $node->value->name->name; |
There was a problem hiding this comment.
The fallback to $node->value->name->name for php-parser v5 is incorrect. The name property on PhpParser\Node\Name was removed in v5. For compatibility, you should use the public parts property instead.
$value = implode('\', method_exists($node->value->name, 'getParts') ? $node->value->name->getParts() : $node->value->name->parts);| $parts = method_exists($node->name, 'getParts') | ||
| ? $node->name->getParts() | ||
| : [$node->name->name]; |
There was a problem hiding this comment.
The fallback to [$node->name->name] for php-parser v5 is incorrect. The name property on PhpParser\Node\Name was removed in v5. You should use the public parts property instead, which is an array of name parts, just like the return value of getParts().
$parts = method_exists($node->name, 'getParts') ? $node->name->getParts() : $node->name->parts;There was a problem hiding this comment.
Pull request overview
This PR aims to make the library compatible with both nikic/php-parser v4.18+ and v5 by widening the Composer constraint and updating a few call sites that were removed in v5.
Changes:
- Widen
nikic/php-parserrequirement to^4.18 || ^5. - Switch parser instantiation to
ParserFactory::createForNewestSupportedVersion(). - Add v5-safe fallbacks for
Node::getLine()andName::getParts()in a couple of locations.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| composer.json | Widens nikic/php-parser constraint to allow v5. |
| src/voku/SimplePhpParser/Parsers/PhpCodeParser.php | Uses createForNewestSupportedVersion() to avoid removed factory APIs in v5. |
| src/voku/SimplePhpParser/Model/BasePHPElement.php | Avoids Node::getLine() by preferring getStartLine(). |
| src/voku/SimplePhpParser/Parsers/Helper/Utils.php | Guards Name::getParts() for v5 compatibility in getPhpParserValueFromNode(). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "phpstan/phpdoc-parser": "~1.23", | ||
| "voku/simple-cache": "~5.0", | ||
| "nikic/php-parser": "~4.18" | ||
| "nikic/php-parser": "^4.18 || ^5" |
There was a problem hiding this comment.
nikic/php-parser is now allowed at ^5, but the codebase still calls APIs removed in v5 (notably PhpParser\Node\Name::getParts()). I found remaining call sites in src/voku/SimplePhpParser/Model/BasePHPElement.php, Model/PHPClass.php, Model/PHPInterface.php, Model/PHPEnum.php, Model/PHPConst.php, and Parsers/Visitors/ASTVisitor.php. With php-parser v5 installed these will fatal at runtime, so the constraint change should be paired with updating those usages (e.g., prefer Name::toString() or a small compat helper that uses getParts() when available and falls back to ->name/toString()).
| "nikic/php-parser": "^4.18 || ^5" | |
| "nikic/php-parser": "^4.18" |
| $this->line = method_exists($node, 'getStartLine') | ||
| ? $node->getStartLine() | ||
| : $node->getLine(); // @phpstan-ignore-line getLine() was removed in PHP-Parser v5 |
There was a problem hiding this comment.
This file still uses Name::getParts() (e.g., in getConstantFQN() and getFQN()), which was removed in php-parser v5. Even though prepareNode() now avoids Node::getLine(), these remaining getParts() calls will still cause fatal errors under v5; please update them to use ->toString() or a compat wrapper (similar to the approach taken in Utils::getPhpParserValueFromNode()).
| $this->line = method_exists($node, 'getStartLine') | ||
| ? $node->getStartLine() | ||
| : $node->getLine(); // @phpstan-ignore-line getLine() was removed in PHP-Parser v5 |
There was a problem hiding this comment.
getFQN() currently contains a var_dump($node->name); debug statement. This will emit output during normal library usage (breaking CLI output, JSON responses, etc.) and should be removed or replaced with a proper logger behind an explicit debug flag.
…allbacks Agent-Logs-Url: https://github.com/voku/Simple-PHP-Code-Parser/sessions/a7ff18d8-62d8-4ae9-bfdb-f5873a470e41 Co-authored-by: voku <264695+voku@users.noreply.github.com>
…back Agent-Logs-Url: https://github.com/voku/Simple-PHP-Code-Parser/sessions/a7ff18d8-62d8-4ae9-bfdb-f5873a470e41 Co-authored-by: voku <264695+voku@users.noreply.github.com>
|
|
@copilot apply changes based on the comments in this thread |
All the changes from that review thread were already applied in commits
All 38 tests pass with php-parser v5.7.0. |



method_exists+wrong->namefallback with->toString()in ClassConstFetch path; use->getParts()directly in ConstFetch path (works in both v4 and v5)implode('\\', $x->getParts())with->toString(); removevar_dump; guard getFQN fallback:$node->name instanceof Name ? $node->name->toString() : (string) $node->namegetParts()with->toString()(×2)getParts()with->toString()getParts()with->toString()getParts()with->toString()->getParts()[0](works in both v4 and v5)