Skip to content

fix: prioritize explicit @field over inherited table<K,V> index in infer_generic_member#1105

Open
ShenzhenGopher wants to merge 1 commit into
EmmyLuaLs:mainfrom
ShenzhenGopher:fix/generic-field-override-by-table-index
Open

fix: prioritize explicit @field over inherited table<K,V> index in infer_generic_member#1105
ShenzhenGopher wants to merge 1 commit into
EmmyLuaLs:mainfrom
ShenzhenGopher:fix/generic-field-override-by-table-index

Conversation

@ShenzhenGopher

@ShenzhenGopher ShenzhenGopher commented Jun 11, 2026

Copy link
Copy Markdown

Summary

Fixes #xxx

When a generic class inherits table<K,V> (K=string), explicit @field declarations are overridden by the parent's index semantics, causing call-non-callable false positives.

Root Cause

In infer_generic_member (crates/emmylua_code_analysis/src/semantic/infer/infer_index/mod.rs, line 688), infer_generic_members_from_super_generics is called before checking the type's own @field declarations. When K=string and the lookup key is a string (e.g. "create"), the parent's table<string, V> index rule returns Some(V) immediately, making the subsequent @field check unreachable.

Changes

File Change
crates/emmylua_code_analysis/src/semantic/infer/infer_index/mod.rs In infer_generic_member, move explicit @field lookup (get_member_item + resolve_type) before infer_generic_members_from_super_generics call (+9 lines)

Rationale

Explicit @field declarations represent the developer's intentional type specification for a specific member. They should always take precedence over inherited generic index semantics, following the standard OOP principle that subclass members override parent behavior.

Example

---@class Container<K,V> : table<K,V>
---@field create fun(data: table): Container<K,V>
local Container = {}
function Container:create(_data) return self end

---@type Container<string, boolean>
local c
c.create({})

Verification

Before fix (commit d7d5dcbf):

---  [1 warning]
warning: Cannot call expression of type `boolean`. [call-non-callable]
  --> :7:1

  6 | local c
  7 | c.create({})

Summary
  1 warning

Check completed with warnings
Check finished

After fix:

No issues found
Check finished

…fer_generic_member

When a generic class inherits table<K,V> (K=string), infer_generic_member calls infer_generic_members_from_super_generics before checking the type's own @field declarations. This causes string-keyed lookups to return V instead of the declared @field type, producing call-non-callable false positives.

Move the explicit @field lookup before the super-generics check so that intentional field declarations always take precedence over inherited table index 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: infer_generic_member Changes

Issue Identified:

Logic Error - Missing Return on Success
The new code block attempts to find a member directly on the type, but if the lookup succeeds, it returns the result. However, if the lookup fails (member not found), execution falls through to the original infer_generic_members_from_super_generics call. This is correct behavior.

Potential Issue: Redundant Lookup
The original code already calls infer_member_by_lookup which may perform similar lookups. Adding this explicit check before the original logic could cause:

  • Duplicate work when the member is found via the new path
  • Different behavior if infer_member_by_lookup has additional logic (e.g., error handling, type coercion)

Missing Error Handling
The get_member_item call returns Option, and the resolve_type call returns Result. If resolve_type fails, the error is silently ignored and execution continues to the original logic. This might hide real issues.

Recommendations:

  1. Consider merging with existing logic - Instead of adding a separate early return, integrate this check into infer_member_by_lookup or the existing flow to avoid duplication.

  2. Add error logging - If resolve_type fails, log the error for debugging:

    if let Ok(member_type) = member_item.resolve_type(db) {
        return Ok(instantiate_type_generic(db, &member_type, &substitutor));
    } else {
        // Log error: failed to resolve member type
    }
  3. Verify priority semantics - Ensure this change doesn't break cases where inherited table index semantics should take priority (e.g., metatable __index). The comment says "explicit @field declarations take priority," but confirm this matches the intended design.

  4. Add tests - Include test cases for:

    • Explicit @field declarations on generic types
    • Cases where both @field and inherited index methods exist
    • Error cases (e.g., unresolvable member types)

Security/Performance:

  • No security issues identified
  • Performance impact is minimal (one extra hash lookup per generic member inference)

@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 updates the generic member inference logic to prioritize explicit @field declarations directly on the type over inherited table index semantics. The reviewer suggested propagating errors from resolve_type using the ? operator rather than silently falling back to super generics, ensuring robust error handling for explicit declarations.

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 +711 to +716
let owner = LuaMemberOwner::Type(base_type_decl_id.clone());
if let Some(member_item) = db.get_member_index().get_member_item(&owner, &lookup.key) {
if let Ok(member_type) = member_item.resolve_type(db) {
return Ok(instantiate_type_generic(db, &member_type, &substitutor));
}
}

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.

medium

If resolve_type fails (returns an Err), the current implementation silently ignores the error and falls back to infer_generic_members_from_super_generics. However, since the member is explicitly declared on the type itself, any resolution failure should be propagated rather than falling back to the parent's generic index semantics (which would re-introduce the exact overriding/shadowing issue this PR aims to fix).

Using the ? operator to propagate the error is consistent with how member resolution is handled in other parts of the codebase (e.g., infer_custom_type_member).

        let owner = LuaMemberOwner::Type(base_type_decl_id.clone());
        if let Some(member_item) = db.get_member_index().get_member_item(&owner, &lookup.key) {
            let member_type = member_item.resolve_type(db)?;
            return Ok(instantiate_type_generic(db, &member_type, &substitutor));
        }

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