Skip to content
This repository has been archived by the owner on Oct 19, 2024. It is now read-only.

Commit

Permalink
refactor(solc): improve error message for unresolved imports
Browse files Browse the repository at this point in the history
  • Loading branch information
mattsse committed Aug 1, 2022
1 parent 140b90d commit a6679f3
Show file tree
Hide file tree
Showing 2 changed files with 57 additions and 16 deletions.
51 changes: 41 additions & 10 deletions ethers-solc/src/report/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -134,8 +134,9 @@ pub trait Reporter: 'static + std::fmt::Debug {
/// Invoked after a [`Solc`] installation failed
fn on_solc_installation_error(&self, _version: &Version, _error: &str) {}

/// Invoked if the import couldn't be resolved with these remappings
fn on_unresolved_import(&self, _import: &Path, _remappings: &[Remapping]) {}
/// Invoked if imports couldn't be resolved with the given remappings, where `imports` is the
/// list of all import paths and the file they occurred in: `(import stmt, file)`
fn on_unresolved_imports(&self, _imports: &[(&Path, &Path)], _remappings: &[Remapping]) {}

/// If `self` is the same type as the provided `TypeId`, returns an untyped
/// [`NonNull`] pointer to that type. Otherwise, returns `None`.
Expand Down Expand Up @@ -213,8 +214,8 @@ pub(crate) fn solc_installation_error(version: &Version, error: &str) {
get_default(|r| r.reporter.on_solc_installation_error(version, error));
}

pub(crate) fn unresolved_import(import: &Path, remappings: &[Remapping]) {
get_default(|r| r.reporter.on_unresolved_import(import, remappings));
pub(crate) fn unresolved_imports(imports: &[(&Path, &Path)], remappings: &[Remapping]) {
get_default(|r| r.reporter.on_unresolved_imports(imports, remappings));
}

fn get_global() -> Option<&'static Report> {
Expand Down Expand Up @@ -388,15 +389,28 @@ impl Reporter for BasicStdoutReporter {
eprintln!("Failed to install solc {}: {}", version, error);
}

fn on_unresolved_import(&self, import: &Path, remappings: &[Remapping]) {
println!(
"Unable to resolve import: \"{}\" with remappings:\n {}",
import.display(),
remappings.iter().map(|r| r.to_string()).collect::<Vec<_>>().join("\n ")
);
fn on_unresolved_imports(&self, imports: &[(&Path, &Path)], remappings: &[Remapping]) {
if imports.is_empty() {
return
}
println!("{}", format_unresolved_imports(imports, remappings))
}
}

/// Creates a meaningful message for all unresolved imports
pub fn format_unresolved_imports(imports: &[(&Path, &Path)], remappings: &[Remapping]) -> String {
let info = imports
.iter()
.map(|(import, file)| format!("\"{}\" in \"{}\"", import.display(), file.display()))
.collect::<Vec<_>>()
.join("\n ");
format!(
"Unable to resolve imports:\n {}\nwith remappings:\n {}",
info,
remappings.iter().map(|r| r.to_string()).collect::<Vec<_>>().join("\n ")
)
}

/// Returned if setting the global reporter fails.
#[derive(Debug)]
pub struct SetGlobalReporterError {
Expand Down Expand Up @@ -473,6 +487,7 @@ fn set_global_reporter(report: Report) -> Result<(), SetGlobalReporterError> {
#[cfg(test)]
mod tests {
use super::*;
use std::str::FromStr;

#[test]
fn scoped_reporter_works() {
Expand Down Expand Up @@ -502,4 +517,20 @@ mod tests {

get_default(|reporter| assert!(reporter.is::<BasicStdoutReporter>()))
}

#[test]
fn test_unresolved_message() {
let unresolved = vec![(Path::new("./src/Import.sol"), Path::new("src/File.col"))];

let remappings = vec![Remapping::from_str("oz=a/b/c/d").unwrap()];

assert_eq!(
format_unresolved_imports(&unresolved, &remappings).trim(),
r#"
Unable to resolve imports:
"./src/Import.sol" in "src/File.col"
with remappings:
oz=a/b/c/d/"#.trim()
)
}
}
22 changes: 16 additions & 6 deletions ethers-solc/src/resolver/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,8 @@ pub struct GraphEdges {
/// Combined with the `indices` this way we can determine if a file was original added to the
/// graph as input or was added as resolved import, see [`Self::is_input_file()`]
num_input_files: usize,
/// tracks all imports that we failed to resolve
unresolved_imports: HashSet<PathBuf>,
/// tracks all imports that we failed to resolve for a file
unresolved_imports: HashSet<(PathBuf, PathBuf)>,
}

impl GraphEdges {
Expand All @@ -114,7 +114,7 @@ impl GraphEdges {
}

/// Returns all imports that we failed to resolve
pub fn unresolved_imports(&self) -> &HashSet<PathBuf> {
pub fn unresolved_imports(&self) -> &HashSet<(PathBuf, PathBuf)> {
&self.unresolved_imports
}

Expand Down Expand Up @@ -354,9 +354,7 @@ impl Graph {
add_node(&mut unresolved, &mut index, &mut resolved_imports, import)?;
}
Err(err) => {
if unresolved_imports.insert(import_path.to_path_buf()) {
crate::report::unresolved_import(import_path, &paths.remappings);
}
unresolved_imports.insert((import_path.to_path_buf(), node.path.clone()));
tracing::trace!(
"failed to resolve import component \"{:?}\" for {:?}",
err,
Expand All @@ -369,6 +367,18 @@ impl Graph {
nodes.push(node);
edges.push(resolved_imports);
}

if !unresolved_imports.is_empty() {
// notify on all unresolved imports
crate::report::unresolved_imports(
&unresolved_imports
.iter()
.map(|(i, f)| (i.as_path(), f.as_path()))
.collect::<Vec<_>>(),
&paths.remappings,
);
}

let edges = GraphEdges {
edges,
rev_indices: index.iter().map(|(k, v)| (*v, k.clone())).collect(),
Expand Down

0 comments on commit a6679f3

Please sign in to comment.