diff --git a/protox/src/compile/mod.rs b/protox/src/compile/mod.rs index b1d6db7..ea029bd 100644 --- a/protox/src/compile/mod.rs +++ b/protox/src/compile/mod.rs @@ -1,5 +1,5 @@ use std::{ - collections::HashMap, + collections::{HashMap, HashSet}, fmt::{self, Write}, path::{Path, PathBuf}, }; @@ -148,11 +148,16 @@ impl Compiler { } let mut import_stack = vec![name.clone()]; + let mut already_imported = HashSet::new(); for (i, import) in file.descriptor.dependency.iter().enumerate() { + if !already_imported.insert(import) { + return Err(Error::from_kind(ErrorKind::DuplicatedThing { + name: import.to_owned(), + }).into_import_error(&file, i)); + } self.add_import(import, &mut import_stack) .map_err(|e| e.into_import_error(&file, i))?; } - drop(import_stack); let path = self.check_file(file)?; self.files.insert( @@ -270,7 +275,13 @@ impl Compiler { let file = self.resolver.open_file(file_name)?; import_stack.push(file_name.to_owned()); + let mut already_imported = HashSet::new(); for (i, import) in file.descriptor.dependency.iter().enumerate() { + if !already_imported.insert(import) { + return Err(Error::from_kind(ErrorKind::DuplicatedThing { + name: import.to_owned(), + }).into_import_error(&file, i)); + } self.add_import(import, import_stack) .map_err(|e| e.into_import_error(&file, i))?; } diff --git a/protox/src/error.rs b/protox/src/error.rs index 8a31736..64ba429 100644 --- a/protox/src/error.rs +++ b/protox/src/error.rs @@ -56,6 +56,17 @@ pub(crate) enum ErrorKind { path: PathBuf, shadow: PathBuf, }, + /// This variant is intermediate and should not be present in the final error. + #[error("'{name}' was listed twice")] + DuplicatedThing { name: String }, + #[error("import '{name}' was listed twice")] + DuplicateImport { + #[label("imported here")] + span: Option, + #[source_code] + source_code: NamedSource, + name: String, + }, #[error(transparent)] Custom(Box), } @@ -83,6 +94,7 @@ impl Error { match &*self.kind { ErrorKind::Parse { err } => Some(err.file()), ErrorKind::Check { err } => err.file(), + ErrorKind::DuplicatedThing { .. } => None, ErrorKind::OpenFile { name, .. } | ErrorKind::FileTooLarge { name } | ErrorKind::FileInvalidUtf8 { name } @@ -91,7 +103,8 @@ impl Error { | ErrorKind::FileShadowed { name, .. } => Some(name), ErrorKind::FileNotIncluded { .. } => None, ErrorKind::Custom(_) => None, - ErrorKind::ImportNotFound { source_code, .. } => Some(source_code.name()), + ErrorKind::ImportNotFound { source_code, .. } + | ErrorKind::DuplicateImport { source_code, .. } => Some(source_code.name()), } } @@ -157,17 +170,19 @@ impl Error { } None } + let source_code = + NamedSource::new(file.name(), file.source().unwrap_or_default().to_owned()); match *self.kind { - ErrorKind::FileNotFound { name } => { - let source_code: NamedSource = - NamedSource::new(file.name(), file.source().unwrap_or_default().to_owned()); - let span = find_span(file, import_idx); - Error::from_kind(ErrorKind::ImportNotFound { - span, - source_code, - name, - }) - } + ErrorKind::FileNotFound { name } => Error::from_kind(ErrorKind::ImportNotFound { + span: find_span(file, import_idx), + source_code, + name, + }), + ErrorKind::DuplicatedThing { name } => Error::from_kind(ErrorKind::DuplicateImport { + span: find_span(file, import_idx), + source_code, + name, + }), _ => self, } } @@ -202,9 +217,13 @@ impl fmt::Debug for Error { | ErrorKind::FileNotFound { .. } | ErrorKind::CircularImport { .. } | ErrorKind::FileNotIncluded { .. } + | ErrorKind::DuplicatedThing { .. } | ErrorKind::FileShadowed { .. } => write!(f, "{}", self), ErrorKind::Custom(err) => err.fmt(f), - ErrorKind::ImportNotFound { + ErrorKind::DuplicateImport { + span, source_code, .. + } + | ErrorKind::ImportNotFound { span, source_code, .. } => { write!(f, "{}:", source_code.name())?; diff --git a/protox/tests/compiler.rs b/protox/tests/compiler.rs index 3d19eb5..b06d4e5 100644 --- a/protox/tests/compiler.rs +++ b/protox/tests/compiler.rs @@ -44,10 +44,9 @@ fn check(files: &'static [(&'static str, &'static str)]) -> Result