Merge pull request #1039 from lewis6991/fix/missing-return-dynamic-loops

fix(flow): separate dynamic loop divergence from infinite loops

Author: CppCXY

crates/emmylua_code_analysis/src/compilation/analyzer/lua/func_body.rs

@@ -24,25 +24,32 @@ enum ConditionState {
 #[derive(Debug, Default)]
 struct ReturnFlow {
     return_points: Vec<LuaReturnPoint>,
-    can_continue: bool,
+    can_fall_through: bool,
     can_break: bool,
-    // `can_stall` means control can get stuck without returning, e.g. `while true do end`.
-    can_stall: bool,
+    // Some reachable path is proven to loop forever instead of returning or
+    // reaching the following statements, e.g. `while true do end`.
+    is_infinite: bool,
+    // Some reachable path can stay inside a runtime-dependent loop without
+    // proving the body itself is infinite. Keep this separate from
+    // reachability so stricter loop diagnostics can opt into it later without
+    // affecting inferred return types.
+    may_diverge: bool,
 }
 
 impl ReturnFlow {
-    fn continue_flow() -> Self {
+    fn fallthrough() -> Self {
         Self {
-            can_continue: true,
+            can_fall_through: true,
             ..Default::default()
         }
     }
 
     fn merge_choice(&mut self, mut other: Self) {
         self.return_points.append(&mut other.return_points);
-        self.can_continue |= other.can_continue;
+        self.can_fall_through |= other.can_fall_through;
         self.can_break |= other.can_break;
-        self.can_stall |= other.can_stall;
+        self.is_infinite |= other.is_infinite;
+        self.may_diverge |= other.may_diverge;
     }
 }
 
@@ -54,22 +61,22 @@ where
     F: FnMut(&LuaExpr) -> Result<LuaType, InferFailReason>,
 {
     let mut flow = analyze_block_returns(body, infer_expr_type)?;
-    if flow.can_continue || flow.can_break {
+    if flow.can_fall_through || flow.can_break {
         flow.return_points.push(LuaReturnPoint::Nil);
     }
 
     Ok(flow.return_points)
 }
 
-pub fn does_func_body_always_return_or_exit<F>(
+pub(in crate::compilation::analyzer) fn analyze_func_body_missing_return_flags_with<F>(
     body: LuaBlock,
     infer_expr_type: &mut F,
-) -> Result<bool, InferFailReason>
+) -> Result<(bool, bool, bool), InferFailReason>
 where
     F: FnMut(&LuaExpr) -> Result<LuaType, InferFailReason>,
 {
     let flow = analyze_block_returns(body, infer_expr_type)?;
-    Ok(!flow.can_continue && !flow.can_break && !flow.can_stall)
+    Ok((flow.can_fall_through, flow.can_break, flow.is_infinite))
 }
 
 fn analyze_block_returns<F>(
@@ -80,21 +87,22 @@ where
     F: FnMut(&LuaExpr) -> Result<LuaType, InferFailReason>,
 {
     let mut flow = ReturnFlow::default();
-    let mut can_continue = true;
+    let mut can_fall_through = true;
 
     for stat in block.get_stats() {
-        if !can_continue {
+        if !can_fall_through {
             break;
         }
 
         let stat_flow = analyze_stat_returns(stat, infer_expr_type)?;
         flow.return_points.extend(stat_flow.return_points);
         flow.can_break |= stat_flow.can_break;
-        flow.can_stall |= stat_flow.can_stall;
-        can_continue = stat_flow.can_continue;
+        flow.is_infinite |= stat_flow.is_infinite;
+        flow.may_diverge |= stat_flow.may_diverge;
+        can_fall_through = stat_flow.can_fall_through;
     }
 
-    flow.can_continue = can_continue;
+    flow.can_fall_through = can_fall_through;
     Ok(flow)
 }
 
@@ -107,7 +115,7 @@ where
 {
     match block {
         Some(block) => analyze_block_returns(block, infer_expr_type),
-        None => Ok(ReturnFlow::continue_flow()),
+        None => Ok(ReturnFlow::fallthrough()),
     }
 }
 
@@ -135,7 +143,7 @@ where
             ..Default::default()
         }),
         LuaStat::ReturnStat(return_stat) => Ok(analyze_return_stat_returns(return_stat)),
-        _ => Ok(ReturnFlow::continue_flow()),
+        _ => Ok(ReturnFlow::fallthrough()),
     }
 }
 
@@ -159,15 +167,31 @@ where
     let condition_state =
         analyze_condition_state(while_stat.get_condition_expr(), infer_expr_type)?;
     match condition_state {
-        ConditionState::Falsy => Ok(ReturnFlow::continue_flow()),
+        ConditionState::Falsy => Ok(ReturnFlow::fallthrough()),
         ConditionState::Never => Ok(ReturnFlow::default()),
-        ConditionState::Truthy | ConditionState::Dynamic => {
+        ConditionState::Truthy => {
             let body = analyze_optional_block_returns(while_stat.get_block(), infer_expr_type)?;
             Ok(ReturnFlow {
                 return_points: body.return_points,
-                can_continue: matches!(condition_state, ConditionState::Dynamic) || body.can_break,
+                can_fall_through: body.can_break,
                 can_break: false,
-                can_stall: body.can_continue || body.can_stall,
+                is_infinite: (body.can_fall_through && !body.can_break) || body.is_infinite,
+                may_diverge: (body.can_fall_through && body.can_break) || body.may_diverge,
+            })
+        }
+        ConditionState::Dynamic => {
+            let body = analyze_optional_block_returns(while_stat.get_block(), infer_expr_type)?;
+            Ok(ReturnFlow {
+                return_points: body.return_points,
+                can_fall_through: true,
+                can_break: false,
+                // A dynamic condition may skip the loop entirely, but a body
+                // that is already proven infinite remains infinite once
+                // entered.
+                is_infinite: body.is_infinite,
+                // We keep dynamic loops in `may_diverge` rather than trying to
+                // prove exit from body progress here.
+                may_diverge: body.can_fall_through || body.may_diverge,
             })
         }
     }
@@ -183,25 +207,32 @@ where
     let body = analyze_optional_block_returns(repeat_stat.get_block(), infer_expr_type)?;
     let mut flow = ReturnFlow {
         return_points: body.return_points,
-        can_continue: body.can_break,
+        can_fall_through: body.can_break,
         can_break: false,
-        can_stall: body.can_stall,
+        is_infinite: body.is_infinite,
+        may_diverge: body.may_diverge,
     };
 
-    if !body.can_continue {
+    if !body.can_fall_through {
         return Ok(flow);
     }
 
     match analyze_condition_state(repeat_stat.get_condition_expr(), infer_expr_type)? {
         ConditionState::Truthy => {
-            flow.can_continue = true;
+            flow.can_fall_through = true;
         }
         ConditionState::Falsy => {
-            flow.can_stall = true;
+            if body.can_break {
+                flow.may_diverge = true;
+            } else {
+                flow.is_infinite = true;
+            }
         }
         ConditionState::Dynamic => {
-            flow.can_continue = true;
-            flow.can_stall = true;
+            flow.can_fall_through = true;
+            // Same rule as dynamic `while`: the loop body is reachable, but
+            // the exit still depends on runtime state.
+            flow.may_diverge = true;
         }
         ConditionState::Never => {}
     }
@@ -217,7 +248,7 @@ where
     F: FnMut(&LuaExpr) -> Result<LuaType, InferFailReason>,
 {
     let mut flow = analyze_optional_block_returns(for_stat.get_block(), infer_expr_type)?;
-    flow.can_continue = true;
+    flow.can_fall_through = true;
     flow.can_break = false;
     Ok(flow)
 }
@@ -286,7 +317,7 @@ where
     }
 
     if can_reach_next_clause {
-        flow.can_continue = true;
+        flow.can_fall_through = true;
     }
 
     Ok(flow)
@@ -300,7 +331,7 @@ where
     F: FnMut(&LuaExpr) -> Result<LuaType, InferFailReason>,
 {
     let mut flow = analyze_optional_block_returns(for_range_stat.get_block(), infer_expr_type)?;
-    flow.can_continue = true;
+    flow.can_fall_through = true;
     flow.can_break = false;
     Ok(flow)
 }
@@ -309,7 +340,7 @@ fn analyze_call_expr_stat_returns(
     call_expr_stat: LuaCallExprStat,
 ) -> Result<ReturnFlow, InferFailReason> {
     let Some(call_expr) = call_expr_stat.get_call_expr() else {
-        return Ok(ReturnFlow::continue_flow());
+        return Ok(ReturnFlow::fallthrough());
     };
 
     if call_expr.is_error() {
@@ -319,7 +350,7 @@ fn analyze_call_expr_stat_returns(
         });
     }
 
-    Ok(ReturnFlow::continue_flow())
+    Ok(ReturnFlow::fallthrough())
 }
 
 fn analyze_return_stat_returns(return_stat: LuaReturnStat) -> ReturnFlow {

crates/emmylua_code_analysis/src/compilation/analyzer/lua/mod.rs

@@ -1,7 +1,7 @@
 mod call;
 mod closure;
 mod for_range_stat;
-mod func_body;
+pub(in crate::compilation::analyzer) mod func_body;
 mod metatable;
 mod module;
 mod stats;
@@ -13,9 +13,7 @@ pub use closure::analyze_return_point;
 use emmylua_parser::{LuaAst, LuaAstNode, LuaExpr};
 use for_range_stat::analyze_for_range_stat;
 pub use for_range_stat::infer_for_range_iter_expr_func;
-pub use func_body::{
-    LuaReturnPoint, analyze_func_body_returns_with, does_func_body_always_return_or_exit,
-};
+pub use func_body::{LuaReturnPoint, analyze_func_body_returns_with};
 use metatable::analyze_setmetatable;
 use module::analyze_chunk_return;
 use stats::{

crates/emmylua_code_analysis/src/compilation/analyzer/mod.rs

@@ -6,19 +6,27 @@ mod infer_cache_manager;
 mod lua;
 mod unresolve;
 
-pub(crate) use lua::does_func_body_always_return_or_exit;
-
 use crate::{
-    Emmyrc, FileId, InFiled, InferFailReason,
+    Emmyrc, FileId, InFiled, InferFailReason, LuaType,
     db_index::{DbIndex, WorkspaceId},
     profile::Profile,
 };
-use emmylua_parser::LuaChunk;
+use emmylua_parser::{LuaBlock, LuaChunk, LuaExpr};
 use hashbrown::{HashMap, HashSet};
 use infer_cache_manager::InferCacheManager;
 use std::sync::Arc;
 use unresolve::UnResolve;
 
+pub(super) fn analyze_func_body_missing_return_flags_with<F>(
+    body: LuaBlock,
+    infer_expr_type: &mut F,
+) -> Result<(bool, bool, bool), InferFailReason>
+where
+    F: FnMut(&LuaExpr) -> Result<LuaType, InferFailReason>,
+{
+    lua::func_body::analyze_func_body_missing_return_flags_with(body, infer_expr_type)
+}
+
 pub fn analyze(db: &mut DbIndex, need_analyzed_files: Vec<InFiled<LuaChunk>>, config: Arc<Emmyrc>) {
     if need_analyzed_files.is_empty() {
         return;

crates/emmylua_code_analysis/src/compilation/mod.rs

@@ -3,11 +3,21 @@ mod test;
 
 use std::sync::Arc;
 
-pub(crate) use analyzer::does_func_body_always_return_or_exit;
-
 use crate::{
-    Emmyrc, FileId, InFiled, LuaIndex, LuaInferCache, db_index::DbIndex, semantic::SemanticModel,
+    Emmyrc, FileId, InFiled, InferFailReason, LuaIndex, LuaInferCache, LuaType, db_index::DbIndex,
+    semantic::SemanticModel,
 };
+use emmylua_parser::{LuaBlock, LuaExpr};
+
+pub(crate) fn analyze_func_body_missing_return_flags_with<F>(
+    body: LuaBlock,
+    infer_expr_type: &mut F,
+) -> Result<(bool, bool, bool), InferFailReason>
+where
+    F: FnMut(&LuaExpr) -> Result<LuaType, InferFailReason>,
+{
+    analyzer::analyze_func_body_missing_return_flags_with(body, infer_expr_type)
+}
 
 #[derive(Debug)]
 pub struct LuaCompilation {

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

@@ -301,6 +301,44 @@ mod test {
         );
     }
 
+    #[test]
+    fn test_inferred_return_from_truthy_while_with_break_before_return() {
+        assert_inferred_return_without_nil(
+            r#"
+        local function f(done)
+            while true do
+                if done then
+                    break
+                end
+            end
+
+            return 1
+        end
+
+        result = f()
+        "#,
+        );
+    }
+
+    #[test]
+    fn test_inferred_return_from_infinite_repeat_with_break_before_return() {
+        assert_inferred_return_without_nil(
+            r#"
+        local function f(done)
+            repeat
+                if done then
+                    break
+                end
+            until false
+
+            return 1
+        end
+
+        result = f()
+        "#,
+        );
+    }
+
     #[test]
     fn test_return_flow_keeps_local_while_call_condition_dynamic() {
         let mut ws = VirtualWorkspace::new();

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

@@ -5,7 +5,7 @@ use emmylua_parser::{
 
 use crate::{
     DiagnosticCode, LuaSignatureId, LuaType, SemanticModel, SignatureReturnStatus,
-    compilation::does_func_body_always_return_or_exit,
+    compilation::analyze_func_body_missing_return_flags_with,
 };
 
 use super::{Checker, DiagnosticContext, get_return_stats};
@@ -111,13 +111,20 @@ fn check_missing_return(
     // 检测缺少返回语句需要处理 if while
     if min_expected_return_count > 0 {
         let range = if let Some(block) = closure_expr.get_block() {
-            if does_func_body_always_return_or_exit(block.clone(), &mut |expr: &LuaExpr| {
-                Ok(semantic_model
-                    .infer_expr(expr.clone())
-                    .unwrap_or(LuaType::Unknown))
-            })
-            .ok()?
-            {
+            let (can_fall_through, can_break, is_infinite) =
+                analyze_func_body_missing_return_flags_with(
+                    block.clone(),
+                    &mut |expr: &LuaExpr| {
+                        Ok(semantic_model
+                            .infer_expr(expr.clone())
+                            .unwrap_or(LuaType::Unknown))
+                    },
+                )
+                .ok()?;
+
+            // `MissingReturn` currently ignores runtime-dependent divergence if
+            // a later `return` is still reachable.
+            if !can_fall_through && !can_break && !is_infinite {
                 return Some(());
             }