Repository: gakonst/ethers-rs
Pull request #1041 (2022-03-16 13:10:11)

fix(solc): use lowercase when comparing paths
<!--
Thank you for your Pull Request. Please provide a description above and review
the requirements below.

Bug fixes and new features should include tests.

Contributors guide: https://github.com/gakonst/ethers-rs/blob/master/CONTRIBUTING.md

The contributors guide includes instructions for running rustfmt and building the
documentation.
-->

## Motivation
https://github.com/odyslam/foundry-upgrades/tree/feat/core highlighted an issue in which errors went undetected, turns out solc not necessarily emits the exact file name, e.g. `src/utils/upgradeProxy.sol` is emitted as `src/utils/UpgradeProxy.sol` so we ended up swallowing some errors.

needs a followup with some tests.

<!--
Explain the context and why you're making that change. What is the problem
you're trying to solve? In some cases there is not a problem and this can be
thought of as being the motivation for your change.
-->

## Solution
use lowercase
<!--
Summarize the solution and provide any necessary context needed to understand
the code change.
-->

## PR Checklist

- [ ] Added Tests
- [ ] Added Documentation
- [ ] Updated the changelog


mattsse: test failures unrelated, version error will be addressed #1045 

Patch:
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<Item = &'a str>,
     {
-        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/mod.rs b/ethers-solc/src/compile/mod.rs
index 63efba68..c7c4d136 100644
--- a/ethers-solc/src/compile/mod.rs
+++ b/ethers-solc/src/compile/mod.rs
@@ -742,7 +742,7 @@ mod tests {
             // update this test whenever there's a new sol
             // version. that's ok! good reminder to check the
             // patch notes.
-            (">=0.5.0", "0.8.12"),
+            (">=0.5.0", "0.8.13"),
             // range
             (">=0.4.0 <0.5.0", "0.4.26"),
         ]
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::<Vec<_>>()
                     );
                     (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<str>) -> R
 }
 
 fn contract_file_name(name: impl AsRef<str>) -> String {
-    let name = name.as_ref();
+    let name = name.as_ref().trim();
     if name.ends_with(".sol") {
         name.to_string()
     } else {


Test patch:
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();