Skip to content

Commit 9bf1d43

Browse files
committed
[REF] symbol path handling with SymbolPath enum
This commit replaces the Vec<String> return type in Symbol::paths() (now renamed to path) with a new SymbolPath enum that explicitly represents three cases: Single path, Multiple paths, or None. The purpose of this change is to force the caller handle the different cases explicitly, rather than doing [0] on the vector previously returned by paths(). This commit also renames get_symbol_first_path() to get_file_path() for clarity, as it only applies to file types (File, Package, XMLFile and CSVFile) and specifically returns the actual file path including __init__.py for packages.
1 parent a189e43 commit 9bf1d43

18 files changed

+230
-213
lines changed

server/src/core/csv_arch_builder.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ impl CsvArchBuilder {
2121
pub fn load_csv(&mut self, session: &mut SessionInfo, csv_symbol: Rc<RefCell<Symbol>>, content: &String) -> Vec<Diagnostic> {
2222
let diagnostics = vec![];
2323
csv_symbol.borrow_mut().set_build_status(BuildSteps::ARCH, BuildStatus::IN_PROGRESS);
24-
let model_name_pb = PathBuf::from(&csv_symbol.borrow().paths()[0]);
24+
let model_name_pb = PathBuf::from(&csv_symbol.borrow().path().as_single());
2525
let model_name = Sy!(model_name_pb.file_stem().unwrap().to_str().unwrap().to_string());
2626
let csv_module = csv_symbol.borrow().find_module();
2727
let Some(csv_module) = &csv_module else {

server/src/core/entry_point.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -203,7 +203,7 @@ impl EntryPointMgr {
203203
} else {
204204
// There was an __init__.py, that was renamed or deleted.
205205
// Another notification will come for the deletion of the file, so we just warn here.
206-
warn_or_panic!("Trying to create a custom entrypoint on a namespace symbol: {:?}", new_sym.borrow().paths());
206+
warn_or_panic!("Trying to create a custom entrypoint on a namespace symbol: {:?}", new_sym.borrow().path().as_multiple());
207207
}
208208
return false;
209209
}

server/src/core/import_resolver.rs

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,9 @@ fn resolve_import_stmt_hook(alias: &Alias, from_symbol: &Option<Rc<RefCell<Symbo
5757
* Helper to manually import a symbol. Do not forget to use level instead of '.' in the from_stmt parameter.
5858
*/
5959
pub fn manual_import(session: &mut SessionInfo, source_file_symbol: &Rc<RefCell<Symbol>>, from_stmt:Option<String>, name: &str, asname: Option<String>, level: u32, diagnostics: &mut Option<&mut Vec<Diagnostic>>) -> Vec<ImportResult> {
60+
if source_file_symbol.borrow().path().as_vec().len() > 1 {
61+
panic!("Importing from a multi-path namespace symbol is not supported in manual_import");
62+
}
6063
let name_aliases = vec![Alias {
6164
name: Identifier { id: Name::new(name), range: TextRange::new(TextSize::new(0), TextSize::new(0)), node_index: AtomicNodeIndex::default() },
6265
asname: match asname {
@@ -82,7 +85,8 @@ pub fn resolve_from_stmt(session: &mut SessionInfo, source_file_symbol: &Rc<RefC
8285
level,
8386
from_stmt);
8487
drop(_source_file_symbol_lock);
85-
let source_path = source_file_symbol.borrow().paths()[0].clone();
88+
// source_file_symbol is either a file or a namespace with a single path
89+
let source_path = source_file_symbol.borrow().path().as_vec()[0].clone();
8690
let mut start_symbol = None;
8791
if level != 0 {
8892
//if level is 0, resolve_packages already built a full tree, so we can start from root
@@ -139,7 +143,8 @@ pub fn resolve_import_stmt(session: &mut SessionInfo, source_file_symbol: &Rc<Re
139143
} else {
140144
vec![]
141145
};
142-
let source_path = source_file_symbol.borrow().paths()[0].clone();
146+
// source_file_symbol is either a file or a namespace with a single path
147+
let source_path = source_file_symbol.borrow().path().as_vec()[0].clone();
143148
let name_last_name: Vec<OYarn> = vec![name_split.last().unwrap().clone()];
144149
let (mut next_symbol, mut fallback_sym) = _get_or_create_symbol(
145150
session,
@@ -223,7 +228,7 @@ pub fn resolve_import_stmt(session: &mut SessionInfo, source_file_symbol: &Rc<Re
223228
}
224229

225230
pub fn find_module(session: &mut SessionInfo, odoo_addons: Rc<RefCell<Symbol>>, name: &OYarn) -> Option<Rc<RefCell<Symbol>>> {
226-
let paths = (*odoo_addons).borrow().paths().clone();
231+
let paths = (*odoo_addons).borrow().path().as_vec();
227232
for path in paths.iter() {
228233
let full_path = Path::new(path.as_str()).join(name.as_str());
229234
if !is_dir_cs(full_path.sanitize()) {
@@ -243,7 +248,7 @@ fn _resolve_packages(from_file: &Symbol, level: u32, from_stmt: Option<&Identifi
243248
let mut first_part_tree: Vec<OYarn> = vec![];
244249
if level > 0 {
245250
let mut lvl = level;
246-
if lvl > Path::new(&from_file.paths()[0]).components().count() as u32 {
251+
if lvl > Path::new(&from_file.path().as_single()).components().count() as u32 {
247252
panic!("Level is too high!")
248253
}
249254
if matches!(from_file.typ(), SymType::PACKAGE(_)) {
@@ -390,7 +395,7 @@ fn _resolve_new_symbol(session: &mut SessionInfo, parent: Rc<RefCell<Symbol>>, n
390395
if (*parent).borrow().typ() == SymType::COMPILED {
391396
return Ok((*parent).borrow_mut().add_new_compiled(session, &sym_name, &S!("")));
392397
}
393-
let paths = (*parent).borrow().paths().clone();
398+
let paths = (*parent).borrow().path().as_vec();
394399
for path in paths.iter() {
395400
let mut full_path = Path::new(path.as_str()).join(name.to_string());
396401
for stub in session.sync_odoo.stubs_dirs.iter() {
@@ -477,7 +482,7 @@ pub fn get_all_valid_names(session: &mut SessionInfo, source_file_symbol: &Rc<Re
477482
let source_root = source_file_symbol.borrow().get_root().as_ref().unwrap().upgrade().unwrap();
478483
let entry = source_root.borrow().get_entry().unwrap();
479484
let (mut from_symbol, _fallback_sym, file_tree) = resolve_from_stmt(session, source_file_symbol, identifier_from.as_ref(), level);
480-
let source_path = source_file_symbol.borrow().paths()[0].clone();
485+
let source_path = source_file_symbol.borrow().path().as_single();
481486
let mut result = HashMap::new();
482487
let mut symbols_to_browse = vec![];
483488
if from_symbol.is_none() {
@@ -548,15 +553,16 @@ fn valid_names_for_a_symbol(_session: &mut SessionInfo, symbol: &Rc<RefCell<Symb
548553
}
549554
res.extend(valid_name_from_symbol(symbol, start_filter));
550555
},
551-
SymType::NAMESPACE | SymType::DISK_DIR => {
552-
for path in symbol.borrow().paths().iter() {
556+
SymType::NAMESPACE => {
557+
for path in symbol.borrow().path().as_multiple().iter() {
553558
res.extend(valid_name_from_disk(path, start_filter));
554559
}
555560
},
561+
SymType::DISK_DIR => {
562+
res.extend(valid_name_from_disk(&symbol.borrow().path().as_single(), start_filter));
563+
}
556564
SymType::PACKAGE(_) => {
557-
for path in symbol.borrow().paths().iter() {
558-
res.extend(valid_name_from_disk(path, start_filter));
559-
}
565+
res.extend(valid_name_from_disk(&symbol.borrow().path().as_single(), start_filter));
560566
if only_on_disk {
561567
return res;
562568
}

server/src/core/odoo.rs

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
use crate::core::diagnostics::{create_diagnostic, DiagnosticCode};
22
use crate::core::entry_point::EntryPointType;
33
use crate::core::file_mgr::AstType;
4+
use crate::core::symbols::symbol::SymbolPath;
45
use crate::core::xml_data::OdooData;
56
use crate::core::xml_validation::XmlValidator;
67
use crate::features::document_symbols::DocumentSymbolFeature;
@@ -468,8 +469,10 @@ impl SyncOdoo {
468469
fn build_modules(session: &mut SessionInfo) {
469470
{
470471
let addons_symbol = session.sync_odoo.get_symbol(session.sync_odoo.config.odoo_path.as_ref().unwrap(), &tree(vec!["odoo", "addons"], vec![]), u32::MAX)[0].clone();
471-
let addons_path = addons_symbol.borrow().paths().clone();
472-
for addon_path in addons_path.iter() {
472+
let SymbolPath::Multiple(addons_paths) = addons_symbol.borrow().path() else {
473+
panic!("Odoo addons symbol paths should be a namespace");
474+
};
475+
for addon_path in addons_paths.iter() {
473476
info!("searching modules in {}", addon_path);
474477
if PathBuf::from(addon_path).exists() {
475478
//browse all dir in path
@@ -756,7 +759,7 @@ impl SyncOdoo {
756759

757760
pub fn add_to_rebuild_arch(&mut self, symbol: Rc<RefCell<Symbol>>) {
758761
if DEBUG_THREADS {
759-
trace!("ADDED TO ARCH - {}", symbol.borrow().paths().first().unwrap_or(&symbol.borrow().name().to_string()));
762+
trace!("ADDED TO ARCH - {} - {:?}", symbol.borrow().name(), symbol.borrow().path());
760763
}
761764
if symbol.borrow().build_status(BuildSteps::ARCH) != BuildStatus::IN_PROGRESS {
762765
let sym_clone = symbol.clone();
@@ -770,7 +773,7 @@ impl SyncOdoo {
770773

771774
pub fn add_to_rebuild_arch_eval(&mut self, symbol: Rc<RefCell<Symbol>>) {
772775
if DEBUG_THREADS {
773-
trace!("ADDED TO EVAL - {}", symbol.borrow().paths().first().unwrap_or(&symbol.borrow().name().to_string()));
776+
trace!("ADDED TO EVAL - {} - {:?}", symbol.borrow().name(), symbol.borrow().path());
774777
}
775778
if symbol.borrow().build_status(BuildSteps::ARCH_EVAL) != BuildStatus::IN_PROGRESS {
776779
let sym_clone = symbol.clone();
@@ -783,7 +786,7 @@ impl SyncOdoo {
783786

784787
pub fn add_to_validations(&mut self, symbol: Rc<RefCell<Symbol>>) {
785788
if DEBUG_THREADS {
786-
trace!("ADDED TO VALIDATION - {}", symbol.borrow().paths().first().unwrap_or(&symbol.borrow().name().to_string()));
789+
trace!("ADDED TO VALIDATION - {} - {:?}", symbol.borrow().name(), symbol.borrow().path());
787790
}
788791
if symbol.borrow().build_status(BuildSteps::VALIDATION) != BuildStatus::IN_PROGRESS {
789792
symbol.borrow_mut().set_build_status(BuildSteps::VALIDATION, BuildStatus::PENDING);
@@ -805,10 +808,10 @@ impl SyncOdoo {
805808
}
806809
if DEBUG_REBUILD_NOW {
807810
if symbol.borrow().build_status(step) == BuildStatus::INVALID {
808-
panic!("Trying to build an invalid symbol: {}", symbol.borrow().paths().first().unwrap_or(&symbol.borrow().name().to_string()));
811+
panic!("Trying to build an invalid symbol: {} - {:?}", symbol.borrow().name(), symbol.borrow().path());
809812
}
810813
if symbol.borrow().build_status(step) == BuildStatus::IN_PROGRESS && !session.sync_odoo.is_in_rebuild(&symbol, step) {
811-
error!("Trying to build a symbol that is NOT in the queue: {}", symbol.borrow().paths().first().unwrap_or(&symbol.borrow().name().to_string()));
814+
error!("Trying to build a symbol that is NOT in the queue: {} - {:?}", symbol.borrow().name(), symbol.borrow().path());
812815
}
813816
}
814817
if symbol.borrow().build_status(step) == BuildStatus::PENDING && symbol.borrow().previous_step_done(step) {
@@ -822,7 +825,7 @@ impl SyncOdoo {
822825
} else if step == BuildSteps::ARCH_EVAL {
823826
if DEBUG_REBUILD_NOW {
824827
if symbol.borrow().build_status(BuildSteps::ARCH) != BuildStatus::DONE {
825-
panic!("An evaluation has been requested on a non-arched symbol: {}", symbol.borrow().paths().first().unwrap_or(&symbol.borrow().name().to_string()));
828+
panic!("An evaluation has been requested on a non-arched symbol: {} - {:?}", symbol.borrow().name(), symbol.borrow().path());
826829
}
827830
}
828831
let mut builder = PythonArchEval::new(entry_point, symbol.clone());
@@ -831,7 +834,7 @@ impl SyncOdoo {
831834
} else if step == BuildSteps::VALIDATION {
832835
if DEBUG_REBUILD_NOW {
833836
if symbol.borrow().build_status(BuildSteps::ARCH) != BuildStatus::DONE || symbol.borrow().build_status(BuildSteps::ARCH_EVAL) != BuildStatus::DONE {
834-
panic!("An evaluation has been requested on a non-arched symbol: {}", symbol.borrow().paths().first().unwrap_or(&symbol.borrow().name().to_string()));
837+
panic!("An evaluation has been requested on a non-arched symbol: {} - {:?}", symbol.borrow().name(), symbol.borrow().path());
835838
}
836839
}
837840
let mut validator = PythonValidator::new(entry_point, symbol.clone());
@@ -947,7 +950,10 @@ impl SyncOdoo {
947950
let mut to_del = Vec::from_iter(path_symbol.borrow().all_module_symbol().map(|x| x.clone()));
948951
let mut index = 0;
949952
while index < to_del.len() {
950-
FileMgr::delete_path(session, &to_del[index].borrow().paths()[0]);
953+
match to_del[index].borrow().path() {
954+
SymbolPath::Single(p) => FileMgr::delete_path(session, &p),
955+
SymbolPath::Multiple(_) | SymbolPath::None => {}
956+
}
951957
let mut to_del_child = Vec::from_iter(to_del[index].borrow().all_module_symbol().map(|x| x.clone()));
952958
to_del.append(&mut to_del_child);
953959
index += 1;

server/src/core/python_arch_builder.rs

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -68,10 +68,10 @@ impl PythonArchBuilder {
6868
self.current_step = if self.file_mode {BuildSteps::ARCH} else {BuildSteps::VALIDATION};
6969
}
7070
if DEBUG_STEPS && (!DEBUG_STEPS_ONLY_INTERNAL || !symbol.borrow().is_external()) {
71-
trace!("building {} - {}", self.file.borrow().paths().first().unwrap_or(&S!("No path found")), symbol.borrow().name());
71+
trace!("building {} - {}", self.file.borrow().path().as_single(), symbol.borrow().name());
7272
}
7373
symbol.borrow_mut().set_build_status(BuildSteps::ARCH, BuildStatus::IN_PROGRESS);
74-
let path = self.file.borrow().get_symbol_first_path();
74+
let path = self.file.borrow().get_file_path();
7575
if self.file_mode {
7676
let in_workspace = (self.file.borrow().parent().is_some() &&
7777
self.file.borrow().parent().as_ref().unwrap().upgrade().is_some() &&
@@ -189,24 +189,24 @@ impl PythonArchBuilder {
189189
if value.is_some() {
190190
let (nf, parse_error) = self.extract_all_symbol_eval_values(&value.as_ref());
191191
if parse_error {
192-
warn!("error during parsing __all__ import in file {}", (*import_result.symbol).borrow().paths()[0] )
192+
warn!("error during parsing __all__ import in file {}", (*import_result.symbol).borrow().path().as_single())
193193
}
194194
name_filter = nf;
195195
all_name_allowed = false;
196196
} else {
197-
warn!("invalid __all__ import in file {} - no value found", (*import_result.symbol).borrow().paths()[0])
197+
warn!("invalid __all__ import in file {} - no value found", (*import_result.symbol).borrow().path().as_single())
198198
}
199199
} else {
200-
warn!("invalid __all__ import in file {} - multiple evaluation found", (*import_result.symbol).borrow().paths()[0])
200+
warn!("invalid __all__ import in file {} - multiple evaluation found", (*import_result.symbol).borrow().path().as_single())
201201
}
202202
} else {
203-
warn!("invalid __all__ import in file {} - localizedSymbol not found", (*import_result.symbol).borrow().paths()[0])
203+
warn!("invalid __all__ import in file {} - localizedSymbol not found", (*import_result.symbol).borrow().path().as_single())
204204
}
205205
} else {
206-
warn!("invalid __all__ import in file {} - expired symbol", (*import_result.symbol).borrow().paths()[0])
206+
warn!("invalid __all__ import in file {} - expired symbol", (*import_result.symbol).borrow().path().as_single())
207207
}
208208
} else {
209-
warn!("invalid __all__ import in file {} - no symbol found", (*import_result.symbol).borrow().paths()[0])
209+
warn!("invalid __all__ import in file {} - no symbol found", (*import_result.symbol).borrow().path().as_single())
210210
}
211211
}
212212
let mut dep_to_add = vec![];

server/src/core/python_arch_builder_hooks.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -177,15 +177,15 @@ impl PythonArchBuilderHooks {
177177
let name = symbol.borrow().name().clone();
178178
if name == "release" {
179179
if symbol.borrow().get_main_entry_tree(session) == (vec![Sy!("odoo"), Sy!("release")], vec![]) {
180-
let (maj, min, mic) = SyncOdoo::read_version(session, PathBuf::from(symbol.borrow().paths()[0].clone()));
180+
let (maj, min, mic) = SyncOdoo::read_version(session, PathBuf::from(symbol.borrow().path().as_single()));
181181
if maj != session.sync_odoo.version_major || min != session.sync_odoo.version_minor || mic != session.sync_odoo.version_micro {
182182
session.sync_odoo.need_rebuild = true;
183183
}
184184
}
185185
} else if name == "init" {
186186
if compare_semver(session.sync_odoo.full_version.as_str(), "18.1") != Ordering::Less {
187187
if symbol.borrow().get_main_entry_tree(session) == (vec![Sy!("odoo"), Sy!("init")], vec![]) {
188-
let odoo_namespace = session.sync_odoo.get_symbol(symbol.borrow().paths()[0].as_str(), &(vec![Sy!("odoo")], vec![]), u32::MAX);
188+
let odoo_namespace = session.sync_odoo.get_symbol(&symbol.borrow().path().as_single(), &(vec![Sy!("odoo")], vec![]), u32::MAX);
189189
if let Some(odoo_namespace) = odoo_namespace.get(0) {
190190
// create _ and Command as ext_symbols
191191
let owner = symbol.clone();
@@ -199,7 +199,7 @@ impl PythonArchBuilderHooks {
199199
} else if name == "werkzeug" {
200200
if symbol.borrow().get_main_entry_tree(session) == (vec![Sy!("odoo"), Sy!("_monkeypatches"), Sy!("werkzeug")], vec![]) {
201201
//doing this patch like this imply that an odoo project will make these functions available for all entrypoints, but heh
202-
let werkzeug_url = session.sync_odoo.get_symbol(symbol.borrow().paths()[0].as_str(), &(vec![Sy!("werkzeug"), Sy!("urls")], vec![]), u32::MAX);
202+
let werkzeug_url = session.sync_odoo.get_symbol(&symbol.borrow().path().as_single(), &(vec![Sy!("werkzeug"), Sy!("urls")], vec![]), u32::MAX);
203203
if let Some(werkzeug_url) = werkzeug_url.first() {
204204
let url_join = werkzeug_url.borrow().get_symbol(&(vec![], vec![Sy!("url_join")]), u32::MAX);
205205
if url_join.is_empty() { //else, installed version is already patched

server/src/core/python_arch_eval.rs

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -72,13 +72,10 @@ impl PythonArchEval {
7272
self.current_step = if self.file_mode {BuildSteps::ARCH_EVAL} else {BuildSteps::VALIDATION};
7373
}
7474
if DEBUG_STEPS && (!DEBUG_STEPS_ONLY_INTERNAL || !symbol.borrow().is_external()) {
75-
trace!("evaluating {} - {}", self.file.borrow().paths().first().unwrap_or(&S!("No path found")), symbol.borrow().name());
75+
trace!("evaluating {} - {}", self.file.borrow().path().as_single(), symbol.borrow().name());
7676
}
7777
symbol.borrow_mut().set_build_status(BuildSteps::ARCH_EVAL, BuildStatus::IN_PROGRESS);
78-
if self.file.borrow().paths().len() != 1 {
79-
panic!("Trying to eval_arch a symbol without any path")
80-
}
81-
let path = self.file.borrow().get_symbol_first_path();
78+
let path = self.file.borrow().get_file_path();
8279
let Some(file_info_rc) = session.sync_odoo.get_file_mgr().borrow().get_file_info(&path).clone() else {
8380
warn!("File info not found for {}", path);
8481
return;
@@ -515,7 +512,7 @@ impl PythonArchEval {
515512
if let Some(sym) = evaluation.symbol.get_symbol_as_weak(session, &mut None, &mut self.diagnostics, None).weak.upgrade() {
516513
if Rc::ptr_eq(&sym, &variable_rc){
517514
// TODO: investigate deps, and fix cyclic evals
518-
let file_path = parent.borrow().get_file().and_then(|file| file.upgrade()).and_then(|file| file.borrow().paths().first().cloned());
515+
let file_path = parent.borrow().get_file().and_then(|file| file.upgrade()).and_then(|file| Some(file.borrow().path().as_single()));
519516
warn!("Found cyclic evaluation symbol: {}, parent: {}, file: {}", var_name, parent.borrow().name(), file_path.unwrap_or(S!("N/A")));
520517
evaluations.remove(ix);
521518
continue;

0 commit comments

Comments
 (0)