From 030488eca5d4fe4b5afaa71abdabff16013cea7a Mon Sep 17 00:00:00 2001 From: Matthias Seitz Date: Thu, 2 Jun 2022 19:31:02 +0200 Subject: [PATCH] fix(solc): invalidate cache on unresolve error (#1337) --- ethers-solc/src/cache.rs | 19 +++++++++++--- ethers-solc/src/resolver/mod.rs | 13 ++++++++-- ethers-solc/tests/project.rs | 45 ++++++++++++++++++++++++++++++++- 3 files changed, 70 insertions(+), 7 deletions(-) diff --git a/ethers-solc/src/cache.rs b/ethers-solc/src/cache.rs index 087c38de..4e837bda 100644 --- a/ethers-solc/src/cache.rs +++ b/ethers-solc/src/cache.rs @@ -781,12 +781,17 @@ pub(crate) enum ArtifactsCache<'a, T: ArtifactOutput> { impl<'a, T: ArtifactOutput> ArtifactsCache<'a, T> { pub fn new(project: &'a Project, edges: GraphEdges) -> Result { - /// returns the [SolFilesCache] to use - fn get_cache(project: &Project) -> SolFilesCache { + /// Returns the [SolFilesCache] to use + /// + /// Returns a new empty cache if the cache does not exist or `invalidate_cache` is set. + fn get_cache( + project: &Project, + invalidate_cache: bool, + ) -> SolFilesCache { // the currently configured paths let paths = project.paths.paths_relative(); - if project.cache_path().exists() { + if !invalidate_cache && project.cache_path().exists() { if let Ok(cache) = SolFilesCache::read_joined(&project.paths) { if cache.paths == paths { // unchanged project paths @@ -794,13 +799,19 @@ impl<'a, T: ArtifactOutput> ArtifactsCache<'a, T> { } } } + // new empty cache SolFilesCache::new(Default::default(), paths) } let cache = if project.cached { + // we only read the existing cache if we were able to resolve the entire graph + // if we failed to resolve an import we invalidate the cache so don't get any false + // positives + let invalidate_cache = !edges.unresolved_imports().is_empty(); + // read the cache file if it already exists - let mut cache = get_cache(project); + let mut cache = get_cache(project, invalidate_cache); cache.remove_missing_files(); diff --git a/ethers-solc/src/resolver/mod.rs b/ethers-solc/src/resolver/mod.rs index 15fb77fb..571e3602 100644 --- a/ethers-solc/src/resolver/mod.rs +++ b/ethers-solc/src/resolver/mod.rs @@ -88,6 +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, } impl GraphEdges { @@ -111,6 +113,11 @@ impl GraphEdges { self.files().skip(self.num_input_files) } + /// Returns all imports that we failed to resolve + pub fn unresolved_imports(&self) -> &HashSet { + &self.unresolved_imports + } + /// Returns a list of nodes the given node index points to for the given kind. pub fn imported_nodes(&self, from: usize) -> &[usize] { &self.edges[from] @@ -277,6 +284,7 @@ impl Graph { self.nodes.iter().take(self.edges.num_input_files) } + /// Returns all files imported by the given file pub fn imports(&self, path: impl AsRef) -> HashSet<&PathBuf> { self.edges.imports(path) } @@ -327,7 +335,7 @@ impl Graph { // keep track of all unique paths that we failed to resolve to not spam the reporter with // the same path - let mut unresolved_paths = HashSet::new(); + let mut unresolved_imports = HashSet::new(); // now we need to resolve all imports for the source file and those imported from other // locations @@ -346,7 +354,7 @@ impl Graph { add_node(&mut unresolved, &mut index, &mut resolved_imports, import)?; } Err(err) => { - if unresolved_paths.insert(import_path.to_path_buf()) { + if unresolved_imports.insert(import_path.to_path_buf()) { crate::report::unresolved_import(import_path, &paths.remappings); } tracing::trace!( @@ -372,6 +380,7 @@ impl Graph { .map(|(idx, node)| (idx, node.data.version_req.clone())) .collect(), data: Default::default(), + unresolved_imports, }; Ok(Graph { nodes, edges, root: paths.root.clone() }) } diff --git a/ethers-solc/tests/project.rs b/ethers-solc/tests/project.rs index 5aaad5aa..8d4b1b30 100644 --- a/ethers-solc/tests/project.rs +++ b/ethers-solc/tests/project.rs @@ -2,7 +2,7 @@ use std::{ collections::{BTreeMap, HashMap, HashSet}, - io, + fs, io, path::{Path, PathBuf}, str::FromStr, }; @@ -1768,3 +1768,46 @@ contract D { } cache.files.keys().cloned().collect::>() ); } + +#[test] +fn test_failure_after_removing_file() { + let project = TempProject::dapptools().unwrap(); + project + .add_source( + "A", + r#" +pragma solidity ^0.8.10; +import "./B.sol"; +contract A { } +"#, + ) + .unwrap(); + + project + .add_source( + "B", + r#" +pragma solidity ^0.8.10; +import "./C.sol"; +contract B { } +"#, + ) + .unwrap(); + + let c = project + .add_source( + "C", + r#" +pragma solidity ^0.8.10; +contract C { } +"#, + ) + .unwrap(); + + let compiled = project.compile().unwrap(); + assert!(!compiled.has_compiler_errors()); + + fs::remove_file(c).unwrap(); + let compiled = project.compile().unwrap(); + assert!(compiled.has_compiler_errors()); +}