[IMP] server: check for field validity on xml record

fda-odoo · fda-odoo · commit 99582874a656 · 2025-08-06T17:46:33.000+02:00
diff --git a/server/src/core/model.rs b/server/src/core/model.rs
@@ -185,26 +185,43 @@ impl Model {
 
     /* Return all symbols that build this model.
         It returns the symbol and an optional string that represents the module name that should be added to dependencies to be used.
+        if with_inheritance is true, it will also return symbols from inherited models (NOT Base classes).
     */
-    pub fn all_symbols(&self, session: &mut SessionInfo, from_module: Option<Rc<RefCell<Symbol>>>) -> Vec<(Rc<RefCell<Symbol>>, Option<OYarn>)> {
-        let mut symbol = Vec::new();
+    pub fn all_symbols(&self, session: &mut SessionInfo, from_module: Option<Rc<RefCell<Symbol>>>, with_inheritance: bool) -> Vec<(Rc<RefCell<Symbol>>, Option<OYarn>)> {
+        self.all_symbols_helper(session, from_module, with_inheritance, &mut HashSet::new())
+    }
+
+    fn all_symbols_helper(&self, session: &mut SessionInfo, from_module: Option<Rc<RefCell<Symbol>>>, with_inheritance: bool, seen_inherited_models: &mut HashSet<OYarn>) -> Vec<(Rc<RefCell<Symbol>>, Option<OYarn>)> {
+        let mut symbols = Vec::new();
         for s in self.symbols.iter() {
             if let Some(from_module) = from_module.as_ref() {
                 let module = s.borrow().find_module();
                 if let Some(module) = module {
                     if ModuleSymbol::is_in_deps(session, &from_module, &module.borrow().as_module_package().dir_name) {
-                        symbol.push((s, None));
+                        symbols.push((s.clone(), None));
                     } else {
-                        symbol.push((s, Some(module.borrow().as_module_package().dir_name.clone())));
+                        symbols.push((s.clone(), Some(module.borrow().as_module_package().dir_name.clone())));
                     }
                 } else {
                     session.log_message(MessageType::WARNING, "A model should be declared in a module.".to_string());
                 }
             } else {
-                symbol.push((s.clone(), None));
+                symbols.push((s.clone(), None));
+            }
+            if with_inheritance {
+                let inherited_models = s.borrow().as_class_sym()._model.as_ref().unwrap().inherit.clone();
+                for inherited_model in inherited_models.iter() {
+                    if !seen_inherited_models.contains(inherited_model) {
+                        seen_inherited_models.insert(inherited_model.clone());
+                        let model = session.sync_odoo.models.get(inherited_model).cloned();
+                        if let Some(model) = model {
+                            symbols.extend(model.borrow().all_symbols_helper(session, from_module.clone(), true, seen_inherited_models));
+                        }
+                    }
+                }
             }
         }
-        symbol
+        symbols
     }
 
     pub fn add_dependent(&mut self, symbol: &Rc<RefCell<Symbol>>) {
diff --git a/server/src/core/python_arch_eval.rs b/server/src/core/python_arch_eval.rs
@@ -541,7 +541,7 @@ impl PythonArchEval {
                     let Some(model) = session.sync_odoo.models.get(&model_data.name).cloned() else {
                         continue;
                     };
-                    let model_classes = model.borrow().all_symbols(session, parent_class.find_module());
+                    let model_classes = model.borrow().all_symbols(session, parent_class.find_module(), false);
                     let fn_name = self.sym_stack[0].borrow().name().clone();
                     let allowed_fields: HashSet<_> = model_classes.iter().filter_map(|(sym, _)| sym.borrow().as_class_sym()._model.as_ref().unwrap().computes.get(&fn_name).cloned()).flatten().collect();
                     if allowed_fields.is_empty() {
diff --git a/server/src/core/python_odoo_builder.rs b/server/src/core/python_odoo_builder.rs
@@ -97,6 +97,7 @@ impl PythonOdooBuilder {
                             end: 1,
                         }),
                         xml_id: Some(xml_id_model_name),
+                        fields: vec![]
                     }));
                 });
                 session.sync_odoo.models.insert(model_name.clone(), Rc::new(RefCell::new(model)));
diff --git a/server/src/core/symbols/symbol.rs b/server/src/core/symbols/symbol.rs
@@ -2295,7 +2295,7 @@ impl Symbol {
                     let model_data =  sym.as_class_sym()._model.as_ref();
                     if let Some(model_data) = model_data {
                         if let Some(model) = session.sync_odoo.models.get(&model_data.name).cloned() {
-                            for (model_sym, dependency) in model.borrow().all_symbols(session, from_module.clone()) {
+                            for (model_sym, dependency) in model.borrow().all_symbols(session, from_module.clone(), true) {
                                 if dependency.is_none() && !Rc::ptr_eq(symbol, &model_sym) {
                                     for s in model_sym.borrow().all_symbols() {
                                         if only_fields && !s.borrow().is_field(session){
@@ -2318,7 +2318,7 @@ impl Symbol {
                 }
                 let bases = symbol.borrow().as_class_sym().bases.clone();
                 for base in bases.iter() {
-                    //no comodel as we will process only model in base class (overrided _name?)
+                    //no comodel as we will search for co-model from original class (what about overrided _name?)
                     if let Some(base) = base.upgrade() {
                         Symbol::all_members(&base, session, result, false, only_fields, only_methods, from_module.clone(), acc, false);
                     }
diff --git a/server/src/core/xml_arch_builder_rng_validation.rs b/server/src/core/xml_arch_builder_rng_validation.rs
@@ -1,10 +1,11 @@
+use std::rc::Rc;
+
 use lsp_types::{Diagnostic, Position, Range};
 use once_cell::sync::Lazy;
 use regex::Regex;
 use roxmltree::Node;
 
-use crate::{core::diagnostics::{create_diagnostic, DiagnosticCode}};
-use crate::{constants::{BuildStatus, BuildSteps, OYarn, EXTENSION_NAME}, core::xml_data::{XmlData, XmlDataActWindow, XmlDataDelete, XmlDataMenuItem, XmlDataRecord, XmlDataReport, XmlDataTemplate}, oyarn, threads::SessionInfo, Sy, S};
+use crate::{constants::{BuildStatus, BuildSteps, OYarn, EXTENSION_NAME}, core::{diagnostics::{create_diagnostic, DiagnosticCode}, xml_data::{XmlData, XmlDataActWindow, XmlDataDelete, XmlDataField, XmlDataMenuItem, XmlDataRecord, XmlDataReport, XmlDataTemplate}}, oyarn, threads::SessionInfo, Sy, S};
 
 use super::xml_arch_builder::XmlArchBuilder;
 
@@ -213,9 +214,16 @@ impl XmlArchBuilder {
             }
             return false;
         }
-
+        let mut data = XmlDataRecord {
+            file_symbol: Rc::downgrade(&self.xml_symbol),
+            model: (oyarn!("{}", node.attribute("model").unwrap()), node.attribute_node("model").unwrap().range()),
+            xml_id: found_id.clone().map(|id| oyarn!("{}", id)),
+            fields: vec![]
+        };
         for child in node.children().filter(|n| n.is_element()) {
-            if !self.load_field(session, &child, diagnostics) {
+            if let Some(field) = self.load_field(session, &child, diagnostics) {
+                data.fields.push(field);
+            } else {
                 if let Some(diagnostic) = create_diagnostic(session, DiagnosticCode::OLS05015, &[child.tag_name().name()]) {
                     diagnostics.push(Diagnostic {
                         range: Range { start: Position::new(child.range().start as u32, 0), end: Position::new(child.range().end as u32, 0) },
@@ -224,17 +232,13 @@ impl XmlArchBuilder {
                 }
             }
         }
-        let data = XmlData::RECORD(XmlDataRecord {
-            file_symbol: Rc::downgrade(&self.xml_symbol),
-            model: (oyarn!("{}", node.attribute("model").unwrap()), node.attribute_node("model").unwrap().range()),
-            xml_id: found_id.clone().map(|id| oyarn!("{}", id)),
-        });
+        let data = XmlData::RECORD(data);
         self.on_operation_creation(session, found_id, node, data, diagnostics);
         true
     }
 
-    fn load_field(&mut self, session: &mut SessionInfo, node: &Node, diagnostics: &mut Vec<Diagnostic>) -> bool {
-        if node.tag_name().name() != "field" { return false; }
+    fn load_field(&mut self, session: &mut SessionInfo, node: &Node, diagnostics: &mut Vec<Diagnostic>) -> Option<XmlDataField> {
+        if node.tag_name().name() != "field" { return None; }
         if node.attribute("name").is_none() {
             if let Some(diagnostic) = create_diagnostic(session, DiagnosticCode::OLS05016, &[]) {
                 diagnostics.push(Diagnostic {
@@ -255,7 +259,7 @@ impl XmlArchBuilder {
                     ..diagnostic.clone()
                 });
             }
-            return false;
+            return None;
         }
         let mut is_xml_or_html = false;
         if let Some(field_type) = node.attribute("type") {
@@ -365,7 +369,10 @@ impl XmlArchBuilder {
                 }
             }
         }
-        true
+        Some(XmlDataField {
+            name: oyarn!("{}", node.attribute("name").unwrap()),
+            range: node.attribute_node("name").unwrap().range(),
+        })
     }
 
     fn load_value(&mut self, session: &mut SessionInfo, node: &Node, diagnostics: &mut Vec<Diagnostic>) -> bool {
diff --git a/server/src/core/xml_data.rs b/server/src/core/xml_data.rs
@@ -20,6 +20,13 @@ pub struct XmlDataRecord {
     pub file_symbol: Weak<RefCell<Symbol>>,
     pub model: (OYarn, Range<usize>),
     pub xml_id: Option<OYarn>,
+    pub fields: Vec<XmlDataField>,
+}
+
+#[derive(Debug, Clone)]
+pub struct XmlDataField {
+    pub name: OYarn,
+    pub range: Range<usize>,
 }
 
 #[derive(Debug, Clone)]
diff --git a/server/src/core/xml_validation.rs b/server/src/core/xml_validation.rs
@@ -3,7 +3,7 @@ use std::{cell::RefCell, collections::HashMap, hash::Hash, path::PathBuf, rc::Rc
 use lsp_types::{Diagnostic, Position, Range};
 use tracing::{info, trace};
 
-use crate::{constants::{BuildSteps, SymType, DEBUG_STEPS}, core::{entry_point::{EntryPoint, EntryPointType}, file_mgr::FileInfo, odoo::SyncOdoo, symbols::symbol::Symbol, xml_data::{XmlData, XmlDataActWindow, XmlDataDelete, XmlDataMenuItem, XmlDataRecord, XmlDataReport, XmlDataTemplate}}, threads::SessionInfo};
+use crate::{constants::{BuildSteps, SymType, DEBUG_STEPS, EXTENSION_NAME}, core::{entry_point::{EntryPoint, EntryPointType}, file_mgr::FileInfo, model::Model, odoo::SyncOdoo, symbols::symbol::Symbol, xml_data::{XmlData, XmlDataActWindow, XmlDataDelete, XmlDataMenuItem, XmlDataRecord, XmlDataReport, XmlDataTemplate}}, threads::SessionInfo, S};
 
 
 
@@ -35,74 +35,97 @@ impl XmlValidator {
         }
         let module = self.xml_symbol.borrow().find_module().unwrap();
         let mut dependencies = vec![];
+        let mut model_dependencies = vec![];
+        let mut diagnostics = vec![];
         for xml_ids in self.xml_symbol.borrow().as_xml_file_sym().xml_ids.values() {
             for xml_id in xml_ids.iter() {
-                self.validate_xml_id(session, &module, xml_id, &mut dependencies);
+                self.validate_xml_id(session, &module, xml_id, &mut diagnostics, &mut dependencies, &mut model_dependencies);
             }
         }
-        for mut dep in dependencies.iter_mut() {
+        for dep in dependencies.iter_mut() {
             self.xml_symbol.borrow_mut().add_dependency(&mut dep.borrow_mut(), BuildSteps::VALIDATION, BuildSteps::ARCH_EVAL);
         }
+        for model in model_dependencies.iter() {
+            self.xml_symbol.borrow_mut().add_model_dependencies(&model);
+        }
         let file_info = self.get_file_info(&mut session.sync_odoo);
+        file_info.borrow_mut().replace_diagnostics(BuildSteps::VALIDATION, diagnostics);
         file_info.borrow_mut().publish_diagnostics(session);
     }
 
-    pub fn validate_xml_id(&self, session: &mut SessionInfo, module: &Rc<RefCell<Symbol>>, data: &XmlData, dependencies: &mut Vec<Rc<RefCell<Symbol>>>) {
-        let path = data.get_file_symbol().unwrap().upgrade().unwrap().borrow().paths()[0].clone();
-        let mut file_info = session.sync_odoo.get_file_mgr().borrow().get_file_info(&path).unwrap();
-        let mut diagnostics = vec![];
+    pub fn validate_xml_id(&self, session: &mut SessionInfo, module: &Rc<RefCell<Symbol>>, data: &XmlData, diagnostics: &mut Vec<Diagnostic>, dependencies: &mut Vec<Rc<RefCell<Symbol>>>, model_dependencies: &mut Vec<Rc<RefCell<Model>>>) {
+        let Some(xml_file) = data.get_xml_file_symbol() else {
+            return;
+        };
+        let path = xml_file.borrow().paths()[0].clone();
         match data {
-            XmlData::RECORD(xml_data_record) => self.validate_record(session, module, xml_data_record, &mut diagnostics, dependencies),
-            XmlData::MENUITEM(xml_data_menu_item) => self.validate_menu_item(session, module, xml_data_menu_item, &mut diagnostics, dependencies),
-            XmlData::TEMPLATE(xml_data_template) => self.validate_template(session, module, xml_data_template, &mut diagnostics, dependencies),
-            XmlData::DELETE(xml_data_delete) => self.validate_delete(session, module, xml_data_delete, &mut diagnostics, dependencies),
-            XmlData::ACT_WINDOW(xml_data_act_window) => self.validate_act_window(session, module, xml_data_act_window, &mut diagnostics, dependencies),
-            XmlData::REPORT(xml_data_report) => self.validate_report(session, module, xml_data_report, &mut diagnostics, dependencies),
+            XmlData::RECORD(xml_data_record) => self.validate_record(session, module, xml_data_record, diagnostics, dependencies, model_dependencies),
+            XmlData::MENUITEM(xml_data_menu_item) => self.validate_menu_item(session, module, xml_data_menu_item, diagnostics, dependencies, model_dependencies),
+            XmlData::TEMPLATE(xml_data_template) => self.validate_template(session, module, xml_data_template, diagnostics, dependencies, model_dependencies),
+            XmlData::DELETE(xml_data_delete) => self.validate_delete(session, module, xml_data_delete, diagnostics, dependencies, model_dependencies),
+            XmlData::ACT_WINDOW(xml_data_act_window) => self.validate_act_window(session, module, xml_data_act_window, diagnostics, dependencies, model_dependencies),
+            XmlData::REPORT(xml_data_report) => self.validate_report(session, module, xml_data_report, diagnostics, dependencies, model_dependencies),
         }
-        file_info.borrow_mut().update_validation_diagnostics(HashMap::from([(BuildSteps::VALIDATION, diagnostics)]));
     }
 
-    fn validate_record(&self, session: &mut SessionInfo, module: &Rc<RefCell<Symbol>>, xml_data_record: &XmlDataRecord, diagnostics: &mut Vec<Diagnostic>, dependencies: &mut Vec<Rc<RefCell<Symbol>>>) {
-        let mut model_ok = false;
-        let model = session.sync_odoo.models.get(&xml_data_record.model.0).cloned();
-        if let Some(model) = model {
-            self.xml_symbol.borrow_mut().add_model_dependencies(&model);
-            for main_sym in model.borrow().get_main_symbols(session, Some(module.clone())).iter() {
-                model_ok = true;
-                dependencies.push(main_sym.borrow().get_file().unwrap().upgrade().unwrap());
-            }
-        } else {
+    fn validate_record(&self, session: &mut SessionInfo, module: &Rc<RefCell<Symbol>>, xml_data_record: &XmlDataRecord, diagnostics: &mut Vec<Diagnostic>, dependencies: &mut Vec<Rc<RefCell<Symbol>>>, model_dependencies: &mut Vec<Rc<RefCell<Model>>>) {
+        let Some(model) = session.sync_odoo.models.get(&xml_data_record.model.0).cloned() else {
             //TODO register to not_found_models
-        }
-        if !model_ok {
-            diagnostics.push(Diagnostic {
-                range: Range::new(Position::new(xml_data_record.model.1.start.try_into().unwrap(), 0), Position::new(xml_data_record.model.1.end.try_into().unwrap(), 0)),
-                severity: Some(lsp_types::DiagnosticSeverity::ERROR),
-                message: format!("Model '{}' not found in module '{}'", xml_data_record.model.0, module.borrow().name()),
-                source: Some("OdooLS".to_string()),
-                ..Default::default()
-            });
+            diagnostics.push(Diagnostic::new(
+                Range::new(Position::new(xml_data_record.model.1.start.try_into().unwrap(), 0), Position::new(xml_data_record.model.1.end.try_into().unwrap(), 0)),
+                Some(lsp_types::DiagnosticSeverity::ERROR),
+                Some(lsp_types::NumberOrString::String(S!("OLS30450"))),
+                Some(EXTENSION_NAME.to_string()),
+                format!("Model '{}' not found in module '{}'", xml_data_record.model.0, module.borrow().name()),
+                None,
+                None
+            ));
             info!("Model '{}' not found in module '{}'", xml_data_record.model.0, module.borrow().name());
+            return;
+        };
+        model_dependencies.push(model.clone());
+        let main_symbols = model.borrow().get_main_symbols(session, Some(module.clone()));
+        for main_sym in main_symbols.iter() {
+            dependencies.push(main_sym.borrow().get_file().unwrap().upgrade().unwrap());
+        }
+        let mut all_fields = HashMap::new();
+        Symbol::all_members(&main_symbols[0], session, &mut all_fields, true, true, false, Some(module.clone()), &mut None, false);
+        for field in &xml_data_record.fields {
+            let declared_field = all_fields.get(&field.name);
+            if let Some(declared_field) = declared_field {
+                //TODO Check type
+            } else {
+                
+                diagnostics.push(Diagnostic::new(
+                    Range::new(Position::new(field.range.start.try_into().unwrap(), 0), Position::new(field.range.end.try_into().unwrap(), 0)),
+                    Some(lsp_types::DiagnosticSeverity::ERROR),
+                    Some(lsp_types::NumberOrString::String(S!("OLS30451"))),
+                    Some(EXTENSION_NAME.to_string()),
+                    format!("Field '{}' not found in model '{}'", field.name, xml_data_record.model.0),
+                    None,
+                    None
+                ));
+            }
         }
     }
 
-    fn validate_menu_item(&self, session: &mut SessionInfo, module: &Rc<RefCell<Symbol>>, xml_data_menu_item: &XmlDataMenuItem, diagnostics: &mut Vec<Diagnostic>, dependencies: &mut Vec<Rc<RefCell<Symbol>>>) {
+    fn validate_menu_item(&self, session: &mut SessionInfo, module: &Rc<RefCell<Symbol>>, xml_data_menu_item: &XmlDataMenuItem, diagnostics: &mut Vec<Diagnostic>, dependencies: &mut Vec<Rc<RefCell<Symbol>>>, model_dependencies: &mut Vec<Rc<RefCell<Model>>>) {
         
     }
 
-    fn validate_template(&self, session: &mut SessionInfo, module: &Rc<RefCell<Symbol>>, xml_data_template: &XmlDataTemplate, diagnostics: &mut Vec<Diagnostic>, dependencies: &mut Vec<Rc<RefCell<Symbol>>>) {
+    fn validate_template(&self, session: &mut SessionInfo, module: &Rc<RefCell<Symbol>>, xml_data_template: &XmlDataTemplate, diagnostics: &mut Vec<Diagnostic>, dependencies: &mut Vec<Rc<RefCell<Symbol>>>, model_dependencies: &mut Vec<Rc<RefCell<Model>>>) {
         
     }
 
-    fn validate_delete(&self, session: &mut SessionInfo, module: &Rc<RefCell<Symbol>>, xml_data_delete: &XmlDataDelete, diagnostics: &mut Vec<Diagnostic>, dependencies: &mut Vec<Rc<RefCell<Symbol>>>) {
+    fn validate_delete(&self, session: &mut SessionInfo, module: &Rc<RefCell<Symbol>>, xml_data_delete: &XmlDataDelete, diagnostics: &mut Vec<Diagnostic>, dependencies: &mut Vec<Rc<RefCell<Symbol>>>, model_dependencies: &mut Vec<Rc<RefCell<Model>>>) {
         
     }
 
-    fn validate_act_window(&self, session: &mut SessionInfo, module: &Rc<RefCell<Symbol>>, xml_data_act_window: &XmlDataActWindow, diagnostics: &mut Vec<Diagnostic>, dependencies: &mut Vec<Rc<RefCell<Symbol>>>) {
+    fn validate_act_window(&self, session: &mut SessionInfo, module: &Rc<RefCell<Symbol>>, xml_data_act_window: &XmlDataActWindow, diagnostics: &mut Vec<Diagnostic>, dependencies: &mut Vec<Rc<RefCell<Symbol>>>, model_dependencies: &mut Vec<Rc<RefCell<Model>>>) {
         
     }
 
-    fn validate_report(&self, session: &mut SessionInfo, module: &Rc<RefCell<Symbol>>, xml_data_report: &XmlDataReport, diagnostics: &mut Vec<Diagnostic>, dependencies: &mut Vec<Rc<RefCell<Symbol>>>) {
+    fn validate_report(&self, session: &mut SessionInfo, module: &Rc<RefCell<Symbol>>, xml_data_report: &XmlDataReport, diagnostics: &mut Vec<Diagnostic>, dependencies: &mut Vec<Rc<RefCell<Symbol>>>, model_dependencies: &mut Vec<Rc<RefCell<Model>>>) {
         
     }
 }
diff --git a/server/src/features/features_utils.rs b/server/src/features/features_utils.rs
