Upgrade nikic/php-parser to v5+

[BrianHenryIE]

PR is to upgrade to php-parser 5.x.

I made some DX changes on my way to updating php-parser library, I will cherry-pick these commits as independent PRs if you prefer.

Tangential commits:

• Mute command not found: nproc – When running tests on MacOS, the stderr output was always printed, so I redirect it to stdio which is then parsed as normal
• Determine number of cores on macOS – intention is to add the corresponding function for the missing nproc
• Add test-coverage Composer script – I wanted an easy way to see the current coverage HTML report so I might identify anywhere that my changes need new tests. Current coverage is 80%.

Proposed Changes

These are the guts of the PR, and much less work than I expected: c55dc7a...a436917

php-parser has a UPGRADE-5.0 guide. I've added checkboxes for each heading it has to try make reviewing as easy as possible:

• PHP version requirements – seems irrelevant since this library already has a PHP 7.4 minimum
• PHP 5 parsing support – as above, NA?
• Changes to the parser factory

c55dc7a Instantiate parser via ::createForNewestSupportedVersion()

The createForNewestSupportedVersion() and creatForHostVersion() are available since PHP-Parser 4.18.0, to allow libraries to support PHP-Parser 4 and 5 at the same time more easily.

composer require "nikic/php-parser":"^4.18 || ^5" (in BrianHenryIE@846e011, BrianHenryIE@63113e2)

• Changes to the throw representation – I searched for exception handling in this library and there is only Exception and ReflectionException, so I don't think there's any changes here
• Changes to the array destructuring representation
• Changes to the name representation – 9356c18 Check method_exists for deprecated ::getParts()
• Changes to the block representation
• Changes to comment assignment
• Renamed nodes – I searched for Scalar and didn't find anything relevant
• Modifiers – modifier/i is not used in this project's code
• Changes to node constructors
• Changes to the pretty printer – no uses
• Changes to precedence handling in the pretty printer – NA
• Changes to token representation – I don't see any use of the fields in the described interface
• Changes to the lexer – Lexer does not appear to be used directly
• Miscellaneous changes – mostly clear; Node::getLine() addressed in a436917

I don't think this is a breaking change, but I haven't used this widely enough to be sure.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[gemini-code-assist]

Hello @BrianHenryIE, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

Summary of Changes

Hello! Gemini here, providing a summary of this pull request. The primary goal of this PR, authored by BrianHenryIE, is to upgrade the nikic/php-parser dependency to support version 5.x, while maintaining compatibility with recent 4.x versions. The author notes that some commits are tangential to the main upgrade (related to CPU core detection and test coverage scripts) and could potentially be cherry-picked into separate PRs. The core changes involve updating the dependency constraint, adjusting how the parser is instantiated to use the newest supported version, and adding compatibility checks for methods (getLine, getParts) that have changed or been deprecated in php-parser v5. Additionally, the PR includes changes to improve CPU core detection, particularly for macOS, and adds a convenient composer script for running test coverage.

Highlights

• Dependency Upgrade: Updates the nikic/php-parser requirement in composer.json to allow versions ^4.18 || ^5.
• PHP Parser Instantiation: Changes the parser creation logic in PhpCodeParser to use ParserFactory::createForNewestSupportedVersion() for better compatibility across php-parser versions.
• PHP Parser API Compatibility: Adds checks (method_exists) for deprecated or changed methods like getLine (now getStartLine) and getParts in BasePHPElement and Utils to ensure compatibility with both php-parser v4 and v5 APIs.
• Test Coverage Script: Adds a test-coverage script to composer.json for easily generating and viewing code coverage reports via PHPUnit.
• CPU Core Detection Improvement: Enhances the getCpuCores utility function to handle nproc output more robustly (redirecting stderr) and adds support for detecting cores on macOS using sysctl.

Changelog

Click here to see the changelog

• .gitignore
  • Add entry /.phpunit.result/ to ignore the directory used for phpunit results (line 8).
• composer.json
  • Update nikic/php-parser dependency from ~4.16 to ^4.18 || ^5 (line 27).
  • Add scripts and scripts-descriptions sections, including the test-coverage script for running PHPUnit with coverage (lines 42-52).
• src/voku/SimplePhpParser/Model/BasePHPElement.php
  • Modify prepareNode to use getStartLine() if available, falling back to getLine() for compatibility with older php-parser versions (lines 100-107).
• src/voku/SimplePhpParser/Parsers/Helper/Utils.php
  • In getPhpParserValueFromNode, add method_exists check for getParts() before calling it, accommodating changes in php-parser v5 (lines 115-117).
  • In getCpuCores, redirect stderr to stdout for the nproc command to prevent errors from being printed (line 473).
  • Add logic to getCpuCores to detect CPU cores on macOS using sysctl -n hw.ncpu (lines 503-514).
• src/voku/SimplePhpParser/Parsers/PhpCodeParser.php
  • Remove unused use PhpParser\Lexer\Emulative; statement (line 8).
  • Change parser instantiation in process to use (new ParserFactory())->createForNewestSupportedVersion() (line 202).

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in issue comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

---

Parse the code tree,
Upgrade the library,
New versions arrive.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request effectively upgrades nikic/php-parser to support v5.x, which is a crucial update. The changes made for compatibility, such as updating the parser factory usage and handling deprecated methods like getLine() and getParts(), are well-targeted. The DX improvements, including the test coverage script and macOS core detection, are also valuable additions.

I've found one area in src/voku/SimplePhpParser/Parsers/Helper/Utils.php concerning the name representation handling for php-parser v5 that could be improved for robustness. Details are in the specific comment.

Overall, great work on tackling this upgrade!

Summary of Findings

• PHP-Parser v5 Name Representation Handling: In src/voku/SimplePhpParser/Parsers/Helper/Utils.php, the fallback logic for obtaining string representation of PhpParser\Node\Name objects for PHP-Parser v5 uses $node->value->name->name. This is not robust for fully qualified names. It's recommended to use $node->value->name->toString() for PHP-Parser v5 compatibility, as it correctly handles all types of Name nodes.

Merge Readiness

The pull request is a significant step forward in modernizing the dependency on php-parser. However, there is a high-severity issue identified in src/voku/SimplePhpParser/Parsers/Helper/Utils.php related to name resolution with PHP-Parser v5 that should be addressed to ensure correctness. Once this is resolved, the PR should be in good shape for merging. As a reviewer, I am not authorized to approve pull requests, so please ensure further review and approval by authorized maintainers after addressing the feedback.

[gemini-code-assist]

This change correctly adapts for php-parser v5 by checking for getParts(). However, the fallback : $node->value->name->name for the v5 path might not be robust for all cases, especially for fully qualified names.

1. PHP-Parser v5 Name Handling: For php-parser v5, PhpParser\Node\Name objects (including FullyQualified, Relative, etc.) provide a toString() method for reliable string conversion. Using $node->value->name->name can be problematic because:

   • For fully qualified names (e.g., new \My\Namespace\MyClass()), the name property might not exist on the FullyQualified object, or it might only contain the last part of the name, not the full FQN.
   • $node->value->name->toString() is the recommended way to get the complete string representation of any Name node in v5.

2. Original Elvis Operator: It appears the original Elvis operator ?: $node->value->name->name (which was part of implode(...) ?: $node->value->name->name) has been removed from the v4 path in this PR (the true branch of the ternary). If implode('\\', $node->value->name->getParts()) results in an empty string, the behavior for the v4 path will now differ from the original code. Was this change in behavior for the v4 path intentional?

For the v5 path, could we use $node->value->name->toString() for more reliable name resolution? This would align better with the php-parser v5 API.

                    $value = method_exists($node->value->name,'getParts')
                        ? implode('\\', $node->value->name->getParts()) // v4 path (Note: original Elvis operator was removed in this PR)
                        : $node->value->name->toString(); // v5 path: use toString() for robust name resolution