diff --git a/server/src/core/evaluation.rs b/server/src/core/evaluation.rs index e3d21135..25d2cc6d 100644 --- a/server/src/core/evaluation.rs +++ b/server/src/core/evaluation.rs @@ -1149,7 +1149,7 @@ impl Evaluation { evals.push(Evaluation::new_unbound(name)); } }, - ExprOrIdent::Expr(Expr::Subscript(sub)) => 'subscript_block: { + ExprOrIdent::Expr(Expr::Subscript(sub)) => { let (eval_left, diags) = Evaluation::eval_from_ast(session, &sub.value, parent.clone(), max_infer, false, required_dependencies); diagnostics.extend(diags); // TODO handle multiple eval_left @@ -1161,73 +1161,74 @@ impl Evaluation { return AnalyzeAstResult::from_only_diagnostics(diagnostics); } let bases = Symbol::follow_ref(&base, session, &mut None, false, false, None); - if bases.len() != 1 { + if bases.is_empty() { return AnalyzeAstResult::from_only_diagnostics(diagnostics); } - let base = &bases[0]; - match base { - EvaluationSymbolPtr::WEAK(base_sym_weak_eval) if base_sym_weak_eval.instance == Some(false) => { - if let Some(SymType::CLASS) = base.upgrade_weak().map(|s| s.borrow().typ()) { - // This is a Generic type (Field[int], or List[int]), for now we just return the main type/Class (Field/List) - // TODO: handle generic types - let mut new_base = base.clone(); - if for_annotation { - new_base.as_mut_weak().instance = Some(true); + for base in &bases { + match base { + EvaluationSymbolPtr::WEAK(base_sym_weak_eval) if base_sym_weak_eval.instance == Some(false) => { + if let Some(SymType::CLASS) = base.upgrade_weak().map(|s| s.borrow().typ()) { + // This is a Generic type (Field[int], or List[int]), for now we just return the main type/Class (Field/List) + // TODO: handle generic types + let mut new_base = base.clone(); + if for_annotation { + new_base.as_mut_weak().instance = Some(true); + } + evals.push(Evaluation { + symbol: EvaluationSymbol { + sym: new_base, + get_symbol_hook: None, + }, + value: None, + range: Some(sub.range()) + }); + continue; } - evals.push(Evaluation { - symbol: EvaluationSymbol { - sym: new_base, - get_symbol_hook: None, - }, - value: None, - range: Some(sub.range()) - }); - break 'subscript_block; } + _ => {} } - _ => {} - } - let value = Evaluation::expr_to_str(session, &sub.slice, parent.clone(), max_infer, false, &mut diagnostics); - diagnostics.extend(value.1); - if let Some(value) = value.0 { - if !base.is_weak() { - return AnalyzeAstResult::from_only_diagnostics(diagnostics); - } - let parent_file_or_func = parent.clone().borrow().parent_file_or_function().as_ref().unwrap().upgrade().unwrap(); - let is_in_validation = match parent_file_or_func.borrow().typ().clone() { - SymType::FILE | SymType::PACKAGE(_) | SymType::FUNCTION => { - parent_file_or_func.borrow().build_status(BuildSteps::VALIDATION) == BuildStatus::IN_PROGRESS - }, - _ => {false} - }; - let base = base.upgrade_weak().unwrap(); - let get_item = base.borrow().get_content_symbol("__getitem__", u32::MAX).symbols; - if get_item.len() == 1 { - let get_item = &get_item[0]; - let get_item = get_item.borrow(); - if get_item.evaluations().is_some() && get_item.evaluations().unwrap().len() == 1 { - let get_item_eval = &get_item.evaluations().unwrap()[0]; - if let Some(hook) = get_item_eval.symbol.get_symbol_hook { - context.as_mut().unwrap().insert(S!("args"), ContextValue::STRING(value)); - let old_range = context.as_mut().unwrap().remove(&S!("range")); - context.as_mut().unwrap().insert(S!("range"), ContextValue::RANGE(sub.slice.range())); - context.as_mut().unwrap().insert(S!("is_in_validation"), ContextValue::BOOLEAN(is_in_validation)); - let hook_result = hook(session, &get_item_eval.symbol, context, &mut diagnostics, Some(parent.clone())); - if let Some(hook_result) = hook_result { - match hook_result { - EvaluationSymbolPtr::WEAK(ref weak) => { - if !weak.weak.is_expired() { + let value = Evaluation::expr_to_str(session, &sub.slice, parent.clone(), max_infer, false, &mut diagnostics); + diagnostics.extend(value.1); + if let Some(value) = value.0 { + if !base.is_weak() { + continue; + } + let parent_file_or_func = parent.clone().borrow().parent_file_or_function().as_ref().unwrap().upgrade().unwrap(); + let is_in_validation = match parent_file_or_func.borrow().typ().clone() { + SymType::FILE | SymType::PACKAGE(_) | SymType::FUNCTION => { + parent_file_or_func.borrow().build_status(BuildSteps::VALIDATION) == BuildStatus::IN_PROGRESS + }, + _ => {false} + }; + let base = base.upgrade_weak().unwrap(); + let get_item = base.borrow().get_content_symbol("__getitem__", u32::MAX).symbols; + if get_item.len() == 1 { + let get_item = &get_item[0]; + let get_item = get_item.borrow(); + if get_item.evaluations().is_some() && get_item.evaluations().unwrap().len() == 1 { + let get_item_eval = &get_item.evaluations().unwrap()[0]; + if let Some(hook) = get_item_eval.symbol.get_symbol_hook { + context.as_mut().unwrap().insert(S!("args"), ContextValue::STRING(value)); + let old_range = context.as_mut().unwrap().remove(&S!("range")); + context.as_mut().unwrap().insert(S!("range"), ContextValue::RANGE(sub.slice.range())); + context.as_mut().unwrap().insert(S!("is_in_validation"), ContextValue::BOOLEAN(is_in_validation)); + let hook_result = hook(session, &get_item_eval.symbol, context, &mut diagnostics, Some(parent.clone())); + if let Some(hook_result) = hook_result { + match hook_result { + EvaluationSymbolPtr::WEAK(ref weak) => { + if !weak.weak.is_expired() { + evals.push(Evaluation::eval_from_ptr(&hook_result)); + } + }, + _ => { evals.push(Evaluation::eval_from_ptr(&hook_result)); } - }, - _ => { - evals.push(Evaluation::eval_from_ptr(&hook_result)); } } + context.as_mut().unwrap().remove(&S!("args")); + context.as_mut().unwrap().remove(&S!("is_in_validation")); + context.as_mut().unwrap().insert(S!("range"), old_range.unwrap()); } - context.as_mut().unwrap().remove(&S!("args")); - context.as_mut().unwrap().remove(&S!("is_in_validation")); - context.as_mut().unwrap().insert(S!("range"), old_range.unwrap()); } } } diff --git a/server/src/core/python_arch_builder_hooks.rs b/server/src/core/python_arch_builder_hooks.rs index 77a95304..505f7d78 100644 --- a/server/src/core/python_arch_builder_hooks.rs +++ b/server/src/core/python_arch_builder_hooks.rs @@ -38,14 +38,27 @@ static arch_class_hooks: Lazy> = Lazy::new(|| {vec![ let mut range = symbol.borrow().range().clone(); let slots = symbol.borrow().get_symbol(&(vec![], vec![Sy!("__slots__")]), u32::MAX); if slots.len() == 1 { - if slots.len() == 1 { - range = slots[0].borrow().range().clone(); - } + range = slots[0].borrow().range().clone(); } symbol.borrow_mut().add_new_variable(session, Sy!("env"), &range); } } }, + PythonArchClassHook { + odoo_entry: true, + trees: vec![ + (Sy!("15.3"), Sy!("999.0"), (vec![Sy!("odoo"), Sy!("http")], vec![Sy!("Request")])) + ], + func: |session: &mut SessionInfo, _entry_point: &Rc>, request_class: Rc>| { + // ----------- Request.env ------------ + let has_env = !request_class.borrow().get_content_symbol(&Sy!("env"), u32::MAX).symbols.is_empty(); + if has_env { + return; + } + let range = request_class.borrow().range().clone(); + request_class.borrow_mut().add_new_variable(session, Sy!("env"), &range); + } + }, PythonArchClassHook { odoo_entry: true, trees: vec![ diff --git a/server/src/core/python_arch_eval_hooks.rs b/server/src/core/python_arch_eval_hooks.rs index ef5b7b8d..79fc1c07 100644 --- a/server/src/core/python_arch_eval_hooks.rs +++ b/server/src/core/python_arch_eval_hooks.rs @@ -64,6 +64,54 @@ static arch_eval_file_hooks: Lazy> = Lazy::new(|| {v env.set_doc_string(Some(S!(""))); } }}, + PythonArchEvalFileHook {odoo_entry: true, + trees: vec![(Sy!("0.0"), Sy!("15.3"), (vec![Sy!("odoo"), Sy!("http")], vec![Sy!("request")]))], + if_exist_only: true, + func: |_odoo: &mut SessionInfo, _entry: &Rc>, file_symbol: Rc>, symbol: Rc>| { + // --------- request: WebRequest (before 15.3) --------- + let web_request_class = file_symbol.borrow().get_symbol(&(vec![], vec![Sy!("WebRequest")]), u32::MAX); + let Some(web_request_class) = web_request_class.last() else { + return; + }; + let mut request = symbol.borrow_mut(); + request.set_evaluations(vec![Evaluation::eval_from_symbol(&Rc::downgrade(web_request_class), Some(true))]); + }}, + PythonArchEvalFileHook {odoo_entry: true, + trees: vec![(Sy!("15.3"), Sy!("999.0"), (vec![Sy!("odoo"), Sy!("http")], vec![Sy!("request")]))], + if_exist_only: true, + func: |_odoo: &mut SessionInfo, _entry: &Rc>, file_symbol: Rc>, symbol: Rc>| { + // --------- request: Request (15.3+) --------- + let request_class = file_symbol.borrow().get_symbol(&(vec![], vec![Sy!("Request")]), u32::MAX); + let Some(request_class) = request_class.last() else { + return; + }; + let mut request = symbol.borrow_mut(); + request.set_evaluations(vec![Evaluation::eval_from_symbol(&Rc::downgrade(request_class), Some(true))]); + }}, + PythonArchEvalFileHook {odoo_entry: true, + trees: vec![ + (Sy!("0"), Sy!("15.3"), (vec![Sy!("odoo"), Sy!("http")], vec![Sy!("WebRequest"), Sy!("env")])), + (Sy!("15.3"), Sy!("999.0"), (vec![Sy!("odoo"), Sy!("http")], vec![Sy!("Request"), Sy!("env")])) + ], + if_exist_only: true, + func: |odoo: &mut SessionInfo, _entry: &Rc>, file_symbol: Rc>, symbol: Rc>| { + // --------- (Web)Request.env: Environment | None --------- + let env_file = odoo.sync_odoo.get_symbol(odoo.sync_odoo.config.odoo_path.as_ref().unwrap(), &(vec![Sy!("odoo"), Sy!("api")], vec![]), u32::MAX); + let Some(env_file) = env_file.last() else { + return; + }; + let env_class = env_file.borrow().get_symbol(&(vec![], vec![Sy!("Environment")]), u32::MAX); + let Some(env_class) = env_class.last() else { + return; + }; + // env is a property (function) before 15.3, and an instance variable in 15.3. + // In both cases the evaluation is Environment | None. + symbol.borrow_mut().set_evaluations(vec![ + Evaluation::eval_from_symbol(&Rc::downgrade(env_class), Some(true)), + Evaluation::new_none() + ]); + file_symbol.borrow_mut().add_dependency(&mut env_file.borrow_mut(), BuildSteps::ARCH_EVAL, BuildSteps::ARCH); + }}, PythonArchEvalFileHook {odoo_entry: true, trees: vec![(Sy!("0.0"), Sy!("18.1"), (vec![Sy!("odoo"), Sy!("models")], vec![Sy!("BaseModel"), Sy!("ids")])), (Sy!("18.1"), Sy!("999.0"), (vec![Sy!("odoo"), Sy!("orm"), Sy!("models")], vec![Sy!("BaseModel"), Sy!("ids")]))], diff --git a/server/tests/data/addons/module_1/__init__.py b/server/tests/data/addons/module_1/__init__.py index f49c07d5..d3f3705d 100644 --- a/server/tests/data/addons/module_1/__init__.py +++ b/server/tests/data/addons/module_1/__init__.py @@ -1,2 +1,3 @@ from . import models -from . import data \ No newline at end of file +from . import data +from . import controllers \ No newline at end of file diff --git a/server/tests/data/addons/module_1/controllers/__init__.py b/server/tests/data/addons/module_1/controllers/__init__.py new file mode 100644 index 00000000..757b12a1 --- /dev/null +++ b/server/tests/data/addons/module_1/controllers/__init__.py @@ -0,0 +1,2 @@ +# -*- coding: utf-8 -*- +from . import main diff --git a/server/tests/data/addons/module_1/controllers/main.py b/server/tests/data/addons/module_1/controllers/main.py new file mode 100644 index 00000000..bfed1939 --- /dev/null +++ b/server/tests/data/addons/module_1/controllers/main.py @@ -0,0 +1,36 @@ +# -*- coding: utf-8 -*- +from odoo import http +from odoo.http import request + + +class TestController(http.Controller): + + @http.route('/test/request', type='http', auth='public') + def test_request_type(self): + """Test that request has correct type.""" + # Hovering over 'request' should show Request class + req = request + + # Hovering over 'request.env' should show Environment | None + env = request.env + + # Accessing models via request.env + if request.env: + partner = request.env['res.partner'] + users = request.env['res.users'] + + return "OK" + + @http.route('/test/request/search', type='http', auth='user') + def test_request_env_usage(self): + """Test using request.env in typical scenarios.""" + # Direct model access + partners = request.env['res.partner'].search([]) + + # With sudo + admin_partners = request.env['res.partner'].sudo().search([]) + + # Accessing user + current_user = request.env.user + + return "OK" diff --git a/server/tests/test_controller.rs b/server/tests/test_controller.rs new file mode 100644 index 00000000..1031e739 --- /dev/null +++ b/server/tests/test_controller.rs @@ -0,0 +1,152 @@ +use std::env; +use std::path::PathBuf; +use std::cell::RefCell; +use std::rc::Rc; + +use odoo_ls_server::core::odoo::SyncOdoo; +use odoo_ls_server::core::symbols::symbol::Symbol; +use odoo_ls_server::core::file_mgr::FileInfo; +use odoo_ls_server::utils::PathSanitizer; +use odoo_ls_server::threads::SessionInfo; + +mod setup; +mod test_utils; + +/// Test that odoo.http.request and request.env hooks work correctly. +/// Tests hover and definition. +#[test] +fn test_controller() { + let (mut odoo, config) = setup::setup::setup_server(true); + let mut session = setup::setup::create_init_session(&mut odoo, config); + + // This test only applies to Odoo 15.3+. Skip otherwise. + if session.sync_odoo.version_major < 15 || (session.sync_odoo.version_major == 15 && session.sync_odoo.version_minor < 3) { + return; + } + + let test_file = PathBuf::from(env!("CARGO_MANIFEST_DIR")) + .join("tests/data/addons/module_1/controllers/main.py") + .sanitize(); + + let Some(file_symbol) = SyncOdoo::get_symbol_of_opened_file(&mut session, &PathBuf::from(&test_file)) else { + panic!("Failed to get file symbol for {}", test_file); + }; + + let file_mgr = session.sync_odoo.get_file_mgr(); + let file_info = file_mgr.borrow().get_file_info(&test_file).unwrap(); + + test_request_type_hover(&mut session, &file_symbol, &file_info); + test_request_env_definition(&mut session, &file_symbol, &file_info); + test_request_env_subscript(&mut session, &file_symbol, &file_info); +} + +/// Test that hovering over 'request' shows Request class +/// and that hovering over 'request.env' shows Environment type +fn test_request_type_hover( + session: &mut SessionInfo, + file_symbol: &Rc>, + file_info: &Rc> +) { + // Test 1: Hover over 'request' variable (line 11: req = request) + // Should show Request class + let hover_text = test_utils::get_hover_markdown(session, file_symbol, file_info, 11, 14) + .expect("Should get hover text for request"); + + assert!( + hover_text.contains("Request"), + "Hover over 'request' should show Request class. Got: {}", + hover_text + ); + + // Test 2: Hover over 'request.env' (line 14: env = request.env) + // Should show Environment | None + let hover_text = test_utils::get_hover_markdown(session, file_symbol, file_info, 14, 21) + .expect("Should get hover text for request.env"); + + assert!( + hover_text.contains("Environment"), + "Hover over 'request.env' should show Environment type. Got: {}", + hover_text + ); + assert!( + hover_text.contains("None"), + "Hover over 'request.env' should show None. Got: {}", + hover_text + ); +} + +/// Test that request.env provides correct definitions +fn test_request_env_definition( + session: &mut SessionInfo, + file_symbol: &Rc>, + file_info: &Rc> +) { + // Test 1: Go-to-definition on 'request' import (line 3: from odoo.http import request) + // Should navigate to request variable in odoo.http + let definitions = test_utils::get_definition_locs(session, file_symbol, file_info, 2, 22); + + assert!( + !definitions.is_empty(), + "Should find definition for 'request' import" + ); + + // Verify it points to odoo/http.py or similar + let target_uri = &definitions[0].target_uri.to_string(); + assert!( + target_uri.contains("odoo/http.py"), + "Definition should point to http module. Got: {:?}", + target_uri + ); + + // Test 2: Go-to-definition on 'request.env' (line 15: env = request.env) + // Should navigate to Environment class or env attribute + let definitions = test_utils::get_definition_locs(session, file_symbol, file_info, 14, 21); + + assert!( + !definitions.is_empty(), + "Should find definition for 'request.env'" + ); + + // Test 3: Hover on request.env.user (line 34: current_user = request.env.user) + // Should show proper type for user + let hover_text = test_utils::get_hover_markdown(session, file_symbol, file_info, 33, 35) + .expect("Should get hover text for 'user'"); + + // env.user should have a type (typically res.users or User) + assert!( + !hover_text.is_empty(), + "request.env.user should have hover information" + ); +} + +/// Test that request.env["model_name"] resolves to Model instance +fn test_request_env_subscript( + session: &mut SessionInfo, + file_symbol: &Rc>, + file_info: &Rc> +) { + let partner_class = test_utils::PARTNER_CLASS_NAME(&session.sync_odoo.full_version); + + // Test 1: Hover over 'partner' variable (line 19: partner = request.env['res.partner']) + // Should show Partner/ResPartner class + let hover_text = test_utils::get_hover_markdown(session, file_symbol, file_info, 18, 12) + .expect("Should get hover text for 'partner' variable"); + + assert!( + hover_text.contains(partner_class), + "Hover over 'partner' should show {} class. Got: {}", + partner_class, + hover_text + ); + + // Test 2: Hover over .search on request.env['res.partner'].search (line 29) + // Should display markdown content for the search method + let hover_text = test_utils::get_hover_markdown(session, file_symbol, file_info, 27, 46) + .expect("Should get hover text for .search method"); + + assert!( + hover_text.contains("def search"), + "Hover over .search should have markdown content and mention 'search'. Got: {}", + hover_text + ); +}