From 88cfc64f85704be8734f062902437e3db9c587e2 Mon Sep 17 00:00:00 2001 From: "Henning P. Schmiedehausen" Date: Mon, 12 Oct 2026 22:50:37 -0700 Subject: [PATCH 1/2] fix(diagnostics): respect narrowed owners in union member arguments --- .../src/check/test/mod.rs | 1 + .../check/test/union_member_argument_test.rs | 143 ++++++++++++++++++ .../src/semantic_model/access.rs | 4 +- 3 files changed, 147 insertions(+), 1 deletion(-) create mode 100644 crates/emmylua_code_analysis/src/check/test/union_member_argument_test.rs diff --git a/crates/emmylua_code_analysis/src/check/test/mod.rs b/crates/emmylua_code_analysis/src/check/test/mod.rs index 94f378922..73d7473b7 100644 --- a/crates/emmylua_code_analysis/src/check/test/mod.rs +++ b/crates/emmylua_code_analysis/src/check/test/mod.rs @@ -43,6 +43,7 @@ mod unbalanced_assignments_test; mod undefined_doc_param_test; mod undefined_field_test; mod undefined_global_test; +mod union_member_argument_test; mod unknown_doc_tag_test; mod unnecessary_assert_test; mod unnecessary_if_test; diff --git a/crates/emmylua_code_analysis/src/check/test/union_member_argument_test.rs b/crates/emmylua_code_analysis/src/check/test/union_member_argument_test.rs new file mode 100644 index 000000000..a3e45c2a2 --- /dev/null +++ b/crates/emmylua_code_analysis/src/check/test/union_member_argument_test.rs @@ -0,0 +1,143 @@ +use crate::{DiagnosticCode, VirtualWorkspace}; +use googletest::prelude::*; +use lsp_types::Diagnostic; +use tokio_util::sync::CancellationToken; + +const TYPES: &str = r#" +---@alias uint32 integer +---@class OneGoal +---@field kind "one" +---@field start_index? uint32 +---@class AllGoals +---@field kind "all" +---@field goals integer[] +---@return OneGoal|AllGoals +local function request_path() end +---@return OneGoal +local function request_one() end +---@param index integer +local function accept_index(index) end +"#; + +fn mismatches(source: &str) -> Vec { + let mut ws = VirtualWorkspace::new_with_init_std_lib(); + ws.analysis + .diagnostic + .enable_only(DiagnosticCode::ParamTypeMismatch); + let file = ws.def(&format!("{TYPES}\n{source}")); + ws.analysis + .diagnose_file(file, CancellationToken::new()) + .expect("parameter diagnostics") +} + +#[gtest] +fn cast_owner_before_passing_optional_member() { + let diagnostics = mismatches( + r#" + ---@param values string[] + local function remove_value(values) + local result = request_path() + ---@cast result OneGoal + ---@cast result.start_index uint32 + table.remove(values, result.start_index) + if values[1] then values[2] = "updated" end + table.remove(values, result.start_index) + end + "#, + ); + expect_that!(diagnostics, is_empty()); +} + +#[gtest] +fn guard_and_assignment_narrow_member_owner() { + let diagnostics = mismatches( + r#" + local result = request_path() + if result.kind == "one" then + ---@cast result.start_index uint32 + accept_index(result.start_index) + end + result = request_one() + ---@cast result.start_index uint32 + accept_index(result.start_index) + "#, + ); + expect_that!(diagnostics, is_empty()); +} + +#[gtest] +fn narrowed_owner_union_checks_only_remaining_components() { + let diagnostics = mismatches( + r#" + ---@class AnotherGoal + ---@field start_index uint32 + ---@param result OneGoal|AllGoals|AnotherGoal + local function narrowed_union(result) + ---@cast result OneGoal|AnotherGoal + table.remove({}, result.start_index) + end + "#, + ); + expect_that!(diagnostics, is_empty()); +} + +#[gtest] +fn nested_owner_cast_is_respected() { + let diagnostics = mismatches( + r#" + ---@class Wrapper + ---@field result OneGoal|AllGoals + ---@param wrapper Wrapper + local function remove_value(wrapper) + ---@cast wrapper.result OneGoal + table.remove({}, wrapper.result.start_index) + end + "#, + ); + expect_that!(diagnostics, is_empty()); +} + +#[gtest] +fn unnarrowed_union_still_reports_missing_member() { + let diagnostics = mismatches( + r#" + local result = request_path() + accept_index(result.start_index) + "#, + ); + assert_that!(diagnostics.len(), eq(1)); + expect_that!(diagnostics[0].message, contains_substring("nil")); +} + +#[gtest] +fn branch_guard_does_not_hide_missing_members_on_other_paths() { + let diagnostics = mismatches( + r#" + local result = request_path() + if result.kind == "one" then + ---@cast result.start_index uint32 + accept_index(result.start_index) + else + accept_index(result.start_index) + end + accept_index(result.start_index) + "#, + ); + expect_that!(diagnostics.len(), eq(2)); +} + +#[gtest] +fn narrowed_owner_still_checks_the_actual_field_type() { + let diagnostics = mismatches( + r#" + ---@param text string + local function accept_text(text) end + local result = request_path() + ---@cast result OneGoal + ---@cast result.start_index uint32 + accept_text(result.start_index) + "#, + ); + assert_that!(diagnostics.len(), eq(1)); + expect_that!(diagnostics[0].message, contains_substring("string")); +} diff --git a/crates/emmylua_code_analysis/src/semantic_model/access.rs b/crates/emmylua_code_analysis/src/semantic_model/access.rs index 5e719e848..c0b29f185 100644 --- a/crates/emmylua_code_analysis/src/semantic_model/access.rs +++ b/crates/emmylua_code_analysis/src/semantic_model/access.rs @@ -516,7 +516,9 @@ impl<'db> SemanticModel<'db> { let Some(prefix) = index_expr.get_prefix_expr() else { return false; }; - let prefix_ty = self.prefix_type_for_member_resolution(&prefix); + // Argument diagnostics must use the owner after casts, guards and assignments. + // Keep the non-flow helper for member resolution itself to avoid recursive lookup. + let prefix_ty = self.type_of_expr_at(prefix.get_syntax_id(), index_expr.get_position()); let LuaType::Union(union) = &prefix_ty else { return false; }; From 3b742971bf8a6fe162f8ed058094c6efc2c3141d Mon Sep 17 00:00:00 2001 From: "Henning P. Schmiedehausen" Date: Mon, 12 Oct 2026 22:57:47 -0700 Subject: [PATCH 2/2] test(diagnostics): cover narrowed union edge cases --- .../check/test/union_member_argument_test.rs | 113 +++++++++++++++--- .../src/semantic_model/access.rs | 5 +- 2 files changed, 102 insertions(+), 16 deletions(-) diff --git a/crates/emmylua_code_analysis/src/check/test/union_member_argument_test.rs b/crates/emmylua_code_analysis/src/check/test/union_member_argument_test.rs index a3e45c2a2..af2c63a55 100644 --- a/crates/emmylua_code_analysis/src/check/test/union_member_argument_test.rs +++ b/crates/emmylua_code_analysis/src/check/test/union_member_argument_test.rs @@ -1,8 +1,9 @@ use crate::{DiagnosticCode, VirtualWorkspace}; use googletest::prelude::*; -use lsp_types::Diagnostic; +use lsp_types::{Diagnostic, NumberOrString}; use tokio_util::sync::CancellationToken; +// Each test gets a fresh workspace; these shared classes are defined exactly once. const TYPES: &str = r#" ---@alias uint32 integer ---@class OneGoal @@ -25,9 +26,15 @@ fn mismatches(source: &str) -> Vec { .diagnostic .enable_only(DiagnosticCode::ParamTypeMismatch); let file = ws.def(&format!("{TYPES}\n{source}")); - ws.analysis + let diagnostics = ws + .analysis .diagnose_file(file, CancellationToken::new()) - .expect("parameter diagnostics") + .expect("parameter diagnostics"); + let code = Some(NumberOrString::String( + DiagnosticCode::ParamTypeMismatch.get_name().to_owned(), + )); + assert!(diagnostics.iter().all(|diagnostic| diagnostic.code == code)); + diagnostics } #[gtest] @@ -45,7 +52,7 @@ fn cast_owner_before_passing_optional_member() { end "#, ); - expect_that!(diagnostics, is_empty()); + assert_that!(diagnostics, is_empty()); } #[gtest] @@ -62,7 +69,7 @@ fn guard_and_assignment_narrow_member_owner() { accept_index(result.start_index) "#, ); - expect_that!(diagnostics, is_empty()); + assert_that!(diagnostics, is_empty()); } #[gtest] @@ -72,13 +79,14 @@ fn narrowed_owner_union_checks_only_remaining_components() { ---@class AnotherGoal ---@field start_index uint32 ---@param result OneGoal|AllGoals|AnotherGoal - local function narrowed_union(result) + ---@param values string[] + local function narrowed_union(result, values) ---@cast result OneGoal|AnotherGoal - table.remove({}, result.start_index) + table.remove(values, result.start_index) end "#, ); - expect_that!(diagnostics, is_empty()); + assert_that!(diagnostics, is_empty()); } #[gtest] @@ -88,13 +96,14 @@ fn nested_owner_cast_is_respected() { ---@class Wrapper ---@field result OneGoal|AllGoals ---@param wrapper Wrapper - local function remove_value(wrapper) + ---@param values string[] + local function remove_value(wrapper, values) ---@cast wrapper.result OneGoal - table.remove({}, wrapper.result.start_index) + table.remove(values, wrapper.result.start_index) end "#, ); - expect_that!(diagnostics, is_empty()); + assert_that!(diagnostics, is_empty()); } #[gtest] @@ -106,7 +115,7 @@ fn unnarrowed_union_still_reports_missing_member() { "#, ); assert_that!(diagnostics.len(), eq(1)); - expect_that!(diagnostics[0].message, contains_substring("nil")); + assert_that!(diagnostics[0].message, contains_substring("nil")); } #[gtest] @@ -123,7 +132,7 @@ fn branch_guard_does_not_hide_missing_members_on_other_paths() { accept_index(result.start_index) "#, ); - expect_that!(diagnostics.len(), eq(2)); + assert_that!(diagnostics.len(), eq(2)); } #[gtest] @@ -139,5 +148,81 @@ fn narrowed_owner_still_checks_the_actual_field_type() { "#, ); assert_that!(diagnostics.len(), eq(1)); - expect_that!(diagnostics[0].message, contains_substring("string")); + assert_that!(diagnostics[0].message, contains_substring("string")); +} + +#[gtest] +fn cast_can_widen_non_union_owner_to_union_with_missing_member() { + let diagnostics = mismatches( + r#" + ---@param result OneGoal + local function widened_owner(result) + ---@cast result OneGoal|AllGoals + accept_index(result.start_index) + end + "#, + ); + assert_that!(diagnostics.len(), eq(1)); + assert_that!(diagnostics[0].message, contains_substring("nil")); +} + +#[gtest] +fn cast_to_unrelated_owner_with_incompatible_field_is_rejected() { + let diagnostics = mismatches( + r#" + ---@class TextGoal + ---@field start_index string + local result = request_path() + ---@cast result TextGoal + accept_index(result.start_index) + "#, + ); + assert_that!(diagnostics.len(), eq(1)); +} + +#[gtest] +fn wide_union_cast_keeps_only_selected_owner() { + let mut source = String::new(); + let mut types = vec!["OneGoal".to_owned()]; + for index in 0..16 { + source.push_str(&format!( + "---@class OtherGoal{index}\n---@field other{index} boolean\n" + )); + types.push(format!("OtherGoal{index}")); + } + source.push_str(&format!( + r#" + ---@param result {} + ---@param values string[] + local function wide_union(result, values) + ---@cast result OneGoal + table.remove(values, result.start_index) + end + "#, + types.join("|") + )); + assert_that!(mismatches(&source), is_empty()); +} + +#[test] +#[ntest::timeout(5000)] +fn recursive_class_union_member_queries_terminate() { + let prefix = format!("node{}", ".next".repeat(16)); + let source = format!( + r#" + ---@class WithIndex + ---@field next WithIndex|WithoutIndex + ---@field start_index integer + ---@class WithoutIndex + ---@field next WithIndex|WithoutIndex + ---@field other boolean + ---@param node WithIndex|WithoutIndex + local function recursive_owner(node) + ---@cast {prefix} WithIndex + accept_index({prefix}.start_index) + end + "# + ); + let diagnostics = mismatches(&source); + assert!(diagnostics.is_empty(), "{diagnostics:#?}"); } diff --git a/crates/emmylua_code_analysis/src/semantic_model/access.rs b/crates/emmylua_code_analysis/src/semantic_model/access.rs index c0b29f185..92dd18ae2 100644 --- a/crates/emmylua_code_analysis/src/semantic_model/access.rs +++ b/crates/emmylua_code_analysis/src/semantic_model/access.rs @@ -516,8 +516,9 @@ impl<'db> SemanticModel<'db> { let Some(prefix) = index_expr.get_prefix_expr() else { return false; }; - // Argument diagnostics must use the owner after casts, guards and assignments. - // Keep the non-flow helper for member resolution itself to avoid recursive lookup. + // This argument-only check needs the owner after casts, guards and assignments. + // type_of_expr_at caches by syntax/offset. Do not skip declared non-unions: + // casts and assignments can widen them into unions with missing members. let prefix_ty = self.type_of_expr_at(prefix.get_syntax_id(), index_expr.get_position()); let LuaType::Union(union) = &prefix_ty else { return false;