Skip to content

fix: add Union branch to check_generic_type_compact to prevent param-type-mismatch false positives#1107

Open
ShenzhenGopher wants to merge 1 commit into
EmmyLuaLs:mainfrom
ShenzhenGopher:fix/union-generic-type-check
Open

fix: add Union branch to check_generic_type_compact to prevent param-type-mismatch false positives#1107
ShenzhenGopher wants to merge 1 commit into
EmmyLuaLs:mainfrom
ShenzhenGopher:fix/union-generic-type-check

Conversation

@ShenzhenGopher

Copy link
Copy Markdown

Summary

Fixes #1106

When a method returning a generic type has both @field declaration and function implementation, the return value is internally wrapped in a Union type. check_generic_type_compact lacks a LuaType::Union branch, causing false param-type-mismatch / assign-type-mismatch warnings even when types are identical.

Root Cause

In check_generic_type_compact (crates/emmylua_code_analysis/src/semantic/type_check/generic_type.rs, line 99), the match statement handles GenericType, TableConst, Ref/Def, but has no LuaType::Union branch. When compact_type is a Union (produced by merging @field and function sources in resolve_member_type), it falls through to _ => Err(TypeCheckFailReason::TypeNotMatch).

Changes

File Change
crates/emmylua_code_analysis/src/semantic/type_check/generic_type.rs Add LuaType::Union branch in check_generic_type_compact that iterates all union members and checks each against the source generic type (+13 lines)

Code Change

Insert before the _ => Err(TypeCheckFailReason::TypeNotMatch) arm in check_generic_type_compact:

        LuaType::Union(union_type) => {
            // When compact_type is a Union, every member of the union must be
            // assignable to the source generic type for the assignment to be safe.
            for member_type in union_type.into_vec() {
                check_generic_type_compact(
                    context,
                    source_generic,
                    &member_type,
                    check_guard.next_level()?,
                )?;
            }
            Ok(())
        }

Rationale

A Union type A | B assigned to a generic parameter T requires all members to satisfy T's constraints for the assignment to be safe. This is standard covariant semantics — the Union is a "wider" type, so each member must individually be valid.

Example

---@class Item
---@field id integer
---@class Box<T>
---@field value T
---@class Service
---@field execute fun(self:Service, box:Box<Item>)
---@field fetch fun(self:Service):Box<Item>
local Service = {}
function Service:execute(_box) end
function Service:fetch() return {} --[[@as Box<Item>]] end
function Service:run()
    local b = self:fetch()
    self:execute(b)
end

Verification

Before fix (commit d7d5dcbf):

---  [1 warning]
warning: expected `Box<Item>` but found `Box<Item>`. [param-type-mismatch]
  --> :17:18

 16 |     local b = self:fetch()
 17 |     self:execute(b)

Summary
  1 warning

Check completed with warnings
Check finished

After fix:

No issues found
Check finished

When a method has both @field declaration and function implementation, resolve_member_type merges them into a Union type. check_generic_type_compact lacked a LuaType::Union branch, causing param-type-mismatch / assign-type-mismatch
false positives even when all union members satisfy the generic constraint.

Add a Union arm that iterates all members and checks each against the source generic type, following standard covariant semantics.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review: generic_type.rs Changes

Issue Found: Potential Infinite Recursion

Problem: The new Union case calls check_generic_type_compact recursively for each member of the union. If a union member is itself a Union type, this could lead to infinite recursion or stack overflow.

Example scenario:

// If union_type contains nested unions
Union([TypeA, Union([TypeB, TypeC])])
// This would recursively call check_generic_type_compact on the inner union
// which would then iterate over its members again

Recommendation

Add a guard to prevent processing the same union type multiple times, or flatten nested unions before processing:

LuaType::Union(union_type) => {
    // Flatten nested unions to prevent infinite recursion
    let flat_members = flatten_union(union_type);
    for member_type in flat_members {
        check_generic_type_compact(
            context,
            source_generic,
            &member_type,
            check_guard.next_level()?,
        )?;
    }
    Ok(())
}

Where flatten_union would recursively extract all non-union types from nested unions.

Additional Consideration

  • Performance: For large unions, this could be expensive. Consider adding a depth limit or early exit if a type mismatch is found (current implementation does exit early via ? operator, which is good).
  • Error Messages: When a union member fails, the error won't indicate which member failed. Consider wrapping errors with context about which union member caused the failure.

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

Copy link
Copy Markdown
Contributor

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 adds support for checking union types against generic types in check_generic_type_compact. However, the current implementation recursively calls check_generic_type_compact directly on union members, which bypasses critical type-checking logic like alias resolution and wildcard checks. It is recommended to wrap the generic type and delegate to check_general_type_compact instead, and to add a corresponding test case to verify this behavior.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +102 to +114
LuaType::Union(union_type) => {
// When compact_type is a Union, every member of the union must be
// assignable to the source generic type for the assignment to be safe.
for member_type in union_type.into_vec() {
check_generic_type_compact(
context,
source_generic,
&member_type,
check_guard.next_level()?,
)?;
}
Ok(())
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

Calling check_generic_type_compact directly on union members bypasses critical type-checking logic defined in check_general_type_compact, such as is_like_any (which handles any and unknown), escape_type (which resolves aliases), and fast_eq_check.

If a union contains unknown or an alias type, calling check_generic_type_compact directly will result in a false positive TypeNotMatch error because those types are not explicitly handled in the match statement of check_generic_type_compact.

To fix this, we should construct a LuaType::Generic from source_generic and call check_general_type_compact for each member of the union.

Additionally, it is highly recommended to add a test case to crates/emmylua_code_analysis/src/semantic/type_check/test.rs to verify this behavior:

#[test]
fn test_issue_1106() {
    let mut ws = VirtualWorkspace::new();
    ws.def(
        r#"
        ---@class Item
        ---@field id integer
        ---@class Box<T>
        ---@field value T
        ---@class Service
        ---@field execute fun(self:Service, box:Box<Item>)
        ---@field fetch fun(self:Service):Box<Item>
        "#,
    );

    assert!(ws.has_no_diagnostic(
        DiagnosticCode::ParamTypeMismatch,
        r#"
        local Service = {}
        function Service:execute(_box) end
        function Service:fetch() return {} --[[@as Box<Item>]] end
        function Service:run()
            local b = self:fetch()
            self:execute(b)
        end
        "#
    ));
}
Suggested change
LuaType::Union(union_type) => {
// When compact_type is a Union, every member of the union must be
// assignable to the source generic type for the assignment to be safe.
for member_type in union_type.into_vec() {
check_generic_type_compact(
context,
source_generic,
&member_type,
check_guard.next_level()?,
)?;
}
Ok(())
}
LuaType::Union(union_type) => {
// When compact_type is a Union, every member of the union must be
// assignable to the source generic type for the assignment to be safe.
let source_type = LuaType::Generic(source_generic.clone().into());
for member_type in union_type.into_vec() {
check_general_type_compact(
context,
&source_type,
&member_type,
check_guard.next_level()?,
)?;
}
Ok(())
}

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.

param-type-mismatch / assign-type-mismatch false positive when a method returning a generic type has both @field declaration and function implementation

1 participant