Skip to content

[IMP] server: Handle invalid asts and improve completions #291

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
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
2 changes: 1 addition & 1 deletion server/src/core/evaluation.rs
Original file line number Diff line number Diff line change
@@ -1037,7 +1037,7 @@ impl Evaluation {
let name = expr.id.to_string();
(Symbol::infer_name(odoo, &parent, &name, Some(expr.range.end().to_u32())), name)
},
_ => unreachable!("NamedExpr can only have an identifier")
_ => return AnalyzeAstResult::from_only_diagnostics(diagnostics)
}
},
ExprOrIdent::Ident(expr) => {
2 changes: 1 addition & 1 deletion server/src/core/odoo.rs
Original file line number Diff line number Diff line change
@@ -1476,7 +1476,7 @@ impl Odoo {
session.log_message(MessageType::INFO, format!("File Change Event: {}, version {}", path.to_str().unwrap(), version));
let (file_updated, file_info) = session.sync_odoo.get_file_mgr().borrow_mut().update_file_info(session, &path.sanitize(), content, Some(version), false);
file_info.borrow_mut().publish_diagnostics(session); //To push potential syntax errors or refresh previous one
return (file_info.borrow().valid && (!file_info.borrow().opened || version >= 0), file_updated);
return (!file_info.borrow().opened || version >= 0, file_updated);
}
(false, false)
}
70 changes: 39 additions & 31 deletions server/src/core/python_arch_builder.rs
Original file line number Diff line number Diff line change
@@ -91,10 +91,6 @@ impl PythonArchBuilder {
false => {session.sync_odoo.get_file_mgr().borrow().get_file_info(&path).unwrap()}
};
self.file_info = Some(file_info_rc.clone());
if !file_info_rc.borrow().valid {
symbol.borrow_mut().set_build_status(BuildSteps::ARCH, BuildStatus::PENDING);
return
}
if self.file_mode {
//diagnostics for functions are stored directly on funcs
let mut file_info = file_info_rc.borrow_mut();
@@ -743,17 +739,19 @@ impl PythonArchBuilder {
let mut last_test_section = test_section.index;

self.visit_expr(session, &if_stmt.test);
scope.borrow_mut().as_mut_symbol_mgr().add_section( // first body section
if_stmt.body.first().unwrap().range().start(),
None // Take preceding section (if test)
);
self.ast_indexes.push(0 as u16); //0 for body
self.visit_node(session, &if_stmt.body)?;
self.ast_indexes.pop();

let body_section = SectionIndex::INDEX(scope.borrow().as_symbol_mgr().get_last_index());
let mut stmt_sections = if if_stmt.body.is_empty() {
vec![]
} else {
scope.borrow_mut().as_mut_symbol_mgr().add_section( // first body section
if_stmt.body[0].range().start(),
None // Take preceding section (if test)
);
self.ast_indexes.push(0 as u16); //0 for body
self.visit_node(session, &if_stmt.body)?;
self.ast_indexes.pop();
vec![ SectionIndex::INDEX(scope.borrow().as_symbol_mgr().get_last_index())]
};

let mut stmt_sections = vec![body_section];
let mut else_clause_exists = false;

let stmt_clauses_iter = if_stmt.elif_else_clauses.iter().enumerate().map(|(index, elif_else_clause)|{
@@ -767,18 +765,21 @@ impl PythonArchBuilder {
},
None => else_clause_exists = true
}
if elif_else_clause.body.is_empty() {
return Ok::<Option<SectionIndex>, Error>(None);
}
scope.borrow_mut().as_mut_symbol_mgr().add_section(
elif_else_clause.body.first().unwrap().range().start(),
elif_else_clause.body[0].range().start(),
Some(SectionIndex::INDEX(last_test_section))
);
self.ast_indexes.push((index + 1) as u16); //0 for body, so index + 1
self.visit_node(session, &elif_else_clause.body)?;
self.ast_indexes.pop();
let clause_section = SectionIndex::INDEX(scope.borrow().as_symbol_mgr().get_last_index());
Ok::<SectionIndex, Error>(clause_section)
Ok::<Option<SectionIndex>, Error>(Some(clause_section))
});

stmt_sections.extend(stmt_clauses_iter.collect::<Result<Vec<_>, _>>()?);
stmt_sections.extend(stmt_clauses_iter.collect::<Result<Vec<_>, _>>()?.into_iter().filter_map(|x| x).collect::<Vec<_>>());

if !else_clause_exists{
// If there is no else clause, the there is an implicit else clause
@@ -804,15 +805,17 @@ impl PythonArchBuilder {
AssignTargetType::Name(ref name_expr) => {
scope.borrow_mut().add_new_variable(session, oyarn!("{}", name_expr.id), &name_expr.range);
},
AssignTargetType::Attribute(ref attr_expr) => {
AssignTargetType::Attribute(_) => {
}
}
}
let previous_section = SectionIndex::INDEX(scope.borrow().as_symbol_mgr().get_last_index());
scope.borrow_mut().as_mut_symbol_mgr().add_section(
for_stmt.body.first().unwrap().range().start(),
None
);
if let Some(first_body_stmt) = for_stmt.body.first() {
scope.borrow_mut().as_mut_symbol_mgr().add_section(
first_body_stmt.range().start(),
None
);
}

self.ast_indexes.push(0 as u16);
self.visit_node(session, &for_stmt.body)?;
@@ -821,7 +824,7 @@ impl PythonArchBuilder {

if !for_stmt.orelse.is_empty(){
scope.borrow_mut().as_mut_symbol_mgr().add_section(
for_stmt.orelse.first().unwrap().range().start(),
for_stmt.orelse[0].range().start(),
Some(previous_section.clone())
);
self.ast_indexes.push(1 as u16);
@@ -859,8 +862,11 @@ impl PythonArchBuilder {
match handler {
ruff_python_ast::ExceptHandler::ExceptHandler(h) => {
if !catch_all_except_exists { catch_all_except_exists = h.type_.is_none()};
if h.body.is_empty() {
continue;
}
scope.borrow_mut().as_mut_symbol_mgr().add_section(
h.body.first().unwrap().range().start(),
h.body[0].range().start(),
Some(previous_section.clone())
);
self.ast_indexes.push(index as u16);
@@ -876,7 +882,7 @@ impl PythonArchBuilder {
stmt_sections.remove(0);
}
scope.borrow_mut().as_mut_symbol_mgr().add_section(
try_stmt.orelse.first().unwrap().range().start(),
try_stmt.orelse[0].range().start(),
Some(previous_section.clone())
);
self.ast_indexes.push(1 as u16);
@@ -975,11 +981,13 @@ impl PythonArchBuilder {
fn visit_while(&mut self, session: &mut SessionInfo, while_stmt: &StmtWhile) -> Result<(), Error> {
// TODO: Handle breaks for sections
let scope = self.sym_stack.last().unwrap().clone();
let body_section = scope.borrow_mut().as_mut_symbol_mgr().add_section(
while_stmt.body.first().unwrap().range().start(),
None
);
let previous_section = SectionIndex::INDEX(body_section.index - 1);
let previous_section = SectionIndex::INDEX(scope.borrow().as_symbol_mgr().get_last_index());
if let Some(first_body_stmt) = while_stmt.body.first() {
scope.borrow_mut().as_mut_symbol_mgr().add_section(
first_body_stmt.range().start(),
None
);
}
self.visit_expr(session, &while_stmt.test);
self.ast_indexes.push(0 as u16); // 0 for body
self.visit_node(session, &while_stmt.body)?;
@@ -988,7 +996,7 @@ impl PythonArchBuilder {
let mut stmt_sections = vec![body_section];
if !while_stmt.orelse.is_empty(){
scope.borrow_mut().as_mut_symbol_mgr().add_section(
while_stmt.orelse.first().unwrap().range().start(),
while_stmt.orelse[0].range().start(),
Some(previous_section.clone())
);
self.ast_indexes.push(1 as u16); // 1 for else
2 changes: 1 addition & 1 deletion server/src/core/python_validator.rs
Original file line number Diff line number Diff line change
@@ -90,7 +90,7 @@ impl PythonValidator {
self.sym_stack[0].borrow_mut().set_build_status(BuildSteps::VALIDATION, BuildStatus::INVALID);
return;
}
if file_info.file_info_ast.borrow().ast.is_some() && file_info.valid {
if file_info.file_info_ast.borrow().ast.is_some() {
let old_noqa = session.current_noqa.clone();
session.current_noqa = self.sym_stack[0].borrow().get_noqas();
let file_info_ast = file_info.file_info_ast.borrow();
37 changes: 27 additions & 10 deletions server/src/core/symbols/symbol.rs
Original file line number Diff line number Diff line change
@@ -1234,6 +1234,18 @@ impl Symbol {
}
}

/// Return all symbols before the given position that are visible in the body of this symbol.
pub fn get_all_visible_symbols(&self, name_prefix: &String, position: u32) -> HashMap<OYarn, Vec<Rc<RefCell<Symbol>>>> {
match self {
Symbol::Class(c) => c.get_all_visible_symbols(name_prefix, position),
Symbol::File(f) => f.get_all_visible_symbols(name_prefix, position),
Symbol::Package(PackageSymbol::Module(m)) => m.get_all_visible_symbols(name_prefix, position),
Symbol::Package(PackageSymbol::PythonPackage(p)) => p.get_all_visible_symbols(name_prefix, position),
Symbol::Function(f) => f.get_all_visible_symbols(name_prefix, position),
_ => HashMap::new(),
}
}

/**
* Return a symbol that can be called from outside of the body of the symbol
*/
@@ -2100,22 +2112,27 @@ impl Symbol {
/*
Return all the symbols that are available at a given position or in a scope for a given start name
*/
pub fn get_all_inferred_names(on_symbol: &Rc<RefCell<Symbol>>, name: &String, position: Option<u32>) -> Vec<Rc<RefCell<Symbol>>> {
fn helper(on_symbol: &Rc<RefCell<Symbol>>, name: &String, position: Option<u32>, acc: &mut Vec<Rc<RefCell<Symbol>>>) {
pub fn get_all_inferred_names(on_symbol: &Rc<RefCell<Symbol>>, name: &String, position: u32) -> HashMap<OYarn, Vec<Rc<RefCell<Symbol>>>> {
fn helper(
on_symbol: &Rc<RefCell<Symbol>>, name: &String, position: u32, acc: &mut HashMap<OYarn, Vec<Rc<RefCell<Symbol>>>>
) {
// Add symbols from files and functions
if matches!(on_symbol.borrow().typ(), SymType::FILE | SymType::FUNCTION){
acc.extend(on_symbol.borrow().all_symbols().filter(|sym|
sym.borrow().name().starts_with(name) && (position.is_none() || !sym.borrow().has_range() || position.unwrap() > sym.borrow().range().end().to_u32())
))
};
if matches!(on_symbol.borrow().typ(), SymType::FILE | SymType::FUNCTION) {
let symbols_map = on_symbol.borrow().get_all_visible_symbols(name, position);
for (sym_name, sym_vec) in symbols_map {
acc.entry(sym_name)
.or_default()
.extend(sym_vec);
}
}
// Traverse upwards if we are under a class or a function
if matches!(on_symbol.borrow().typ(), SymType::CLASS | SymType::FUNCTION){
if matches!(on_symbol.borrow().typ(), SymType::CLASS | SymType::FUNCTION) {
if let Some(parent) = on_symbol.borrow().parent().as_ref().and_then(|parent_weak| parent_weak.upgrade()) {
helper(&parent, name, position, acc);
}
};
}
}
let mut results: Vec<Rc<RefCell<Symbol>>> = vec![];
let mut results= HashMap::new();
helper(on_symbol, name, position, &mut results);
results
}
19 changes: 19 additions & 0 deletions server/src/core/symbols/symbol_mgr.rs
Original file line number Diff line number Diff line change
@@ -37,6 +37,7 @@ pub trait SymbolMgr {
fn get_ext_symbol(&self, name: OYarn) -> Option<&Vec<Rc<RefCell<Symbol>>>>;
fn _init_symbol_mgr(&mut self);
fn _get_loc_symbol(&self, map: &HashMap<u32, Vec<Rc<RefCell<Symbol>>>>, position: u32, index: &SectionIndex, acc: &mut HashSet<u32>) -> ContentSymbols;
fn get_all_visible_symbols(&self, name_prefix: &String, position: u32) -> HashMap<OYarn, Vec<Rc<RefCell<Symbol>>>>;
}


@@ -156,6 +157,24 @@ macro_rules! impl_section_mgr_for {
res
}

fn get_all_visible_symbols(&self, name_prefix: &String, position: u32) -> HashMap<OYarn, Vec<Rc<RefCell<Symbol>>>> {
let mut result = HashMap::new();
let current_section = self.get_section_for(position);
let current_index = SectionIndex::INDEX(current_section.index);

for (name, section_map) in self.symbols.iter() {
if !name.starts_with(name_prefix) {
continue;
}
let mut seen = HashSet::new();
let content = self._get_loc_symbol(section_map, position, &current_index, &mut seen);

if !content.symbols.is_empty() {
result.insert(name.clone(), content.symbols);
}
}
result
}
}
)+)
}
Loading