Repository: foundry-rs/foundry (language: rust)

Issue:
forge flatten sort contracts alphabetically
### Component

Forge

### Describe the feature you would like

We rely on flatten quite heavily to diff code.
One huge pain point with this is that flattening two versions of a contract can result in dramatically different code as import order might have changed.

Therefore it would be a huge help/improvement if flatten would sort contacts alphabetically, either as default or when passing a flag.

### Additional context

_No response_
forge flatten sort contracts alphabetically
### Component

Forge

### Describe the feature you would like

We rely on flatten quite heavily to diff code.
One huge pain point with this is that flattening two versions of a contract can result in dramatically different code as import order might have changed.

Therefore it would be a huge help/improvement if flatten would sort contacts alphabetically, either as default or when passing a flag.

### Additional context

_No response_


Discussion / hints:
per foundry compilers comment
```
/// We can't just sort files alphabetically as it might break compilation, because Solidity
/// does not allow base class definitions to appear after derived contract
/// definitions.
///
/// Instead, we sort files by the number of their dependencies (imports of any depth) in ascending
/// order. If files have the same number of dependencies, we sort them alphabetically.
/// Target file is always placed last.
```

simple test yields solc failure with `Error (2449): Definition of base has to precede definition of derived contract`

@klkvr could you please chime in?
@grandizzy interesting, i was not aware.

For us this would still be insanely useful 😅 - we don't actually want to build/test or deploy the flattened code. We just want to compare it.
The realworld problem we have is for example [here](https://github.com/bgd-labs/collector-upgrade-rev6/blob/main/flattened/1/0x80f2c02224a2E548FC67c0bF705eBFA825dd5439.sol) vs [here](https://github.com/bgd-labs/collector-upgrade-rev6/blob/main/flattened/Collector.sol).

In the first `Address` comes first, in the second `Initializable` comes first. Both i think don't have dependencies.

---

Perhaps this overall approach is a bit suboptimal and there might be better ways to do this?
Would be curious on your opinions, I guess contract diffing could be a quite useful feature on foundry itself as i guess everyone working with proxies would be interested in this.
@sakulstra this was initially addressed in scope of https://github.com/foundry-rs/compilers/pull/52 to resolve https://github.com/foundry-rs/foundry/issues/6539

Sorting contracs alphabetically is not feasible because of the `base has to precede definition of derived contract` error. When working on this I was mostly optimizing for diff readability which I think is good enough now as order of files is pretty much deterministic as long as imported fileds not change. 

> The realworld problem we have is for example [here](https://github.com/bgd-labs/collector-upgrade-rev6/blob/main/flattened/1/0x80f2c02224a2E548FC67c0bF705eBFA825dd5439.sol) vs [here](https://github.com/bgd-labs/collector-upgrade-rev6/blob/main/flattened/Collector.sol).
> 
> In the first `Address` comes first, in the second `Initializable` comes first. Both i think don't have dependencies.

This is happening because in the second file, `Initializable.sol` is coming from `lib/aave-v3-origin/..` which is lexicographically higher than `Address.sol` path
> This is happening because in the second file, Initializable.sol is coming from lib/aave-v3-origin/.. which is lexicographically higher than Address.sol path

Sorry, i don't see it 😅 

How is Initializable lexicographically higher than Address? Or is the path considered for the sorting... which would be weird, no?
As in this case changing from e.g. submodules to soldeer or npm would completely change the output i guess?
> Or is the path considered for the sorting... which would be weird, no?

yeah path is considered right now

> As in this case changing from e.g. submodules to soldeer or npm would completely change the output i guess?

yeah tbh I'd say this is an edge case I just didn't consider at the time :) I guess we can use path as secondary sorting key, should be fixed in https://github.com/foundry-rs/compilers/pull/247
going to reopen as a placeholder for bumping compiler version
sending patch @grandizzy 
patch is out
per foundry compilers comment
```
/// We can't just sort files alphabetically as it might break compilation, because Solidity
/// does not allow base class definitions to appear after derived contract
/// definitions.
///
/// Instead, we sort files by the number of their dependencies (imports of any depth) in ascending
/// order. If files have the same number of dependencies, we sort them alphabetically.
/// Target file is always placed last.
```

simple test yields solc failure with `Error (2449): Definition of base has to precede definition of derived contract`

@klkvr could you please chime in?
@grandizzy interesting, i was not aware.

For us this would still be insanely useful 😅 - we don't actually want to build/test or deploy the flattened code. We just want to compare it.
The realworld problem we have is for example [here](https://github.com/bgd-labs/collector-upgrade-rev6/blob/main/flattened/1/0x80f2c02224a2E548FC67c0bF705eBFA825dd5439.sol) vs [here](https://github.com/bgd-labs/collector-upgrade-rev6/blob/main/flattened/Collector.sol).

In the first `Address` comes first, in the second `Initializable` comes first. Both i think don't have dependencies.

---

Perhaps this overall approach is a bit suboptimal and there might be better ways to do this?
Would be curious on your opinions, I guess contract diffing could be a quite useful feature on foundry itself as i guess everyone working with proxies would be interested in this.
@sakulstra this was initially addressed in scope of https://github.com/foundry-rs/compilers/pull/52 to resolve https://github.com/foundry-rs/foundry/issues/6539

Sorting contracs alphabetically is not feasible because of the `base has to precede definition of derived contract` error. When working on this I was mostly optimizing for diff readability which I think is good enough now as order of files is pretty much deterministic as long as imported fileds not change. 

> The realworld problem we have is for example [here](https://github.com/bgd-labs/collector-upgrade-rev6/blob/main/flattened/1/0x80f2c02224a2E548FC67c0bF705eBFA825dd5439.sol) vs [here](https://github.com/bgd-labs/collector-upgrade-rev6/blob/main/flattened/Collector.sol).
> 
> In the first `Address` comes first, in the second `Initializable` comes first. Both i think don't have dependencies.

This is happening because in the second file, `Initializable.sol` is coming from `lib/aave-v3-origin/..` which is lexicographically higher than `Address.sol` path
> This is happening because in the second file, Initializable.sol is coming from lib/aave-v3-origin/.. which is lexicographically higher than Address.sol path

Sorry, i don't see it 😅 

How is Initializable lexicographically higher than Address? Or is the path considered for the sorting... which would be weird, no?
As in this case changing from e.g. submodules to soldeer or npm would completely change the output i guess?
> Or is the path considered for the sorting... which would be weird, no?

yeah path is considered right now

> As in this case changing from e.g. submodules to soldeer or npm would completely change the output i guess?

yeah tbh I'd say this is an edge case I just didn't consider at the time :) I guess we can use path as secondary sorting key, should be fixed in https://github.com/foundry-rs/compilers/pull/247
going to reopen as a placeholder for bumping compiler version
sending patch @grandizzy 
patch is out

Fix patch:
diff --git a/Cargo.lock b/Cargo.lock
index 06196f4cf276..df5e5a80a8a0 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -3756,9 +3756,9 @@ dependencies = [
 
 [[package]]
 name = "foundry-compilers"
-version = "0.13.1"
+version = "0.13.2"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "203e96bd596350ec8d9aedfca7eff573e9347e2b2fe50eedfbf78532dabe418e"
+checksum = "de23802550de5204eec1a6297296d2fef6b86ea61f33b1667eebd886577caa34"
 dependencies = [
  "alloy-json-abi",
  "alloy-primitives",
@@ -3793,9 +3793,9 @@ dependencies = [
 
 [[package]]
 name = "foundry-compilers-artifacts"
-version = "0.13.1"
+version = "0.13.2"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "be8ba2ccf8a4bf7730b2ad2815984c1af8e5b8c492420f1cf1d26a8be29cc9e4"
+checksum = "27f065a1c785b3ec556a7a49a27016fc723bd9a4d8623f521c326f8396df64a6"
 dependencies = [
  "foundry-compilers-artifacts-solc",
  "foundry-compilers-artifacts-vyper",
@@ -3803,9 +3803,9 @@ dependencies = [
 
 [[package]]
 name = "foundry-compilers-artifacts-solc"
-version = "0.13.1"
+version = "0.13.2"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "5889eeb7d6729afbbbb1313b22e8f58aa909fcf38aa6b2f9c9b2443ca0765c59"
+checksum = "127e0e788765bc0103eb6b92067843c6ffe660090b2f6fbff1a1441a51939aa0"
 dependencies = [
  "alloy-json-abi",
  "alloy-primitives",
@@ -3827,9 +3827,9 @@ dependencies = [
 
 [[package]]
 name = "foundry-compilers-artifacts-vyper"
-version = "0.13.1"
+version = "0.13.2"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "b01048f354b4e98bf5fb4810e0607c87d4b8cc7fe6305a42103ce192e33b2da7"
+checksum = "8764f450332267305440ec399560919e01aa98f23af2c36ac952f9216449cdca"
 dependencies = [
  "alloy-json-abi",
  "alloy-primitives",
@@ -3842,9 +3842,9 @@ dependencies = [
 
 [[package]]
 name = "foundry-compilers-core"
-version = "0.13.1"
+version = "0.13.2"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "079b80a4f188af9c273b081547d1a95a0a33f782851a99e21aa4eba06e47fa34"
+checksum = "c59caebbeaeb0e34564c1a404081478a0aadaa8999b57f0befd85965252e074a"
 dependencies = [
  "alloy-primitives",
  "cfg-if",
diff --git a/Cargo.toml b/Cargo.toml
index f152898f64c2..9d14ed5925e3 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -187,7 +187,7 @@ foundry-linking = { path = "crates/linking" }
 
 # solc & compilation utilities
 foundry-block-explorers = { version = "0.11.0", default-features = false }
-foundry-compilers = { version = "0.13.0", default-features = false }
+foundry-compilers = { version = "0.13.2", default-features = false }
 foundry-fork-db = "0.11.0"
 solang-parser = "=0.3.3"
 solar-parse = { version = "=0.1.1", default-features = false }


Test patch: