Skip to content

Commit 3e26dbd

Browse files
committed
fix: separate unreachable conditions from never types
Truthiness checks for call conditions treated an impossible branch as `Result(Never)`. That made the queried symbol look like `never` while walking the branch, so unrelated callable values inside impossible `if`/`else` bodies could be narrowed to non-callable. Add an explicit unreachable condition action and handle it by query mode. Normal point queries continue past an unreachable condition edge so unrelated symbols keep their declared types. Merge-branch queries finish with `never`, so unreachable branch predecessors still drop out of the merged type. Preserve merge-branch mode through assignment, tag-cast, and correlated condition subqueries. That prevents assignments and casts inside impossible branches from contributing their statement-local type effects to the final merge. Rename flow modes around their actual use and keep single-predecessor branch labels out of merge mode. Add coverage for plain call conditions, always-false and always-true call-condition branches, and unreachable assignment/cast merge contributions. Assisted-by: Codex
1 parent d7d5dcb commit 3e26dbd

4 files changed

Lines changed: 199 additions & 34 deletions

File tree

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

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -304,6 +304,112 @@ mod test {
304304
assert_eq!(ws.expr_ty("after_guard"), ws.ty("string"));
305305
}
306306

307+
#[test]
308+
fn test_plain_call_condition_keeps_inner_call_prefix_type() {
309+
let mut ws = VirtualWorkspace::new();
310+
let code = r#"
311+
local function a() end
312+
local function b() end
313+
314+
b()
315+
if a() then
316+
b()
317+
inner = b
318+
end
319+
"#;
320+
ws.def(code);
321+
322+
let ty = ws.expr_ty("inner");
323+
assert!(ty.is_function());
324+
325+
let mut diag_ws = VirtualWorkspace::new();
326+
assert!(diag_ws.has_no_diagnostic(DiagnosticCode::CallNonCallable, code));
327+
}
328+
329+
#[test]
330+
fn test_false_call_condition_keeps_inner_unrelated_type() {
331+
let mut ws = VirtualWorkspace::new();
332+
ws.def(
333+
r#"
334+
---@return false
335+
local function always_false()
336+
return false
337+
end
338+
339+
---@type string
340+
local value = "ok"
341+
if always_false() then
342+
inner = value
343+
end
344+
"#,
345+
);
346+
347+
assert_eq!(ws.expr_ty("inner"), ws.ty("string"));
348+
}
349+
350+
#[test]
351+
fn test_true_call_condition_keeps_else_call_prefix_type() {
352+
let mut ws = VirtualWorkspace::new();
353+
let code = r#"
354+
---@return true
355+
local function always_true()
356+
return true
357+
end
358+
359+
local function b() end
360+
if always_true() then
361+
else
362+
b()
363+
end
364+
"#;
365+
366+
assert!(ws.has_no_diagnostic(DiagnosticCode::CallNonCallable, code));
367+
}
368+
369+
#[test]
370+
fn test_false_call_condition_assignment_does_not_contribute_to_merge() {
371+
let mut ws = VirtualWorkspace::new();
372+
ws.def(
373+
r#"
374+
---@return false
375+
local function always_false()
376+
return false
377+
end
378+
379+
local value = "before"
380+
if always_false() then
381+
value = 1
382+
end
383+
after = value
384+
"#,
385+
);
386+
387+
let after = ws.expr_ty("after");
388+
assert_eq!(ws.humanize_type(after), "string");
389+
}
390+
391+
#[test]
392+
fn test_false_call_condition_tag_cast_does_not_contribute_to_merge() {
393+
let mut ws = VirtualWorkspace::new();
394+
ws.def(
395+
r#"
396+
---@return false
397+
local function always_false()
398+
return false
399+
end
400+
401+
local value = "before"
402+
if always_false() then
403+
---@cast value integer
404+
end
405+
after = value
406+
"#,
407+
);
408+
409+
let after = ws.expr_ty("after");
410+
assert_eq!(ws.humanize_type(after), r#""before""#);
411+
}
412+
307413
#[test]
308414
fn test_branch_join_keeps_union_when_only_one_side_narrows() {
309415
let mut ws = VirtualWorkspace::new();

crates/emmylua_code_analysis/src/semantic/cache/mod.rs

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -26,14 +26,12 @@ pub(in crate::semantic) struct FlowAssignmentInfo {
2626

2727
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
2828
pub(in crate::semantic) enum FlowMode {
29-
WithConditions,
30-
WithoutConditions,
31-
}
32-
33-
impl FlowMode {
34-
pub fn uses_conditions(self) -> bool {
35-
matches!(self, Self::WithConditions)
36-
}
29+
Normal,
30+
// Query one predecessor of a merge label; if that walk reaches an
31+
// unreachable condition edge, it contributes `never` to the merged type.
32+
MergeBranch,
33+
// Re-query assignment antecedents without applying condition narrows.
34+
IgnoreConditions,
3735
}
3836

3937
#[derive(Debug, Default)]

crates/emmylua_code_analysis/src/semantic/infer/narrow/condition_flow/mod.rs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,7 @@ pub(in crate::semantic) enum CorrelatedDiscriminantNarrow {
121121
#[derive(Debug, Clone)]
122122
pub(in crate::semantic) enum ConditionFlowAction {
123123
Continue,
124+
Unreachable,
124125
Result(LuaType),
125126
Pending(PendingConditionNarrow),
126127
NeedExprType {
@@ -712,12 +713,12 @@ pub(in crate::semantic::infer::narrow) fn resolve_expr_type_continuation(
712713
condition_flow,
713714
),
714715
ExprTypeContinuation::Truthiness { condition_flow } => Ok(match condition_flow {
715-
_ if expr_type.is_never() => ConditionFlowAction::Result(LuaType::Never),
716+
_ if expr_type.is_never() => ConditionFlowAction::Unreachable,
716717
InferConditionFlow::TrueCondition if expr_type.is_always_falsy() => {
717-
ConditionFlowAction::Result(LuaType::Never)
718+
ConditionFlowAction::Unreachable
718719
}
719720
InferConditionFlow::FalseCondition if expr_type.is_always_truthy() => {
720-
ConditionFlowAction::Result(LuaType::Never)
721+
ConditionFlowAction::Unreachable
721722
}
722723
_ => ConditionFlowAction::Continue,
723724
}),

crates/emmylua_code_analysis/src/semantic/infer/narrow/get_type_at_flow.rs

Lines changed: 83 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,7 @@ use crate::{
3636
};
3737

3838
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
39-
// One cached flow query: one ref at one flow node, optionally without replaying
40-
// pending condition narrows.
39+
// One cached flow query: one ref at one flow node under one flow mode.
4140
// Example: "what is `x` at flow 42, with current guards applied?"
4241
struct FlowQuery {
4342
var_ref_id: VarRefId,
@@ -52,7 +51,7 @@ impl FlowQuery {
5251
var_ref_id: var_ref_id.clone(),
5352
var_cache_idx: get_flow_cache_var_ref_id(cache, var_ref_id),
5453
flow_id,
55-
mode: FlowMode::WithConditions,
54+
mode: FlowMode::Normal,
5655
}
5756
}
5857

@@ -583,10 +582,15 @@ impl<'a> FlowTypeEngine<'a> {
583582
// Branches are resumed from the end because the initial merge setup
584583
// schedules the last incoming branch first.
585584
let branch_idx = next_pending_idx - 1;
585+
// The remaining predecessor is still a merge contribution. Preserve
586+
// MergeBranch so an unreachable condition edge contributes `never`
587+
// instead of continuing to the type from before the merge.
588+
let branch_mode = match walk.query.mode {
589+
FlowMode::Normal | FlowMode::MergeBranch => FlowMode::MergeBranch,
590+
FlowMode::IgnoreConditions => FlowMode::IgnoreConditions,
591+
};
586592
Ok(SchedulerStep::StartQuery {
587-
query: walk
588-
.query
589-
.at_flow(branch_flow_ids[branch_idx], walk.query.mode),
593+
query: walk.query.at_flow(branch_flow_ids[branch_idx], branch_mode),
590594
continuation: Some(Continuation::Merge {
591595
walk,
592596
branch_flow_ids,
@@ -613,12 +617,18 @@ impl<'a> FlowTypeEngine<'a> {
613617
Err(err) => return self.fail_query(&walk.query, err),
614618
};
615619

620+
// In MergeBranch mode, `never` from the antecedent query means this
621+
// predecessor already crossed an unreachable condition edge.
622+
if matches!(walk.query.mode, FlowMode::MergeBranch) && antecedent_type.is_never() {
623+
return Ok(self.finish_walk(walk, LuaType::Never));
624+
}
625+
616626
if reuse_antecedent_narrowing
617627
&& !can_reuse_narrowed_assignment_source(self.db, &antecedent_type, &expr_type)
618628
{
619629
let next_query = walk
620630
.query
621-
.at_flow(antecedent_flow_id, FlowMode::WithoutConditions);
631+
.at_flow(antecedent_flow_id, FlowMode::IgnoreConditions);
622632
return Ok(SchedulerStep::StartQuery {
623633
query: next_query,
624634
continuation: Some(Continuation::AssignmentAntecedent {
@@ -826,6 +836,11 @@ impl<'a> FlowTypeEngine<'a> {
826836
// should still be narrowed by applying the cast from `unknown`.
827837
Err(_) => LuaType::Unknown,
828838
};
839+
// In MergeBranch mode, `never` from the antecedent query means this
840+
// predecessor already crossed an unreachable condition edge.
841+
if matches!(walk.query.mode, FlowMode::MergeBranch) && cast_input_type.is_never() {
842+
return Ok(self.finish_walk(walk, LuaType::Never));
843+
}
829844
for cast_op_type in cast_op_types {
830845
cast_input_type = match cast_type(
831846
self.db,
@@ -1116,14 +1131,17 @@ impl<'a> FlowTypeEngine<'a> {
11161131
// Broad RHS types replace the previous runtime type. The old path still
11171132
// queried the antecedent and then discarded it in finish_assignment_result.
11181133
let reuse_antecedent_narrowing = preserves_assignment_expr_type(&expr_type);
1119-
if !expr_type.is_unknown() && !reuse_antecedent_narrowing {
1134+
if !expr_type.is_unknown()
1135+
&& !reuse_antecedent_narrowing
1136+
&& !matches!(walk.query.mode, FlowMode::MergeBranch)
1137+
{
11201138
return Ok(self.finish_walk(walk, expr_type));
11211139
}
11221140

1123-
let mode = if reuse_antecedent_narrowing {
1124-
FlowMode::WithConditions
1125-
} else {
1126-
FlowMode::WithoutConditions
1141+
let mode = match (walk.query.mode, reuse_antecedent_narrowing) {
1142+
(FlowMode::MergeBranch, _) => FlowMode::MergeBranch,
1143+
(_, true) => FlowMode::Normal,
1144+
(_, false) => FlowMode::IgnoreConditions,
11271145
};
11281146
let subquery = walk.query.at_flow(antecedent_flow_id, mode);
11291147
Ok(SchedulerStep::StartQuery {
@@ -1179,7 +1197,7 @@ impl<'a> FlowTypeEngine<'a> {
11791197
condition_flow: InferConditionFlow,
11801198
) -> Result<SchedulerStep, InferFailReason> {
11811199
let antecedent_flow_id = get_single_antecedent(flow_node)?;
1182-
if !walk.query.mode.uses_conditions() {
1200+
if matches!(walk.query.mode, FlowMode::IgnoreConditions) {
11831201
walk.antecedent_flow_id = antecedent_flow_id;
11841202
return Ok(SchedulerStep::ContinueWalk(walk));
11851203
}
@@ -1233,6 +1251,7 @@ impl<'a> FlowTypeEngine<'a> {
12331251
if cached_action {
12341252
return match action {
12351253
ConditionFlowAction::Continue => Ok(SchedulerStep::ContinueWalk(walk)),
1254+
ConditionFlowAction::Unreachable => Ok(self.finish_unreachable_condition(walk)),
12361255
ConditionFlowAction::Result(result_type) => Ok(self.finish_walk(walk, result_type)),
12371256
ConditionFlowAction::Pending(pending_condition_narrow) => {
12381257
let mut walk = walk;
@@ -1275,9 +1294,11 @@ impl<'a> FlowTypeEngine<'a> {
12751294
}
12761295

12771296
let antecedent_flow_id = get_single_antecedent(flow_node)?;
1278-
let subquery = walk
1279-
.query
1280-
.at_flow(antecedent_flow_id, FlowMode::WithConditions);
1297+
let mode = match walk.query.mode {
1298+
FlowMode::MergeBranch => FlowMode::MergeBranch,
1299+
FlowMode::Normal | FlowMode::IgnoreConditions => FlowMode::Normal,
1300+
};
1301+
let subquery = walk.query.at_flow(antecedent_flow_id, mode);
12811302
Ok(SchedulerStep::StartQuery {
12821303
query: subquery,
12831304
continuation: Some(Continuation::TagCastAntecedent {
@@ -1315,13 +1336,28 @@ impl<'a> FlowTypeEngine<'a> {
13151336
} else {
13161337
Arc::<[FlowId]>::from(get_multi_antecedents(self.tree, flow_node)?)
13171338
};
1318-
let Some(next_pending_idx) = branch_flow_ids.len().checked_sub(1) else {
1319-
return Ok(self.finish_walk(walk, LuaType::Never));
1320-
};
1339+
match branch_flow_ids.len() {
1340+
0 => return Ok(self.finish_walk(walk, LuaType::Never)),
1341+
// A single predecessor is just a branch-entry label, not a merge.
1342+
// Keep the current mode so queries inside an impossible then/else
1343+
// body can still resolve unrelated symbols from their declarations.
1344+
1 => {
1345+
walk.antecedent_flow_id = branch_flow_ids[0];
1346+
continue;
1347+
}
1348+
_ => {}
1349+
}
1350+
let next_pending_idx = branch_flow_ids.len() - 1;
13211351
let q = &walk.query;
1322-
let next_query = q.at_flow(branch_flow_ids[next_pending_idx], q.mode);
1352+
// Multiple predecessors make this a merge. Query each
1353+
// predecessor as a merge contribution so an unreachable
1354+
// condition edge contributes `never` to the final union.
1355+
let branch_mode = match q.mode {
1356+
FlowMode::Normal | FlowMode::MergeBranch => FlowMode::MergeBranch,
1357+
FlowMode::IgnoreConditions => FlowMode::IgnoreConditions,
1358+
};
13231359
return Ok(SchedulerStep::StartQuery {
1324-
query: next_query,
1360+
query: q.at_flow(branch_flow_ids[next_pending_idx], branch_mode),
13251361
continuation: Some(Continuation::Merge {
13261362
walk,
13271363
branch_flow_ids,
@@ -1453,6 +1489,15 @@ impl<'a> FlowTypeEngine<'a> {
14531489
);
14541490
Ok(SchedulerStep::ContinueWalk(walk))
14551491
}
1492+
ConditionFlowAction::Unreachable => {
1493+
get_flow_var_cache(self.cache, walk.query.var_cache_idx)
1494+
.condition_cache
1495+
.insert(
1496+
(flow_id, condition_flow),
1497+
CacheEntry::Cache(ConditionFlowAction::Unreachable),
1498+
);
1499+
Ok(self.finish_unreachable_condition(walk))
1500+
}
14561501
ConditionFlowAction::Result(result_type) => {
14571502
get_flow_var_cache(self.cache, walk.query.var_cache_idx)
14581503
.condition_cache
@@ -1498,9 +1543,13 @@ impl<'a> FlowTypeEngine<'a> {
14981543
self.start_field_literal_sibling_subquery(walk, flow_id, condition_flow, subquery)
14991544
),
15001545
ConditionFlowAction::NeedCorrelated(pending_correlated_condition) => {
1546+
let mode = match walk.query.mode {
1547+
FlowMode::MergeBranch => FlowMode::MergeBranch,
1548+
FlowMode::Normal | FlowMode::IgnoreConditions => FlowMode::Normal,
1549+
};
15011550
let subquery = walk.query.at_flow(
15021551
pending_correlated_condition.current_search_root_flow_id,
1503-
FlowMode::WithConditions,
1552+
mode,
15041553
);
15051554
Ok(SchedulerStep::StartQuery {
15061555
query: subquery,
@@ -1515,14 +1564,25 @@ impl<'a> FlowTypeEngine<'a> {
15151564
}
15161565
}
15171566

1567+
fn finish_unreachable_condition(&mut self, walk: QueryWalk) -> SchedulerStep {
1568+
// Unreachable describes the condition edge, not the queried symbol. A
1569+
// merge query asks what this branch contributes, so the answer is
1570+
// `never`; point queries continue to the antecedent so unrelated
1571+
// symbols inside impossible branches keep their declared types.
1572+
match walk.query.mode {
1573+
FlowMode::MergeBranch => self.finish_walk(walk, LuaType::Never),
1574+
FlowMode::Normal | FlowMode::IgnoreConditions => SchedulerStep::ContinueWalk(walk),
1575+
}
1576+
}
1577+
15181578
fn finish_walk(&mut self, walk: QueryWalk, narrow_type: LuaType) -> SchedulerStep {
15191579
let QueryWalk {
15201580
query,
15211581
pending_condition_narrows,
15221582
..
15231583
} = walk;
15241584
let mut final_type = narrow_type;
1525-
if query.mode.uses_conditions() {
1585+
if !matches!(query.mode, FlowMode::IgnoreConditions) {
15261586
for pending_condition_narrow in pending_condition_narrows.into_iter().rev() {
15271587
final_type = pending_condition_narrow.apply(self.db, self.cache, final_type);
15281588
}

0 commit comments

Comments
 (0)