From 2fce9bda3bda251d71142ab80801aeac2623fcbe Mon Sep 17 00:00:00 2001 From: Shunsuke Shibayama Date: Thu, 4 May 2023 19:05:27 +0900 Subject: [PATCH] fix(els): eliminate `unwrap`s --- crates/els/completion.rs | 18 ++++++++++----- crates/els/diagnostics.rs | 9 +++++--- crates/els/file_cache.rs | 16 ++++++++++--- crates/els/hir_visitor.rs | 4 ++-- crates/els/hover.rs | 29 ++++++++++++++++-------- crates/els/references.rs | 4 +++- crates/els/rename.rs | 45 ++++++++++++++++++++++++++++--------- crates/els/sig_help.rs | 2 +- crates/erg_compiler/lint.rs | 4 +++- crates/erg_parser/lex.rs | 6 ++--- 10 files changed, 98 insertions(+), 39 deletions(-) diff --git a/crates/els/completion.rs b/crates/els/completion.rs index 6d6005c03..d0683fb96 100644 --- a/crates/els/completion.rs +++ b/crates/els/completion.rs @@ -162,7 +162,11 @@ impl<'b> CompletionOrderSetter<'b> { pub fn mangle(&self) -> String { let score = self.score(); - format!("{}_{}", char::from_u32(score as u32).unwrap(), self.label) + format!( + "{}_{}", + char::from_u32(score as u32).unwrap_or(CompletionOrder::STD_ITEM), + self.label + ) } fn set(&self, item: &mut CompletionItem) { @@ -380,7 +384,9 @@ impl Server { .map(|t| matches!(t.kind, Dot | DblColon)) .unwrap_or(false) { - let dot_pos = util::loc_to_pos(prev_token.unwrap().loc()).unwrap(); + let Some(dot_pos) = util::loc_to_pos(prev_token.unwrap().loc()) else { + return Ok(None); + }; self.get_receiver_ctxs(&uri, dot_pos)? } else { self.get_local_ctx(&uri, pos) @@ -444,9 +450,9 @@ impl Server { } // only show static methods, if the receiver is a type if vi.t.is_method() - && receiver_t - .as_ref() - .map_or(true, |t| !mod_ctx.subtype_of(t, vi.t.self_t().unwrap())) + && receiver_t.as_ref().map_or(true, |t| { + !mod_ctx.subtype_of(t, vi.t.self_t().unwrap_or(Type::OBJ)) + }) { continue; } @@ -494,7 +500,7 @@ impl Server { send_log(format!("completion resolve requested: {item:?}"))?; if let Some(data) = &item.data { let mut contents = vec![]; - let Ok(def_loc) = data.as_str().unwrap().parse::() else { + let Ok(def_loc) = data.as_str().unwrap_or_default().parse::() else { return Ok(item); }; self.show_doc_comment(None, &mut contents, &def_loc)?; diff --git a/crates/els/diagnostics.rs b/crates/els/diagnostics.rs index 3ed63c4a1..421ca73b6 100644 --- a/crates/els/diagnostics.rs +++ b/crates/els/diagnostics.rs @@ -115,10 +115,13 @@ impl Server { let mut uri_and_diags: Vec<(Url, Vec)> = vec![]; for err in errors.into_iter() { let loc = err.core.get_loc_with_fallback(); - let err_uri = if let Some(path) = err.input.path() { - Url::from_file_path(path).unwrap() + let res_uri = if let Some(path) = err.input.path() { + Url::from_file_path(path) } else { - uri.clone().raw() + Ok(uri.clone().raw()) + }; + let Ok(err_uri) = res_uri else { + continue; }; let mut message = remove_style(&err.core.main_message); for sub in err.core.sub_messages { diff --git a/crates/els/file_cache.rs b/crates/els/file_cache.rs index 91ae16547..a220397d9 100644 --- a/crates/els/file_cache.rs +++ b/crates/els/file_cache.rs @@ -15,6 +15,7 @@ use erg_common::traits::DequeStream; use erg_compiler::erg_parser::lex::Lexer; use erg_compiler::erg_parser::token::{Token, TokenStream}; +use crate::_log; use crate::server::ELSResult; use crate::util::{self, NormalizedUrl}; @@ -258,7 +259,9 @@ impl FileCache { } let mut code = entry.code.clone(); for change in params.content_changes { - let range = change.range.unwrap(); + let Some(range) = change.range else { + continue; + }; let start = util::pos_to_byte_index(&code, range.start); let end = util::pos_to_byte_index(&code, range.end); code.replace_range(start..end, &change.text); @@ -276,9 +279,16 @@ impl FileCache { pub fn rename_files(&mut self, params: &RenameFilesParams) -> ELSResult<()> { for file in ¶ms.files { - let old_uri = NormalizedUrl::parse(&file.old_uri).unwrap(); - let new_uri = NormalizedUrl::parse(&file.new_uri).unwrap(); + let Ok(old_uri) = NormalizedUrl::parse(&file.old_uri) else { + _log!("failed to parse old uri: {}", file.old_uri); + continue; + }; + let Ok(new_uri) = NormalizedUrl::parse(&file.new_uri) else { + _log!("failed to parse new uri: {}", file.new_uri); + continue; + }; let Some(entry) = self.files.borrow_mut().remove(&old_uri) else { + _log!("failed to find old uri: {}", file.old_uri); continue; }; self.files.borrow_mut().insert(new_uri, entry); diff --git a/crates/els/hir_visitor.rs b/crates/els/hir_visitor.rs index ddc9d166a..0a53de15d 100644 --- a/crates/els/hir_visitor.rs +++ b/crates/els/hir_visitor.rs @@ -36,10 +36,10 @@ impl<'a> HIRVisitor<'a> { .path() .split('/') .last() - .unwrap() + .unwrap_or_default() .split('.') .next() - .unwrap(); + .unwrap_or_default(); let namespace = vec![Str::rc(name)]; if let Some(ns) = self.get_exprs_ns(namespace.clone(), self.hir.module.iter(), pos) { ns diff --git a/crates/els/hover.rs b/crates/els/hover.rs index 7f3ece4ad..0a2b676e6 100644 --- a/crates/els/hover.rs +++ b/crates/els/hover.rs @@ -101,7 +101,9 @@ impl Server { match self.get_definition(&uri, &token)? { Some(vi) => { if let Some(line) = vi.def_loc.loc.ln_begin() { - let file_path = vi.def_loc.module.as_ref().unwrap(); + let Some(file_path) = vi.def_loc.module.as_ref() else { + return Ok(None); + }; let mut code_block = if cfg!(not(windows)) { let relative = file_path .strip_prefix(&self.home) @@ -112,15 +114,20 @@ impl Server { } else { // windows' file paths are case-insensitive, so we need to normalize them let lower = file_path.as_os_str().to_ascii_lowercase(); - let verbatim_removed = lower.to_str().unwrap().replace("\\\\?\\", ""); + let verbatim_removed = + lower.to_str().unwrap_or_default().replace("\\\\?\\", ""); let relative = verbatim_removed .strip_prefix( - self.home.as_os_str().to_ascii_lowercase().to_str().unwrap(), + self.home + .as_os_str() + .to_ascii_lowercase() + .to_str() + .unwrap_or_default(), ) - .unwrap_or_else(|| file_path.as_path().to_str().unwrap()) + .unwrap_or_else(|| file_path.as_path().to_str().unwrap_or_default()) .trim_start_matches(['\\', '/']); let relative = relative - .strip_prefix(self.erg_path.to_str().unwrap()) + .strip_prefix(self.erg_path.to_str().unwrap_or_default()) .unwrap_or(relative); format!("# {relative}, line {line}\n") }; @@ -180,10 +187,14 @@ impl Server { let mut defs = "".to_string(); for inner_t in vi.t.inner_ts() { if let Some(path) = &vi.def_loc.module { - let def_uri = util::NormalizedUrl::try_from(path.as_path()).unwrap(); - let module = { - self.quick_check_file(def_uri.clone()).unwrap(); - self.modules.get(&def_uri).unwrap() + let Ok(def_uri) = util::NormalizedUrl::try_from(path.as_path()) else { + continue; + }; + let Some(module) = ({ + self.quick_check_file(def_uri.clone())?; + self.modules.get(&def_uri) + }) else { + continue; }; if let Some((_, vi)) = module.context.get_type_info(&inner_t) { if let Some(uri) = diff --git a/crates/els/references.rs b/crates/els/references.rs index 5e3f5842a..fe12aa407 100644 --- a/crates/els/references.rs +++ b/crates/els/references.rs @@ -37,7 +37,9 @@ impl Server { if let (Some(path), Some(range)) = (&referrer.module, util::loc_to_range(referrer.loc)) { - let ref_uri = Url::from_file_path(path).unwrap(); + let Ok(ref_uri) = Url::from_file_path(path) else { + continue; + }; refs.push(lsp_types::Location::new(ref_uri, range)); } } diff --git a/crates/els/rename.rs b/crates/els/rename.rs index ce5e6ce1a..d606ebd26 100644 --- a/crates/els/rename.rs +++ b/crates/els/rename.rs @@ -108,8 +108,13 @@ impl Server { new_name: String, ) { if let Some(path) = &abs_loc.module { - let def_uri = Url::from_file_path(path).unwrap(); - let edit = TextEdit::new(util::loc_to_range(abs_loc.loc).unwrap(), new_name); + let Ok(def_uri) = Url::from_file_path(path) else { + return; + }; + let Some(range) = util::loc_to_range(abs_loc.loc) else { + return; + }; + let edit = TextEdit::new(range, new_name); if let Some(edits) = changes.get_mut(&def_uri) { edits.push(edit); } else { @@ -122,7 +127,7 @@ impl Server { urls.map(|url| { let timestamp = util::get_metadata_from_uri(url) .and_then(|md| Ok(md.modified()?)) - .unwrap(); + .unwrap_or(SystemTime::now()); (url.clone(), timestamp) }) .collect() @@ -218,7 +223,9 @@ impl Server { let Some(Expr::Call(import_call)) = def.body.block.first() else { return vec![]; }; - let module_name = import_call.args.get_left_or_key("Path").unwrap(); + let Some(module_name) = import_call.args.get_left_or_key("Path") else { + return vec![]; + }; match module_name { Expr::Lit(lit) if lit @@ -245,9 +252,15 @@ impl Server { new_uri: &NormalizedUrl, ) { if old_uri.as_str().ends_with(".d.er") { + let Ok(old_uri) = Url::parse(&old_uri.as_str().replace(".d.er", ".py")) else { + return; + }; + let Ok(new_uri) = Url::parse(&new_uri.as_str().replace(".d.er", ".py")) else { + return; + }; let rename = DocumentChangeOperation::Op(ResourceOp::Rename(RenameFile { - old_uri: Url::parse(&old_uri.as_str().replace(".d.er", ".py")).unwrap(), - new_uri: Url::parse(&new_uri.as_str().replace(".d.er", ".py")).unwrap(), + old_uri, + new_uri, options: None, annotation_id: None, })); @@ -255,9 +268,15 @@ impl Server { } else if old_uri.as_str().ends_with(".py") { let d_er_file = PathBuf::from(old_uri.as_str().replace(".py", ".d.er")); if d_er_file.exists() { + let Ok(old_uri) = Url::from_file_path(&d_er_file) else { + return; + }; + let Ok(new_uri) = Url::parse(&new_uri.as_str().replace(".py", ".d.er")) else { + return; + }; let rename = DocumentChangeOperation::Op(ResourceOp::Rename(RenameFile { - old_uri: Url::from_file_path(&d_er_file).unwrap(), - new_uri: Url::parse(&new_uri.as_str().replace(".py", ".d.er")).unwrap(), + old_uri, + new_uri, options: None, annotation_id: None, })); @@ -274,8 +293,14 @@ impl Server { let mut edits = HashMap::new(); let mut renames = vec![]; for file in ¶ms.files { - let old_uri = NormalizedUrl::new(Url::parse(&file.old_uri).unwrap()); - let new_uri = NormalizedUrl::new(Url::parse(&file.new_uri).unwrap()); + let Ok(old) = Url::parse(&file.old_uri) else { + continue; + }; + let old_uri = NormalizedUrl::new(old); + let Ok(new) = Url::parse(&file.new_uri) else { + continue; + }; + let new_uri = NormalizedUrl::new(new); edits.extend(self.collect_module_changes(&old_uri, &new_uri)); self.rename_linked_files(&mut renames, &old_uri, &new_uri); let Some(entry) = self.artifacts.remove(&old_uri) else { diff --git a/crates/els/sig_help.rs b/crates/els/sig_help.rs index af1fd38e8..5d5c21100 100644 --- a/crates/els/sig_help.rs +++ b/crates/els/sig_help.rs @@ -86,7 +86,7 @@ impl Server { args_loc: erg_common::error::Location, token: &Token, ) -> usize { - let tks = self.file_cache.get_token_stream(uri).unwrap(); + let tks = self.file_cache.get_token_stream(uri).unwrap_or_default(); // we should use the latest commas let commas = tks .iter() diff --git a/crates/erg_compiler/lint.rs b/crates/erg_compiler/lint.rs index 06488191f..db47423ee 100644 --- a/crates/erg_compiler/lint.rs +++ b/crates/erg_compiler/lint.rs @@ -8,6 +8,7 @@ use erg_common::traits::{Locational, Runnable, Stream}; use erg_common::Str; use erg_parser::ast::AST; use erg_parser::build_ast::ASTBuilder; +use erg_parser::lex::Lexer; use crate::context::ContextKind; use crate::link_ast::ASTLinker; @@ -171,7 +172,8 @@ impl ASTLowerer { for (referee, value) in self.module.context.index().iter() { let code = referee.code(); let name = code.as_ref().map(|s| &s[..]).unwrap_or(""); - let name_is_auto = name == "_"; // || name.starts_with(['%']); + let name_is_auto = + name == "_" || !Lexer::is_valid_start_symbol_ch(name.chars().next().unwrap_or(' ')); if value.referrers.is_empty() && value.vi.vis.is_private() && !name_is_auto { let input = referee .module diff --git a/crates/erg_parser/lex.rs b/crates/erg_parser/lex.rs index 05ca6a0c3..7c83397ae 100644 --- a/crates/erg_parser/lex.rs +++ b/crates/erg_parser/lex.rs @@ -235,11 +235,11 @@ impl Lexer /*<'a>*/ { Some(Err(LexError::feature_error(0, token.loc(), feat_name))) } - fn is_valid_start_symbol_ch(c: char) -> bool { - c == '_' || c.is_xid_start() + pub fn is_valid_start_symbol_ch(c: char) -> bool { + c == '_' || c == '\'' || c.is_xid_start() } - fn is_valid_continue_symbol_ch(c: char) -> bool { + pub fn is_valid_continue_symbol_ch(c: char) -> bool { c.is_xid_continue() && !('０'..='９').contains(&c) }