From 75835a928023d81ce55d6d071e2697d8259309d3 Mon Sep 17 00:00:00 2001 From: Matthias Seitz Date: Mon, 23 May 2022 00:19:59 +0200 Subject: [PATCH] feat(solc): emit artifacts for standalone source files (#1296) * feat(solc): emit artifact files for sources with no contracts * test(solc): add tests for emitting standalone sources * chore: update CHANGELOG * style: check ast is some --- CHANGELOG.md | 2 + .../src/artifact_output/configurable.rs | 13 ++ ethers-solc/src/artifact_output/mod.rs | 89 +++++++++- ethers-solc/src/artifacts/mod.rs | 16 ++ ethers-solc/src/compile/project.rs | 26 +-- ethers-solc/src/hh.rs | 10 +- ethers-solc/src/lib.rs | 10 +- ethers-solc/tests/project.rs | 154 ++++++++++++++++++ 8 files changed, 305 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b7db676d..51b9830a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -96,6 +96,8 @@ - Bundle svm, svm-builds and sha2 dependencies in new `svm-solc` feature [#1071](https://github.com/gakonst/ethers-rs/pull/1071) +- Emit artifact files for source files without any ContractDefinition + [#1296](https://github.com/gakonst/ethers-rs/pull/1296) - Wrap `ethabi::Contract` into new type `LosslessAbi` and `abi: Option` with `abi: Option` in `ConfigurableContractArtifact` [#952](https://github.com/gakonst/ethers-rs/pull/952) - Let `Project` take ownership of `ArtifactOutput` and change trait interface diff --git a/ethers-solc/src/artifact_output/configurable.rs b/ethers-solc/src/artifact_output/configurable.rs index ea373cf7..c3dea9ea 100644 --- a/ethers-solc/src/artifact_output/configurable.rs +++ b/ethers-solc/src/artifact_output/configurable.rs @@ -11,6 +11,7 @@ use crate::{ Ast, CompactContractBytecodeCow, DevDoc, Evm, Ewasm, FunctionDebugData, GasEstimates, GeneratedSource, LosslessAbi, Metadata, Offsets, Settings, StorageLayout, UserDoc, }, + sources::VersionedSourceFile, ArtifactOutput, SolcConfig, SolcError, SourceFile, }; use serde::{Deserialize, Serialize}; @@ -329,6 +330,18 @@ impl ArtifactOutput for ConfigurableArtifacts { generated_sources: generated_sources.unwrap_or_default(), } } + + fn standalone_source_file_to_artifact( + &self, + _path: &str, + file: &VersionedSourceFile, + ) -> Option { + file.source_file.ast.clone().map(|ast| ConfigurableContractArtifact { + id: Some(file.source_file.id), + ast: Some(ast), + ..Default::default() + }) + } } /// Determines the additional values to include in the contract's artifact file diff --git a/ethers-solc/src/artifact_output/mod.rs b/ethers-solc/src/artifact_output/mod.rs index fa736bc3..d73c8f22 100644 --- a/ethers-solc/src/artifact_output/mod.rs +++ b/ethers-solc/src/artifact_output/mod.rs @@ -9,7 +9,7 @@ use semver::Version; use serde::{de::DeserializeOwned, Serialize}; use std::{ borrow::Cow, - collections::btree_map::BTreeMap, + collections::{btree_map::BTreeMap, HashSet}, fmt, fs, io, path::{Path, PathBuf}, }; @@ -23,6 +23,7 @@ use crate::{ }, compile::output::{contracts::VersionedContracts, sources::VersionedSourceFiles}, sourcemap::{SourceMap, SyntaxError}, + sources::VersionedSourceFile, }; pub use configurable::*; @@ -632,14 +633,25 @@ pub trait ArtifactOutput { sources: &VersionedSourceFiles, ) -> Artifacts { let mut artifacts = ArtifactsMap::new(); + + // this tracks all the `SourceFile`s that we successfully mapped to a contract + let mut non_standalone_sources = HashSet::new(); + + // loop over all files and their contracts for (file, contracts) in contracts.as_ref().iter() { let mut entries = BTreeMap::new(); + + // loop over all contracts and their versions for (name, versioned_contracts) in contracts { let mut contracts = Vec::with_capacity(versioned_contracts.len()); // check if the same contract compiled with multiple solc versions for contract in versioned_contracts { let source_file = sources.find_file_and_version(file, &contract.version); + if let Some(source) = source_file { + non_standalone_sources.insert((source.id, &contract.version)); + } + let artifact_path = if versioned_contracts.len() > 1 { Self::output_file_versioned(file, name, &contract.version) } else { @@ -664,8 +676,67 @@ pub trait ArtifactOutput { artifacts.insert(file.to_string(), entries); } + // extend with standalone source files and convert them to artifacts + for (file, sources) in sources.as_ref().iter() { + for source in sources { + if !non_standalone_sources.contains(&(source.source_file.id, &source.version)) { + // scan the ast as a safe measure to ensure this file does not include any + // source units + // there's also no need to create a standalone artifact for source files that + // don't contain an ast + if source.source_file.contains_contract_definition() || + source.source_file.ast.is_none() + { + continue + } + + // we use file and file stem + if let Some(name) = Path::new(file).file_stem().and_then(|stem| stem.to_str()) { + if let Some(artifact) = + self.standalone_source_file_to_artifact(file, source) + { + let artifact_path = if sources.len() > 1 { + Self::output_file_versioned(file, name, &source.version) + } else { + Self::output_file(file, name) + }; + + let entries = artifacts + .entry(file.to_string()) + .or_default() + .entry(name.to_string()) + .or_default(); + + if entries.iter().all(|entry| entry.version != source.version) { + entries.push(ArtifactFile { + artifact, + file: artifact_path, + version: source.version.clone(), + }); + } + } + } + } + } + } + Artifacts(artifacts) } + + /// This converts a `SourceFile` that doesn't contain _any_ contract definitions (interfaces, + /// contracts, libraries) to an artifact. + /// + /// We do this because not all `SourceFile`s emitted by solc have at least 1 corresponding entry + /// in the `contracts` + /// section of the solc output. For example for an `errors.sol` that only contains custom error + /// definitions and no contract, no `Contract` object will be generated by solc. However, we + /// still want to emit an `Artifact` for that file that may include the `ast`, docs etc., + /// because other tools depend on this, such as slither. + fn standalone_source_file_to_artifact( + &self, + _path: &str, + _file: &VersionedSourceFile, + ) -> Option; } /// An `Artifact` implementation that uses a compact representation @@ -695,6 +766,14 @@ impl ArtifactOutput for MinimalCombinedArtifacts { ) -> Self::Artifact { Self::Artifact::from(contract) } + + fn standalone_source_file_to_artifact( + &self, + _path: &str, + _file: &VersionedSourceFile, + ) -> Option { + None + } } /// An Artifacts handler implementation that works the same as `MinimalCombinedArtifacts` but also @@ -739,6 +818,14 @@ impl ArtifactOutput for MinimalCombinedArtifactsHardhatFallback { ) -> Self::Artifact { MinimalCombinedArtifacts::default().contract_to_artifact(file, name, contract, source_file) } + + fn standalone_source_file_to_artifact( + &self, + path: &str, + file: &VersionedSourceFile, + ) -> Option { + MinimalCombinedArtifacts::default().standalone_source_file_to_artifact(path, file) + } } #[cfg(test)] diff --git a/ethers-solc/src/artifacts/mod.rs b/ethers-solc/src/artifacts/mod.rs index 1547313f..ca31c1da 100644 --- a/ethers-solc/src/artifacts/mod.rs +++ b/ethers-solc/src/artifacts/mod.rs @@ -1634,6 +1634,22 @@ pub struct SourceFile { pub ast: Option, } +// === impl SourceFile === + +impl SourceFile { + /// Returns `true` if the source file contains at least 1 `ContractDefinition` such as + /// `contract`, `abstract contract`, `interface` or `library` + pub fn contains_contract_definition(&self) -> bool { + if let Some(ref ast) = self.ast { + // contract definitions are only allowed at the source-unit level + return ast.nodes.iter().any(|node| node.node_type == NodeType::ContractDefinition) + // abstract contract, interfaces: ContractDefinition + } + + false + } +} + /// A wrapper type for a list of source files /// `path -> SourceFile` #[derive(Clone, Debug, Serialize, Deserialize, Eq, PartialEq)] diff --git a/ethers-solc/src/compile/project.rs b/ethers-solc/src/compile/project.rs index 22126ca6..59b6774e 100644 --- a/ethers-solc/src/compile/project.rs +++ b/ethers-solc/src/compile/project.rs @@ -265,23 +265,25 @@ impl<'a, T: ArtifactOutput> CompiledState<'a, T> { fn write_artifacts(self) -> Result> { let CompiledState { output, cache } = self; - // write all artifacts via the handler but only if the build succeeded - let compiled_artifacts = if cache.project().no_artifacts { - cache - .project() - .artifacts_handler() - .output_to_artifacts(&output.contracts, &output.sources) + let project = cache.project(); + // write all artifacts via the handler but only if the build succeeded and project wasn't + // configured with `no_artifacts == true` + let compiled_artifacts = if project.no_artifacts { + project.artifacts_handler().output_to_artifacts(&output.contracts, &output.sources) } else if output.has_error() { tracing::trace!("skip writing cache file due to solc errors: {:?}", output.errors); - cache - .project() - .artifacts_handler() - .output_to_artifacts(&output.contracts, &output.sources) + project.artifacts_handler().output_to_artifacts(&output.contracts, &output.sources) } else { - cache.project().artifacts_handler().on_output( + tracing::trace!( + "handling artifact output for {} contracts and {} sources", + output.contracts.len(), + output.sources.len() + ); + // this emits the artifacts via the project's artifacts handler + project.artifacts_handler().on_output( &output.contracts, &output.sources, - &cache.project().paths, + &project.paths, )? }; diff --git a/ethers-solc/src/hh.rs b/ethers-solc/src/hh.rs index c71fbeb1..a67fb2f2 100644 --- a/ethers-solc/src/hh.rs +++ b/ethers-solc/src/hh.rs @@ -6,7 +6,7 @@ use crate::{ contract::{CompactContract, CompactContractBytecode, Contract, ContractBytecode}, CompactContractBytecodeCow, LosslessAbi, Offsets, }, - ArtifactOutput, SourceFile, + ArtifactOutput, SourceFile, VersionedSourceFile, }; use serde::{Deserialize, Serialize}; use std::{borrow::Cow, collections::btree_map::BTreeMap}; @@ -135,6 +135,14 @@ impl ArtifactOutput for HardhatArtifacts { deployed_link_references, } } + + fn standalone_source_file_to_artifact( + &self, + _path: &str, + _file: &VersionedSourceFile, + ) -> Option { + None + } } #[cfg(test)] diff --git a/ethers-solc/src/lib.rs b/ethers-solc/src/lib.rs index ac0e8827..bf6c41fc 100644 --- a/ethers-solc/src/lib.rs +++ b/ethers-solc/src/lib.rs @@ -36,7 +36,7 @@ use crate::{ artifacts::Sources, cache::SolFilesCache, error::{SolcError, SolcIoError}, - sources::VersionedSourceFiles, + sources::{VersionedSourceFile, VersionedSourceFiles}, }; use artifacts::contract::Contract; use compile::output::contracts::VersionedContracts; @@ -830,6 +830,14 @@ impl ArtifactOutput for Project { ) -> Artifacts { self.artifacts_handler().output_to_artifacts(contracts, sources) } + + fn standalone_source_file_to_artifact( + &self, + path: &str, + file: &VersionedSourceFile, + ) -> Option { + self.artifacts_handler().standalone_source_file_to_artifact(path, file) + } } #[cfg(test)] diff --git a/ethers-solc/tests/project.rs b/ethers-solc/tests/project.rs index be809dea..3a3ee65b 100644 --- a/ethers-solc/tests/project.rs +++ b/ethers-solc/tests/project.rs @@ -1304,6 +1304,160 @@ fn can_recompile_unchanged_with_empty_files() { assert!(compiled.find("C").is_some()); } +#[test] +fn can_emit_empty_artifacts() { + let tmp = TempProject::dapptools().unwrap(); + + let top_level = tmp + .add_source( + "top_level", + r#" + function test() {} + "#, + ) + .unwrap(); + + tmp.add_source( + "Contract", + r#" +// SPDX-License-Identifier: UNLICENSED +pragma solidity 0.8.10; + +import "./top_level.sol"; + +contract Contract { + function a() public{ + test(); + } +} + "#, + ) + .unwrap(); + + let compiled = tmp.compile().unwrap(); + assert!(!compiled.has_compiler_errors()); + assert!(compiled.find("Contract").is_some()); + assert!(compiled.find("top_level").is_some()); + let mut artifacts = tmp.artifacts_snapshot().unwrap(); + + assert_eq!(artifacts.artifacts.as_ref().len(), 2); + + let mut top_level = + artifacts.artifacts.as_mut().remove(top_level.to_string_lossy().as_ref()).unwrap(); + + assert_eq!(top_level.len(), 1); + + let artifact = top_level.remove("top_level").unwrap().remove(0); + assert!(artifact.artifact.ast.is_some()); + + // recompile + let compiled = tmp.compile().unwrap(); + assert!(compiled.is_unchanged()); + + // modify standalone file + + tmp.add_source( + "top_level", + r#" + error MyError(); + function test() {} + "#, + ) + .unwrap(); + let compiled = tmp.compile().unwrap(); + assert!(!compiled.is_unchanged()); +} + +#[test] +fn can_detect_contract_def_source_files() { + let tmp = TempProject::dapptools().unwrap(); + + let mylib = tmp + .add_source( + "MyLib", + r#" + pragma solidity 0.8.10; + library MyLib { + } + "#, + ) + .unwrap(); + + let myinterface = tmp + .add_source( + "MyInterface", + r#" + pragma solidity 0.8.10; + interface MyInterface {} + "#, + ) + .unwrap(); + + let mycontract = tmp + .add_source( + "MyContract", + r#" + pragma solidity 0.8.10; + contract MyContract {} + "#, + ) + .unwrap(); + + let myabstract_contract = tmp + .add_source( + "MyAbstractContract", + r#" + pragma solidity 0.8.10; + contract MyAbstractContract {} + "#, + ) + .unwrap(); + + let myerr = tmp + .add_source( + "MyError", + r#" + pragma solidity 0.8.10; + error MyError(); + "#, + ) + .unwrap(); + + let myfunc = tmp + .add_source( + "MyFunction", + r#" + pragma solidity 0.8.10; + function abc(){} + "#, + ) + .unwrap(); + + let compiled = tmp.compile().unwrap(); + println!("{}", compiled); + assert!(!compiled.has_compiler_errors()); + + let mut sources = compiled.output().sources; + let myfunc = sources.remove_by_path(myfunc.to_string_lossy()).unwrap(); + assert!(!myfunc.contains_contract_definition()); + + let myerr = sources.remove_by_path(myerr.to_string_lossy()).unwrap(); + assert!(!myerr.contains_contract_definition()); + + let mylib = sources.remove_by_path(mylib.to_string_lossy()).unwrap(); + assert!(mylib.contains_contract_definition()); + + let myabstract_contract = + sources.remove_by_path(myabstract_contract.to_string_lossy()).unwrap(); + assert!(myabstract_contract.contains_contract_definition()); + + let myinterface = sources.remove_by_path(myinterface.to_string_lossy()).unwrap(); + assert!(myinterface.contains_contract_definition()); + + let mycontract = sources.remove_by_path(mycontract.to_string_lossy()).unwrap(); + assert!(mycontract.contains_contract_definition()); +} + #[test] fn can_compile_sparse_with_link_references() { let tmp = TempProject::dapptools().unwrap();