Skip to content

Commit 499195d

Browse files
authored
Merge pull request #1098 from lewis6991/fix/separate-unreachable-condition-flow
fix: separate unreachable conditions from never types
2 parents 88170e8 + aa80085 commit 499195d

13 files changed

Lines changed: 784 additions & 73 deletions

File tree

crates/emmylua_code_analysis/src/compilation/analyzer/flow/bind_analyze/stats.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,9 @@ pub fn bind_call_expr_stat(
199199
if let Some(ast) = LuaAst::cast(call_expr.syntax().clone()) {
200200
bind_each_child(binder, ast, current);
201201
}
202-
current
202+
let flow_id = binder.create_node(FlowNodeKind::CallExprStat(call_expr_stat.to_ptr()));
203+
binder.add_antecedent(flow_id, current);
204+
flow_id
203205
}
204206
}
205207

crates/emmylua_code_analysis/src/compilation/test/flow.rs

Lines changed: 227 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -307,6 +307,180 @@ mod test {
307307
assert_eq!(ws.expr_ty("after_guard"), ws.ty("string"));
308308
}
309309

310+
#[test]
311+
fn test_plain_call_condition_keeps_inner_call_prefix_type() {
312+
let mut ws = VirtualWorkspace::new();
313+
let code = r#"
314+
local function a() end
315+
local function b() end
316+
317+
b()
318+
if a() then
319+
b()
320+
inner = b
321+
end
322+
"#;
323+
ws.def(code);
324+
325+
let ty = ws.expr_ty("inner");
326+
assert!(ty.is_function());
327+
328+
let mut diag_ws = VirtualWorkspace::new();
329+
assert!(diag_ws.has_no_diagnostic(DiagnosticCode::CallNonCallable, code));
330+
}
331+
332+
#[test]
333+
fn test_false_call_condition_keeps_inner_unrelated_type() {
334+
let mut ws = VirtualWorkspace::new();
335+
ws.def(
336+
r#"
337+
---@return false
338+
local function always_false()
339+
return false
340+
end
341+
342+
---@type string
343+
local value = "ok"
344+
if always_false() then
345+
inner = value
346+
end
347+
"#,
348+
);
349+
350+
assert_eq!(ws.expr_ty("inner"), ws.ty("string"));
351+
}
352+
353+
#[test]
354+
fn test_true_call_condition_keeps_else_call_prefix_type() {
355+
let mut ws = VirtualWorkspace::new();
356+
let code = r#"
357+
---@return true
358+
local function always_true()
359+
return true
360+
end
361+
362+
local function b() end
363+
if always_true() then
364+
else
365+
b()
366+
end
367+
"#;
368+
369+
assert!(ws.has_no_diagnostic(DiagnosticCode::CallNonCallable, code));
370+
}
371+
372+
#[test]
373+
fn test_false_call_condition_assignment_does_not_contribute_to_merge() {
374+
let mut ws = VirtualWorkspace::new();
375+
ws.def(
376+
r#"
377+
---@return false
378+
local function always_false()
379+
return false
380+
end
381+
382+
local value = "before"
383+
if always_false() then
384+
value = 1
385+
end
386+
after = value
387+
"#,
388+
);
389+
390+
let after = ws.expr_ty("after");
391+
assert_eq!(ws.humanize_type(after), "string");
392+
}
393+
394+
#[test]
395+
fn test_false_call_condition_missing_field_assignment_does_not_contribute_to_merge() {
396+
let mut ws = VirtualWorkspace::new();
397+
ws.def(
398+
r#"
399+
---@return false
400+
local function is_windows()
401+
return false
402+
end
403+
404+
local command = "ls"
405+
local config = {}
406+
if is_windows() then
407+
command = config.windows_command
408+
end
409+
after = command
410+
"#,
411+
);
412+
413+
let after = ws.expr_ty("after");
414+
assert_eq!(ws.humanize_type(after), "string");
415+
}
416+
417+
#[test]
418+
fn test_reachable_assignment_over_never_value_contributes_to_merge() {
419+
let mut ws = VirtualWorkspace::new();
420+
ws.def(
421+
r#"
422+
---@class NeverBox
423+
---@field value never
424+
425+
---@return NeverBox
426+
local function make_box() end
427+
428+
local value = make_box().value
429+
local cond ---@type boolean
430+
if cond then
431+
value = 1
432+
end
433+
after = value
434+
"#,
435+
);
436+
437+
assert_eq!(ws.expr_ty("after"), LuaType::IntegerConst(1));
438+
}
439+
440+
#[test]
441+
fn test_false_call_condition_tag_cast_does_not_contribute_to_merge() {
442+
let mut ws = VirtualWorkspace::new();
443+
ws.def(
444+
r#"
445+
---@return false
446+
local function always_false()
447+
return false
448+
end
449+
450+
local value = "before"
451+
if always_false() then
452+
---@cast value integer
453+
end
454+
after = value
455+
"#,
456+
);
457+
458+
let after = ws.expr_ty("after");
459+
assert_eq!(ws.humanize_type(after), r#""before""#);
460+
}
461+
462+
#[test]
463+
fn test_false_call_condition_doc_assignment_does_not_contribute_to_merge() {
464+
let mut ws = VirtualWorkspace::new();
465+
ws.def(
466+
r#"
467+
---@return false
468+
local function always_false()
469+
return false
470+
end
471+
472+
local value = "before"
473+
if always_false() then
474+
---@type integer
475+
value = 1
476+
end
477+
after = value
478+
"#,
479+
);
480+
481+
assert_eq!(ws.expr_ty("after"), ws.ty("string"));
482+
}
483+
310484
#[test]
311485
fn test_branch_join_keeps_union_when_only_one_side_narrows() {
312486
let mut ws = VirtualWorkspace::new();
@@ -1616,6 +1790,59 @@ end
16161790
assert_eq!(b, LuaType::String);
16171791
}
16181792

1793+
#[test]
1794+
fn test_issue_877_never_return_call_narrows_after_guard() {
1795+
let mut ws = VirtualWorkspace::new();
1796+
1797+
let source = r#"
1798+
---@param _num integer
1799+
local function accept_num(_num)
1800+
end
1801+
1802+
---@return never
1803+
local function panic()
1804+
error("panic!")
1805+
end
1806+
1807+
---@type integer?
1808+
local num
1809+
if num == nil then
1810+
panic()
1811+
end
1812+
1813+
accept_num(num)
1814+
after_guard = num
1815+
"#;
1816+
1817+
assert!(ws.has_no_diagnostic(DiagnosticCode::ParamTypeMismatch, source));
1818+
assert_eq!(ws.expr_ty("after_guard"), LuaType::Integer);
1819+
}
1820+
1821+
#[test]
1822+
fn test_never_return_call_after_branch_statement_narrows_after_guard() {
1823+
let mut ws = VirtualWorkspace::new();
1824+
1825+
ws.def(
1826+
r#"
1827+
---@return never
1828+
local function panic()
1829+
error("panic!")
1830+
end
1831+
1832+
---@type integer?
1833+
local num
1834+
if num == nil then
1835+
local reason = "bad"
1836+
panic()
1837+
end
1838+
1839+
after_guard = num
1840+
"#,
1841+
);
1842+
1843+
assert_eq!(ws.expr_ty("after_guard"), LuaType::Integer);
1844+
}
1845+
16191846
#[test]
16201847
fn test_unknown_type() {
16211848
let mut ws = VirtualWorkspace::new();

crates/emmylua_code_analysis/src/db_index/flow/flow_node.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use emmylua_parser::{
2-
LuaAssignStat, LuaAstNode, LuaAstPtr, LuaChunk, LuaClosureExpr, LuaDocTagCast, LuaExpr,
3-
LuaForStat, LuaFuncStat, LuaSyntaxKind, LuaSyntaxNode,
2+
LuaAssignStat, LuaAstNode, LuaAstPtr, LuaCallExprStat, LuaChunk, LuaClosureExpr, LuaDocTagCast,
3+
LuaExpr, LuaForStat, LuaFuncStat, LuaSyntaxKind, LuaSyntaxNode,
44
};
55
use internment::ArcIntern;
66
use rowan::{TextRange, TextSize};
@@ -44,6 +44,8 @@ pub enum FlowNodeKind {
4444
DeclPosition(TextSize),
4545
/// Variable assignment
4646
Assignment(LuaAstPtr<LuaAssignStat>),
47+
/// Call expression statement
48+
CallExprStat(LuaAstPtr<LuaCallExprStat>),
4749
/// Conditional flow (type guards, existence checks)
4850
TrueCondition(LuaAstPtr<LuaExpr>),
4951
/// Conditional flow (type guards, existence checks)

crates/emmylua_code_analysis/src/db_index/flow/flow_tree.rs

Lines changed: 59 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,18 @@
1+
use std::collections::VecDeque;
2+
13
use hashbrown::{HashMap, HashSet};
24

35
use emmylua_parser::{LuaAstPtr, LuaCallExpr, LuaExpr, LuaSyntaxId};
46
use rowan::TextSize;
57

6-
use crate::{FlowAntecedent, FlowId, FlowNode, LuaDeclId};
8+
use crate::{FlowAntecedent, FlowId, FlowNode, FlowNodeKind, LuaDeclId};
79

810
#[derive(Debug)]
911
pub struct FlowTree {
1012
decl_bind_expr_ref: HashMap<LuaDeclId, LuaAstPtr<LuaExpr>>,
1113
decl_multi_return_ref: HashMap<LuaDeclId, Vec<DeclMultiReturnRefAt>>,
1214
flow_nodes: Vec<FlowNode>,
15+
has_tag_cast: bool,
1316
multiple_antecedents: Vec<Vec<FlowId>>,
1417
// labels: HashMap<LuaClosureId, HashMap<SmolStr, FlowId>>,
1518
bindings: HashMap<LuaSyntaxId, FlowId>,
@@ -24,10 +27,14 @@ impl FlowTree {
2427
// labels: HashMap<LuaClosureId, HashMap<SmolStr, FlowId>>,
2528
bindings: HashMap<LuaSyntaxId, FlowId>,
2629
) -> Self {
30+
let has_tag_cast = flow_nodes
31+
.iter()
32+
.any(|node| matches!(node.kind, FlowNodeKind::TagCast(_)));
2733
Self {
2834
decl_bind_expr_ref,
2935
decl_multi_return_ref,
3036
flow_nodes,
37+
has_tag_cast,
3138
multiple_antecedents,
3239
bindings,
3340
}
@@ -41,12 +48,34 @@ impl FlowTree {
4148
self.flow_nodes.get(flow_id.0 as usize)
4249
}
4350

51+
pub fn has_tag_cast(&self) -> bool {
52+
self.has_tag_cast
53+
}
54+
4455
pub fn get_multi_antecedents(&self, id: u32) -> Option<&[FlowId]> {
4556
self.multiple_antecedents
4657
.get(id as usize)
4758
.map(|v| v.as_slice())
4859
}
4960

61+
/// Returns the first backward-reachable flow node shared by all starting flows.
62+
pub fn get_nearest_common_antecedent(&self, flow_ids: &[FlowId]) -> Option<FlowId> {
63+
let (first_flow_id, rest_flow_ids) = flow_ids.split_first()?;
64+
let first_antecedents = self.collect_antecedents(*first_flow_id);
65+
let rest_antecedents = rest_flow_ids
66+
.iter()
67+
.map(|flow_id| {
68+
self.collect_antecedents(*flow_id)
69+
.into_iter()
70+
.collect::<HashSet<_>>()
71+
})
72+
.collect::<Vec<_>>();
73+
74+
first_antecedents
75+
.into_iter()
76+
.find(|flow_id| rest_antecedents.iter().all(|set| set.contains(flow_id)))
77+
}
78+
5079
pub fn get_decl_ref_expr(&self, decl_id: &LuaDeclId) -> Option<LuaAstPtr<LuaExpr>> {
5180
self.decl_bind_expr_ref.get(decl_id).cloned()
5281
}
@@ -263,6 +292,35 @@ impl FlowTree {
263292
}
264293
}
265294
}
295+
296+
fn collect_antecedents(&self, flow_id: FlowId) -> Vec<FlowId> {
297+
let mut antecedents = Vec::new();
298+
let mut pending = VecDeque::from([flow_id]);
299+
let mut visited = HashSet::new();
300+
while let Some(flow_id) = pending.pop_front() {
301+
if !visited.insert(flow_id) {
302+
continue;
303+
}
304+
305+
antecedents.push(flow_id);
306+
let Some(flow_node) = self.get_flow_node(flow_id) else {
307+
continue;
308+
};
309+
match flow_node.antecedent.as_ref() {
310+
Some(FlowAntecedent::Single(antecedent_flow_id)) => {
311+
pending.push_back(*antecedent_flow_id);
312+
}
313+
Some(FlowAntecedent::Multiple(multi_id)) => {
314+
if let Some(branch_flow_ids) = self.get_multi_antecedents(*multi_id) {
315+
pending.extend(branch_flow_ids.iter().copied());
316+
}
317+
}
318+
None => {}
319+
}
320+
}
321+
322+
antecedents
323+
}
266324
}
267325

268326
#[derive(Debug, Clone)]

crates/emmylua_code_analysis/src/diagnostic/checker/assign_type_mismatch.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,10 @@ fn check_name_expr(
104104
}
105105
_ => None,
106106
};
107+
let source_type = source_type.map(|source_type| {
108+
semantic_model
109+
.apply_assignment_target_casts(LuaExpr::NameExpr(name_expr.clone()), source_type)
110+
});
107111
check_assign_type_mismatch(
108112
context,
109113
semantic_model,
@@ -139,6 +143,10 @@ fn check_index_expr(
139143
false,
140144
)
141145
.ok();
146+
let source_type = source_type.map(|source_type| {
147+
semantic_model
148+
.apply_assignment_target_casts(LuaExpr::IndexExpr(index_expr.clone()), source_type)
149+
});
142150

143151
check_assign_type_mismatch(
144152
context,

0 commit comments

Comments
 (0)