Skip to content

Commit

Permalink
chore: Refactor DefCollector duplicate errors (#2231)
Browse files Browse the repository at this point in the history 
* chore: Refactor DefCollector duplicate errors

* Better message

* Convert duplicate type to enum

---------

Co-authored-by: Yordan Madzhunkov <ymadzhunkov@gmail.com>
  • Loading branch information
@yordanmadzhunkov @ymadzhunkov
yordanmadzhunkov and ymadzhunkov authored Aug 10, 2023
1 parent 650937a commit 1e6da05
Show file tree
Hide file tree
Showing 3 changed files with 76 additions and 68 deletions.
15 changes: 11 additions & 4 deletions crates/noirc_frontend/src/hir/def_collector/dc_crate.rs
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use super::dc_mod::collect_defs;
use super::errors::DefCollectorErrorKind;
use super::errors::{DefCollectorErrorKind, DuplicateType};
use crate::graph::CrateId;
use crate::hir::def_map::{CrateDefMap, LocalModuleId, ModuleId};
use crate::hir::resolution::errors::ResolverError;
Expand Down Expand Up @@ -151,7 +151,11 @@ impl DefCollector {
.import(name.clone(), ns);

if let Err((first_def, second_def)) = result {
let err = DefCollectorErrorKind::DuplicateImport { first_def, second_def };
let err = DefCollectorErrorKind::Duplicate {
typ: DuplicateType::Import,
first_def,
second_def,
};
errors.push(err.into_file_diagnostic(root_file_id));
}
}
Expand Down Expand Up @@ -256,8 +260,11 @@ fn collect_impls(
let result = module.declare_function(method.name_ident().clone(), *method_id);

if let Err((first_def, second_def)) = result {
let err =
DefCollectorErrorKind::DuplicateFunction { first_def, second_def };
let err = DefCollectorErrorKind::Duplicate {
typ: DuplicateType::Function,
first_def,
second_def,
};
errors.push(err.into_file_diagnostic(unresolved.file_id));
}
}
Expand Down
32 changes: 26 additions & 6 deletions crates/noirc_frontend/src/hir/def_collector/dc_mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use crate::{

use super::{
dc_crate::{DefCollector, UnresolvedFunctions, UnresolvedGlobal, UnresolvedTypeAlias},
errors::DefCollectorErrorKind,
errors::{DefCollectorErrorKind, DuplicateType},
};
use crate::hir::def_map::{parse_file, LocalModuleId, ModuleData, ModuleId, ModuleOrigin};
use crate::hir::resolution::import::ImportDirective;
Expand Down Expand Up @@ -82,7 +82,11 @@ impl<'a> ModCollector<'a> {
self.def_collector.def_map.modules[self.module_id.0].declare_global(name, stmt_id);

if let Err((first_def, second_def)) = result {
let err = DefCollectorErrorKind::DuplicateGlobal { first_def, second_def };
let err = DefCollectorErrorKind::Duplicate {
typ: DuplicateType::Global,
first_def,
second_def,
};
errors.push(err.into_file_diagnostic(self.file_id));
}

Expand Down Expand Up @@ -142,7 +146,11 @@ impl<'a> ModCollector<'a> {
.declare_function(name, func_id);

if let Err((first_def, second_def)) = result {
let error = DefCollectorErrorKind::DuplicateFunction { first_def, second_def };
let error = DefCollectorErrorKind::Duplicate {
typ: DuplicateType::Function,
first_def,
second_def,
};
errors.push(error.into_file_diagnostic(self.file_id));
}
}
Expand Down Expand Up @@ -172,7 +180,11 @@ impl<'a> ModCollector<'a> {
self.def_collector.def_map.modules[self.module_id.0].declare_struct(name, id);

if let Err((first_def, second_def)) = result {
let err = DefCollectorErrorKind::DuplicateFunction { first_def, second_def };
let err = DefCollectorErrorKind::Duplicate {
typ: DuplicateType::TypeDefinition,
first_def,
second_def,
};
errors.push(err.into_file_diagnostic(self.file_id));
}

Expand Down Expand Up @@ -211,7 +223,11 @@ impl<'a> ModCollector<'a> {
.declare_type_alias(name, type_alias_id);

if let Err((first_def, second_def)) = result {
let err = DefCollectorErrorKind::DuplicateFunction { first_def, second_def };
let err = DefCollectorErrorKind::Duplicate {
typ: DuplicateType::Function,
first_def,
second_def,
};
errors.push(err.into_file_diagnostic(self.file_id));
}

Expand Down Expand Up @@ -323,7 +339,11 @@ impl<'a> ModCollector<'a> {
if let Err((first_def, second_def)) =
modules[self.module_id.0].declare_child_module(mod_name.to_owned(), mod_id)
{
let err = DefCollectorErrorKind::DuplicateModuleDecl { first_def, second_def };
let err = DefCollectorErrorKind::Duplicate {
typ: DuplicateType::Module,
first_def,
second_def,
};
errors.push(err.into_file_diagnostic(self.file_id));
return None;
}
Expand Down
97 changes: 39 additions & 58 deletions crates/noirc_frontend/src/hir/def_collector/errors.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,20 @@ use noirc_errors::FileDiagnostic;
use noirc_errors::Span;
use thiserror::Error;

use std::fmt;

pub enum DuplicateType {
Function,
Module,
Global,
TypeDefinition,
Import,
}

#[derive(Error, Debug)]
pub enum DefCollectorErrorKind {
#[error("duplicate function found in namespace")]
DuplicateFunction { first_def: Ident, second_def: Ident },
#[error("duplicate function found in namespace")]
DuplicateModuleDecl { first_def: Ident, second_def: Ident },
#[error("duplicate import")]
DuplicateImport { first_def: Ident, second_def: Ident },
#[error("duplicate global found in namespace")]
DuplicateGlobal { first_def: Ident, second_def: Ident },
#[error("duplicate {typ:?} found in namespace")]
Duplicate { typ: DuplicateType, first_def: Ident, second_def: Ident },
#[error("unresolved import")]
UnresolvedModuleDecl { mod_name: Ident },
#[error("path resolution error")]
Expand All @@ -32,60 +36,37 @@ impl DefCollectorErrorKind {
}
}

impl fmt::Debug for DuplicateType {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
match *self {
DuplicateType::Function => write!(f, "function"),
DuplicateType::Module => write!(f, "module"),
DuplicateType::Global => write!(f, "global"),
DuplicateType::TypeDefinition => write!(f, "type definition"),
DuplicateType::Import => write!(f, "import"),
}
}
}

impl From<DefCollectorErrorKind> for Diagnostic {
fn from(error: DefCollectorErrorKind) -> Diagnostic {
match error {
DefCollectorErrorKind::DuplicateFunction { first_def, second_def } => {
let first_span = first_def.0.span();
let second_span = second_def.0.span();
let func_name = &first_def.0.contents;

let mut diag = Diagnostic::simple_error(
format!("duplicate definitions of {func_name} function found"),
"first definition found here".to_string(),
first_span,
);
diag.add_secondary("second definition found here".to_string(), second_span);
diag
}
DefCollectorErrorKind::DuplicateModuleDecl { first_def, second_def } => {
let first_span = first_def.0.span();
let second_span = second_def.0.span();
let mod_name = &first_def.0.contents;

let mut diag = Diagnostic::simple_error(
format!("module {mod_name} has been declared twice"),
"first declaration found here".to_string(),
first_span,
);
diag.add_secondary("second declaration found here".to_string(), second_span);
diag
}
DefCollectorErrorKind::DuplicateImport { first_def, second_def } => {
let first_span = first_def.0.span();
let second_span = second_def.0.span();
let import_name = &first_def.0.contents;

let mut diag = Diagnostic::simple_error(
format!("the name `{import_name}` is defined multiple times"),
"first import found here".to_string(),
first_span,
);
diag.add_secondary("second import found here".to_string(), second_span);
diag
}
DefCollectorErrorKind::DuplicateGlobal { first_def, second_def } => {
let first_span = first_def.0.span();
let second_span = second_def.0.span();
let import_name = &first_def.0.contents;

let mut diag = Diagnostic::simple_error(
format!("the name `{import_name}` is defined multiple times"),
"first global declaration found here".to_string(),
first_span,
DefCollectorErrorKind::Duplicate { typ, first_def, second_def } => {
let primary_message = format!(
"duplicate definitions of {:?} with name {} found",
&typ, &first_def.0.contents
);
diag.add_secondary("second global declaration found here".to_string(), second_span);
diag
{
let first_span = first_def.0.span();
let second_span = second_def.0.span();
let mut diag = Diagnostic::simple_error(
primary_message,
format!("first {:?} found here", &typ),
first_span,
);
diag.add_secondary(format!("second {:?} found here", &typ), second_span);
diag
}
}
DefCollectorErrorKind::UnresolvedModuleDecl { mod_name } => {
let span = mod_name.0.span();
Expand Down

0 comments on commit 1e6da05

Please sign in to comment.