Skip to content

Commit

Permalink
fix: errors in other modules are ignored
Browse files Browse the repository at this point in the history
  • Loading branch information
@mtshiba
mtshiba committed Jun 22, 2023
1 parent 7ba874a commit eaeb659
Show file tree
Hide file tree
Showing 13 changed files with 214 additions and 75 deletions.
41 changes: 29 additions & 12 deletions crates/els/diagnostics.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,21 +34,21 @@ impl<Checker: BuildRunnable, Parser: Parsable> Server<Checker, Parser> {
} else {
"exec"
};
let mut checker = self.get_checker(path);
match checker.build(code.into(), mode) {
let mut checker = self.get_checker(path.clone());
let artifact = match checker.build(code.into(), mode) {
Ok(artifact) => {
send_log(format!("checking {uri} passed"))?;
send_log(format!(
"checking {uri} passed, found warns: {}",
artifact.warns.len()
))?;
let uri_and_diags = self.make_uri_and_diags(uri.clone(), artifact.warns.clone());
// clear previous diagnostics
self.send_diagnostics(uri.clone().raw(), vec![])?;
for (uri, diags) in uri_and_diags.into_iter() {
send_log(format!("{uri}, warns: {}", diags.len()))?;
self.send_diagnostics(uri, diags)?;
}
if let Some(module) = self.get_ast(&uri) {
self.analysis_result
.insert(uri.clone(), AnalysisResult::new(module, artifact.into()));
}
artifact.into()
}
Err(artifact) => {
send_log(format!("found errors: {}", artifact.errors.len()))?;
Expand All @@ -67,12 +67,28 @@ impl<Checker: BuildRunnable, Parser: Parsable> Server<Checker, Parser> {
send_log(format!("{uri}, errs & warns: {}", diags.len()))?;
self.send_diagnostics(uri, diags)?;
}
if let Some(module) = self.get_ast(&uri) {
self.analysis_result
.insert(uri.clone(), AnalysisResult::new(module, artifact));
}
artifact
}
};
if let Some(shared) = self.get_shared() {
if mode == "declare" {
shared.py_mod_cache.register(
path,
artifact.object.clone(),
checker.get_context().unwrap().clone(),
);
} else {
shared.mod_cache.register(
path,
artifact.object.clone(),
checker.get_context().unwrap().clone(),
);
}
}
if let Some(module) = self.get_ast(&uri) {
self.analysis_result
.insert(uri.clone(), AnalysisResult::new(module, artifact));
}
if let Some(module) = checker.pop_context() {
send_log(format!("{uri}: {}", module.context.name))?;
self.modules.insert(uri.clone(), module);
Expand Down Expand Up @@ -118,11 +134,12 @@ impl<Checker: BuildRunnable, Parser: Parsable> Server<Checker, Parser> {
for err in errors.into_iter() {
let loc = err.core.get_loc_with_fallback();
let res_uri = if let Some(path) = err.input.path() {
Url::from_file_path(path)
Url::from_file_path(path.canonicalize().unwrap())
} else {
Ok(uri.clone().raw())
};
let Ok(err_uri) = res_uri else {
crate::_log!("failed to get uri: {}", err.input.path().unwrap().display());
continue;
};
let mut message = remove_style(&err.core.main_message);
Expand Down
4 changes: 2 additions & 2 deletions crates/els/server.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -314,6 +314,7 @@ pub struct Server<Checker: BuildRunnable = HIRBuilder, Parser: Parsable = Simple
pub(crate) opt_features: Vec<OptionalFeatures>,
pub(crate) file_cache: FileCache,
pub(crate) comp_cache: CompletionCache,
// TODO: remove modules, analysis_result, and add `shared: SharedCompilerResource`
pub(crate) modules: ModuleCache,
pub(crate) analysis_result: AnalysisResultCache,
pub(crate) current_sig: Option<Expr>,
Expand Down Expand Up @@ -723,8 +724,7 @@ impl<Checker: BuildRunnable, Parser: Parsable> Server<Checker, Parser> {
pub(crate) fn get_checker(&self, path: PathBuf) -> Checker {
if let Some(shared) = self.get_shared() {
let shared = shared.clone();
shared.mod_cache.remove(&path);
shared.py_mod_cache.remove(&path);
shared.clear(&path);
Checker::inherit(self.cfg.inherit(path), shared)
} else {
Checker::new(self.cfg.inherit(path))
Expand Down
6 changes: 6 additions & 0 deletions crates/erg_common/set.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,12 @@ impl<T: Hash + Eq> Set<T> {
self.elems.extend(other.elems);
self
}

/// remove all elements for which the predicate returns false
#[inline]
pub fn retain(&mut self, f: impl FnMut(&T) -> bool) {
self.elems.retain(f);
}
}

impl<T: Hash + Eq + Clone> Set<T> {
Expand Down
37 changes: 20 additions & 17 deletions crates/erg_compiler/context/register.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ use ast::{
};
use erg_parser::ast;

use crate::artifact::ErrorArtifact;
use crate::ty::constructors::{
free_var, func, func0, func1, proc, ref_, ref_mut, tp_enum, unknown_len_array_t, v_enum,
};
Expand Down Expand Up @@ -225,7 +224,7 @@ impl Context {
py_name,
self.absolutize(ident.name.loc()),
);
self.index().register(&vi);
self.index().register(ident.inspect().clone(), &vi);
self.future_defined_locals.insert(ident.name.clone(), vi);
Ok(())
}
Expand Down Expand Up @@ -271,7 +270,7 @@ impl Context {
py_name,
self.absolutize(sig.ident.name.loc()),
);
self.index().register(&vi);
self.index().register(sig.ident.inspect().clone(), &vi);
if self
.remove_class_attr(name)
.is_some_and(|(_, decl)| !decl.kind.is_auto())
Expand Down Expand Up @@ -505,7 +504,7 @@ impl Context {
None,
self.absolutize(name.loc()),
);
self.index().register(&vi);
self.index().register(name.inspect().clone(), &vi);
sig.vi = vi.clone();
self.params.push((Some(name.clone()), vi));
if errs.is_empty() {
Expand Down Expand Up @@ -1272,7 +1271,7 @@ impl Context {
None,
self.absolutize(ident.name.loc()),
);
self.index().register(&vi);
self.index().register(ident.inspect().clone(), &vi);
self.decls.insert(ident.name.clone(), vi);
self.consts.insert(ident.name.clone(), other);
Ok(())
Expand Down Expand Up @@ -1619,7 +1618,7 @@ impl Context {
None,
self.absolutize(name.loc()),
);
self.index().register(&vi);
self.index().register(name.inspect().clone(), &vi);
self.decls.insert(name.clone(), vi);
self.consts.insert(name.clone(), val);
Ok(())
Expand Down Expand Up @@ -1667,7 +1666,7 @@ impl Context {
None,
self.absolutize(name.loc()),
);
self.index().register(&vi);
self.index().register(name.inspect().clone(), &vi);
self.decls.insert(name.clone(), vi);
self.consts.insert(name.clone(), val);
self.register_methods(&t, &ctx);
Expand Down Expand Up @@ -1727,7 +1726,7 @@ impl Context {
None,
self.absolutize(name.loc()),
);
self.index().register(&vi);
self.index().register(name.inspect().clone(), &vi);
self.decls.insert(name.clone(), vi);
self.consts.insert(name.clone(), val);
self.register_methods(&t, &ctx);
Expand Down Expand Up @@ -1940,18 +1939,16 @@ impl Context {
Some(artifact.object),
builder.pop_mod_ctx().unwrap(),
);
// shared.warns.extend(artifact.warns);
Ok(())
shared.warns.extend(artifact.warns);
}
Err(artifact) => {
if let Some(hir) = artifact.object {
shared
.mod_cache
.register(_path, Some(hir), builder.pop_mod_ctx().unwrap());
}
// shared.warns.extend(artifact.warns);
// shared.errors.extend(artifact.errors);
Err(ErrorArtifact::new(artifact.errors, artifact.warns))
shared.warns.extend(artifact.warns);
shared.errors.extend(artifact.errors);
}
}
};
Expand Down Expand Up @@ -2233,9 +2230,15 @@ impl Context {
Ok(())
}

pub(crate) fn inc_ref<L: Locational>(&self, vi: &VarInfo, name: &L, namespace: &Context) {
pub(crate) fn inc_ref<L: Locational>(
&self,
name: &Str,
vi: &VarInfo,
loc: &L,
namespace: &Context,
) {
if let Some(index) = self.opt_index() {
index.inc_ref(vi, namespace.absolutize(name.loc()));
index.inc_ref(name, vi, namespace.absolutize(loc.loc()));
}
}

Expand Down Expand Up @@ -2288,7 +2291,7 @@ impl Context {
&self.cfg.input,
self,
) {
self.inc_ref(&vi, &ident.name, namespace);
self.inc_ref(ident.inspect(), &vi, &ident.name, namespace);
true
} else {
false
Expand All @@ -2307,7 +2310,7 @@ impl Context {
&self.cfg.input,
self,
) {
self.inc_ref(&vi, &local.name, namespace);
self.inc_ref(local.inspect(), &vi, &local.name, namespace);
true
} else {
&local.inspect()[..] == "module" || &local.inspect()[..] == "global"
Expand Down
20 changes: 12 additions & 8 deletions crates/erg_compiler/lint.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -161,19 +161,23 @@ impl ASTLowerer {
}
}

pub(crate) fn inc_ref<L: Locational>(&self, vi: &VarInfo, name: &L) {
self.module.context.inc_ref(vi, name, &self.module.context);
pub(crate) fn inc_ref<L: Locational>(&self, name: &Str, vi: &VarInfo, loc: &L) {
self.module
.context
.inc_ref(name, vi, loc, &self.module.context);
}

pub(crate) fn warn_unused_vars(&mut self, mode: &str) {
pub(crate) fn warn_unused_local_vars(&mut self, mode: &str) {
if mode == "eval" {
return;
}
let self_path = self.module.context.module_path();
for (referee, value) in self.module.context.index().members().iter() {
let code = referee.code();
let name = code.as_ref().map(|s| &s[..]).unwrap_or("");
let name_is_auto =
name == "_" || !Lexer::is_valid_start_symbol_ch(name.chars().next().unwrap_or(' '));
if referee.module.as_deref() != self_path {
continue;
}
let name_is_auto = &value.name[..] == "_"
|| !Lexer::is_valid_start_symbol_ch(value.name.chars().next().unwrap_or(' '));
if value.referrers.is_empty() && value.vi.vis.is_private() && !name_is_auto {
let input = referee
.module
Expand All @@ -183,7 +187,7 @@ impl ASTLowerer {
input,
line!() as usize,
referee.loc,
name,
&value.name,
self.module.context.caused_by(),
);
self.warns.push(warn);
Expand Down
37 changes: 28 additions & 9 deletions crates/erg_compiler/lower.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
use std::mem;

use erg_common::config::{ErgConfig, ErgMode};
use erg_common::consts::{ERG_MODE, PYTHON_MODE};
use erg_common::consts::{ELS, ERG_MODE, PYTHON_MODE};
use erg_common::dict;
use erg_common::dict::Dict;
use erg_common::error::{Location, MultiErrorDisplay};
Expand Down Expand Up @@ -659,7 +659,7 @@ impl ASTLowerer {
VarInfo::ILLEGAL
}
};
self.inc_ref(&vi, &attr.ident.name);
self.inc_ref(attr.ident.inspect(), &vi, &attr.ident.name);
let ident = hir::Identifier::new(attr.ident, None, vi);
let acc = hir::Accessor::Attr(hir::Attribute::new(obj, ident));
Ok(acc)
Expand Down Expand Up @@ -728,7 +728,7 @@ impl ASTLowerer {
.map(|ctx| ctx.first().unwrap().name.clone()),
)
};
self.inc_ref(&vi, &ident.name);
self.inc_ref(ident.inspect(), &vi, &ident.name);
let ident = hir::Identifier::new(ident, __name__, vi);
Ok(ident)
}
Expand Down Expand Up @@ -998,7 +998,7 @@ impl ASTLowerer {
}
}
let attr_name = if let Some(attr_name) = call.attr_name {
self.inc_ref(&vi, &attr_name.name);
self.inc_ref(attr_name.inspect(), &vi, &attr_name.name);
Some(hir::Identifier::new(attr_name, None, vi))
} else {
if let hir::Expr::Call(call) = &obj {
Expand Down Expand Up @@ -1251,13 +1251,21 @@ impl ASTLowerer {
// suppress warns of lambda types, e.g. `(x: Int, y: Int) -> Int`
if self.module.context.subtype_of(body.ref_t(), &Type::Type) {
for param in params.non_defaults.iter() {
self.inc_ref(&param.vi, param);
self.inc_ref(param.inspect().unwrap_or(&"_".into()), &param.vi, param);
}
if let Some(var_param) = params.var_params.as_deref() {
self.inc_ref(&var_param.vi, var_param);
self.inc_ref(
var_param.inspect().unwrap_or(&"_".into()),
&var_param.vi,
var_param,
);
}
for default in params.defaults.iter() {
self.inc_ref(&default.sig.vi, &default.sig);
self.inc_ref(
default.sig.inspect().unwrap_or(&"_".into()),
&default.sig.vi,
&default.sig,
);
}
}
let (non_default_params, default_params): (Vec<_>, Vec<_>) = self
Expand Down Expand Up @@ -2456,7 +2464,7 @@ impl ASTLowerer {
log!(info "the AST lowering process has started.");
log!(info "the type-checking process has started.");
if let Some(path) = self.cfg.input.path() {
let graph = &self.module.context.shared.as_ref().unwrap().graph;
let graph = &self.module.context.shared().graph;
graph.add_node_if_none(path);
}
let ast = ASTLinker::new(self.cfg.clone())
Expand Down Expand Up @@ -2511,8 +2519,19 @@ impl ASTLowerer {
};
self.warn_implicit_union(&hir);
self.warn_unused_expr(&hir.module, mode);
self.warn_unused_vars(mode);
self.check_doc_comments(&hir);
if &self.module.context.name[..] == "<module>" || ELS {
self.warn_unused_local_vars(mode);
if ELS {
self.module.context.shared().promises.join_children();
} else {
self.module.context.shared().promises.join_all();
}
let errs = self.module.context.shared().errors.take();
let warns = self.module.context.shared().warns.take();
self.errs.extend(errs);
self.warns.extend(warns);
}
if self.errs.is_empty() {
log!(info "the AST lowering process has completed.");
Ok(CompleteArtifact::new(
Expand Down
6 changes: 6 additions & 0 deletions crates/erg_compiler/module/errors.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,10 @@ impl SharedCompileErrors {
pub fn take(&self) -> CompileErrors {
self.0.borrow_mut().take_all().into()
}

pub fn clear(&self) {
self.0.borrow_mut().clear();
}
}

pub type SharedCompileWarnings = SharedCompileErrors;
Loading

0 comments on commit eaeb659

Please sign in to comment.