Reliable prefixing of global functions #197
Conversation
There was a problem hiding this comment.
Summary of Changes
Hello @0zd0, I'm Gemini Code Assist1! 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!
This pull request significantly enhances the reliability of global function prefixing by migrating from a regex-based approach to an Abstract Syntax Tree (AST) driven one. This change addresses previous limitations, ensuring that only global functions are prefixed, even when their names conflict with class methods, and improves the handling of function names used as string literals in callable contexts.
Highlights
- Improved Global Function Prefixing: The core logic for prefixing global functions has been significantly refactored in
src/Pipeline/Prefixer.php. It now leverages PHP-Parser's Abstract Syntax Tree (AST) traversal instead of regular expressions. This change ensures more reliable and accurate identification and modification of global function declarations and calls. - Accurate Scope Handling: The updated prefixing mechanism correctly differentiates between global functions and class methods that might share the same name. This prevents unintended prefixing of methods, ensuring only truly global functions are modified.
- Enhanced Callable String Handling: The new AST-based approach improves the handling of global functions when they are passed as string arguments to functions like
function_existsorcall_user_func, ensuring these instances are also correctly prefixed. - Code Modernization and Testing: The
Prefixer.phpfile now explicitly imports variousPhpParsernode types, simplifying internal code references. A new unit test has been added totests/Unit/PrefixerTest.phpto specifically validate the correct distinction between global functions and class methods during prefixing.
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.
Footnotes
-
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. ↩
There was a problem hiding this comment.
Code Review
The pull request focuses on improving the reliability of prefixing global functions. It introduces changes to the Prefixer class to handle function prefixing more effectively, especially in cases where functions are used as callable values. Additionally, a new test case is added to ensure the absence of function prefixes within classes.
|
CI is failing because it's trying to add a comment to this thread with the code coverage report. I've fixed it in 5963023. Check out my other project, php-diff-test. Thank you very much for the PR, this is exactly the direction I see the project moving in. At a 20 minute look, I think the PR is good to merge, but I'm going to take another look soon. The global functions replacements is relatively new code (0.21.0, Jan 2025) and I don't think it's particularly well covered by tests. I'm actually not mad on prefixing globals, I think a better approach would be to move every file into a namespace, but whatever, I haven't given it thorough consideration: #126 |
Fixes #195
Fixes #196