Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 25, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 11 additions & 3 deletions server/src/core/python_arch_eval_hooks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
22 changes: 17 additions & 5 deletions server/src/features/completion.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use std::cmp::Ordering;
use std::collections::{HashMap, HashSet};
use std::{cell::RefCell, rc::Rc};
use itertools::Itertools;
Expand All @@ -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;
Expand Down Expand Up @@ -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)]
Expand Down
67 changes: 42 additions & 25 deletions server/src/features/features_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand All @@ -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(
Expand Down Expand Up @@ -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;
Expand Down
23 changes: 15 additions & 8 deletions server/tests/test_get_symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand All @@ -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();
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand All @@ -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();
Expand Down
27 changes: 25 additions & 2 deletions server/tests/test_utils.rs
Original file line number Diff line number Diff line change
@@ -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> {
Expand Down