fix: support python @classmethod decorator#106
Conversation
CrackTC
requested review from
AntJiuFo,
Arielwyy and
alipaydeshui
as code owners
Summary of ChangesHello @CrackTC, 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 addresses a bug where Python's Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe 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 by creating a comment using either
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 Limitations & Feedback Gemini Code Assist 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. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. 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
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds support for Python's @classmethod decorator by overriding the executeSingleCall method. The implementation correctly identifies classmethod calls and injects the class object as the first argument if it's missing. The logic is sound. My feedback includes suggestions to improve the documentation and type safety of the new code for better maintainability.
| /** | ||
| * | ||
| * @param fclos | ||
| * @param argvalues | ||
| * @param state | ||
| * @param node | ||
| * @param scope | ||
| */ |
There was a problem hiding this comment.
The JSDoc for this method is incomplete. It should explain that this method is overridden to handle Python's @classmethod decorator, where the first argument (cls) is implicitly passed.
/**
* Overrides the parent `executeSingleCall` to provide special handling for Python's `@classmethod`.
* For a classmethod, the first argument is the class itself. This method ensures that if the
* first argument is not defined, it is correctly set to the class object (`fclos._this`).
*
* @param fclos The function closure being called.
* @param argvalues The arguments for the function call.
* @param state The current analysis state.
* @param node The AST node for the call expression.
* @param scope The current scope.
*/| * @param node | ||
| * @param scope | ||
| */ | ||
| executeSingleCall(fclos: any, argvalues: any, state: any, node: any, scope: any) { |
There was a problem hiding this comment.
The method signature uses any for all its parameters. This reduces type safety and makes the code harder to understand and maintain. While this matches the parent class, consider using more specific types. For example, fclos could be FunctionValue, argvalues an array of SymbolValue, and scope a Scope.
| * @param scope | ||
| */ | ||
| executeSingleCall(fclos: any, argvalues: any, state: any, node: any, scope: any) { | ||
| if (fclos.decorators?.some((d: any) => d.name === 'classmethod') && argvalues[0]?.vtype === 'undefine') { |
There was a problem hiding this comment.
The decorator object d is typed as any. To improve type safety, you could use a more specific inline type like { name: string } or define a Decorator interface.
if (fclos.decorators?.some((d: { name: string }) => d.name === 'classmethod') && argvalues[0]?.vtype === 'undefine') {![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
This PR is being reviewed by Cursor Bugbot
Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| * @param scope | ||
| */ | ||
| executeSingleCall(fclos: any, argvalues: any, state: any, node: any, scope: any) { | ||
| if (fclos.decorators?.some((d: any) => d.name === 'classmethod') && argvalues[0]?.vtype === 'undefine') { |
There was a problem hiding this comment.
Decorator check uses wrong property name
High Severity
The decorator check d.name === 'classmethod' will always fail because the processed decorator object doesn't have a name property. When an unresolved identifier like classmethod is processed, processIdentifier sets _id and _sid to the identifier name, but not name. The condition should check d._id === 'classmethod' or d._sid === 'classmethod' instead.
1 participant
Note
Medium Risk
Touches Python call execution semantics by mutating call arguments for decorated functions; could affect how method calls are modeled across analyses if decorator detection or
undefinehandling is wrong.Overview
Adds a
PythonAnalyzer.executeSingleCalloverride to special-case functions decorated with@classmethod.When the first argument is missing/
undefine, it now injects the captured class context (fclos._this) as the implicitclsargument before delegating to the baseAnalyzercall execution.Written by Cursor Bugbot for commit fdc679f. This will update automatically on new commits. Configure here.