Fix VB With block conversion with null-conditional operator#21
Conversation
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
Summary of ChangesHello, 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 critical bug in the VB.NET to C# code converter, specifically concerning the translation of 'With' blocks that incorporate null-conditional operators. The core fix involves refining the logic for detecting and processing member access expressions within conditional access chains, ensuring that the generated C# code correctly handles null propagation. Additionally, the PR includes a new regression test to validate the fix and prevent recurrence, alongside minor cleanup of the test project's dependencies. 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 effectively addresses an issue with the conversion of Visual Basic With blocks that use the null-conditional operator. The updated logic in IsSubPartOfConditionalAccess is a clear improvement, correctly traversing the syntax tree to handle nested expressions. The addition of a regression test is valuable for ensuring this fix remains stable. I've also noted the project dependency cleanup, which is a good maintenance improvement. I have one minor suggestion regarding code formatting.
| child = parent; | ||
| parent = parent.Parent; | ||
| continue; |
There was a problem hiding this comment.
There's a minor indentation issue in this block. These lines appear to be indented one space too many. Please correct the indentation for consistency and readability.
child = parent;
parent = parent.Parent;
continue;
GrahamTheCoder
force-pushed
the
fix-1174-vb-with-block-null-conditional-12294486853097276888
branch
from
3d4cc80 to
8031033
Compare
Modified IsSubPartOfConditionalAccess to recursively check if a member access is part of the WhenNotNull branch of a ConditionalAccessExpression, handling nested conditional accesses. This ensures that the expression part (LHS) of a conditional access is correctly identified as not being part of the binding chain, allowing it to be resolved to the With block variable. Added regression test WithBlockWithNullConditionalAccessAsync. Co-authored-by: GrahamTheCoder <[email protected]>
GrahamTheCoder
force-pushed
the
fix-1174-vb-with-block-null-conditional-12294486853097276888
branch
from
8031033 to
0532ed5
Compare
1 participant
This PR fixes an issue where converting VB code with a
Withblock containing a null-conditional operator (e.g.,.x?.Name) resulted in incorrect C# code (e.g.,x = .x?.Name;).The fix involves updating
IsSubPartOfConditionalAccessinNameExpressionNodeVisitor.csto correctly identify whether aMemberAccessExpressionis part of theWhenNotNullbranch of aConditionalAccessExpression. This ensures that the left-hand side of the null-conditional operator (the expression being checked for null) is treated as a regular expression and resolved against theWithblock variable, while the right-hand side is treated as a binding expression.The logic was improved to recursively check up the syntax tree to handle nested conditional access expressions (e.g.,
.Prop?.Prop?.Prop).A regression test
WithBlockWithNullConditionalAccessAsynchas been added toMethodStatementTests.cs.PR created automatically by Jules for task 12294486853097276888 started by @GrahamTheCoder