Skip to content

Commit b87cd9d

Browse files
mmahroussfda-odoo
authored andcommitted
[IMP] server: handle diagnostics for multiple fn signatures
1 parent 20e1f96 commit b87cd9d

File tree

2 files changed

+52
-21
lines changed

2 files changed

+52
-21
lines changed

server/src/core/diagnostic_codes_list.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,10 @@ OLS01007, DiagnosticSetting::Error, "{0} takes {1} positional arguments but {2}
5050
* You gave a named parameter that is not present in the function definition.
5151
*/
5252
OLS01008, DiagnosticSetting::Error, "{0} got an unexpected keyword argument '{1}'",
53+
/**
54+
* Arguments are not valid for all function or method definitions
55+
*/
56+
OLS01009, DiagnosticSetting::Warning, "Arguments are not valid for all function or method definitions",
5357
/**
5458
* Check your python environment, the effective your sys.path and your addon paths.
5559
*/

server/src/core/evaluation.rs

Lines changed: 48 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -740,7 +740,7 @@ impl Evaluation {
740740
evals.push(Evaluation::new_dict(odoo, values, expr.range));
741741
},
742742
ExprOrIdent::Expr(Expr::Call(expr)) => {
743-
let (base_eval, diags) = Evaluation::eval_from_ast(session, &expr.func, parent.clone(), max_infer, false, required_dependencies);
743+
let (base_evals, diags) = Evaluation::eval_from_ast(session, &expr.func, parent.clone(), max_infer, false, required_dependencies);
744744
diagnostics.extend(diags);
745745
//TODO actually we only evaluate if there is only one function behind the evaluation.
746746
// we could evaluate the result of each function and filter results by signature matching.
@@ -764,13 +764,25 @@ impl Evaluation {
764764
765765
print(c) <= string/int with value 5. if we had a parameter to 'other_test', only string with value 5
766766
*/
767-
if base_eval.len() == 0 {
767+
if base_evals.len() == 0 {
768768
/*TODO if multiple evals are found, we could maybe try to validate that they all have the same signature in case of diamond inheritance?
769769
However, other cases should be handled by arch step or syntax? */
770770
return AnalyzeAstResult::from_only_diagnostics(diagnostics);
771771
}
772-
let base_sym_weak_eval_base= base_eval[0].symbol.get_symbol_weak_transformed(session, context, &mut diagnostics, None);
773-
let base_eval_ptrs = Symbol::follow_ref(&base_sym_weak_eval_base, session, context, true, false, None, &mut diagnostics);
772+
let base_eval_ptrs: Vec<EvaluationSymbolPtr> = base_evals.iter().map(|base_eval| {
773+
let base_sym_weak_eval_base = base_eval.symbol.get_symbol_weak_transformed(session, context, &mut diagnostics, None);
774+
Symbol::follow_ref(&base_sym_weak_eval_base, session, context, true, false, None, &mut diagnostics)
775+
}).flatten().collect();
776+
777+
let parent_file_or_func = parent.clone().borrow().parent_file_or_function().as_ref().unwrap().upgrade().unwrap();
778+
let is_in_validation = match parent_file_or_func.borrow().typ().clone() {
779+
SymType::FILE | SymType::PACKAGE(_) | SymType::FUNCTION => {
780+
parent_file_or_func.borrow().build_status(BuildSteps::VALIDATION) == BuildStatus::IN_PROGRESS
781+
},
782+
_ => {false}
783+
};
784+
785+
let mut call_argument_diagnostics = Vec::new();
774786
for base_eval_ptr in base_eval_ptrs.iter() {
775787
let EvaluationSymbolPtr::WEAK(base_sym_weak_eval) = base_eval_ptr else {continue};
776788
let Some(base_sym) = base_sym_weak_eval.weak.upgrade() else {continue};
@@ -904,15 +916,6 @@ impl Evaluation {
904916
}
905917
//It allows us to check parameters validity too if we are in validation step
906918
/*let parent_file_or_func = parent.borrow().parent_file_or_function().as_ref().unwrap().upgrade().unwrap();
907-
let is_in_validation = match parent_file_or_func.borrow().typ().clone() {
908-
SymType::FILE | SymType::PACKAGE(_) => {
909-
parent_file_or_func.borrow().build_status(BuildSteps::VALIDATION) == BuildStatus::IN_PROGRESS
910-
},
911-
SymType::FUNCTION => {
912-
true //functions are always evaluated at validation step
913-
}
914-
_ => {false}
915-
};
916919
if is_in_validation {
917920
let from_module = parent.borrow().find_module();
918921
diagnostics.extend(Evaluation::validate_call_arguments(session,
@@ -972,20 +975,13 @@ impl Evaluation {
972975
}
973976
}
974977
if base_sym.borrow().evaluations().is_some() {
975-
let parent_file_or_func = parent.clone().borrow().parent_file_or_function().as_ref().unwrap().upgrade().unwrap();
976-
let is_in_validation = match parent_file_or_func.borrow().typ().clone() {
977-
SymType::FILE | SymType::PACKAGE(_) | SymType::FUNCTION => {
978-
parent_file_or_func.borrow().build_status(BuildSteps::VALIDATION) == BuildStatus::IN_PROGRESS
979-
},
980-
_ => {false}
981-
};
982978
let call_parent = match base_sym_weak_eval.context.get(&S!("base_attr")){
983979
Some(ContextValue::SYMBOL(s)) => s.clone(),
984980
_ => Weak::new()
985981
};
986982
if is_in_validation {
987983
let on_instance = base_sym_weak_eval.context.get(&S!("is_attr_of_instance")).map(|v| v.as_bool());
988-
diagnostics.extend(Evaluation::validate_call_arguments(session,
984+
call_argument_diagnostics.extend(Evaluation::validate_call_arguments(session,
989985
&base_sym.borrow().as_func(),
990986
expr,
991987
call_parent.clone(),
@@ -1013,6 +1009,7 @@ impl Evaluation {
10131009
}
10141010
}
10151011
}
1012+
diagnostics.extend(Evaluation::process_argument_diagnostics(&session, expr, call_argument_diagnostics, base_eval_ptrs.len()));
10161013
},
10171014
ExprOrIdent::Expr(Expr::Attribute(expr)) => {
10181015
let (base_evals, diags) = Evaluation::eval_from_ast(session, &expr.value, parent.clone(), max_infer, false, required_dependencies);
@@ -1428,6 +1425,36 @@ impl Evaluation {
14281425
diagnostics
14291426
}
14301427

1428+
fn process_argument_diagnostics(session: &SessionInfo, expr_call: &ExprCall, diagnostics: Vec<Diagnostic>, eval_count: usize) -> Vec<Diagnostic> {
1429+
let mut filtered_diagnostics = vec![];
1430+
let mut arg_diagnostic = diagnostics.iter().filter(|d|
1431+
d.code == Some(lsp_types::NumberOrString::String(DiagnosticCode::OLS01007.to_string())) ||
1432+
d.code == Some(lsp_types::NumberOrString::String(DiagnosticCode::OLS01008.to_string()))
1433+
);
1434+
let arg_diagnostic_count = arg_diagnostic.clone().count();
1435+
if arg_diagnostic_count > 0 {
1436+
if arg_diagnostic_count == eval_count {
1437+
// if all evaluations have some argument error, we keep the first one
1438+
filtered_diagnostics.push(arg_diagnostic.next().unwrap().clone());
1439+
} else {
1440+
// if not all evaluations have it, it means at least one is valid.
1441+
// We use a different code warning
1442+
if let Some(diagnostic) = create_diagnostic(session, DiagnosticCode::OLS01009, &[]) {
1443+
filtered_diagnostics.push(Diagnostic {
1444+
range: Range::new(Position::new(expr_call.range().start().to_u32(), 0), Position::new(expr_call.range().end().to_u32(), 0)),
1445+
..diagnostic
1446+
});
1447+
}
1448+
}
1449+
}
1450+
// we add the rest of the diagnostics as is
1451+
filtered_diagnostics.extend(diagnostics.into_iter().filter(|d| {
1452+
d.code != Some(lsp_types::NumberOrString::String(DiagnosticCode::OLS01007.to_string())) &&
1453+
d.code != Some(lsp_types::NumberOrString::String(DiagnosticCode::OLS01008.to_string()))
1454+
}));
1455+
filtered_diagnostics
1456+
}
1457+
14311458
fn validate_domain(session: &mut SessionInfo, on_object: Weak<RefCell<Symbol>>, from_module: Option<Rc<RefCell<Symbol>>>, value: &Expr) -> Vec<Diagnostic> {
14321459
let mut diagnostics = vec![];
14331460
if !matches!(value, Expr::List(_)) {

0 commit comments

Comments
 (0)