[FIX] fix duplicate references and order in tests

Author: fda-odoo

server/src/core/evaluation.rs

@@ -12,7 +12,7 @@ use std::rc::{Rc, Weak};
 use std::cell::RefCell;
 use crate::core::diagnostics::{create_diagnostic, DiagnosticCode};
 use crate::utils::NoHashBuilder;
-use crate::{constants::*, Sy};
+use crate::{Sy, constants::*, oyarn};
 use crate::core::odoo::SyncOdoo;
 use crate::threads::SessionInfo;
 use crate::S;
@@ -1394,7 +1394,7 @@ impl Evaluation {
                                         let class_bw = evaluation_search.borrow();
                                         let class = class_bw.as_class_sym();
                                         if let Some(model_data) = class._model.as_ref() {
-                                            if s.value.to_str() == model_data.name {
+                                            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 {

server/src/features/references.rs

@@ -4,7 +4,7 @@ use std::{cell::RefCell, path::PathBuf, rc::Rc};
 use lsp_types::{Location, Range};
 use ruff_python_ast::{Alias, Expr, Identifier, Stmt, StmtAnnAssign, StmtAssert, StmtAssign, StmtAugAssign, StmtClassDef, StmtIf, StmtMatch, StmtRaise, StmtReturn, StmtTry, StmtTypeAlias, StmtWith};
 use ruff_text_size::{Ranged, TextRange, TextSize};
-use tracing::error;
+use tracing::{error, warn};
 use weak_table::PtrWeakHashSet;
 
 use crate::S;
@@ -36,7 +36,6 @@ impl ReferenceFeature {
                     let mut to_check: PtrWeakHashSet<Weak<RefCell<Symbol>>> = PtrWeakHashSet::new();
                     let symbol_name = target_symbol.borrow().name().to_string();
 
-                    locations.extend(ReferenceFeature::references_in_file(session, file_symbol, file_info, &symbol_name, &target_symbol));
                     to_check.insert(file_symbol.clone());
 
                     //take arch and arch_eval dependents
@@ -60,14 +59,18 @@ impl ReferenceFeature {
                             }
                         }
                     }
+                    let mut files_to_check: PtrWeakHashSet<Weak<RefCell<Symbol>>> = PtrWeakHashSet::new(); //only iter on files
                     for sym in to_check.iter() {
                         let Some(Some(file)) = sym.borrow().get_file().map(|x| x.upgrade()) else { //to be sure we are on a file
                             continue;
                         };
+                        files_to_check.insert(file.clone());
+                    }
+                    for file in files_to_check.iter() {
                         let Some(dep_file_info) = session.sync_odoo.get_file_mgr().borrow().get_file_info(&file.borrow().paths()[0]) else {
                             continue;
                         };
-                        locations.extend(ReferenceFeature::references_in_file(session, &sym, &dep_file_info, &symbol_name, &target_symbol));
+                        locations.extend(ReferenceFeature::references_in_file(session, &file, &dep_file_info, &symbol_name, &target_symbol));
                     }
                 },
                 _ => continue,
@@ -333,14 +336,6 @@ impl ReferenceVisitor {
     }
 
     fn visit_assign(&mut self, session: &mut SessionInfo, assign: &StmtAssign) {
-        match *assign.value {
-            Expr::StringLiteral(ref s) => {
-                if s.value.to_str() == "pygls.tests.base_test_model" {
-                    error!("here");
-                }
-            },
-            _=> {}
-        }
         self.visit_expr(session, &assign.value, &assign.range.start());
     }
 

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

@@ -46,4 +46,6 @@ def _get_partner_id(self):
 return_funcBase2 = baseInstance2.get_test_int()
 
 class NoBaseModel(models.Model):
-    _inherit = "module_1.no_base_model"
+    _inherit = "module_1.no_base_model"
+
+basic_var = 42

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

@@ -1,4 +1,7 @@
 from odoo import models, fields
+from .base_test_models import basic_var
+
+print(basic_var)
 
 class model_model(models.Model):
     pass

server/tests/test_references.rs

@@ -18,51 +18,59 @@ fn test_references() {
 
     let test_file = test_addons_path.join("module_1").join("models").join("base_test_models.py").sanitize();
     //1. reference of a model
-    let references = get_references(&mut session, &test_file, Position::new(4, 29));
-    for r in references.iter() {
-        error!("Reference found at {}:{}:{}", r.uri.as_str(), r.range.start.line, r.range.start.character);
-    }
-    assert!(references.len() >= 10, "Expected at least 10 references, got {}", references.len());
-    for i in 0..9 {
-        assert!(references[i].uri.as_str().ends_with("base_test_models.py"), "Expected reference in base_test_models.py, got {}", references[i].uri.as_str());
-    }
-    assert!(references[0].range.start.line == 4 && references[0].range.start.character == 12, "Expected 1th reference at line 4, character 12, got line {}, character {}", references[0].range.start.line, references[0].range.start.character);
-    assert!(references[1].range.start.line == 15 && references[1].range.start.character == 8, "Expected 2nd reference at line 15, character 8, got line {}, character {}", references[1].range.start.line, references[1].range.start.character);
-    assert!(references[2].range.start.line == 32 && references[2].range.start.character == 17, "Expected 3th reference at line 32, character 17, got line {}, character {}", references[2].range.start.line, references[2].range.start.character);
-    assert!(references[3].range.start.line == 33 && references[3].range.start.character == 8, "Expected 4th reference at line 33, character 8, got line {}, character {}", references[3].range.start.line, references[3].range.start.character);
-    assert!(references[4].range.start.line == 34 && references[4].range.start.character == 18, "Expected 5th reference at line 34, character 18, got line {}, character {}", references[4].range.start.line, references[4].range.start.character);
-    assert!(references[5].range.start.line == 40 && references[5].range.start.character == 16, "Expected 6th reference at line 40, character 16, got line {}, character {}", references[5].range.start.line, references[5].range.start.character);
-    assert!(references[6].range.start.line == 41 && references[6].range.start.character == 16, "Expected 7th reference at line 41, character 16, got line {}, character {}", references[6].range.start.line, references[6].range.start.character);
-    assert!(references[7].range.start.line == 42 && references[7].range.start.character == 16, "Expected 8th reference at line 42, character 16, got line {}, character {}", references[7].range.start.line, references[7].range.start.character);
-    assert!(references[8].range.start.line == 43 && references[8].range.start.character == 16, "Expected 9th reference at line 43, character 16, got line {}, character {}", references[8].range.start.line, references[8].range.start.character);
-    assert!(references[9].uri.as_str().ends_with("module_2/models/base_test_models.py"), "Expected reference 10 to be in module2/base_test_models.py, got {}", references[9].uri.as_str());
-    assert!(references[10].range.start.line == 44 && references[10].range.start.character == 16, "Expected ninth reference at line 44, character 16, got line {}, character {}", references[10].range.start.line, references[10].range.start.character);
-    assert!(references.len() == 11, "Expected only 10 references, got unexpected additional references: {}",
-        references.iter().skip(11).map(|r| format!("{}:{}:{}", r.uri.as_str(), r.range.start.line, r.range.start.character)).collect::<Vec<String>>().join(", ")
+    let mut references = get_references(&mut session, &test_file, Position::new(4, 29));
+    //as order is not guaranteed (usage of set), check if the expected reference is somewhere in the result list.
+    assert_in_result(&mut references, "module_1/models/base_test_models.py", 4, 12);
+    assert_in_result(&mut references, "module_1/models/base_test_models.py", 15, 8);
+    assert_in_result(&mut references, "module_1/models/base_test_models.py", 32, 17);
+    assert_in_result(&mut references, "module_1/models/base_test_models.py", 33, 8);
+    assert_in_result(&mut references, "module_1/models/base_test_models.py", 34, 18);
+    assert_in_result(&mut references, "module_1/models/base_test_models.py", 40, 16);
+    assert_in_result(&mut references, "module_1/models/base_test_models.py", 41, 16);
+    assert_in_result(&mut references, "module_1/models/base_test_models.py", 42, 16);
+    assert_in_result(&mut references, "module_1/models/base_test_models.py", 43, 16);
+    assert_in_result(&mut references, "module_1/models/diagnostics.py", 9, 34);
+    assert_in_result(&mut references, "module_1/models/diagnostics.py", 17, 34);
+    assert_in_result(&mut references, "module_1/models/diagnostics.py", 18, 34);
+    assert_in_result(&mut references, "module_1/models/diagnostics.py", 19, 34);
+    assert_in_result(&mut references, "module_1/models/diagnostics.py", 20, 34);
+    assert_in_result(&mut references, "module_1/models/diagnostics.py", 22, 34);
+    assert_in_result(&mut references, "module_2/models/base_test_models.py", 5, 15);
+    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(", ")
     );
 
     // reference of an attribute
-    let references = get_references(&mut session, &test_file, Position::new(9, 8));
-    for r in references.iter() {
-        error!("Reference found at {}:{}:{}", r.uri.as_str(), r.range.start.line, r.range.start.character);
-    }
-    assert!(references.len() >= 8, "Expected at least 8 references, got {}", references.len());
-    for i in 0..8 {
-        assert!(references[i].uri.as_str().ends_with("base_test_models.py"), "Expected reference in base_test_models.py, got {}", references[i].uri.as_str());
-    }
-    assert!(references[0].range.start.line == 15 && references[0].range.start.character == 8, "Expected first reference at line 15, character 8, got line {}, character {}", references[0].range.start.line, references[0].range.start.character);
-    assert!(references[1].range.start.line == 32 && references[1].range.start.character == 8, "Expected second reference at line 32, character 8, got line {}, character {}", references[1].range.start.line, references[1].range.start.character);
-    assert!(references[2].range.start.line == 33 && references[2].range.start.character == 8, "Expected third reference at line 33, character 8, got line {}, character {}", references[2].range.start.line, references[2].range.start.character);
-    assert!(references[3].range.start.line == 34 && references[3].range.start.character == 18, "Expected fourth reference at line 34, character 18, got line {}, character {}", references[3].range.start.line, references[3].range.start.character);
-    assert!(references[4].range.start.line == 40 && references[4].range.start.character == 16, "Expected fifth reference at line 40, character 16, got line {}, character {}", references[4].range.start.line, references[4].range.start.character);
-    assert!(references[5].range.start.line == 41 && references[5].range.start.character == 16, "Expected sixth reference at line 41, character 16, got line {}, character {}", references[5].range.start.line, references[5].range.start.character);
-    assert!(references[6].range.start.line == 42 && references[6].range.start.character == 16, "Expected seventh reference at line 42, character 16, got line {}, character {}", references[6].range.start.line, references[6].range.start.character);
-    assert!(references[7].range.start.line == 43 && references[7].range.start.character == 16, "Expected eighth reference at line 43, character 16, got line {}, character {}", references[7].range.start.line, references[7].range.start.character);
-    assert!(references.len() == 8, "Expected only 8 references, got unexpected additional references: {}",
-        references.iter().skip(8).map(|r| format!("{}:{}:{}", r.uri.as_str(), r.range.start.line, r.range.start.character)).collect::<Vec<String>>().join(", ")
+    let mut references = get_references(&mut session, &test_file, Position::new(9, 8));
+    assert_in_result(&mut references, "module_1/models/base_test_models.py", 37, 18);
+    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(", ")
     );
 
     //reference of a simple variable
+    let mut references = get_references(&mut session, &test_file, Position::new(50, 4));
+    assert_in_result(&mut references, "module_1/models/models.py", 1, 30);
+    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(", ")
+    );
+    
+    // for r in references.iter() {
+    //     error!("Reference found at {}:{}:{}", r.uri.as_str(), r.range.start.line, r.range.start.character);
+    // }
+}
+
+fn assert_in_result(references: &mut Vec<Location>, end_path: &str, line: u32, character: u32) {
+    let mut index = None;
+    for (i, r) in references.iter().enumerate() {
+        if r.uri.as_str().ends_with(end_path) && r.range.start.line == line && r.range.start.character == character {
+            index = Some(i);
+        }
+    }
+    if let Some(i) = index {
+        references.remove(i);
+    } else {
+        assert!(false, "Expected reference not found: {}:{}:{}", end_path, line, character);
+    }
 }
 
 fn get_references(session: &mut SessionInfo, path: &String, position: Position)-> Vec<Location> {