From 990c60938422a28d18992bec021117ea2bbb97e6 Mon Sep 17 00:00:00 2001 From: Arsenii Kulikov Date: Tue, 13 Feb 2024 17:39:07 +0300 Subject: [PATCH] Small flattener features (#75) This PR implements two small flattener features mentioned in https://github.com/makerdao/spells-mainnet/pull/391#issuecomment-1926898173 1. Combine pragmas from different sources into one pragma so that version restrictions are kept the same before and after flattening 2. Comments specifying original source file of the flattened code (`// src/File.sol`) --- Cargo.toml | 1 + src/config.rs | 34 ++- src/flatten.rs | 93 ++++++-- src/resolver/mod.rs | 2 +- .../test-flatten-duplicates/contracts/Bar.sol | 4 - .../test-flatten-duplicates/contracts/Foo.sol | 6 - .../contracts/FooBar.sol | 7 - .../contracts/Contract.sol | 9 - .../contracts/Lib.sol | 4 - tests/project.rs | 222 +++++++++++++----- 10 files changed, 264 insertions(+), 118 deletions(-) delete mode 100644 test-data/test-flatten-duplicates/contracts/Bar.sol delete mode 100644 test-data/test-flatten-duplicates/contracts/Foo.sol delete mode 100644 test-data/test-flatten-duplicates/contracts/FooBar.sol delete mode 100644 test-data/test-flatten-solang-failure/contracts/Contract.sol delete mode 100644 test-data/test-flatten-solang-failure/contracts/Lib.sol diff --git a/Cargo.toml b/Cargo.toml index 4866925b..df3a5906 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -57,6 +57,7 @@ home = "0.5" svm = { package = "svm-rs", version = "0.3", default-features = false, optional = true } svm-builds = { package = "svm-rs-builds", version = "0.3", default-features = false, optional = true } sha2 = { version = "0.10", default-features = false, optional = true } +itertools = "0.12" [dev-dependencies] alloy-primitives = { version = "0.6", features = ["serde", "rand"] } diff --git a/src/config.rs b/src/config.rs index 3871b45e..03ef7ae1 100644 --- a/src/config.rs +++ b/src/config.rs @@ -2,7 +2,7 @@ use crate::{ artifacts::{output_selection::ContractOutputSelection, Settings}, cache::SOLIDITY_FILES_CACHE_FILENAME, error::{Result, SolcError, SolcIoError}, - flatten::collect_ordered_deps, + flatten::{collect_ordered_deps, combine_version_pragmas}, remappings::Remapping, resolver::{Graph, SolImportAlias}, utils, Source, Sources, @@ -412,6 +412,8 @@ impl ProjectPathsConfig { let ordered_deps = collect_ordered_deps(&flatten_target, self, &graph)?; let mut sources = Vec::new(); + let mut experimental_pragma = None; + let mut version_pragmas = Vec::new(); let mut result = String::new(); @@ -420,7 +422,7 @@ impl ProjectPathsConfig { SolcError::msg(format!("cannot resolve file at {}", path.display())) })?; let node = graph.node(*node_id); - let content = node.content().to_owned(); + let content = node.content(); // Firstly we strip all licesnses, verson pragmas // We keep target file pragma and license placing them in the beginning of the result. @@ -434,17 +436,14 @@ impl ProjectPathsConfig { } } if let Some(version) = node.version() { + let content = &content[version.loc()]; ranges_to_remove.push(version.loc()); - if *path == flatten_target { - result.push_str(&content[version.loc()]); - result.push('\n'); - } + version_pragmas.push(content); } if let Some(experimental) = node.experimental() { ranges_to_remove.push(experimental.loc()); - if *path == flatten_target { - result.push_str(&content[experimental.loc()]); - result.push('\n'); + if experimental_pragma.is_none() { + experimental_pragma = Some(content[experimental.loc()].to_owned()); } } for import in node.imports() { @@ -489,12 +488,27 @@ impl ProjectPathsConfig { } } + let content = format!( + "// {}\n{}", + path.strip_prefix(&self.root).unwrap_or(path).display(), + content + ); + sources.push(content); } + if let Some(version) = combine_version_pragmas(version_pragmas) { + result.push_str(&version); + result.push('\n'); + } + if let Some(experimental) = experimental_pragma { + result.push_str(&experimental); + result.push('\n'); + } + for source in sources { - result.push_str(&source); result.push_str("\n\n"); + result.push_str(&source); } Ok(format!("{}\n", utils::RE_THREE_OR_MORE_NEWLINES.replace_all(&result, "\n\n").trim())) diff --git a/src/flatten.rs b/src/flatten.rs index 6c383575..fc6115e2 100644 --- a/src/flatten.rs +++ b/src/flatten.rs @@ -4,6 +4,8 @@ use std::{ path::{Path, PathBuf}, }; +use itertools::Itertools; + use crate::{ artifacts::{ ast::SourceLocation, @@ -12,7 +14,7 @@ use crate::{ MemberAccess, Source, SourceUnit, SourceUnitPart, Sources, }, error::SolcError, - utils, Graph, Project, ProjectCompileOutput, ProjectPathsConfig, Result, + utils, Graph, Project, ProjectCompileOutput, ProjectPathsConfig, Result, Solc, }; /// Alternative of `SourceLocation` which includes path of the file. @@ -95,11 +97,11 @@ impl Visitor for ReferencesCollector { /// source_path -> (start, end, new_value) type Updates = HashMap>; -struct FlatteningResult<'a> { +pub struct FlatteningResult<'a> { /// Updated source in the order they shoud be written to the output file. sources: Vec, /// Pragmas that should be present in the target file. - pragmas: Vec<&'a str>, + pragmas: Vec, /// License identifier that should be present in the target file. license: Option<&'a str>, } @@ -108,7 +110,7 @@ impl<'a> FlatteningResult<'a> { fn new( flattener: &Flattener, mut updates: Updates, - pragmas: Vec<&'a str>, + pragmas: Vec, license: Option<&'a str>, ) -> Self { let mut sources = Vec::new(); @@ -127,7 +129,12 @@ impl<'a> FlatteningResult<'a> { offset += new_value.len() as isize - (end - start) as isize; } } - sources.push(String::from_utf8(content).unwrap()); + let content = format!( + "// {}\n{}", + path.strip_prefix(&flattener.project_root).unwrap_or(path).display(), + String::from_utf8(content).unwrap() + ); + sources.push(content); } Self { sources, pragmas, license } @@ -143,7 +150,7 @@ impl<'a> FlatteningResult<'a> { result.push_str(&format!("{}\n", pragma)); } for source in &self.sources { - result.push_str(&format!("{}\n\n", source)); + result.push_str(&format!("\n\n{}", source)); } format!("{}\n", utils::RE_THREE_OR_MORE_NEWLINES.replace_all(&result, "\n\n").trim()) @@ -160,6 +167,8 @@ pub struct Flattener { asts: Vec<(PathBuf, SourceUnit)>, /// Sources in the order they should be written to the output file. ordered_sources: Vec, + /// Project root directory. + project_root: PathBuf, } impl Flattener { @@ -177,9 +186,9 @@ impl Flattener { let sources = Source::read_all_files(input_files)?; let graph = Graph::resolve_sources(&project.paths, sources)?; - let ordered_deps = collect_ordered_deps(&target.to_path_buf(), &project.paths, &graph)?; + let ordered_sources = collect_ordered_deps(&target.to_path_buf(), &project.paths, &graph)?; - let sources = Source::read_all(&ordered_deps)?; + let sources = Source::read_all(&ordered_sources)?; // Convert all ASTs from artifacts to strongly typed ASTs let mut asts: Vec<(PathBuf, SourceUnit)> = Vec::new(); @@ -197,7 +206,13 @@ impl Flattener { asts.push((PathBuf::from(path), serde_json::from_str(&serde_json::to_string(ast)?)?)); } - Ok(Flattener { target: target.into(), sources, asts, ordered_sources: ordered_deps }) + Ok(Flattener { + target: target.into(), + sources, + asts, + ordered_sources, + project_root: project.root().clone(), + }) } /// Flattens target file and returns the result as a string @@ -227,7 +242,7 @@ impl Flattener { fn flatten_result<'a>( &'a self, updates: Updates, - target_pragmas: Vec<&'a str>, + target_pragmas: Vec, target_license: Option<&'a str>, ) -> FlatteningResult<'_> { FlatteningResult::new(self, updates, target_pragmas, target_license) @@ -629,25 +644,22 @@ impl Flattener { .collect() } - /// Removes all pragma directives from all sources. Returns Vec of pragmas that were found in - /// target file. - fn process_pragmas(&self, updates: &mut Updates) -> Vec<&str> { - // Pragmas that will be used in the resulted file - let mut target_pragmas = Vec::new(); + /// Removes all pragma directives from all sources. Returns Vec with experimental and combined + /// version pragmas (if present). + fn process_pragmas(&self, updates: &mut Updates) -> Vec { + let mut experimental = None; let pragmas = self.collect_pragmas(); - - let mut seen_experimental = false; + let mut version_pragmas = Vec::new(); for loc in &pragmas { let pragma_content = self.read_location(loc); if pragma_content.contains("experimental") { - if !seen_experimental { - seen_experimental = true; - target_pragmas.push(loc); + if experimental.is_none() { + experimental = Some(self.read_location(loc).to_string()); } - } else if loc.path == self.target { - target_pragmas.push(loc); + } else if pragma_content.contains("solidity") { + version_pragmas.push(pragma_content); } updates.entry(loc.path.clone()).or_default().insert(( @@ -657,8 +669,17 @@ impl Flattener { )); } - target_pragmas.sort_by_key(|loc| loc.start); - target_pragmas.iter().map(|loc| self.read_location(loc)).collect::>() + let mut pragmas = Vec::new(); + + if let Some(version_pragma) = combine_version_pragmas(version_pragmas) { + pragmas.push(version_pragma); + } + + if let Some(pragma) = experimental { + pragmas.push(pragma); + } + + pragmas } // Collects all pragma directives locations. @@ -785,3 +806,27 @@ pub fn collect_ordered_deps( Ok(ordered_deps) } + +pub fn combine_version_pragmas(pragmas: Vec<&str>) -> Option { + let mut versions = pragmas + .into_iter() + .filter_map(|p| { + Solc::version_req( + p.replace("pragma", "").replace("solidity", "").replace(';', "").trim(), + ) + .ok() + }) + .flat_map(|req| req.comparators) + .collect::>() + .into_iter() + .map(|comp| comp.to_string()) + .collect::>(); + + versions.sort(); + + if !versions.is_empty() { + return Some(format!("pragma solidity {};", versions.iter().format(" "))); + } + + None +} diff --git a/src/resolver/mod.rs b/src/resolver/mod.rs index 672d15f5..e86fd6d9 100644 --- a/src/resolver/mod.rs +++ b/src/resolver/mod.rs @@ -151,7 +151,7 @@ impl GraphEdges { /// Returns all files that import the given file pub fn importers(&self, file: impl AsRef) -> HashSet<&PathBuf> { if let Some(start) = self.indices.get(file.as_ref()).copied() { - self.rev_edges[start].iter().map(move |idx| &self.rev_indices[&idx]).collect() + self.rev_edges[start].iter().map(move |idx| &self.rev_indices[idx]).collect() } else { HashSet::new() } diff --git a/test-data/test-flatten-duplicates/contracts/Bar.sol b/test-data/test-flatten-duplicates/contracts/Bar.sol deleted file mode 100644 index 5e9034a5..00000000 --- a/test-data/test-flatten-duplicates/contracts/Bar.sol +++ /dev/null @@ -1,4 +0,0 @@ -//SPDX-License-Identifier: UNLICENSED -pragma solidity >=0.6.0; - -contract Bar {} diff --git a/test-data/test-flatten-duplicates/contracts/Foo.sol b/test-data/test-flatten-duplicates/contracts/Foo.sol deleted file mode 100644 index 1f7086de..00000000 --- a/test-data/test-flatten-duplicates/contracts/Foo.sol +++ /dev/null @@ -1,6 +0,0 @@ -//SPDX-License-Identifier: UNLICENSED -pragma solidity >=0.6.0; - -import { Bar } from './Bar.sol'; - -contract Foo {} diff --git a/test-data/test-flatten-duplicates/contracts/FooBar.sol b/test-data/test-flatten-duplicates/contracts/FooBar.sol deleted file mode 100644 index 58ec146b..00000000 --- a/test-data/test-flatten-duplicates/contracts/FooBar.sol +++ /dev/null @@ -1,7 +0,0 @@ -//SPDX-License-Identifier: UNLICENSED -pragma solidity >=0.6.0; - -import { Bar } from './Bar.sol'; -import { Foo } from './Foo.sol'; - -contract FooBar {} diff --git a/test-data/test-flatten-solang-failure/contracts/Contract.sol b/test-data/test-flatten-solang-failure/contracts/Contract.sol deleted file mode 100644 index c0be0077..00000000 --- a/test-data/test-flatten-solang-failure/contracts/Contract.sol +++ /dev/null @@ -1,9 +0,0 @@ -// SPDX-License-Identifier: UNLICENSED -pragma solidity ^0.8.10; - -import { Lib } from "./Lib.sol"; - -// Intentionally erroneous code -contract Contract { - failure(); -} diff --git a/test-data/test-flatten-solang-failure/contracts/Lib.sol b/test-data/test-flatten-solang-failure/contracts/Lib.sol deleted file mode 100644 index 407fb855..00000000 --- a/test-data/test-flatten-solang-failure/contracts/Lib.sol +++ /dev/null @@ -1,4 +0,0 @@ -// SPDX-License-Identifier: UNLICENSED -pragma solidity ^0.8.10; - -library Lib {} diff --git a/tests/project.rs b/tests/project.rs index 64c31c43..f78a13ec 100644 --- a/tests/project.rs +++ b/tests/project.rs @@ -490,32 +490,6 @@ fn test_flatteners(project: &TempProject, target: &Path, additional_checks: fn(& additional_checks(&result); } -#[test] -fn can_flatten_file() { - let root = PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("test-data/test-contract-libs"); - let target = root.join("src").join("Foo.sol"); - let paths = ProjectPathsConfig::builder() - .sources(root.join("src")) - .lib(root.join("lib1")) - .lib(root.join("lib2")); - - let project = TempProject::::new(paths).unwrap(); - - test_flatteners(&project, &target, |result| { - assert_eq!( - result, - r#"pragma solidity 0.8.6; - -contract Bar {} - -contract Baz {} - -contract Foo is Bar, Baz {} -"# - ); - }); -} - #[test] fn can_flatten_file_with_external_lib() { let root = PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("test-data/hardhat-sample"); @@ -592,10 +566,16 @@ contract C { } result, r#"pragma solidity ^0.8.10; +// src/B.sol + contract B { } +// src/C.sol + contract C { } +// src/A.sol + contract A { } "# ); @@ -649,10 +629,16 @@ contract C { } r"pragma solidity ^0.8.10; pragma experimental ABIEncoderV2; +// src/B.sol + contract B { } +// src/C.sol + contract C { } +// src/A.sol + contract A { } " ); @@ -660,37 +646,35 @@ contract A { } } #[test] -fn can_flatten_file_with_duplicates() { - let root = PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("test-data/test-flatten-duplicates"); - let paths = ProjectPathsConfig::builder().sources(root.join("contracts")); - let project = TempProject::::new(paths).unwrap(); +fn can_flatten_on_solang_failure() { + let project = TempProject::dapptools().unwrap(); - let target = root.join("contracts/FooBar.sol"); + project + .add_source( + "Lib", + r#"// SPDX-License-Identifier: UNLICENSED +pragma solidity ^0.8.10; - test_flatteners(&project, &target, |result| { - assert_eq!( - result, - r"//SPDX-License-Identifier: UNLICENSED -pragma solidity >=0.6.0; +library Lib {} +"#, + ) + .unwrap(); -contract Bar {} + let target = project + .add_source( + "Contract", + r#"// SPDX-License-Identifier: UNLICENSED +pragma solidity ^0.8.10; -contract Foo {} +import { Lib } from "./Lib.sol"; -contract FooBar {} -" - ); - }); +// Intentionally erroneous code +contract Contract { + failure(); } - -#[test] -fn can_flatten_on_solang_failure() { - let root = - PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("test-data/test-flatten-solang-failure"); - let paths = ProjectPathsConfig::builder().sources(root.join("contracts")); - let project = TempProject::::new(paths).unwrap(); - - let target = root.join("contracts/Contract.sol"); +"#, + ) + .unwrap(); let result = project.flatten(target.as_path()); assert!(result.is_ok()); @@ -701,7 +685,11 @@ fn can_flatten_on_solang_failure() { r"// SPDX-License-Identifier: UNLICENSED pragma solidity ^0.8.10; -library Lib {} +// src/Lib.sol + +library Lib {} + +// src/Contract.sol // Intentionally erroneous code contract Contract { @@ -756,11 +744,17 @@ contract C { } result, r"pragma solidity ^0.8.10; +// src/C.sol + contract C { } +// src/Errors.sol + error IllegalArgument(); error IllegalState(); +// src/A.sol + contract A { } " ); @@ -809,12 +803,17 @@ contract C { } result, r"pragma solidity ^0.8.10; +// src/C.sol + contract C { } +// src/B.sol // This is a B Contract contract B { } +// src/A.sol + contract A { } " ); @@ -829,6 +828,7 @@ fn can_flatten_with_alias() { .add_source( "Contract", r#"pragma solidity ^0.8.10; + import { ParentContract as Parent } from "./Parent.sol"; import { AnotherParentContract as AnotherParent } from "./AnotherParent.sol"; import { PeerContract as Peer } from "./Peer.sol"; @@ -860,6 +860,7 @@ contract Contract is Parent, .add_source( "Parent", r"pragma solidity ^0.8.10; + contract ParentContract { } ", ) @@ -869,6 +870,7 @@ contract ParentContract { } .add_source( "AnotherParent", r"pragma solidity ^0.8.10; + contract AnotherParentContract { } ", ) @@ -878,6 +880,7 @@ contract AnotherParentContract { } .add_source( "Peer", r"pragma solidity ^0.8.10; + contract PeerContract { } ", ) @@ -887,6 +890,7 @@ contract PeerContract { } .add_source( "Math", r"pragma solidity ^0.8.10; + library MathLibrary { function minusOne(uint256 val) internal returns (uint256) { return val - 1; @@ -908,6 +912,7 @@ library MathLibrary { .add_source( "SomeLib", r"pragma solidity ^0.8.10; + library SomeLib { } ", ) @@ -918,8 +923,12 @@ library SomeLib { } result, r#"pragma solidity ^0.8.10; +// src/AnotherParent.sol + contract AnotherParentContract { } +// src/Math.sol + library MathLibrary { function minusOne(uint256 val) internal returns (uint256) { return val - 1; @@ -934,12 +943,20 @@ library MathLibrary { } } +// src/Parent.sol + contract ParentContract { } +// src/Peer.sol + contract PeerContract { } +// src/SomeLib.sol + library SomeLib { } +// src/Contract.sol + contract Contract is ParentContract, AnotherParentContract { using MathLibrary for uint256; @@ -1016,12 +1033,20 @@ contract D { } result, r#"pragma solidity ^0.8.10; +// src/C.sol + contract C { } +// src/D.sol + contract D { } +// src/B.sol + contract B { } +// src/A.sol + contract A { } "# ); @@ -1067,6 +1092,8 @@ contract Bar is Foo {} result, r"pragma solidity ^0.8.10; +// src/Foo.sol + contract Foo { function foo() public pure returns (uint256) { return 1; @@ -1075,6 +1102,8 @@ contract Foo { contract Bar_0 is Foo {} +// src/Bar.sol + contract Bar_1 is Foo {} " ); @@ -1169,6 +1198,8 @@ contract D is C_File.B_File.A_File.A { result, r"pragma solidity ^0.8.10; +// src/A.sol + contract A_0 { type SomeCustomValue is uint256; @@ -1183,10 +1214,16 @@ contract A_0 { } } +// src/B.sol + contract A_1 is A_0 {} +// src/C.sol + contract A_2 is A_0 {} +// src/D.sol + A_0.SomeCustomValue constant fileLevelValue = A_0.SomeCustomValue.wrap(1); contract D is A_0 { @@ -1257,6 +1294,8 @@ contract B { result, r#"pragma solidity ^0.8.10; +// src/FileB.sol + interface FooBar_0 { function bar() external; } @@ -1266,6 +1305,8 @@ contract B { } } +// src/FlieA.sol + interface FooBar_1 { function foo() external; } @@ -1311,11 +1352,15 @@ contract B is A {} Flattener::new(project.project(), &project.compile().unwrap(), &target).unwrap().flatten(); assert_eq!( result, - r"pragma solidity 0.6.12; + r"pragma solidity =0.6.12; pragma experimental ABIEncoderV2; +// src/A.sol + contract A {} +// src/B.sol + contract B is A {} " ); @@ -1371,8 +1416,11 @@ pragma solidity ^0.8.10;"#, result, r"pragma solidity ^0.8.10; +// src/A.sol + contract A { } +// src/B.sol contract B is A {} " ); @@ -1425,13 +1473,19 @@ contract B is A { result, r"pragma solidity ^0.8.10; +// src/DuplicateA.sol + contract A_0 {} +// src/A.sol + contract A_1 { /// Documentation function foo() public virtual {} } +// src/B.sol + contract B is A_1 { /// @inheritdoc A_1 function foo() public override {} @@ -1476,11 +1530,15 @@ contract B is Alias { result, r"pragma solidity ^0.8.10; +// src/A.sol + contract A { /// Documentation function foo() public virtual {} } +// src/B.sol + contract B is A { /// @inheritdoc A function foo() public override {} @@ -1550,6 +1608,8 @@ contract Foo { result, r"pragma solidity ^0.8.10; +// src/CustomInt.sol + type CustomInt is int256; function mul_0(CustomInt a, CustomInt b) pure returns(CustomInt) { @@ -1558,6 +1618,8 @@ function mul_0(CustomInt a, CustomInt b) pure returns(CustomInt) { using {mul_0} for CustomInt global; +// src/CustomUint.sol + type CustomUint is uint256; function mul_1(CustomUint a, CustomUint b) pure returns(CustomUint) { @@ -1566,6 +1628,8 @@ function mul_1(CustomUint a, CustomUint b) pure returns(CustomUint) { using {mul_1} for CustomUint global; +// src/Target.sol + contract Foo { function mul(CustomUint a, CustomUint b) public returns(CustomUint) { return a.mul_1(b); @@ -1626,12 +1690,18 @@ contract Foo { result, r"pragma solidity ^0.8.10; +// src/func1.sol + function func_0() view {} +// src/func2.sol + function func_1(uint256 x) view returns(uint256) { return x + 1; } +// src/Target.sol + contract Foo { constructor(uint256 x) { func_0(); @@ -1688,10 +1758,16 @@ contract Foo { result, r"pragma solidity ^0.8.10; +// src/A.sol + uint256 constant a_0 = 1; +// src/B.sol + uint256 constant a_1 = 2; +// src/Target.sol + contract Foo { function test() public returns(uint256 x) { assembly { @@ -1703,6 +1779,46 @@ contract Foo { ); } +#[test] +fn can_flatten_combine_pragmas() { + let project = TempProject::dapptools().unwrap(); + + project + .add_source( + "A", + r"pragma solidity >=0.5.0; + +contract A {}", + ) + .unwrap(); + + let target = project + .add_source( + "B", + r"pragma solidity <0.9.0; +import './A.sol'; + +contract B {}", + ) + .unwrap(); + + test_flatteners(&project, &target, |result| { + assert_eq!( + result, + r"pragma solidity <0.9.0 >=0.5.0; + +// src/A.sol + +contract A {} + +// src/B.sol + +contract B {} +" + ); + }); +} + #[test] fn can_compile_single_files() { let tmp = TempProject::dapptools().unwrap();