refactor generic

[xuhuanzy]

No description provided.

[github-actions]

Here is my code review of the changes:

Summary

This is a significant refactoring of the generic type inference system in an EmmyLua code analysis tool. The changes focus on improving literal type preservation through generic type inference, restructuring the type substitution system, and adding pre-analysis for table fields in call arguments.

Key Changes

1. Literal Type Preservation (Major improvement)

The most impactful change is the introduction of TplCandidateSource enum and finalize_inferred_tpl_candidate function. This allows the type system to distinguish between:

• Plain - regular type inference that should preserve literals at top level
• ConstPreserving - explicit const type references that always preserve literals
• Finalized - already processed types

This fixes the issue where generic functions like id("hello") would return string instead of "hello".

2. TypeSubstitutor Refactoring (Structural improvement)

The TypeSubstitutor has been significantly restructured:

• Replaced multiple insert_* methods with a single bind method using TplBinding enum
• Added finalize_inferred_types method to process all inferred types after inference is complete
• Renamed add_need_infer_tpls to prepare_inference_slots for clarity
• Renamed is_infer_all_tpl to has_unresolved_inference_slots

3. Pre-analysis of Table Fields (Performance optimization)

Added pre_analyze_call_arg_table_fields function that eagerly analyzes table fields in call arguments before the main inference. This helps with type caching and avoids redundant work.

4. Widening Context (New feature)

Introduced WideningContext enum to control how literal types are widened in different contexts:

• Root - preserves literals
• UnionMember, ObjectProperty, etc. - widens literals to their base types

5. Test Updates

• Updated tests to expect literal types instead of widened types
• Added new tests for literal preservation scenarios
• Added #[cfg(feature = "full-test")] to a performance-sensitive test

Potential Issues

1. Performance Concern

The finalize_inferred_types method clones the entire TypeSubstitutor for each generic type parameter:

let substitutor = self.clone();

This could be expensive for functions with many generic parameters. Consider if this clone is necessary or if a reference would suffice.

2. Recursion Risk

The widen_finalized_candidate_type function recursively processes nested types. For deeply nested or recursive types, this could cause stack overflow. Consider adding a recursion depth limit or iterative approach.

3. Missing Error Handling

In resolve_unpack_operands, the function uses unwrap_or_else as a fallback but doesn't log or handle the case where a TplRef lookup fails. This could silently produce incorrect types.

4. Code Duplication

The widen_finalized_candidate_type and widen_type_with_context functions have very similar structure. Consider refactoring to reduce duplication.

5. Test Coverage

While many new tests are added, there's no test for:

• Nested generic types with literal preservation
• Edge cases with recursive type aliases
• Performance regression tests

Recommendations

1. Add recursion guards to widen_finalized_candidate_type and widen_type_with_context to prevent stack overflow on deeply nested types.

2. Consider lazy evaluation for finalize_inferred_types - instead of eagerly finalizing all types, consider finalizing on-demand when types are accessed.

3. Add logging in resolve_unpack_operands when fallback is used, to help debug type inference issues.

4. Refactor widening functions to share common logic:

fn widen_type_internal(ty: LuaType, context: WideningContext, is_finalized: bool) -> LuaType {
    // Common logic here
}

5. Add performance benchmarks for the new inference pipeline, especially for functions with many generic parameters.

Positive Aspects

• The literal type preservation is a significant improvement for type accuracy
• The refactoring makes the type substitution system more maintainable
• The pre-analysis optimization should improve performance for table-heavy code
• Good test coverage for the new literal preservation behavior

Overall, this is a well-structured refactoring that improves type inference accuracy while maintaining backward compatibility. The main concerns are around performance and recursion safety, which should be addressed before merging.

[gemini-code-assist]

Code Review

This pull request refactors the generic type inference system to improve the preservation of literal types when they appear at the top level of return types or are subject to primitive constraints. It introduces a new type widening mechanism, refactors the TypeSubstitutor to include a finalization phase, and enhances the handling of std.Unpack with generic defaults. Feedback from the review identifies a high-severity risk of infinite recursion in the table_const_to_object function and suggests increasing the precision of top-level template detection by recursing into unions and intersections. Other suggestions include optimizing the finalization loop in TypeSubstitutor and extending pre-analysis logic to cover nested call expressions.

[gemini-code-assist]

The table_const_to_object function is susceptible to infinite recursion if a table constant contains a circular reference to itself (e.g., local t = {}; t.self = t). Since widen_finalized_candidate_type calls table_const_to_object for nested TableConst types, this will lead to a stack overflow. Consider adding a recursion guard using a HashSet of visited table_ids.

[gemini-code-assist]

The is_tpl_at_top_level_with_guard function does not recurse into Union or Intersection types. If a template is part of a top-level union in the return type (e.g., ---@return T | nil), it will not be recognized as being at the top level, causing literals inferred for T to be widened unnecessarily. Recursing into these types would improve the precision of literal preservation.

    match ty {
        LuaType::TplRef(tpl) | LuaType::ConstTplRef(tpl) => tpl.get_tpl_id() == tpl_id,
        LuaType::Union(union) => union
            .into_vec()
            .iter()
            .any(|ty| is_tpl_at_top_level_with_guard(db, ty, tpl_id, visited_aliases)),
        LuaType::Intersection(intersection) => intersection
            .get_types()
            .iter()
            .any(|ty| is_tpl_at_top_level_with_guard(db, ty, tpl_id, visited_aliases)),
        LuaType::Generic(generic) => {
            let type_decl_id = generic.get_base_type_id_ref();
            let Some(alias_param) = 
                get_transparent_alias_param_index(db, type_decl_id, visited_aliases)
            else {
                return false;
            };

            generic.get_params().get(alias_param).is_some_and(|param| {
                is_tpl_at_top_level_with_guard(db, param, tpl_id, visited_aliases)
            })
        }
        _ => false,
    }

[gemini-code-assist]

The pre-analysis logic for table fields in call arguments is currently shallow. It only processes TableExpr arguments directly passed to a call, but does not recurse into nested calls or table fields that might contain further calls with table arguments (e.g., f(g({x=1})) or { a = f({y=2}) }). This may result in missing type information during generic inference for complex nested expressions. Consider making these pre-analysis helpers more comprehensive by traversing nested expressions.