Skip to content

Commit b937cb6

Browse files
fda-odoofda-odoo
committed
[FIX] fix duplicate references and order in tests
1 parent 8ab3068 commit b937cb6

File tree

5 files changed

+63
-59
lines changed

5 files changed

+63
-59
lines changed

‎server/src/core/evaluation.rs‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ use itertools::FoldWhile::{Continue, Done};
33
use ruff_python_ast::{Arguments, Expr, ExprCall, Identifier, Number, Operator, Parameter, UnaryOp};
44
use ruff_text_size::{Ranged, TextRange, TextSize};
55
use lsp_types::{Diagnostic, Location, Position, Range};
6-
use tracing::error;
76
use weak_table::traits::WeakElement;
87
use std::cmp::{max, min};
98
use std::collections::{HashMap, HashSet};
@@ -12,7 +11,7 @@ use std::rc::{Rc, Weak};
1211
use std::cell::RefCell;
1312
use crate::core::diagnostics::{create_diagnostic, DiagnosticCode};
1413
use crate::utils::NoHashBuilder;
15-
use crate::{constants::*, Sy};
14+
use crate::{Sy, constants::*, oyarn};
1615
use crate::core::odoo::SyncOdoo;
1716
use crate::threads::SessionInfo;
1817
use crate::S;
@@ -1394,7 +1393,7 @@ impl Evaluation {
13941393
let class_bw = evaluation_search.borrow();
13951394
let class = class_bw.as_class_sym();
13961395
if let Some(model_data) = class._model.as_ref() {
1397-
if s.value.to_str() == model_data.name {
1396+
if oyarn!("{}", s.value.to_str()) == model_data.name {
13981397
let file = parent.borrow().get_file().unwrap().upgrade().unwrap();
13991398
let file_info = session.sync_odoo.get_file_mgr().borrow().get_file_info(&file.borrow().paths()[0]);
14001399
if let Some(file_info) = file_info {

‎server/src/features/references.rs‎

Lines changed: 6 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ use std::{cell::RefCell, path::PathBuf, rc::Rc};
44
use lsp_types::{Location, Range};
55
use ruff_python_ast::{Alias, Expr, Identifier, Stmt, StmtAnnAssign, StmtAssert, StmtAssign, StmtAugAssign, StmtClassDef, StmtIf, StmtMatch, StmtRaise, StmtReturn, StmtTry, StmtTypeAlias, StmtWith};
66
use ruff_text_size::{Ranged, TextRange, TextSize};
7-
use tracing::error;
87
use weak_table::PtrWeakHashSet;
98

109
use crate::S;
@@ -34,9 +33,7 @@ impl ReferenceFeature {
3433
match &definition.source {
3534
GotoSourceType::Symbol(target_symbol) => {
3635
let mut to_check: PtrWeakHashSet<Weak<RefCell<Symbol>>> = PtrWeakHashSet::new();
37-
let symbol_name = target_symbol.borrow().name().to_string();
3836

39-
locations.extend(ReferenceFeature::references_in_file(session, file_symbol, file_info, &symbol_name, &target_symbol));
4037
to_check.insert(file_symbol.clone());
4138

4239
//take arch and arch_eval dependents
@@ -60,14 +57,18 @@ impl ReferenceFeature {
6057
}
6158
}
6259
}
60+
let mut files_to_check: PtrWeakHashSet<Weak<RefCell<Symbol>>> = PtrWeakHashSet::new(); //only iter on files
6361
for sym in to_check.iter() {
6462
let Some(Some(file)) = sym.borrow().get_file().map(|x| x.upgrade()) else { //to be sure we are on a file
6563
continue;
6664
};
65+
files_to_check.insert(file.clone());
66+
}
67+
for file in files_to_check.iter() {
6768
let Some(dep_file_info) = session.sync_odoo.get_file_mgr().borrow().get_file_info(&file.borrow().paths()[0]) else {
6869
continue;
6970
};
70-
locations.extend(ReferenceFeature::references_in_file(session, &sym, &dep_file_info, &symbol_name, &target_symbol));
71+
locations.extend(ReferenceFeature::references_in_file(session, &file, &dep_file_info, &target_symbol));
7172
}
7273
},
7374
_ => continue,
@@ -81,7 +82,7 @@ impl ReferenceFeature {
8182
}
8283
}
8384

84-
fn references_in_file(session: &mut SessionInfo, file_symbol: &Rc<RefCell<Symbol>>, file_info: &Rc<RefCell<FileInfo>>, symbol_name: &String, target_symbol_rc: &Rc<RefCell<Symbol>>) -> Vec<Location> {
85+
fn references_in_file(session: &mut SessionInfo, file_symbol: &Rc<RefCell<Symbol>>, file_info: &Rc<RefCell<FileInfo>>, target_symbol_rc: &Rc<RefCell<Symbol>>) -> Vec<Location> {
8586
let file_info_ast = file_info.borrow().file_info_ast.clone();
8687
let mut visitor = ReferenceVisitor {
8788
sym_stack: vec![],
@@ -333,14 +334,6 @@ impl ReferenceVisitor {
333334
}
334335

335336
fn visit_assign(&mut self, session: &mut SessionInfo, assign: &StmtAssign) {
336-
match *assign.value {
337-
Expr::StringLiteral(ref s) => {
338-
if s.value.to_str() == "pygls.tests.base_test_model" {
339-
error!("here");
340-
}
341-
},
342-
_=> {}
343-
}
344337
self.visit_expr(session, &assign.value, &assign.range.start());
345338
}
346339

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,4 +46,6 @@ def _get_partner_id(self):
4646
return_funcBase2 = baseInstance2.get_test_int()
4747

4848
class NoBaseModel(models.Model):
49-
_inherit = "module_1.no_base_model"
49+
_inherit = "module_1.no_base_model"
50+
51+
basic_var = 42

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,7 @@
11
from odoo import models, fields
2+
from .base_test_models import basic_var
3+
4+
print(basic_var)
25

36
class model_model(models.Model):
47
pass

‎server/tests/test_references.rs‎

Lines changed: 49 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,7 @@
11
use std::path::PathBuf;
22

3-
use lsp_types::{Location, PartialResultParams, Position, TextDocumentContentChangeEvent, WorkDoneProgressParams};
4-
use odoo_ls_server::{core::file_mgr::FileMgr, features::references, threads::SessionInfo, utils::PathSanitizer};
5-
use tracing::error;
3+
use lsp_types::{Location, PartialResultParams, Position, WorkDoneProgressParams};
4+
use odoo_ls_server::{core::file_mgr::FileMgr, threads::SessionInfo, utils::PathSanitizer};
65

76
use crate::setup::setup::{create_init_session, setup_server};
87

@@ -18,51 +17,59 @@ fn test_references() {
1817

1918
let test_file = test_addons_path.join("module_1").join("models").join("base_test_models.py").sanitize();
2019
//1. reference of a model
21-
let references = get_references(&mut session, &test_file, Position::new(4, 29));
22-
for r in references.iter() {
23-
error!("Reference found at {}:{}:{}", r.uri.as_str(), r.range.start.line, r.range.start.character);
24-
}
25-
assert!(references.len() >= 10, "Expected at least 10 references, got {}", references.len());
26-
for i in 0..9 {
27-
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());
28-
}
29-
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);
30-
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);
31-
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);
32-
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);
33-
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);
34-
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);
35-
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);
36-
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);
37-
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);
38-
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());
39-
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);
40-
assert!(references.len() == 11, "Expected only 10 references, got unexpected additional references: {}",
41-
references.iter().skip(11).map(|r| format!("{}:{}:{}", r.uri.as_str(), r.range.start.line, r.range.start.character)).collect::<Vec<String>>().join(", ")
20+
let mut references = get_references(&mut session, &test_file, Position::new(4, 29));
21+
//as order is not guaranteed (usage of set), check if the expected reference is somewhere in the result list.
22+
assert_in_result(&mut references, "module_1/models/base_test_models.py", 4, 12);
23+
assert_in_result(&mut references, "module_1/models/base_test_models.py", 15, 8);
24+
assert_in_result(&mut references, "module_1/models/base_test_models.py", 32, 17);
25+
assert_in_result(&mut references, "module_1/models/base_test_models.py", 33, 8);
26+
assert_in_result(&mut references, "module_1/models/base_test_models.py", 34, 18);
27+
assert_in_result(&mut references, "module_1/models/base_test_models.py", 40, 16);
28+
assert_in_result(&mut references, "module_1/models/base_test_models.py", 41, 16);
29+
assert_in_result(&mut references, "module_1/models/base_test_models.py", 42, 16);
30+
assert_in_result(&mut references, "module_1/models/base_test_models.py", 43, 16);
31+
assert_in_result(&mut references, "module_1/models/diagnostics.py", 9, 34);
32+
assert_in_result(&mut references, "module_1/models/diagnostics.py", 17, 34);
33+
assert_in_result(&mut references, "module_1/models/diagnostics.py", 18, 34);
34+
assert_in_result(&mut references, "module_1/models/diagnostics.py", 19, 34);
35+
assert_in_result(&mut references, "module_1/models/diagnostics.py", 20, 34);
36+
assert_in_result(&mut references, "module_1/models/diagnostics.py", 22, 34);
37+
assert_in_result(&mut references, "module_2/models/base_test_models.py", 5, 15);
38+
assert!(references.len() == 0, "Some references were not expected: {}",
39+
references.iter().map(|r| format!("{}:{}:{}", r.uri.as_str(), r.range.start.line, r.range.start.character)).collect::<Vec<String>>().join(", ")
4240
);
4341

4442
// reference of an attribute
45-
let references = get_references(&mut session, &test_file, Position::new(9, 8));
46-
for r in references.iter() {
47-
error!("Reference found at {}:{}:{}", r.uri.as_str(), r.range.start.line, r.range.start.character);
48-
}
49-
assert!(references.len() >= 8, "Expected at least 8 references, got {}", references.len());
50-
for i in 0..8 {
51-
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());
52-
}
53-
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);
54-
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);
55-
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);
56-
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);
57-
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);
58-
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);
59-
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);
60-
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);
61-
assert!(references.len() == 8, "Expected only 8 references, got unexpected additional references: {}",
62-
references.iter().skip(8).map(|r| format!("{}:{}:{}", r.uri.as_str(), r.range.start.line, r.range.start.character)).collect::<Vec<String>>().join(", ")
43+
let mut references = get_references(&mut session, &test_file, Position::new(9, 8));
44+
assert_in_result(&mut references, "module_1/models/base_test_models.py", 37, 18);
45+
assert!(references.len() == 0, "Some references were not expected: {}",
46+
references.iter().map(|r| format!("{}:{}:{}", r.uri.as_str(), r.range.start.line, r.range.start.character)).collect::<Vec<String>>().join(", ")
6347
);
6448

6549
//reference of a simple variable
50+
let mut references = get_references(&mut session, &test_file, Position::new(50, 4));
51+
assert_in_result(&mut references, "module_1/models/models.py", 1, 30);
52+
assert!(references.len() == 0, "Some references were not expected: {}",
53+
references.iter().map(|r| format!("{}:{}:{}", r.uri.as_str(), r.range.start.line, r.range.start.character)).collect::<Vec<String>>().join(", ")
54+
);
55+
56+
// for r in references.iter() {
57+
// error!("Reference found at {}:{}:{}", r.uri.as_str(), r.range.start.line, r.range.start.character);
58+
// }
59+
}
60+
61+
fn assert_in_result(references: &mut Vec<Location>, end_path: &str, line: u32, character: u32) {
62+
let mut index = None;
63+
for (i, r) in references.iter().enumerate() {
64+
if r.uri.as_str().ends_with(end_path) && r.range.start.line == line && r.range.start.character == character {
65+
index = Some(i);
66+
}
67+
}
68+
if let Some(i) = index {
69+
references.remove(i);
70+
} else {
71+
assert!(false, "Expected reference not found: {}:{}:{}", end_path, line, character);
72+
}
6673
}
6774

6875
fn get_references(session: &mut SessionInfo, path: &String, position: Position)-> Vec<Location> {

0 commit comments

Comments
 (0)