Skip to content

[IMP] server: add diagnostic for model shadowing #353

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 25, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions server/src/core/diagnostic_codes_list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,11 @@ OLS03018, DiagnosticSetting::Error, "Method does not exist on current model",
* Consider marking the modified field with the compute method
*/
OLS03019, DiagnosticSetting::Error, "Compute method not set to modify this field",
/**
* _name is set on a class which creaes a model, but the name is already used by another model.
* Hence, this model is shadowing an existing model.
*/
OLS03020, DiagnosticSetting::Warning, "Model {0} is shadowing an existing model in dependencies",
/**
* A __manifest__.py file should be evaluated with a literal_eval to a single dictionary.
* Do not store any other information in it.
Expand Down
36 changes: 32 additions & 4 deletions server/src/core/python_odoo_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,13 @@ use std::cmp::Ordering;
use std::collections::HashSet;
use std::rc::Rc;
use std::cell::RefCell;
use lsp_types::notification::ShowMessage;
use lsp_types::MessageType;
use ruff_python_ast::Expr;
use lsp_types::{Diagnostic, ShowMessageParams, notification::Notification};
use lsp_types::Diagnostic;
use ruff_text_size::TextRange;
use tracing::error;
use weak_table::PtrWeakHashSet;
use crate::core::diagnostics::{create_diagnostic, DiagnosticCode};
use crate::core::file_mgr::{FileMgr};

use crate::constants::{OYarn, SymType};
use crate::core::model::{Model, ModelData};
Expand Down Expand Up @@ -49,7 +50,34 @@ impl PythonOdooBuilder {
self._add_magic_fields(session);
let model_name = sym.borrow().as_class_sym()._model.as_ref().unwrap().name.clone();
match session.sync_odoo.models.get(&model_name).cloned(){
Some(model) => model.borrow_mut().add_symbol(session, sym.clone()),
Some(model) => {
let inherited_model_names = sym.borrow().as_class_sym()._model.as_ref().unwrap().inherit.clone();
if !inherited_model_names.contains(&model_name)
&& !model.borrow().get_main_symbols(session, sym.borrow().find_module()).is_empty(){
// This a model with a name that already exists in models and in dependencies,
// and it is not inherited, so it is basically shadowing the existing model.
let _name = sym.borrow().get_symbol(&(vec![], vec![Sy!("_name")]), u32::MAX);
if let Some(_name) = _name.last() {
let mut range = _name.borrow().range().clone();
// Try to get the string value range, otherwise stick to _name var range.
if let Some(eval_range) = _name.borrow().evaluations().unwrap().iter().find_map(|e|
match e.follow_ref_and_get_value(session, &mut None, &mut diagnostics) {
Some(EvaluationValue::CONSTANT(Expr::StringLiteral(_))) => e.range,
_ => None,
}
) {
range = TextRange::new(range.start(), eval_range.end());
}
if let Some(diagnostic) = create_diagnostic(&session, DiagnosticCode::OLS03020, &[&model_name]) {
diagnostics.push(Diagnostic {
range: FileMgr::textRange_to_temporary_Range(&range),
..diagnostic
});
}
}
}
model.borrow_mut().add_symbol(session, sym.clone())
},
None => {
let model = Model::new(model_name.clone(), sym.clone());
session.sync_odoo.modules.get("base").map(|module| {
Expand Down