[IMP] Handle lambdas and other missing expressions

Author: mmahrouss

server/src/core/evaluation.rs

@@ -3,7 +3,7 @@ use std::cell::RefCell;
 use std::vec;
 use anyhow::Error;
 use ruff_text_size::{Ranged, TextRange, TextSize};
-use ruff_python_ast::{Alias, AnyRootNodeRef, CmpOp, Expr, ExprNamed, ExprTuple, FStringPart, Identifier, Pattern, Stmt, StmtAnnAssign, StmtAssign, StmtClassDef, StmtFor, StmtFunctionDef, StmtIf, StmtMatch, StmtTry, StmtWhile, StmtWith};
+use ruff_python_ast::{Alias, AnyRootNodeRef, CmpOp, Expr, ExprNamed, ExprTuple, FStringPart, Identifier, Parameters, Pattern, Stmt, StmtAnnAssign, StmtAssign, StmtClassDef, StmtFor, StmtFunctionDef, StmtIf, StmtMatch, StmtTry, StmtWhile, StmtWith};
 use lsp_types::Diagnostic;
 use tracing::{trace, warn};
 use weak_table::traits::WeakElement;
@@ -456,12 +456,16 @@ impl PythonArchBuilder {
                 expr_slice.lower.as_ref().map(|lower_expr| self.visit_expr(session, &lower_expr));
             },
             // Expressions that cannot contained a named expressions are not traversed
-            Expr::Lambda(_todo_lambda_expr) => {
-                // Lambdas can have named expressions, but it is not a common use
-                // Like lambda vals: vals[(x := 0): x + 3]
-                // However x is only in scope in the lambda expression only
-                // It needs adding a new function, ast_indexes, then add the variable inside
-                // I deem it currently unnecessary
+            Expr::Lambda(lambda_expr) => {
+                let sym = self.sym_stack.last().unwrap().borrow_mut().add_new_function(
+                    session, &S!("<lambda>"), &lambda_expr.range, &lambda_expr.body.range().start()
+                );
+                if let Some(parameters) = &lambda_expr.parameters {
+                    PythonArchBuilder::handle_func_args(&sym, session, &parameters);
+                }
+                self.sym_stack.push(sym.clone());
+                self.visit_expr(session, &lambda_expr.body);
+                self.sym_stack.pop();
             },
             Expr::Generator(_todo_expr_generator) => {
                 // generators are lazily evaluated,
@@ -661,6 +665,67 @@ impl PythonArchBuilder {
         }
     }
 
+    fn handle_func_args(fun_sym: &Rc<RefCell<Symbol>>, session: &mut SessionInfo, parameters: &Parameters) {
+        for arg in parameters.posonlyargs.iter() {
+            let param = fun_sym.borrow_mut().add_new_variable(session, oyarn!("{}", arg.parameter.name.id), &arg.range);
+            param.borrow_mut().as_variable_mut().is_parameter = true;
+            let mut default = None;
+            if arg.default.is_some() {
+                default = Some(Evaluation::new_none()); //TODO evaluate default? actually only used to know if there is a default or not
+            }
+            fun_sym.borrow_mut().as_func_mut().args.push(Argument {
+                symbol: Rc::downgrade(&param),
+                default_value: default,
+                arg_type: ArgumentType::POS_ONLY,
+                annotation: arg.parameter.annotation.clone(),
+            });
+        }
+        for arg in parameters.args.iter() {
+            let param = fun_sym.borrow_mut().add_new_variable(session, oyarn!("{}", arg.parameter.name.id), &arg.range);
+            param.borrow_mut().as_variable_mut().is_parameter = true;
+            let mut default = None;
+            if arg.default.is_some() {
+                default = Some(Evaluation::new_none()); //TODO evaluate default? actually only used to know if there is a default or not
+            }
+            fun_sym.borrow_mut().as_func_mut().args.push(Argument {
+                symbol: Rc::downgrade(&param),
+                default_value: default,
+                arg_type: ArgumentType::ARG,
+                annotation: arg.parameter.annotation.clone(),
+            });
+        }
+        if let Some(arg) = &parameters.vararg {
+            let param = fun_sym.borrow_mut().add_new_variable(session, oyarn!("{}", arg.name.id), &arg.range);
+            param.borrow_mut().as_variable_mut().is_parameter = true;
+            fun_sym.borrow_mut().as_func_mut().args.push(Argument {
+                symbol: Rc::downgrade(&param),
+                default_value: None,
+                arg_type: ArgumentType::VARARG,
+                annotation: arg.annotation.clone(),
+            });
+        }
+        for arg in parameters.kwonlyargs.iter() {
+            let param = fun_sym.borrow_mut().add_new_variable(session, oyarn!("{}", arg.parameter.name.id), &arg.range);
+            param.borrow_mut().as_variable_mut().is_parameter = true;
+            fun_sym.borrow_mut().as_func_mut().args.push(Argument {
+                symbol: Rc::downgrade(&param),
+                default_value: arg.default.as_ref().map(|_default| Evaluation::new_none()),
+                arg_type: ArgumentType::KWORD_ONLY,
+                annotation: arg.parameter.annotation.clone(),
+            });
+        }
+        if let Some(arg) = &parameters.kwarg {
+            let param = fun_sym.borrow_mut().add_new_variable(session, oyarn!("{}", arg.name.id), &arg.range);
+            param.borrow_mut().as_variable_mut().is_parameter = true;
+            fun_sym.borrow_mut().as_func_mut().args.push(Argument {
+                symbol: Rc::downgrade(&param),
+                default_value: None,
+                arg_type: ArgumentType::KWARG,
+                annotation: arg.annotation.clone(),
+            });
+        }
+    }
+
     fn visit_func_def(&mut self, session: &mut SessionInfo, func_def: &StmtFunctionDef) -> Result<(), Error> {
         if func_def.body.is_empty() {
             return Ok(()) //if body is empty, it usually means that the ast of the class is invalid. Skip it
@@ -706,64 +771,7 @@ impl PythonArchBuilder {
         }
         drop(sym_bw);
         //add params
-        for arg in func_def.parameters.posonlyargs.iter() {
-            let param = sym.borrow_mut().add_new_variable(session, oyarn!("{}", arg.parameter.name.id), &arg.range);
-            param.borrow_mut().as_variable_mut().is_parameter = true;
-            let mut default = None;
-            if arg.default.is_some() {
-                default = Some(Evaluation::new_none()); //TODO evaluate default? actually only used to know if there is a default or not
-            }
-            sym.borrow_mut().as_func_mut().args.push(Argument {
-                symbol: Rc::downgrade(&param),
-                default_value: default,
-                arg_type: ArgumentType::POS_ONLY,
-                annotation: arg.parameter.annotation.clone(),
-            });
-        }
-        for arg in func_def.parameters.args.iter() {
-            let param = sym.borrow_mut().add_new_variable(session, oyarn!("{}", arg.parameter.name.id), &arg.range);
-            param.borrow_mut().as_variable_mut().is_parameter = true;
-            let mut default = None;
-            if arg.default.is_some() {
-                default = Some(Evaluation::new_none()); //TODO evaluate default? actually only used to know if there is a default or not
-            }
-            sym.borrow_mut().as_func_mut().args.push(Argument {
-                symbol: Rc::downgrade(&param),
-                default_value: default,
-                arg_type: ArgumentType::ARG,
-                annotation: arg.parameter.annotation.clone(),
-            });
-        }
-        if let Some(arg) = &func_def.parameters.vararg {
-            let param = sym.borrow_mut().add_new_variable(session, oyarn!("{}", arg.name.id), &arg.range);
-            param.borrow_mut().as_variable_mut().is_parameter = true;
-            sym.borrow_mut().as_func_mut().args.push(Argument {
-                symbol: Rc::downgrade(&param),
-                default_value: None,
-                arg_type: ArgumentType::VARARG,
-                annotation: arg.annotation.clone(),
-            });
-        }
-        for arg in func_def.parameters.kwonlyargs.iter() {
-            let param = sym.borrow_mut().add_new_variable(session, oyarn!("{}", arg.parameter.name.id), &arg.range);
-            param.borrow_mut().as_variable_mut().is_parameter = true;
-            sym.borrow_mut().as_func_mut().args.push(Argument {
-                symbol: Rc::downgrade(&param),
-                default_value: arg.default.as_ref().map(|_default| Evaluation::new_none()),
-                arg_type: ArgumentType::KWORD_ONLY,
-                annotation: arg.parameter.annotation.clone(),
-            });
-        }
-        if let Some(arg) = &func_def.parameters.kwarg {
-            let param = sym.borrow_mut().add_new_variable(session, oyarn!("{}", arg.name.id), &arg.range);
-            param.borrow_mut().as_variable_mut().is_parameter = true;
-            sym.borrow_mut().as_func_mut().args.push(Argument {
-                symbol: Rc::downgrade(&param),
-                default_value: None,
-                arg_type: ArgumentType::KWARG,
-                annotation: arg.annotation.clone(),
-            });
-        }
+        PythonArchBuilder::handle_func_args(&sym, session, &func_def.parameters);
         let mut add_noqa = false;
         if let Some(noqa_bloc) = self.file_info.as_ref().unwrap().borrow().noqas_blocs.get(&func_def.range.start().to_u32()) {
             session.noqas_stack.push(noqa_bloc.clone());

server/src/core/python_arch_builder.rs

@@ -314,12 +314,26 @@ impl PythonArchEval {
                 expr_slice.lower.as_ref().map(|lower_expr| self.visit_expr(session, &lower_expr));
             },
             // Expressions that cannot contained a named expressions are not traversed
-            Expr::Lambda(_todo_lambda_expr) => {
+            Expr::Lambda(lambda_expr) => {
                 // Lambdas can have named expressions, but it is not a common use
                 // Like lambda vals: vals[(x := 0): x + 3]
                 // However x is only in scope in the lambda expression only
                 // It needs adding a new function, ast_indexes, then add the variable inside
                 // I deem it currently unnecessary
+                let variable: Option<Rc<RefCell<Symbol>>> = self.sym_stack.last().unwrap().borrow().get_positioned_symbol(&Sy!("<lambda>"), &lambda_expr.range);
+                let Some(lambda_fn_sym) = variable else {
+                    return; // can be not found if AST is incomplete
+                };
+                lambda_fn_sym.borrow_mut().as_func_mut().arch_eval_status = BuildStatus::IN_PROGRESS;
+                self.sym_stack.push(lambda_fn_sym.clone());
+                self.visit_expr(session, &lambda_expr.body);
+                let mut deps = vec![vec![], vec![]];
+                let (eval, diags) = Evaluation::eval_from_ast(session, &lambda_expr.body, lambda_fn_sym.clone(), &lambda_expr.body.range().start(), false, &mut deps);
+                self.diagnostics.extend(diags);
+                Symbol::insert_dependencies(&self.file, &mut deps, self.current_step);
+                FunctionSymbol::add_return_evaluations(lambda_fn_sym.clone(), session, eval);
+                self.sym_stack.pop();
+                lambda_fn_sym.borrow_mut().as_func_mut().arch_eval_status = BuildStatus::DONE;
             },
             Expr::Generator(_todo_expr_generator) => {
                 // generators are lazily evaluated,

server/src/core/python_arch_eval.rs

@@ -73,7 +73,7 @@ static arch_eval_file_hooks: Lazy<Vec<PythonArchEvalFileHook>> = Lazy::new(|| {v
         let values: Vec<ruff_python_ast::Expr> = Vec::new();
         let mut id = symbol.borrow_mut();
         let range = id.range().clone();
-        id.set_evaluations(vec![Evaluation::new_list(odoo.sync_odoo, values, range)]);
+        id.set_evaluations(vec![Evaluation::new_list(odoo.sync_odoo, Some(values), range)]);
     }},
     /*PythonArchEvalFileHook {file_tree: vec![Sy!("odoo"), Sy!("models")],
                         content_tree: vec![Sy!("BaseModel"), Sy!("search_count")],

server/src/core/python_arch_eval_hooks.rs

@@ -48,4 +48,16 @@ def _get_partner_id(self):
 class NoBaseModel(models.Model):
     _inherit = "module_1.no_base_model"
 
-basic_var = 42
+basic_var = 42
+lambda_ref = lambda x: basic_var
+fstring_ref = f"value: {basic_var}"
+boolop_ref = basic_var and 1
+compare_ref = basic_var > 0
+listcomp_ref = [basic_var for _ in []]
+dictcomp_ref = {basic_var: 1 for _ in []}
+list_ref = [basic_var, 1]
+tuple_ref = (basic_var, 1)
+set_ref = {basic_var}
+dict_ref = {1: basic_var}
+call_ref = print(basic_var)
+binop_ref = basic_var + 1

server/tests/data/addons/module_1/models/base_test_models.py

@@ -1,4 +1,4 @@
-// Test the hover feature by calling get_hover on various symbols in the test addons.
+// Test the hover feature by calling get_hover on various symbols in the test addons.
 
 use odoo_ls_server::core::odoo::SyncOdoo;
 use odoo_ls_server::utils::{PathSanitizer, ToFilePath};
@@ -278,4 +278,145 @@ fn test_model_subscription() {
         "Resolving a subscript of a model should include the model symbol itself.
         Expected to find BaseTestModel symbol among resolved symbols of `partner = self.search([], limit=2)[-1:]`"
     )
-}
+}
+
+#[test]
+/// Test that symbol resolution (hover/get_symbol) works inside newly-handled AST constructs:
+/// lambda bodies, f-strings, bool operations, comparisons, list comprehensions, and dict comprehensions.
+fn test_get_symbol_in_new_expression_constructs() {
+    let (mut odoo, config) = setup::setup::setup_server(true);
+    let test_addons_path = PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("tests").join("data").join("addons");
+    let test_file = test_addons_path.join("module_1").join("models").join("base_test_models.py").sanitize();
+    assert!(PathBuf::from(&test_file).exists(), "Test file does not exist: {}", test_file);
+    let mut session = setup::setup::create_init_session(&mut odoo, config);
+
+    let file_mgr = session.sync_odoo.get_file_mgr();
+    let file_info = file_mgr.borrow().get_file_info(&test_file).unwrap();
+    let Some(file_symbol) = SyncOdoo::get_symbol_of_opened_file(
+        &mut session,
+        &PathBuf::from(&test_file)
+    ) else {
+        panic!("Failed to get file symbol");
+    };
+
+    // Hover on `basic_var` inside the lambda body: `lambda_ref = lambda x: basic_var`
+    // The reference is at line 51, col 23
+    let hover = test_utils::get_hover_markdown(&mut session, &file_symbol, &file_info, 51, 23)
+        .unwrap_or_default();
+    assert!(
+        hover.contains("basic_var"),
+        "Hover inside lambda body should resolve to basic_var, got: {hover}"
+    );
+    assert!(
+        hover.contains("int"),
+        "basic_var inside lambda body should have type int, got: {hover}"
+    );
+
+    // Hover on the lambda parameter `x` in `lambda x: basic_var`
+    // The parameter is at line 51, col 20
+    let hover_param = test_utils::get_hover_markdown(&mut session, &file_symbol, &file_info, 51, 20)
+        .unwrap_or_default();
+    assert!(
+        !hover_param.is_empty(),
+        "Hover on lambda parameter should return a result"
+    );
+
+    // Hover on `basic_var` inside an f-string: `fstring_ref = f"value: {basic_var}"`
+    // The reference is at line 52, col 24
+    let hover = test_utils::get_hover_markdown(&mut session, &file_symbol, &file_info, 52, 24)
+        .unwrap_or_default();
+    assert!(
+        hover.contains("basic_var"),
+        "Hover inside f-string should resolve to basic_var, got: {hover}"
+    );
+
+    // Hover on `basic_var` inside a bool operation: `boolop_ref = basic_var and 1`
+    // The reference is at line 53, col 13
+    let hover = test_utils::get_hover_markdown(&mut session, &file_symbol, &file_info, 53, 13)
+        .unwrap_or_default();
+    assert!(
+        hover.contains("basic_var"),
+        "Hover inside bool operation should resolve to basic_var, got: {hover}"
+    );
+
+    // Hover on `basic_var` inside a compare expression: `compare_ref = basic_var > 0`
+    // The reference is at line 54, col 14
+    let hover = test_utils::get_hover_markdown(&mut session, &file_symbol, &file_info, 54, 14)
+        .unwrap_or_default();
+    assert!(
+        hover.contains("basic_var"),
+        "Hover inside compare expression should resolve to basic_var, got: {hover}"
+    );
+
+    // Hover on `basic_var` inside a list comprehension: `listcomp_ref = [basic_var for _ in []]`
+    // The reference is at line 55, col 16
+    let hover = test_utils::get_hover_markdown(&mut session, &file_symbol, &file_info, 55, 16)
+        .unwrap_or_default();
+    assert!(
+        hover.contains("basic_var"),
+        "Hover inside list comprehension should resolve to basic_var, got: {hover}"
+    );
+
+    // Hover on `basic_var` in the key position of a dict comprehension: `dictcomp_ref = {basic_var: 1 for _ in []}`
+    // The reference is at line 56, col 16
+    let hover = test_utils::get_hover_markdown(&mut session, &file_symbol, &file_info, 56, 16)
+        .unwrap_or_default();
+    assert!(
+        hover.contains("basic_var"),
+        "Hover inside dict comprehension key should resolve to basic_var, got: {hover}"
+    );
+
+    // Hover on `basic_var` inside a list literal: `list_ref = [basic_var, 1]`
+    // The reference is at line 57, col 12
+    let hover = test_utils::get_hover_markdown(&mut session, &file_symbol, &file_info, 57, 12)
+        .unwrap_or_default();
+    assert!(
+        hover.contains("basic_var"),
+        "Hover inside list literal should resolve to basic_var, got: {hover}"
+    );
+
+    // Hover on `basic_var` inside a tuple literal: `tuple_ref = (basic_var, 1)`
+    // The reference is at line 58, col 13
+    let hover = test_utils::get_hover_markdown(&mut session, &file_symbol, &file_info, 58, 13)
+        .unwrap_or_default();
+    assert!(
+        hover.contains("basic_var"),
+        "Hover inside tuple literal should resolve to basic_var, got: {hover}"
+    );
+
+    // Hover on `basic_var` inside a set literal: `set_ref = {basic_var}`
+    // The reference is at line 59, col 11
+    let hover = test_utils::get_hover_markdown(&mut session, &file_symbol, &file_info, 59, 11)
+        .unwrap_or_default();
+    assert!(
+        hover.contains("basic_var"),
+        "Hover inside set literal should resolve to basic_var, got: {hover}"
+    );
+
+    // Hover on `basic_var` as a dict literal value: `dict_ref = {1: basic_var}`
+    // The reference is at line 60, col 15
+    let hover = test_utils::get_hover_markdown(&mut session, &file_symbol, &file_info, 60, 15)
+        .unwrap_or_default();
+    assert!(
+        hover.contains("basic_var"),
+        "Hover inside dict literal value should resolve to basic_var, got: {hover}"
+    );
+
+    // Hover on `basic_var` as a call argument: `call_ref = print(basic_var)`
+    // The reference is at line 61, col 17
+    let hover = test_utils::get_hover_markdown(&mut session, &file_symbol, &file_info, 61, 17)
+        .unwrap_or_default();
+    assert!(
+        hover.contains("basic_var"),
+        "Hover on call argument should resolve to basic_var, got: {hover}"
+    );
+
+    // Hover on `basic_var` as the left operand of a binary operator: `binop_ref = basic_var + 1`
+    // The reference is at line 62, col 12
+    let hover = test_utils::get_hover_markdown(&mut session, &file_symbol, &file_info, 62, 12)
+        .unwrap_or_default();
+    assert!(
+        hover.contains("basic_var"),
+        "Hover on binary operator operand should resolve to basic_var, got: {hover}"
+    );
+}