[FIX] server: follow ref for decs and version check

Author: mmahrouss

server/src/core/python_arch_eval_hooks.rs

@@ -560,9 +560,17 @@ impl PythonArchEvalHooks {
             let (dec_evals, diags) = Evaluation::eval_from_ast(session, &decorator_base, parent, &func_stmt.range.start(),&mut deps);
             Symbol::insert_dependencies(&file, &mut deps, current_step);
             diagnostics.extend(diags);
-            for decorator_eval in dec_evals.iter(){
-                let EvaluationSymbolPtr::WEAK(decorator_eval_sym_weak) = decorator_eval.symbol.get_symbol(session, &mut None, &mut diagnostics, None)  else {continue};
-                let Some(dec_sym) = decorator_eval_sym_weak.weak.upgrade() else {continue};
+            let mut followed_evals = vec![];
+            for eval in dec_evals {
+                followed_evals.extend(Symbol::follow_ref(&eval.symbol.get_symbol(session, &mut None, &mut vec![], None), session, &mut None, true, false, None, &mut vec![]));
+            }
+            for decorator_eval in followed_evals {
+                let EvaluationSymbolPtr::WEAK(decorator_eval_sym_weak) = decorator_eval else {
+                    continue;
+                };
+                let Some(dec_sym) = decorator_eval_sym_weak.weak.upgrade() else {
+                    continue;
+                };
                 let dec_sym_tree = dec_sym.borrow().get_tree();
                 for hook in arch_eval_decorator_hooks.iter() {
                     for (min_version, max_version, hook_tree) in hook.trees.iter() {

server/src/features/completion.rs

@@ -1,3 +1,4 @@
+use std::cmp::Ordering;
 use std::collections::{HashMap, HashSet};
 use std::{cell::RefCell, rc::Rc};
 use itertools::Itertools;
@@ -12,6 +13,7 @@ use crate::core::odoo::SyncOdoo;
 use crate::core::python_arch_eval_hooks::PythonArchEvalHooks;
 use crate::core::symbols::module_symbol::ModuleSymbol;
 use crate::threads::SessionInfo;
+use crate::utils::compare_semver;
 use crate::{oyarn, Sy, S};
 use crate::core::symbols::symbol::Symbol;
 use crate::features::features_utils::FeaturesUtils;
@@ -504,12 +506,22 @@ fn complete_decorator_call(
     }
     let scope = Symbol::get_scope_symbol(file.clone(), offset as u32, false);
     let dec_evals = Evaluation::eval_from_ast(session, &decorator_base, scope.clone(), max_infer, &mut vec![]).0;
-    for decorator_eval in dec_evals.iter(){
-        let EvaluationSymbolPtr::WEAK(decorator_eval_sym_weak) = decorator_eval.symbol.get_symbol(session, &mut None, &mut vec![], None)  else {continue};
-        let Some(dec_sym) = decorator_eval_sym_weak.weak.upgrade() else {continue};
+    let mut followed_evals = vec![];
+    for eval in dec_evals {
+        followed_evals.extend(Symbol::follow_ref(&eval.symbol.get_symbol(session, &mut None, &mut vec![], None), session, &mut None, true, false, None, &mut vec![]));
+    }
+    for decorator_eval in followed_evals{
+        let EvaluationSymbolPtr::WEAK(decorator_eval_sym_weak) = decorator_eval else {
+            continue;
+        };
+        let Some(dec_sym) = decorator_eval_sym_weak.weak.upgrade() else {
+            continue;
+        };
         let dec_sym_tree = dec_sym.borrow().get_tree();
-        let expected_types = if dec_sym_tree.0.ends_with(&[Sy!("odoo"), Sy!("api")]){
-            if [vec![Sy!("onchange")], vec![Sy!("constrains")]].contains(&dec_sym_tree.1) && SyncOdoo::is_in_main_entry(session, &dec_sym_tree.0){
+        let version_comparison = compare_semver(session.sync_odoo.full_version.as_str(), "18.1.0");
+        let expected_types = if (version_comparison < Ordering::Equal && dec_sym_tree.0.ends_with(&[Sy!("odoo"), Sy!("api")])) ||
+                (version_comparison >= Ordering::Equal && dec_sym_tree.0.ends_with(&[Sy!("odoo"), Sy!("orm"), Sy!("decorators")])) {
+            if [vec![Sy!("onchange")], vec![Sy!("constrains")]].contains(&dec_sym_tree.1) && SyncOdoo::is_in_main_entry(session, &dec_sym_tree.0) {
                 &vec![ExpectedType::SIMPLE_FIELD]
             } else if dec_sym_tree.1 == vec![Sy!("depends")] && SyncOdoo::is_in_main_entry(session, &dec_sym_tree.0){
                 &vec![ExpectedType::NESTED_FIELD(None)]

server/src/features/features_utils.rs

@@ -4,7 +4,8 @@ use ruff_text_size::{Ranged, TextRange, TextSize};
 use crate::core::file_mgr::FileMgr;
 use crate::core::odoo::SyncOdoo;
 use crate::core::symbols::function_symbol::Argument;
-use crate::utils::PathSanitizer;
+use crate::utils::{compare_semver, PathSanitizer};
+use std::cmp::Ordering;
 use std::collections::HashMap;
 use std::path::PathBuf;
 use std::rc::Weak;
@@ -71,12 +72,13 @@ impl FeaturesUtils {
             return vec![];
         }
         let evaluations = Evaluation::eval_from_ast(session, &call_expr.func, scope.clone(), &call_expr.func.range().start(), &mut vec![]).0;
-        if !evaluations.iter().any(|eval|
-            match eval.symbol.get_symbol_as_weak(session, &mut None, &mut vec![], None).weak.upgrade() {
-                Some(sym_rc) => sym_rc.borrow().is_field_class(session),
-                None => false
-            }
-        ){
+        let mut followed_evals = vec![];
+        for eval in evaluations {
+            followed_evals.extend(Symbol::follow_ref(&eval.symbol.get_symbol(session, &mut None, &mut vec![], None), session, &mut None, true, false, None, &mut vec![]));
+        }
+        if !followed_evals.iter().any(|eval|
+            eval.is_weak() && eval.as_weak().weak.upgrade().map(|sym| sym.borrow().is_field_class(session)).unwrap_or(false)
+        ) {
             return vec![];
         }
         parent_class.clone().borrow().get_member_symbol(session, field_value, from_module.clone(), false, false, true, false).0
@@ -188,10 +190,16 @@ impl FeaturesUtils {
     ) -> Vec<Rc<RefCell<Symbol>>>{
         let mut arg_symbols: Vec<Rc<RefCell<Symbol>>> = vec![];
         let callable_evals = Evaluation::eval_from_ast(session, &call_expr.func, scope.clone(), &call_expr.func.range().start(), &mut vec![]).0;
-        for callable_eval in callable_evals.iter() {
-            let callable = callable_eval.symbol.get_symbol_as_weak(session, &mut None, &mut vec![], None);
+        let mut followed_evals = vec![];
+        for eval in callable_evals {
+            followed_evals.extend(Symbol::follow_ref(&eval.symbol.get_symbol(session, &mut None, &mut vec![], None), session, &mut None, true, false, None, &mut vec![]));
+        }
+        for callable_eval in followed_evals {
+            let EvaluationSymbolPtr::WEAK(callable) = callable_eval else {
+                continue;
+            };
             let Some(callable_sym) = callable.weak.upgrade() else {
-                continue
+                continue;
             };
             if callable_sym.borrow().typ() != SymType::FUNCTION {
                 continue;
@@ -200,18 +208,21 @@ impl FeaturesUtils {
 
             // Check if we are in api.onchange/constrains/depends
             let func_sym_tree = func.get_tree();
-            if func_sym_tree.0.ends_with(&[Sy!("odoo"), Sy!("api")]){
-                if [vec![Sy!("onchange")], vec![Sy!("constrains")]].contains(&func_sym_tree.1) && SyncOdoo::is_in_main_entry(session, &func_sym_tree.0){
-                arg_symbols.extend(
-                    FeaturesUtils::find_simple_decorator_field_symbol(session, scope.clone(), from_module.clone(), field_name)
-                );
-                continue;
-            } else if func_sym_tree.1 == vec![Sy!("depends")] && SyncOdoo::is_in_main_entry(session, &func_sym_tree.0){
-                arg_symbols.extend(
-                    FeaturesUtils::find_nested_fields_in_class(session, scope.clone(), from_module.clone(), &field_range, field_name, &offset)
-                );
-                continue;
-            }
+            // TODO: account for change in tree after 18.1 odoo.orm.decorators
+            let version_comparison = compare_semver(session.sync_odoo.full_version.as_str(), "18.1.0");
+            if (version_comparison < Ordering::Equal && func_sym_tree.0.ends_with(&[Sy!("odoo"), Sy!("api")])) ||
+                (version_comparison >= Ordering::Equal && func_sym_tree.0.ends_with(&[Sy!("odoo"), Sy!("orm"), Sy!("decorators")])){
+                if [vec![Sy!("onchange")], vec![Sy!("constrains")]].contains(&func_sym_tree.1) && SyncOdoo::is_in_main_entry(session, &func_sym_tree.0) {
+                    arg_symbols.extend(
+                        FeaturesUtils::find_simple_decorator_field_symbol(session, scope.clone(), from_module.clone(), field_name)
+                    );
+                    continue;
+                } else if func_sym_tree.1 == vec![Sy!("depends")] && SyncOdoo::is_in_main_entry(session, &func_sym_tree.0){
+                    arg_symbols.extend(
+                        FeaturesUtils::find_nested_fields_in_class(session, scope.clone(), from_module.clone(), &field_range, field_name, &offset)
+                    );
+                    continue;
+                }
             }
 
             let func_arg = func.as_func().get_indexed_arg_in_call(
@@ -251,10 +262,16 @@ impl FeaturesUtils {
         }
         let mut arg_symbols: Vec<Rc<RefCell<Symbol>>> = vec![];
         let callable_evals = Evaluation::eval_from_ast(session, &call_expr.func, scope.clone(), &call_expr.func.range().start(), &mut vec![]).0;
-        for callable_eval in callable_evals.iter() {
-            let callable = callable_eval.symbol.get_symbol_as_weak(session, &mut None, &mut vec![], None);
+        let mut followed_evals = vec![];
+        for eval in callable_evals {
+            followed_evals.extend(Symbol::follow_ref(&eval.symbol.get_symbol(session, &mut None, &mut vec![], None), session, &mut None, true, false, None, &mut vec![]));
+        }
+        for callable_eval in followed_evals {
+            let EvaluationSymbolPtr::WEAK(callable) = callable_eval else {
+                continue;
+            };
             let Some(callable_sym) = callable.weak.upgrade() else {
-                continue
+                continue;
             };
             if !callable_sym.borrow().is_field_class(session) {
                 continue;

server/tests/test_get_symbol.rs

@@ -20,6 +20,9 @@ fn test_hover_on_model_field_and_method() {
     assert!(PathBuf::from(&test_file).exists(), "Test file does not exist: {}", test_file);
     let mut session = setup::setup::create_session(&mut odoo);
 
+    // Use Lazy value for partner and country class names
+    let partner_class_name = test_utils::PARTNER_CLASS_NAME(session.sync_odoo.full_version.as_str());
+    let country_class_name = test_utils::COUNTRY_CLASS_NAME(session.sync_odoo.full_version.as_str());
     // Get file symbol and file info
     let file_mgr = session.sync_odoo.get_file_mgr();
     let file_info = file_mgr.borrow().get_file_info(&test_file).unwrap();
@@ -53,12 +56,12 @@ fn test_hover_on_model_field_and_method() {
     // Hover on related field "partner_company_phone_code"
     let hover_partner_id = test_utils::get_hover_markdown(&mut session, &file_symbol, &file_info, 10, 63).unwrap_or_default();
     assert!(
-        hover_partner_id.contains("partner_id: Partner"),
+        hover_partner_id.contains(&format!("partner_id: {}", partner_class_name)),
         "Hover on field_name in related field name should show field name and field type"
     );
     let hover_country_id = test_utils::get_hover_markdown(&mut session, &file_symbol, &file_info, 10, 74).unwrap_or_default();
     assert!(
-        hover_country_id.contains("country_id: Country"),
+        hover_country_id.contains(&format!("country_id: {}", country_class_name)),
         "Hover on field_name in related field name should show field name and field type"
     );
     let hover_phone_code = test_utils::get_hover_markdown(&mut session, &file_symbol, &file_info, 10, 86).unwrap_or_default();
@@ -96,12 +99,12 @@ fn test_hover_on_model_field_and_method() {
     // Hover on depends decorator, on different sections
     let hover_partner_id = test_utils::get_hover_markdown(&mut session, &file_symbol, &file_info, 27, 22).unwrap_or_default();
     assert!(
-        hover_partner_id.contains("partner_id: Partner"),
+        hover_partner_id.contains(&format!("partner_id: {}", partner_class_name)),
         "Hover on field_name in depends should show field name and field type"
     );
     let hover_country_id = test_utils::get_hover_markdown(&mut session, &file_symbol, &file_info, 27, 35).unwrap_or_default();
     assert!(
-        hover_country_id.contains("country_id: Country"),
+        hover_country_id.contains(&format!("country_id: {}", country_class_name)),
         "Hover on field_name in depends should show field name and field type"
     );
     let hover_code = test_utils::get_hover_markdown(&mut session, &file_symbol, &file_info, 27, 43).unwrap_or_default();
@@ -125,12 +128,12 @@ fn test_hover_on_model_field_and_method() {
     // Hover on domains, on different sections
     let hover_partner_id = test_utils::get_hover_markdown(&mut session, &file_symbol, &file_info, 31, 25).unwrap_or_default();
     assert!(
-        hover_partner_id.contains("partner_id: Partner"),
+        hover_partner_id.contains(&format!("partner_id: {}", partner_class_name)),
         "Hover on field_name in search domain should show field name and field type"
     );
     let hover_country_id = test_utils::get_hover_markdown(&mut session, &file_symbol, &file_info, 31, 39).unwrap_or_default();
     assert!(
-        hover_country_id.contains("country_id: Country"),
+        hover_country_id.contains(&format!("country_id: {}", country_class_name)),
         "Hover on field_name in search domain should show field name and field type"
     );
     let hover_code = test_utils::get_hover_markdown(&mut session, &file_symbol, &file_info, 31, 48).unwrap_or_default();
@@ -164,6 +167,10 @@ fn test_definition() {
     assert!(PathBuf::from(&module2_test_file).exists(), "Test file does not exist: {}", module1_test_file);
     let mut session = setup::setup::create_session(&mut odoo);
 
+    // Use Lazy value for partner and Country class names
+    let partner_class_name = test_utils::PARTNER_CLASS_NAME(session.sync_odoo.full_version.as_str());
+    let country_class_name = test_utils::COUNTRY_CLASS_NAME(session.sync_odoo.full_version.as_str());
+
     // Get file symbol and file info
     let file_mgr = session.sync_odoo.get_file_mgr();
     let m1_tf_file_info = file_mgr.borrow().get_file_info(&module1_test_file).unwrap();
@@ -219,7 +226,7 @@ fn test_definition() {
     assert_eq!(file_mgr.borrow().text_range_to_range(&mut session, &module1_test_file, sym_partner_id[0].borrow().range()), partner_id_locs[0].range, "Expected partner_id to be at the same location as the field");
 
     let country_id_locs = test_utils::get_definition_locs(&mut session, &m1_tf_file_symbol, &m1_tf_file_info, 10, 74);
-    let country_id_field_sym = session.sync_odoo.get_symbol(odoo_path, &(vec![Sy!("odoo"), Sy!("addons"), Sy!("base"), Sy!("models"), Sy!("res_partner")], vec![Sy!("Partner"), Sy!("country_id")]), u32::MAX);
+    let country_id_field_sym = session.sync_odoo.get_symbol(odoo_path, &(vec![Sy!("odoo"), Sy!("addons"), Sy!("base"), Sy!("models"), Sy!("res_partner")], vec![Sy!(partner_class_name), Sy!("country_id")]), u32::MAX);
     assert_eq!(country_id_field_sym.len(), 1, "Expected 1 location for country_id");
     let country_id_field_sym = country_id_field_sym[0].clone();
     let country_id_file = country_id_field_sym.borrow().get_file().unwrap().upgrade().unwrap().borrow().paths()[0].clone();
@@ -229,7 +236,7 @@ fn test_definition() {
 
     // now the same for phone_code
     let phone_code_locs = test_utils::get_definition_locs(&mut session, &m1_tf_file_symbol, &m1_tf_file_info, 10, 86);
-    let phone_code_field_sym = session.sync_odoo.get_symbol(odoo_path, &(vec![Sy!("odoo"), Sy!("addons"), Sy!("base"), Sy!("models"), Sy!("res_country")], vec![Sy!("Country"), Sy!("phone_code")]), u32::MAX);
+    let phone_code_field_sym = session.sync_odoo.get_symbol(odoo_path, &(vec![Sy!("odoo"), Sy!("addons"), Sy!("base"), Sy!("models"), Sy!("res_country")], vec![Sy!(country_class_name), Sy!("phone_code")]), u32::MAX);
     assert_eq!(phone_code_field_sym.len(), 1, "Expected 1 location for phone_code");
     let phone_code_field_sym = phone_code_field_sym[0].clone();
     let phone_code_file = phone_code_field_sym.borrow().get_file().unwrap().upgrade().unwrap().borrow().paths()[0].clone();

server/tests/test_utils.rs

@@ -1,8 +1,31 @@
-use std::{cell::RefCell, rc::Rc};
+use once_cell::sync::Lazy;
+use std::{cell::RefCell, rc::Rc, cmp::Ordering};
 
-use odoo_ls_server::{core::{file_mgr::FileInfo, symbols::symbol::Symbol}, threads::SessionInfo};
+use odoo_ls_server::{core::{file_mgr::FileInfo, symbols::symbol::Symbol}, threads::SessionInfo, utils::compare_semver};
 
 
+/// Returns the correct class name for Partner/ResPartner depending on Odoo version
+pub static PARTNER_CLASS_NAME: Lazy<fn(&str) -> &'static str> = Lazy::new(|| {
+    |full_version: &str| {
+        if compare_semver(full_version, "18.1") >= Ordering::Equal {
+            "ResPartner"
+        } else {
+            "Partner"
+        }
+    }
+});
+
+/// Returns the correct class name for Country/ResCountry depending on Odoo version
+pub static COUNTRY_CLASS_NAME: Lazy<fn(&str) -> &'static str> = Lazy::new(|| {
+    |full_version: &str| {
+        if compare_semver(full_version, "18.1") >= Ordering::Equal {
+            "ResCountry"
+        } else {
+            "Country"
+        }
+    }
+});
+
 
 /// Helper to get hover markdown string at a given (line, character)
 pub fn get_hover_markdown(session: &mut SessionInfo, file_symbol: &Rc<RefCell<Symbol>>, file_info: &Rc<RefCell<FileInfo>>, line: u32, character: u32) -> Option<String> {