[IMP] Implement Go To References Part 1 - python code by fda-odoo · Pull Request #531 · odoo/odoo-ls

fda-odoo · 2026-01-28T10:20:40Z

First part of the Go To References.
Include Python searches: python variables, models.
Do not search or work in:

xml/csv files
manifest files
lambda functions

github-actions · 2026-02-19T13:21:25Z

⚠️ The file 'Cargo.toml or constant.rs' has been modified in this PR. Please review the changes carefully.

github-actions · 2026-02-19T13:31:17Z

⚠️ The file 'Cargo.toml or constant.rs' has been modified in this PR. Please review the changes carefully.

github-actions · 2026-02-19T16:05:25Z

⚠️ The file 'Cargo.toml or constant.rs' has been modified in this PR. Please review the changes carefully.

github-actions · 2026-02-19T17:57:18Z

⚠️ The file 'Cargo.toml or constant.rs' has been modified in this PR. Please review the changes carefully.

github-actions · 2026-02-20T17:04:31Z

⚠️ The file 'Cargo.toml or constant.rs' has been modified in this PR. Please review the changes carefully.

github-actions · 2026-02-23T16:10:47Z

⚠️ The file 'Cargo.toml or constant.rs' has been modified in this PR. Please review the changes carefully.

github-actions · 2026-02-23T17:09:07Z

⚠️ The file 'Cargo.toml or constant.rs' has been modified in this PR. Please review the changes carefully.

github-actions · 2026-02-23T17:30:00Z

⚠️ The file 'Cargo.toml or constant.rs' has been modified in this PR. Please review the changes carefully.

github-actions · 2026-02-24T10:06:50Z

⚠️ The file 'Cargo.toml or constant.rs' has been modified in this PR. Please review the changes carefully.

…odels

github-actions · 2026-02-24T10:13:55Z

⚠️ The file 'Cargo.toml or constant.rs' has been modified in this PR. Please review the changes carefully.

github-actions · 2026-02-24T10:37:09Z

⚠️ The file 'Cargo.toml or constant.rs' has been modified in this PR. Please review the changes carefully.

mmahrouss

Looks good, I added some cosmetic suggestions, nothing functional:D

mmahrouss · 2026-02-24T12:17:39Z

server/src/core/evaluation.rs

+                if odoo.evaluation_search.is_some() {
+                    match operator.op {
+                        Operator::Add | Operator::Mult | Operator::Sub | Operator::Div | Operator::BitAnd | Operator::BitOr | Operator::BitXor |Operator::FloorDiv | Operator::LShift | Operator::MatMult | Operator::Mod | Operator::Pow | Operator::RShift => {
+                            Evaluation::eval_from_ast(session, &operator.left, parent.clone(), max_infer, false, required_dependencies);
+                            Evaluation::eval_from_ast(session, &operator.right, parent.clone(), max_infer, false, required_dependencies);
+                        }
+                    }
                }


The match is not needed here, we can drop it

Suggested change

if odoo.evaluation_search.is_some() {

match operator.op {

Operator::Add | Operator::Mult | Operator::Sub | Operator::Div | Operator::BitAnd | Operator::BitOr | Operator::BitXor |Operator::FloorDiv | Operator::LShift | Operator::MatMult | Operator::Mod | Operator::Pow | Operator::RShift => {

Evaluation::eval_from_ast(session, &operator.left, parent.clone(), max_infer, false, required_dependencies);

Evaluation::eval_from_ast(session, &operator.right, parent.clone(), max_infer, false, required_dependencies);

}

}

}

if odoo.evaluation_search.is_some() {

Evaluation::eval_from_ast(session, &operator.left, parent.clone(), max_infer, false, required_dependencies);

Evaluation::eval_from_ast(session, &operator.right, parent.clone(), max_infer, false, required_dependencies);

}

mmahrouss · 2026-02-24T12:27:57Z

server/src/core/odoo.rs

@@ -1468,14 +1604,28 @@ impl Odoo {
                match ast_type {


Suggestion to have less code duplication and readability

Suggested change

match ast_type {

if file_info.borrow().file_info_ast.borrow().indexed_module.is_none() && matches!(ast_type, AstType::Python) {

return Ok(None);

}

let location_fn = match (ast_type, is_declaration) {

(AstType::Python, true) => DeclarationFeature::get_location,

(AstType::Python, false) => DefinitionFeature::get_location,

(AstType::Xml, true) => DeclarationFeature::get_location_xml,

(AstType::Xml, false) => DefinitionFeature::get_location_xml,

(AstType::Csv, true) => DeclarationFeature::get_location_csv,

(AstType::Csv, false) => DefinitionFeature::get_location_csv,

};

return Ok(location_fn(session, &file_symbol, &file_info, params.text_document_position_params.position.line, params.text_document_position_params.position.character));

mmahrouss · 2026-02-24T12:52:39Z

server/src/features/definition.rs

        Some(GotoDefinitionResponse::Link(links))
    }

    pub fn get_location_xml(session: &mut SessionInfo,


could be useful to isolate get_location_xml from declaration and definition to goto_utils, to have less code duplication. what do you think?

mmahrouss · 2026-02-24T13:29:19Z

server/tests/test_references.rs

+    // for r in references.iter() {
+    //     error!("Reference found at {}:{}:{}", r.uri.as_str(), r.range.start.line, r.range.start.character);
+    // }
+}


unneeded old comment?

mmahrouss · 2026-02-24T13:42:56Z

server/src/core/symbols/symbol.rs

+            let Some(symbol) = w.weak.upgrade() else {
+                res.push(current_sym.clone());
+                continue;
+            };


Why are we adding the sym to res in the else? it is an expired weak, so it is better to just continue, right?

If we can't go further in the evaluations, we want to still keep the actual result, so the else is adding it to the list. Seems correct to me.

I was about to comment the same thing. Keeping an EvaluationSymbolPtr::WEAK to an expired weak is useful? Why?

mmahrouss · 2026-02-24T14:14:16Z

server/src/core/evaluation.rs

+            for eval in evals.iter() {
+                if eval.symbol.sym.is_weak() && let Some(weak) = eval.symbol.sym.as_weak().weak.upgrade() {
+                    if Rc::ptr_eq(&weak, evaluation_search) {
+                        if found_one_reference {
+                            //if we have multiple matches, it means that that ast can reference it multiple times, but we only want to know if that ast matches or not
+                            continue;
+                        }


We could move the check of found_one_reference and break early to avoid extra checks

Suggested change

for eval in evals.iter() {

if eval.symbol.sym.is_weak() && let Some(weak) = eval.symbol.sym.as_weak().weak.upgrade() {

if Rc::ptr_eq(&weak, evaluation_search) {

if found_one_reference {

//if we have multiple matches, it means that that ast can reference it multiple times, but we only want to know if that ast matches or not

continue;

}

for eval in evals.iter() {

if found_one_reference {

//if we have multiple matches, it means that that ast can reference it multiple times, but we only want to know if that ast matches or not

break;

}

if eval.symbol.sym.is_weak() && let Some(weak) = eval.symbol.sym.as_weak().weak.upgrade() {

if Rc::ptr_eq(&weak, evaluation_search) {

mmahrouss · 2026-02-24T14:32:43Z

server/src/features/references.rs

+    /// Same-file references only, local vars, params, methods
+    /// TODO: Cross-file references within module
+    /// TODO: All files within workspace
+    /// TODO: Odoo specific (XML field refs, string-based model refs)


This comments are no longer correct, no? perhaps only XML refs are missing?

mmahrouss · 2026-02-24T14:53:06Z

server/src/features/references.rs

+                    let sym = self.sym_stack.last().unwrap().borrow().get_positioned_symbol(&OYarn::from(f.name.to_string()), &f.range);
+                    if let Some(sym) = sym {
+                        self.sym_stack.push(sym);
+                        self.visit_vec_stmt(session, &f.body);
+                        self.sym_stack.pop();
+                    }


We could isolate this into its own method visit_function_def to be consistent with the other branches, what do you think?

mmahrouss · 2026-02-24T14:59:43Z

server/src/features/references.rs

+            let var_name = if alias.asname.is_none() {
+                S!(alias.name.split(".").next().unwrap())
+            } else {
+                alias.asname.as_ref().unwrap().clone().to_string()
+            };


Could be a bit more readable to use match here.

Suggested change

let var_name = if alias.asname.is_none() {

S!(alias.name.split(".").next().unwrap())

} else {

alias.asname.as_ref().unwrap().clone().to_string()

};

let var_name = match alias.asname.as_ref() {

None => S!(alias.name.split(".").next().unwrap()),

Some(asname) => S!(asname),

};

cammarosano

Very nice!! Killer feature, nice approach!
I have added mostly minor style suggestions, feel free to ignore them.

cammarosano · 2026-02-24T22:15:49Z

server/src/core/symbols/class_symbol.rs


    pub fn add_symbol(&mut self, content: &Rc<RefCell<Symbol>>, section: u32) {
-        let sections = self.symbols.entry(content.borrow().name().clone()).or_insert(HashMap::new());
+        let sections = self.symbols.entry(content.borrow().name().clone()).or_insert(HashMap::default());


or simply or_default()

Suggested change

let sections = self.symbols.entry(content.borrow().name().clone()).or_insert(HashMap::default());

let sections = self.symbols.entry(content.borrow().name().clone()).or_default());

(same for function_symbol, file_symbol, etc)

cammarosano · 2026-02-24T22:18:08Z

server/src/utils.rs

+
+impl Hasher for U32IdentityHasher {
+	fn write(&mut self, bytes: &[u8]) {
+		// fallback lent (rare si on n'utilise que u32)


cammarosano · 2026-02-24T22:45:35Z

server/src/utils.rs

+#[derive(Default)]
+pub struct U32IdentityHasher(u64);
+
+impl Hasher for U32IdentityHasher {


This looks cool!
But did you perf? Did it improve?

Have you considered using a Vector instead of Hashmap? If the idea is to use small sequential integers as keys, wouldn't a Vector be simpler (and cache friendlier)?
You could just type alias it to "SymbolMap" or something, ex:

type SymbolMap = Vec<u32, Vec<Rc<RefCell<Symbol>>>>

or maybe wrap the inner Vec in an Option (to signal the absence/presence of entry for a given key), replicating what a map would return for get:

type SymbolMap = Vec<u32, Option<Vec<Rc<RefCell<Symbol>>>>>

Your optimization makes the hashmap behave like a vector (no hashing, direct indexing), so why not go for a vector?

cammarosano · 2026-02-24T22:58:47Z

server/src/core/evaluation.rs

            symbol: EvaluationSymbol {
                sym: EvaluationSymbolPtr::WEAK(EvaluationSymbolWeak{
-                    weak: Rc::downgrade(&odoo.get_symbol("", &(vec![Sy!("builtins")], vec![Sy!("list")]), u32::MAX).last().expect("builtins list not found")),
+                    weak: odoo.get_ts_list(),


Very minor:
You replaced this expect with a unwrap (inside get_ts_list). I would suggest keeping the expect with an informative message.
(same for the other get_ts_*s below)
Also, see other comment about using a macro. That could be a way to bring the expect back.

cammarosano · 2026-02-24T23:04:06Z

server/src/core/odoo.rs

+        }
+        self.typeshed_weak_cache.object.clone()
+    }
+


Maybe a macro?

macro_rules! typeshed_getter { ($fn_name:ident, $field:ident, $($path:expr),+) => { pub fn $fn_name(&mut self) -> Weak<RefCell<Symbol>> { if self.typeshed_weak_cache.$field.is_expired() { self.typeshed_weak_cache.$field = Rc::downgrade( &self.get_symbol("", &(vec![Sy!("builtins")], vec![Sy!($($path),+)]), u32::MAX) .last() .expect(concat!("builtins ", stringify!($field), " not found")) ); } self.typeshed_weak_cache.$field.clone() } }; } impl SyncOdoo { typeshed_getter!(get_ts_dict, dict, "dict"); typeshed_getter!(get_ts_tuple, tuple, "tuple"); typeshed_getter!(get_ts_set, set, "set"); typeshed_getter!(get_ts_list, list, "list"); typeshed_getter!(get_ts_string, string, "str"); typeshed_getter!(get_ts_boolean, boolean, "bool"); typeshed_getter!(get_ts_int, int, "int"); typeshed_getter!(get_ts_float, float, "float"); typeshed_getter!(get_ts_complex, complex, "complex"); typeshed_getter!(get_ts_ellipsis, ellipsis, "Ellipsis"); typeshed_getter!(get_ts_bytes, bytes, "bytes"); typeshed_getter!(get_ts_object, object, "object"); }

cammarosano · 2026-02-25T10:08:05Z

server/src/core/evaluation.rs

+                if let Some(value) = eval.value.as_ref() {
+                    match value {
+                        EvaluationValue::CONSTANT(constant) => {
+                            match constant {
+                                Expr::StringLiteral(s) => {
+                                    if evaluation_search.borrow().typ() == SymType::CLASS {
+                                        let class_bw = evaluation_search.borrow();
+                                        let class = class_bw.as_class_sym();
+                                        if let Some(model_data) = class._model.as_ref() {
+                                            if oyarn!("{}", s.value.to_str()) == model_data.name {
+                                                let file = parent.borrow().get_file().unwrap().upgrade().unwrap();
+                                                let file_info = session.sync_odoo.get_file_mgr().borrow().get_file_info(&file.borrow().paths()[0]);
+                                                if let Some(file_info) = file_info {
+                                                    let transformed_range = file_info.borrow().text_range_to_range(&s.range, session.sync_odoo.encoding);
+                                                    let uri = FileMgr::pathname2uri(&file.borrow().paths().first().unwrap());
+                                                    session.sync_odoo.evaluation_locations.push(Location {
+                                                        uri: uri,
+                                                        range: transformed_range,
+                                                    });
+                                                }
+                                            }
+                                        }
+                                    }
+                                },
+                                _ => {}
+                            }


Every time you write code like this a puppy dies !!!!

😆

I suggest replacing this pattern

match value { the_one_pattern(x) => {...} _ => {} }

By this:

let the_one_pattern(x) = value else { continue; } ...

The same for if let without any code after it -> let ... else { continue }

cammarosano · 2026-02-25T10:14:40Z

server/src/core/evaluation.rs

            }
            _ => {}
        }
+        if let Some(evaluation_search) = session.sync_odoo.evaluation_search.as_ref() {


Beware of early returns in the code above this part!
A possible way to ensure this part always runs would be to make this a sibling function of current analyze_ast:

fn analyze_ast() -> result { result = old_analyze_ast; new_function_for_references(result.evals); return result; }

And then found_one_reference would be declared inside thenew_function_for_references

Edit: maybe make new_function_for_references get called by eval_from_ast, right after the call to analyze_ast?

cammarosano · 2026-02-25T12:13:28Z

server/tests/test_references.rs

+    assert!(references.len() == 0, "Some references were not expected: {}",
+        references.iter().map(|r| format!("{}:{}:{}", r.uri.as_str(), r.range.start.line, r.range.start.character)).collect::<Vec<String>>().join(", ")
+    );
+


I suggest adding more tests 😬
Like functions/methods, shadowing/nested scopes... or when the lack of results is expected (like a literal).

cammarosano · 2026-02-25T12:14:17Z

server/src/features/references.rs

+    sym_stack: Vec<Rc<RefCell<Symbol>>>,
+}
+
+impl ReferenceVisitor {


this is beautiful 💟

cammarosano · 2026-02-25T12:19:44Z

server/src/core/evaluation.rs

+                            let uri = FileMgr::pathname2uri(&file.borrow().paths().first().unwrap());
+                            if session.sync_odoo.evaluation_locations.is_empty() ||
+                            session.sync_odoo.evaluation_locations.last().unwrap().uri != uri ||
+                            session.sync_odoo.evaluation_locations.last().unwrap().range.start.line != transformed_range.start.line{


Maybe add a comment explaining what this deduplication of consecutive matches solves? (env["some_model"].sudo().browse() etc)

andg-odoo · 2026-03-02T22:29:41Z

Hey! 👋
I did a quick test checking out this branch and just working as normal. It's great! 99% of the time it works perfectly with what I would use it for.

However, there is small areas that I'm not sure are broken due to the unimplemented list above in the description or something else.

Here is an example of a function that partially works:

def _log(self, uuid, account_id, data):
  purpose = data['endpoint']
  account_name = account_id.display_name if account_id else 'N/A'
  self._create_account(uuid, account_id, data)
  
  _logger.info('Request on behalf of %s for api_id %s to %s', uuid, account_name, purpose)
  
  http.request.env['proxy.log'].sudo().create({
    'uuid': uuid,
    'account_id': account_id.id if account_id else False,
    'purpose': purpose,
    'is_paid': bool(self._check_paid(data)),
  })

Doing a get reference call on the variables will get the following info:

uuid will find only the call in the parameter to self._create_account.
account_id will only find the calls on the assigning for account_name and self._create_account, but not the account_id inside of the create() call.
data will only find the calsl to assign purpose and self._create_account, but not the call inside of check_paid.

It seems like it can find use in top level calls but nothing within parameters of function calls that aren't on self.

Goto def appears to work for all variables seen here even in areas get refs fail.

Obviously this isn't merged yet so could still change but just wanted to put my experience in for the implementation.

Thanks again, watching the growth of this has been great and excited for this feature once it is merged 😄

fda-odoo · 2026-03-03T13:44:00Z

Hey @andg-odoo ,

Thank you for your feedback :) Indeed this PR is a 'part 1' to review and validate the architecture of the feature, but it is still missing some important things. And it's pointing us to some bugs too :D
Aside of xml_id, manifests and lambda that are missing here, the branch is pointing out that some part of arguments are not processed by the tool. that's the case for some other expressions too. That's will be indeed part of the task once it's stabilized. I hope we'll be able to release it soon :)

fda-odoo self-assigned this Jan 28, 2026

fda-odoo added the enhancement New feature or request label Jan 28, 2026

fda-odoo added this to the 1.3.0 milestone Jan 28, 2026

fda-odoo mentioned this pull request Jan 28, 2026

[IMP] Implement Phase 1 of Go To References (#466) #519

Open

fda-odoo force-pushed the alpha-references-fda branch 2 times, most recently from b6d04ff to 74786b2 Compare February 3, 2026 22:29

fda-odoo force-pushed the alpha branch from 69f5ac3 to a5e855b Compare February 9, 2026 13:29

[IMP] Implement Phase 1 of Go To References (#466)

3951d0d

fda-odoo force-pushed the alpha-references-fda branch from a776bf4 to b0ea6b5 Compare February 10, 2026 10:20

fda-odoo added 7 commits February 24, 2026 11:12

[IMP] refactor definition to get symbol and build location by after

7052fde

[IMP] GotoDeclaration

3a33f4e

[IMP] no_hash hashmap + cache for typeshed types

0475a75

[IMP] improve searches performances by avoiding useless searches in m…

a6fc73b

…odels

[IMP] iai_profiler to gungraun

aaf65d4

[IMP] various compiler warning fixes

7013b1b

[ FIX] tests + remove duplicated references with models

8ab3068

fda-odoo force-pushed the alpha-references-fda branch from b8b89b9 to 5ede711 Compare February 24, 2026 10:13

fda-odoo changed the title ~~[IMP] Implement Go To References~~ [IMP] Implement Go To References Part 1 - python code Feb 24, 2026

[FIX] fix duplicate references and order in tests

b937cb6

fda-odoo force-pushed the alpha-references-fda branch from 5ede711 to b937cb6 Compare February 24, 2026 10:36

fda-odoo requested a review from mmahrouss February 24, 2026 10:38

fda-odoo marked this pull request as ready for review February 24, 2026 10:38

fda-odoo requested a review from cammarosano February 24, 2026 10:38

fda-odoo mentioned this pull request Feb 24, 2026

Alpha ref 2 fda #550

Draft

mmahrouss approved these changes Feb 24, 2026

View reviewed changes

cammarosano approved these changes Feb 25, 2026

View reviewed changes

-                match ast_type {
+                if file_info.borrow().file_info_ast.borrow().indexed_module.is_none() && matches!(ast_type, AstType::Python) {
+                    return Ok(None);
+                }
+                let location_fn = match (ast_type, is_declaration) {
+                    (AstType::Python, true) => DeclarationFeature::get_location,
+                    (AstType::Python, false) => DefinitionFeature::get_location,
+                    (AstType::Xml, true) => DeclarationFeature::get_location_xml,
+                    (AstType::Xml, false) => DefinitionFeature::get_location_xml,
+                    (AstType::Csv, true) => DeclarationFeature::get_location_csv,
+                    (AstType::Csv, false) => DefinitionFeature::get_location_csv,
+                };
+                return Ok(location_fn(session, &file_symbol, &file_info, params.text_document_position_params.position.line, params.text_document_position_params.position.character));

	let sections = self.symbols.entry(content.borrow().name().clone()).or_insert(HashMap::default());
	let sections = self.symbols.entry(content.borrow().name().clone()).or_default());

Conversation

fda-odoo commented Jan 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Feb 19, 2026

Uh oh!

github-actions bot commented Feb 19, 2026

Uh oh!

github-actions bot commented Feb 19, 2026

Uh oh!

github-actions bot commented Feb 19, 2026

Uh oh!

github-actions bot commented Feb 20, 2026

Uh oh!

github-actions bot commented Feb 23, 2026

Uh oh!

github-actions bot commented Feb 23, 2026

Uh oh!

github-actions bot commented Feb 23, 2026

Uh oh!

github-actions bot commented Feb 24, 2026

Uh oh!

github-actions bot commented Feb 24, 2026

Uh oh!

github-actions bot commented Feb 24, 2026

Uh oh!

mmahrouss left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cammarosano left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andg-odoo commented Mar 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fda-odoo commented Mar 3, 2026

Uh oh!

Reviewers

fda-odoo commented Jan 28, 2026 •

edited

Loading

andg-odoo commented Mar 2, 2026 •

edited

Loading