From 5314c4e618afa6daad682adec0677ec03fd0835e Mon Sep 17 00:00:00 2001 From: Matthias Seitz Date: Thu, 17 Mar 2022 09:27:03 +0100 Subject: [PATCH] fix(solc): use lowercase when comparing paths (#1041) * fix(solc): use lowercase when comparing paths * trace keys * test: add lowercase contract test --- ethers-solc/src/artifacts/mod.rs | 44 ++++++++++++++++++--- ethers-solc/src/compile/project.rs | 3 +- ethers-solc/src/project_util/mod.rs | 2 +- ethers-solc/tests/project.rs | 61 +++++++++++++++++++++++++++++ 4 files changed, 103 insertions(+), 7 deletions(-) diff --git a/ethers-solc/src/artifacts/mod.rs b/ethers-solc/src/artifacts/mod.rs index ba745336..ef9d230d 100644 --- a/ethers-solc/src/artifacts/mod.rs +++ b/ethers-solc/src/artifacts/mod.rs @@ -741,12 +741,16 @@ impl CompilerOutput { where I: IntoIterator, { - let files: HashSet<_> = files.into_iter().collect(); - - self.contracts.retain(|f, _| files.contains(f.as_str())); - self.sources.retain(|f, _| files.contains(f.as_str())); + // Note: use `to_lowercase` here because solc not necessarily emits the exact file name, + // e.g. `src/utils/upgradeProxy.sol` is emitted as `src/utils/UpgradeProxy.sol` + let files: HashSet<_> = files.into_iter().map(|s| s.to_lowercase()).collect(); + self.contracts.retain(|f, _| files.contains(f.to_lowercase().as_str())); + self.sources.retain(|f, _| files.contains(f.to_lowercase().as_str())); self.errors.retain(|err| { - err.source_location.as_ref().map(|s| files.contains(s.file.as_str())).unwrap_or(true) + err.source_location + .as_ref() + .map(|s| files.contains(s.file.to_lowercase().as_str())) + .unwrap_or(true) }); } @@ -2050,8 +2054,38 @@ impl SourceFiles { #[cfg(test)] mod tests { use super::*; + use crate::AggregatedCompilerOutput; use std::{fs, path::PathBuf}; + #[test] + fn can_parse_declaration_error() { + let s = r#"{ + "errors": [ + { + "component": "general", + "errorCode": "7576", + "formattedMessage": "DeclarationError: Undeclared identifier. Did you mean \"revert\"?\n --> /Users/src/utils/UpgradeProxy.sol:35:17:\n |\n35 | refert(\"Transparent ERC1967 proxies do not have upgradeable implementations\");\n | ^^^^^^\n\n", + "message": "Undeclared identifier. Did you mean \"revert\"?", + "severity": "error", + "sourceLocation": { + "end": 1623, + "file": "/Users/src/utils/UpgradeProxy.sol", + "start": 1617 + }, + "type": "DeclarationError" + } + ], + "sources": { } +}"#; + + let out: CompilerOutput = serde_json::from_str(s).unwrap(); + assert_eq!(out.errors.len(), 1); + + let mut aggregated = AggregatedCompilerOutput::default(); + aggregated.extend("0.8.12".parse().unwrap(), out); + assert!(!aggregated.is_unchanged()); + } + #[test] fn can_link_bytecode() { // test cases taken from https://github.com/ethereum/solc-js/blob/master/test/linker.js diff --git a/ethers-solc/src/compile/project.rs b/ethers-solc/src/compile/project.rs index 5c68132c..e3df3a3d 100644 --- a/ethers-solc/src/compile/project.rs +++ b/ethers-solc/src/compile/project.rs @@ -331,7 +331,7 @@ impl CompilerSources { tracing::trace!( "Detected {} dirty sources {:?}", sources.dirty().count(), - sources.dirty_files() + sources.dirty_files().collect::>() ); (solc, (version, sources)) }) @@ -454,6 +454,7 @@ fn compile_sequential( let output = solc.compile_exact(&input)?; report::solc_success(&solc, &version, &output); tracing::trace!("compiled input, output has error: {}", output.has_error()); + tracing::trace!("received compiler output: {:?}", output.contracts.keys()); aggregated.extend(version.clone(), output); } } diff --git a/ethers-solc/src/project_util/mod.rs b/ethers-solc/src/project_util/mod.rs index 04ce0651..a35a41aa 100644 --- a/ethers-solc/src/project_util/mod.rs +++ b/ethers-solc/src/project_util/mod.rs @@ -342,7 +342,7 @@ pub(crate) fn create_contract_file(path: PathBuf, content: impl AsRef) -> R } fn contract_file_name(name: impl AsRef) -> String { - let name = name.as_ref(); + let name = name.as_ref().trim(); if name.ends_with(".sol") { name.to_string() } else { diff --git a/ethers-solc/tests/project.rs b/ethers-solc/tests/project.rs index e2fd432f..8871fe98 100644 --- a/ethers-solc/tests/project.rs +++ b/ethers-solc/tests/project.rs @@ -684,6 +684,67 @@ fn can_recompile_with_changes() { assert!(compiled.find("B").is_some()); } +#[test] +fn can_recompile_with_lowercase_names() { + init_tracing(); + let tmp = TempProject::dapptools().unwrap(); + + tmp.add_source( + "deployProxy.sol", + r#" + pragma solidity =0.8.12; + contract DeployProxy {} + "#, + ) + .unwrap(); + + let upgrade = r#" + pragma solidity =0.8.12; + import "./deployProxy.sol"; + import "./ProxyAdmin.sol"; + contract UpgradeProxy {} + "#; + tmp.add_source("upgradeProxy.sol", upgrade).unwrap(); + + tmp.add_source( + "ProxyAdmin.sol", + r#" + pragma solidity =0.8.12; + contract ProxyAdmin {} + "#, + ) + .unwrap(); + + let compiled = tmp.compile().unwrap(); + assert!(!compiled.has_compiler_errors()); + assert!(compiled.find("DeployProxy").is_some()); + assert!(compiled.find("UpgradeProxy").is_some()); + assert!(compiled.find("ProxyAdmin").is_some()); + + let artifacts = tmp.artifacts_snapshot().unwrap(); + assert_eq!(artifacts.artifacts.as_ref().len(), 3); + artifacts.assert_artifacts_essentials_present(); + + let compiled = tmp.compile().unwrap(); + assert!(compiled.find("DeployProxy").is_some()); + assert!(compiled.find("UpgradeProxy").is_some()); + assert!(compiled.find("ProxyAdmin").is_some()); + assert!(compiled.is_unchanged()); + + // modify upgradeProxy.sol + tmp.add_source("upgradeProxy.sol", format!("{}\n", upgrade)).unwrap(); + let compiled = tmp.compile().unwrap(); + assert!(!compiled.has_compiler_errors()); + assert!(!compiled.is_unchanged()); + assert!(compiled.find("DeployProxy").is_some()); + assert!(compiled.find("UpgradeProxy").is_some()); + assert!(compiled.find("ProxyAdmin").is_some()); + + let artifacts = tmp.artifacts_snapshot().unwrap(); + assert_eq!(artifacts.artifacts.as_ref().len(), 3); + artifacts.assert_artifacts_essentials_present(); +} + #[test] fn can_recompile_unchanged_with_empty_files() { let tmp = TempProject::dapptools().unwrap();