Skip to content

Commit

Permalink
fix(els): eliminate unwraps
Browse files Browse the repository at this point in the history
  • Loading branch information
mtshiba committed May 4, 2023
1 parent 078f80e commit 2fce9bd
Show file tree
Hide file tree
Showing 10 changed files with 98 additions and 39 deletions.
18 changes: 12 additions & 6 deletions crates/els/completion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -380,7 +384,9 @@ impl<Checker: BuildRunnable> Server<Checker> {
.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)
Expand Down Expand Up @@ -444,9 +450,9 @@ impl<Checker: BuildRunnable> Server<Checker> {
}
// 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;
}
Expand Down Expand Up @@ -494,7 +500,7 @@ impl<Checker: BuildRunnable> Server<Checker> {
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::<AbsLocation>() else {
let Ok(def_loc) = data.as_str().unwrap_or_default().parse::<AbsLocation>() else {
return Ok(item);
};
self.show_doc_comment(None, &mut contents, &def_loc)?;
Expand Down
9 changes: 6 additions & 3 deletions crates/els/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,10 +115,13 @@ impl<Checker: BuildRunnable> Server<Checker> {
let mut uri_and_diags: Vec<(Url, Vec<Diagnostic>)> = 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 {
Expand Down
16 changes: 13 additions & 3 deletions crates/els/file_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down Expand Up @@ -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);
Expand All @@ -276,9 +279,16 @@ impl FileCache {

pub fn rename_files(&mut self, params: &RenameFilesParams) -> ELSResult<()> {
for file in &params.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);
Expand Down
4 changes: 2 additions & 2 deletions crates/els/hir_visitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
29 changes: 20 additions & 9 deletions crates/els/hover.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,9 @@ impl<Checker: BuildRunnable> Server<Checker> {
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)
Expand All @@ -112,15 +114,20 @@ impl<Checker: BuildRunnable> Server<Checker> {
} 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")
};
Expand Down Expand Up @@ -180,10 +187,14 @@ impl<Checker: BuildRunnable> Server<Checker> {
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) =
Expand Down
4 changes: 3 additions & 1 deletion crates/els/references.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,9 @@ impl<Checker: BuildRunnable> Server<Checker> {
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));
}
}
Expand Down
45 changes: 35 additions & 10 deletions crates/els/rename.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,8 +108,13 @@ impl<Checker: BuildRunnable> Server<Checker> {
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 {
Expand All @@ -122,7 +127,7 @@ impl<Checker: BuildRunnable> Server<Checker> {
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()
Expand Down Expand Up @@ -218,7 +223,9 @@ impl<Checker: BuildRunnable> Server<Checker> {
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
Expand All @@ -245,19 +252,31 @@ impl<Checker: BuildRunnable> Server<Checker> {
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,
}));
renames.push(rename);
} 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,
}));
Expand All @@ -274,8 +293,14 @@ impl<Checker: BuildRunnable> Server<Checker> {
let mut edits = HashMap::new();
let mut renames = vec![];
for file in &params.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 {
Expand Down
2 changes: 1 addition & 1 deletion crates/els/sig_help.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ impl<Checker: BuildRunnable> Server<Checker> {
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()
Expand Down
4 changes: 3 additions & 1 deletion crates/erg_compiler/lint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions crates/erg_parser/lex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() && !('0'..='9').contains(&c)
}

Expand Down

0 comments on commit 2fce9bd

Please sign in to comment.