Skip to content

Commit f11c0e4

Browse files
fda-odoofda-odoo
committed
[IMP] server: python version check in if tests
actually skipping arch steps for these sections, not in arch_eval, leading to some debug logs
1 parent f3719f4 commit f11c0e4

File tree

2 files changed

+157
-13
lines changed

2 files changed

+157
-13
lines changed

‎server/src/core/odoo.rs

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ use lsp_server::ResponseError;
2121
use lsp_types::*;
2222
use request::{RegisterCapability, Request, WorkspaceConfiguration};
2323
use ruff_python_parser::{Mode, ParseOptions};
24+
use serde_json::Value;
2425
use tracing::{error, warn, info, trace};
2526

2627
use std::collections::HashSet;
@@ -63,6 +64,7 @@ pub struct SyncOdoo {
6364
pub version_minor: u32,
6465
pub version_micro: u32,
6566
pub full_version: String,
67+
pub python_version: Vec<u32>,
6668
pub config: ConfigEntry,
6769
pub config_file: Option<ConfigFile>,
6870
pub config_path: Option<String>,
@@ -101,6 +103,7 @@ impl SyncOdoo {
101103
version_minor: 0,
102104
version_micro: 0,
103105
full_version: "0.0.0".to_string(),
106+
python_version: vec![0, 0, 0],
104107
config: ConfigEntry::new(),
105108
config_file: None,
106109
config_path: None,
@@ -194,7 +197,6 @@ impl SyncOdoo {
194197
session.send_notification("$Odoo/loadingStatusUpdate", "stop");
195198
return;
196199
}
197-
session.sync_odoo.has_valid_python = true;
198200
let output = output.unwrap();
199201
if output.status.success() {
200202
let stdout = String::from_utf8_lossy(&output.stdout);
@@ -213,6 +215,30 @@ impl SyncOdoo {
213215
let stderr = String::from_utf8_lossy(&output.stderr);
214216
error!("{}", stderr);
215217
}
218+
let output = Command::new(session.sync_odoo.config.python_path.clone()).args(&["-c", "import sys; import json; print(json.dumps(sys.version_info))"]).output();
219+
if let Err(_output) = &output {
220+
error!("Wrong python command: {}", session.sync_odoo.config.python_path.clone());
221+
session.send_notification("$Odoo/invalid_python_path", ());
222+
session.send_notification("$Odoo/loadingStatusUpdate", "stop");
223+
return;
224+
}
225+
session.sync_odoo.has_valid_python = true;
226+
let output = output.unwrap();
227+
if output.status.success() {
228+
let stdout = String::from_utf8_lossy(&output.stdout);
229+
session.log_message(MessageType::INFO, format!("Detected sys.version_info: {}", stdout));
230+
let version_infos: Value = serde_json::from_str(&stdout).expect("Unable to get python version info with json of sys.version_info output");
231+
session.sync_odoo.python_version = version_infos.as_array()
232+
.expect("Expected JSON array")
233+
.iter()
234+
.filter_map(|v| v.as_u64())
235+
.map(|v| v as u32)
236+
.take(3)
237+
.collect();
238+
} else {
239+
let stderr = String::from_utf8_lossy(&output.stderr);
240+
error!("{}", stderr);
241+
}
216242
}
217243
if SyncOdoo::load_builtins(session) {
218244
session.sync_odoo.state_init = InitState::PYTHON_READY;

‎server/src/core/python_arch_builder.rs

Lines changed: 130 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ use std::cell::RefCell;
44
use std::vec;
55
use anyhow::Error;
66
use ruff_text_size::{Ranged, TextRange, TextSize};
7-
use ruff_python_ast::{Alias, Expr, ExprNamed, FStringPart, Identifier, Pattern, Stmt, StmtAnnAssign, StmtAssign, StmtClassDef, StmtFor, StmtFunctionDef, StmtIf, StmtMatch, StmtTry, StmtWhile, StmtWith};
7+
use ruff_python_ast::{Alias, CmpOp, Expr, ExprNamed, ExprTuple, FStringPart, Identifier, Pattern, Stmt, StmtAnnAssign, StmtAssign, StmtClassDef, StmtFor, StmtFunctionDef, StmtIf, StmtMatch, StmtTry, StmtWhile, StmtWith};
88
use lsp_types::Diagnostic;
99
use tracing::{trace, warn};
1010
use weak_table::traits::WeakElement;
@@ -793,6 +793,102 @@ impl PythonArchBuilder {
793793
}
794794
}
795795

796+
fn check_tuples(&self, version: &Vec<u32>, op: &CmpOp, tuple: &ExprTuple) -> bool {
797+
let mut tuple = tuple.elts.iter().map(|elt| {
798+
if let Expr::NumberLiteral(num) = elt {
799+
if num.value.is_int() {
800+
num.value.as_int().unwrap().as_u32().unwrap()
801+
} else {
802+
0 as u32
803+
}
804+
} else {
805+
0 as u32 // If not a number, treat as 0
806+
}
807+
}).collect::<Vec<u32>>();
808+
// ensure that the vec is sized of 3
809+
tuple.resize(3, 0);
810+
return match op {
811+
CmpOp::Gt => {
812+
version[0] > tuple[0] ||
813+
(version[0] == tuple[0] && version[1] > tuple[1]) ||
814+
(version[0] == tuple[0] && version[1] == tuple[1] && version[2] > tuple[2])
815+
},
816+
CmpOp::GtE => {
817+
version[0] >= tuple[0] ||
818+
(version[0] == tuple[0] && version[1] >= tuple[1]) ||
819+
(version[0] == tuple[0] && version[1] == tuple[1] && version[2] >= tuple[2])
820+
},
821+
CmpOp::Lt => {
822+
version[0] < tuple[0] ||
823+
(version[0] == tuple[0] && version[1] < tuple[1]) ||
824+
(version[0] == tuple[0] && version[1] == tuple[1] && version[2] < tuple[2])
825+
},
826+
CmpOp::LtE => {
827+
version[0] <= tuple[0] ||
828+
(version[0] == tuple[0] && version[1] <= tuple[1]) ||
829+
(version[0] == tuple[0] && version[1] == tuple[1] && version[2] <= tuple[2])
830+
},
831+
CmpOp::Eq => {
832+
version[0] == tuple[0] &&
833+
version[1] == tuple[1] &&
834+
version[2] == tuple[2]
835+
},
836+
CmpOp::NotEq => {
837+
version[0] != tuple[0] ||
838+
version[1] != tuple[1] ||
839+
version[2] != tuple[2]
840+
},
841+
_ => {
842+
false
843+
}
844+
}
845+
}
846+
847+
/** returns
848+
* first bool: true if we can go in the condition, because no version check is preventing it
849+
* second bool: true if there was a version check or false if the condition was unrelated
850+
*/
851+
fn _check_sys_version_condition(&self, session: &mut SessionInfo, expr: &Expr) -> (bool, bool) {
852+
if session.sync_odoo.python_version[0] == 0 {
853+
return (true, false); //unknown python version
854+
}
855+
if let Expr::Compare(expr_comp) = expr {
856+
if expr_comp.comparators.len() == 1 {
857+
let p1 = expr_comp.left.as_ref();
858+
let p2 = expr_comp.comparators.first().unwrap();
859+
if !p1.is_tuple_expr() && !p2.is_tuple_expr() {
860+
return (true, false);
861+
}
862+
if !p1.is_attribute_expr() && !p2.is_attribute_expr() {
863+
return (true, false);
864+
}
865+
let (tuple, attr) = if p1.is_tuple_expr() {
866+
(p1.as_tuple_expr().unwrap(), p2.as_attribute_expr().unwrap())
867+
} else {
868+
(p2.as_tuple_expr().unwrap(), p1.as_attribute_expr().unwrap())
869+
};
870+
if attr.value.is_name_expr() && attr.value.as_name_expr().unwrap().id == "sys" {
871+
if attr.attr.id == "version_info" {
872+
let mut op = expr_comp.ops.first().unwrap();
873+
if p1.is_tuple_expr() { //invert if tuple is in front
874+
if op.is_gt() {
875+
op = &CmpOp::Lt;
876+
} else if op.is_gt_e() {
877+
op = &CmpOp::LtE;
878+
} else if op.is_lt() {
879+
op = &CmpOp::Gt;
880+
} else if op.is_lt_e() {
881+
op = &CmpOp::GtE;
882+
}
883+
}
884+
return (self.check_tuples(&session.sync_odoo.python_version, op, tuple), true)
885+
}
886+
}
887+
}
888+
}
889+
(true, false)
890+
}
891+
796892
fn visit_if(&mut self, session: &mut SessionInfo, if_stmt: &StmtIf) -> Result<(), Error> {
797893
//TODO check platform condition (sys.version > 3.12, etc...)
798894
let scope = self.sym_stack.last().unwrap().clone();
@@ -803,17 +899,26 @@ impl PythonArchBuilder {
803899
let mut last_test_section = test_section.index;
804900

805901
self.visit_expr(session, &if_stmt.test);
902+
let mut body_version_ok = false; //if true, it means we found a condition that is true and contained a version check. Used to avoid else clause
806903
let mut stmt_sections = if if_stmt.body.is_empty() {
807904
vec![]
808905
} else {
809-
scope.borrow_mut().as_mut_symbol_mgr().add_section( // first body section
810-
if_stmt.body[0].range().start(),
811-
None // Take preceding section (if test)
812-
);
813-
self.ast_indexes.push(0 as u16); //0 for body
814-
self.visit_node(session, &if_stmt.body)?;
815-
self.ast_indexes.pop();
816-
vec![ SectionIndex::INDEX(scope.borrow().as_symbol_mgr().get_last_index())]
906+
scope.borrow_mut().as_mut_symbol_mgr().add_section( // first body section
907+
if_stmt.body[0].range().start(),
908+
None // Take preceding section (if test)
909+
);
910+
let check_version = self._check_sys_version_condition(session, if_stmt.test.as_ref());
911+
if check_version.0 {
912+
if check_version.1 {
913+
body_version_ok = true;
914+
}
915+
self.ast_indexes.push(0 as u16); //0 for body
916+
self.visit_node(session, &if_stmt.body)?;
917+
self.ast_indexes.pop();
918+
vec![ SectionIndex::INDEX(scope.borrow().as_symbol_mgr().get_last_index())]
919+
} else {
920+
vec![]
921+
}
817922
};
818923

819924
let mut else_clause_exists = false;
@@ -836,9 +941,22 @@ impl PythonArchBuilder {
836941
elif_else_clause.body[0].range().start(),
837942
Some(SectionIndex::INDEX(last_test_section))
838943
);
839-
self.ast_indexes.push((index + 1) as u16); //0 for body, so index + 1
840-
self.visit_node(session, &elif_else_clause.body)?;
841-
self.ast_indexes.pop();
944+
if elif_else_clause.test.is_some() {
945+
let version_check = self._check_sys_version_condition(session, elif_else_clause.test.as_ref().unwrap());
946+
if version_check.0 {
947+
if version_check.1 {
948+
body_version_ok = true;
949+
}
950+
self.ast_indexes.push((index + 1) as u16); //0 for body, so index + 1
951+
self.visit_node(session, &elif_else_clause.body)?;
952+
self.ast_indexes.pop();
953+
}
954+
}
955+
else if !body_version_ok { //else clause
956+
self.ast_indexes.push((index + 1) as u16); //0 for body, so index + 1
957+
self.visit_node(session, &elif_else_clause.body)?;
958+
self.ast_indexes.pop();
959+
}
842960
let clause_section = SectionIndex::INDEX(scope.borrow().as_symbol_mgr().get_last_index());
843961
Ok::<Option<SectionIndex>, Error>(Some(clause_section))
844962
});

0 commit comments

Comments
 (0)