Skip to content

Conversation

@phanen
Copy link
Contributor

@phanen phanen commented Dec 27, 2025

Most of code are written by ai now, it seems work as my expected.

---@class MyClass
---@field value string|number
---@field data table|nil
local MyClass = {}

---Check if value field is string
---@return_cast self.value string
function MyClass:check_string()
	return type(self.value) == "string"
end

---Check if data field exists
---@return_cast self table else nil
function MyClass:has_data()
	return self.data ~= nil
end

---@param obj MyClass
function test_simple_field(obj)
	if obj:check_string() then
		-- Here obj.value should be narrowed to string
		local v = obj.value -- should be: string
		print(v:upper()) -- string method should work
	else
		-- Here obj.value should remain string|number
		local v = obj.value
		print(v)
	end
end

---@param obj MyClass
function test_with_fallback(obj)
	if obj:has_data() then
		-- Here obj.data should be table
		local d = obj.data -- should be: table
		print(#d)
	else
		-- Here obj.data should be nil (if fallback works)
		local d = obj.data -- should be: nil
		print(d)
	end
end

@gemini-code-assist
Copy link

Summary of Changes

Hello @phanen, 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 type-checking capabilities by extending the @return_cast LuaDoc tag to support type narrowing for specific fields of the self object within methods. This allows the type system to understand and apply more precise types to object properties after conditional checks, leading to more accurate code analysis and a better developer experience. The changes involve updates to the parser, semantic analyzer, type inference logic, and diagnostic checks, ensuring robust and correct behavior for this new feature.

Highlights

  • Enhanced @return_cast for self.field: The @return_cast LuaDoc tag now supports specifying fields of the self object (e.g., ---@return_cast self.value string). This allows for more granular type narrowing within methods based on conditional checks of object properties.
  • Updated Parsing Logic: The parser has been modified to correctly interpret and extract complex expressions like self.xxx within the @return_cast tag, moving beyond simple parameter names.
  • Improved Semantic Analysis and Type Inference: The semantic analyzer and type inference engine have been updated to process these self.field casts. This includes a new helper function extract_name_from_expr to handle multi-level field access and a dedicated function get_type_at_call_expr_by_signature_self_field to apply the correct type narrowing, including fallback types, to the specified field.
  • Refined Undefined Global Diagnostics: The diagnostic checker for undefined globals has been improved to recognize self and self.field within @return_cast tags, preventing false positive warnings in valid method contexts.
  • Comprehensive Test Coverage: New test cases have been added to validate the self.field casting functionality, covering basic narrowing, fallback types, and complex object scenarios, ensuring the feature works as expected.

🧠 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.

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 by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

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 pull request 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 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

  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.

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a valuable feature to support @return_cast on class fields, enabling more precise type narrowing. The changes are well-structured, touching the parser, analyzer, and type inference logic, and are accompanied by comprehensive tests that cover the new functionality well. I've made a few suggestions to improve code readability and reduce minor duplication, but overall, this is a solid implementation.

Comment on lines +120 to +151
for ancestor in name_expr.syntax().ancestors() {
if let Some(return_cast_tag) = LuaDocTagReturnCast::cast(ancestor.clone()) {
// Find the LuaComment that contains this tag
for comment_ancestor in return_cast_tag.syntax().ancestors() {
if let Some(comment) = LuaComment::cast(comment_ancestor) {
// Get the owner (function) of this comment
if let Some(owner) = comment.get_owner() {
if let LuaAst::LuaClosureExpr(closure) = owner {
let sig_id = LuaSignatureId::from_closure(
semantic_model.get_file_id(),
&closure,
);
if let Some(sig) =
semantic_model.get_db().get_signature_index().get(&sig_id)
{
// Check if the owner function is a method
if sig.is_method(semantic_model, None) {
return Some(());
}
// Check if self is a parameter
if sig.find_param_idx("self").is_some() {
return Some(());
}
}
}
}
break;
}
}
break;
}
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The nested for loops for traversing ancestors can be made more concise and readable by using ancestors::<T>().next() and if let chains. This would flatten the code and make the intent clearer.

    if let Some(return_cast_tag) = name_expr.ancestors::<LuaDocTagReturnCast>().next() {
        if let Some(comment) = return_cast_tag.ancestors::<LuaComment>().next() {
            if let Some(owner) = comment.get_owner() {
                if let LuaAst::LuaClosureExpr(closure) = owner {
                    let sig_id = LuaSignatureId::from_closure(
                        semantic_model.get_file_id(),
                        &closure,
                    );
                    if let Some(sig) =
                        semantic_model.get_db().get_signature_index().get(&sig_id)
                    {
                        // Check if the owner function is a method or has a 'self' param
                        if sig.is_method(semantic_model, None) || sig.find_param_idx("self").is_some() {
                            return Some(());
                        }
                    }
                }
            }
        }
    }

Comment on lines +333 to +343
let base_matches = match &self_var_ref_id {
VarRefId::VarRef(self_decl_id) => {
decl_or_member.as_decl_id() == Some(*self_decl_id)
}
VarRefId::SelfRef(self_decl_or_member) => {
decl_or_member == self_decl_or_member
}
VarRefId::IndexRef(self_decl_or_member, _) => {
decl_or_member == self_decl_or_member
}
};

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The match statement for determining base_matches has identical logic for the VarRefId::SelfRef and VarRefId::IndexRef arms. You can combine these two arms using | to reduce code duplication and simplify the logic.

            let base_matches = match &self_var_ref_id {
                VarRefId::VarRef(self_decl_id) => {
                    decl_or_member.as_decl_id() == Some(*self_decl_id)
                }
                VarRefId::SelfRef(self_decl_or_member)
                | VarRefId::IndexRef(self_decl_or_member, _) => {
                    decl_or_member == self_decl_or_member
                }
            };

Comment on lines +376 to +381
match parse_cast_expr(p) {
Ok(_) => {}
Err(e) => {
return Err(e);
}
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The match statement for handling the result of parse_cast_expr can be simplified by using the ? operator. This is more idiomatic and concise for error propagation.

        parse_cast_expr(p)?;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant