[mareklangiewicz]

Summary

Fixes #1319. Neovim declares vim.env as ---@type table<string, string?> over setmetatable({}, { __index = function ... end }), and every if vim.env.X then was reported "always falsy". Three causes, all fixed:

 type_of_member_impl / shell member_type
-  call value's VM type wins; doc_type_syntax never read
+  a declared ---@type wins (as decl_type already does for locals)
 signature_returns
-  a return it cannot type (os.getenv(k)) is merged away → returns only nil
+  an unknown row makes the shell result unknown → VM return inference (string?)
 setmetatable builtin
-  types the __index closure → re-enters a narrowing in progress, caches truncated nil
+  only checks whether __index is a named type (metatable_index_is_closure)

Plus a separate first commit that now only adds a test: a ---@field [string] any class reported inject-field for g.aaa = true. After rebasing onto 1b6df85 the fix was no longer needed, because e1cd626 ("unify index signature resolution") replaced that check with applicable_index_infos. The test still pins the case.

Related: with this PR vim.env.X reads its declared table<string, string?>, but on main an index read of table<string, V> is unknown (#1334), so it shows unknown until that is fixed.

Evidence

• Before (main 1b6df85 plus only this PR's tests): test_issue_1319_declared_type_wins_over_metatable_index, test_issue_1319_guarded_return_keeps_both_paths (unnecessary_if_test.rs) and test_member_declared_object_type_keeps_fields (member_infer_test.rs) fail. test_index_signature_class_allows_name_keys already passes on main.
  After: all pass, and so do the emmylua_code_analysis and emmylua_ls test suites.

Merge Danger

Door: two-way

Blast Radius: member type inference wherever a member has both a ---@type and a call value. This also affects return inference of functions with an untyped return row.

Written with help from Claude Code.

🤖 Generated with Claude Code

https://claude.ai/code/session_01T88qMNChLeJksJGRszr7na

[github-actions]

Code Review

Summary

The changes fix two related type-inference bugs: (1) index-signature detection now uses the is_index_signature flag instead of string-matching [, and (2) declared ---@type annotations on runtime members now take precedence over inferred value types, with a guard against __index closures causing reentrant inference. Tests are added for both.

Issues & Recommendations

1. signature_returns: unknown row poisons the entire result (potential over-correction)

crates/emmylua_code_analysis/src/semantic_db/query.rs

has_unknown_row |= row.is_unknown();
rows.merge(&row);
...
if has_unknown_row {
    rows = TypeShell::unknown();
}

• Concern: If any return statement has an untypeable expression, the whole function's return type becomes Unknown, discarding otherwise-valid inferred returns. This is a broad behavioral change that could regress inference for functions with many returns where only one is unresolvable (e.g., a single return some_std_call() among several well-typed returns).
• Recommendation: Consider whether the fallback should be scoped to the specific arity/slot that is unknown, or whether Unknown should only be forced when the first return slot is unknown (since callers typically consume the first). At minimum, add a test covering a function with one unknown return among several known ones to document the intended behavior.

2. metatable_index_is_closure only handles TableConst literals

crates/emmylua_code_analysis/src/semantic_model/infer/vm.rs

let LuaType::TableConst(table) = metatable else {
    return false;
};

• Concern: The guard only detects __index = function ... end when the metatable is a literal table. If the metatable is a variable (local mt = { __index = function... }) or a class instance, the reentrancy risk described in the comment still exists. This may be acceptable as a targeted fix, but the comment ("never gives the instance a named type") overstates the guarantee.
• Recommendation: Either broaden detection (resolve the metatable type to its members) or soften the comment to clarify it only covers literal metatables. Also consider whether __index assigned a named function reference (not a closure) should be excluded too.

3. member_type: doc-type precedence may shadow legitimate value inference

crates/emmylua_code_analysis/src/semantic_db/query.rs

if !matches!(member.owner, SemanticId::TypeDef(_))
    && let Some(type_syntax) = member.doc_type_syntax
{
    let shell = lower_doc_type(db, file, type_syntax, &[]);
    if !shell.is_unknown() {
        return shell;
    }
}

• Concern: This is consistent with decl_type behavior, which is good. However, the !shell.is_unknown() fallback means a malformed/unsupported doc type silently falls through to value inference — reasonable, but worth a test to lock in that behavior.
• Minor: The comment says "as it does for a local declaration in decl_type" — good for maintainers; consider linking the function name precisely (decl_type vs. the actual symbol) so it's greppable.

4. Duplicated member-lookup pattern

Both check_field.rs and vm.rs now use the same chain:

semantic_model
    .file_facts_of(member_ref.file_id)
    .and_then(|facts| facts.member_by_id(&member_ref.id))

• Recommendation: Consider a helper on SemanticModel (e.g., member_def_of(&MemberRef)) to avoid repeating this boilerplate and to centralize the file_facts_of + member_by_id idiom. This improves readability and reduces the chance of inconsistent handling.

5. Test quality

• test_issue_1319_guarded_return_keeps_both_paths asserts humanize_type(fun).ends_with("-> string?") — ends_with is brittle to formatting changes in humanize_type. Prefer exact equality or a structural type assertion if available.
• The new tests reference issue #1319 in comments but the PR description isn't shown; ensure the issue is linked in the PR body for traceability.
• test_index_signature_class_allows_name_keys is good and directly targets the starts_with('[') → flag change.

6. Behavioral note on is_index_signature

The switch from member.name.starts_with('[') to member.is_index_signature is a correctness improvement (the old check would also match a field literally named [foo] and miss index signatures whose name is the key type). No action needed, but confirm is_index_signature is populated for all index-signature forms (e.g., [integer], [string], and mapped aliases) — the is_mapped_alias branch remains separate, which is fine.

Verdict

The changes are well-targeted and well-commented, with tests. The main risk is item 1 (unknown-row poisoning) being broader than necessary. Items 2–4 are maintainability/robustness suggestions rather than blockers.