Check for duplicate contract names on cli command line (#1238)

* Check for duplicate contract names on cli command line

If we are compiling two files, and they define the same contract name from
multiple locations, error out.

Signed-off-by: Sean Young <sean@mess.org>
Co-authored-by: Lucas Steuernagel <38472950+LucasSte@users.noreply.github.com>

src/bin/solang.rs

@@ -499,9 +499,18 @@ fn compile(matches: &ArgMatches) {
     }
 
     if !errors {
+        let mut seen_contracts = HashMap::new();
+
         for ns in namespaces.iter_mut() {
             for contract_no in 0..ns.contracts.len() {
-                contract_results(contract_no, matches, ns, &mut json_contracts, &opt);
+                contract_results(
+                    contract_no,
+                    matches,
+                    ns,
+                    &mut json_contracts,
+                    &mut seen_contracts,
+                    &opt,
+                );
             }
         }
     }
@@ -578,6 +587,7 @@ fn contract_results(
     matches: &ArgMatches,
     ns: &mut Namespace,
     json_contracts: &mut HashMap<String, JsonContract>,
+    seen_contracts: &mut HashMap<String, String>,
     opt: &Options,
 ) {
     let verbose = *matches.get_one("VERBOSE").unwrap();
@@ -589,6 +599,26 @@ fn contract_results(
         return;
     }
 
+    if ns.top_file_no() != resolved_contract.loc.file_no() {
+        // contracts that were imported should not be considered. For example, if we have a file
+        // a.sol which imports b.sol, and b.sol defines contract B, then:
+        // solang compile a.sol
+        // should not write the results for contract B
+        return;
+    }
+
+    let loc = ns.loc_to_string(true, &resolved_contract.loc);
+
+    if let Some(other_loc) = seen_contracts.get(&resolved_contract.name) {
+        eprintln!(
+            "error: contract {} defined at {other_loc} and {}",
+            resolved_contract.name, loc
+        );
+        exit(1);
+    }
+
+    seen_contracts.insert(resolved_contract.name.to_string(), loc);
+
     if let Some("cfg") = matches.get_one::<String>("EMIT").map(|v| v.as_str()) {
         println!("{}", resolved_contract.print_cfg(ns));
         return;

tests/imports.rs

@@ -118,3 +118,38 @@ fn import() {
 
     assert!(stderr.contains("file not found 'bar.sol'"));
 }
+
+#[test]
+fn contract_name_defined_twice() {
+    let mut cmd = Command::cargo_bin("solang").unwrap();
+
+    let ok = cmd
+        .args(["compile", "--target", "solana", "bar.sol", "rel.sol"])
+        .current_dir("tests/imports_testcases/imports")
+        .assert();
+
+    let output = ok.get_output();
+
+    assert_eq!(String::from_utf8_lossy(&output.stderr), "");
+
+    let mut cmd = Command::cargo_bin("solang").unwrap();
+
+    let not_ok = cmd
+        .args([
+            "compile",
+            "--target",
+            "solana",
+            "relative_import.sol",
+            "rel.sol",
+        ])
+        .current_dir("tests/imports_testcases/imports")
+        .assert();
+
+    let output = not_ok.get_output();
+    let err = String::from_utf8_lossy(&output.stderr);
+
+    // The error contains the absolute paths, so we cannot assert the whole string
+    assert!(err.starts_with("error: contract rel defined at "));
+    assert!(err.contains("relative_import.sol:1:1-16 and "));
+    assert!(err.ends_with("rel.sol:2:1-16\n"));
+}

tests/imports_testcases/imports/rel.sol

@@ -0,0 +1,2 @@
+
+contract rel {}