`forge flatten` does not support qualified imports

[adhusson]

trafficstars

Component

Forge

Have you ensured that all of these are up to date?

• [X] Foundry
• [X] Foundryup

What version of Foundry are you on?

forge 0.2.0 (9ed1c37 2022-08-01T00:09:10.803586Z)

What command(s) is the bug in?

forge flatten

Operating System

macOS (Apple Silicon)

Describe the bug

Issue

When a flattened file contains a qualified import (of the form import path as Q), then:

1. Q is not stripped from accesses to definitions found in path. For instance, Q.structName does not become structName.
2. Names that clash with other names defined in other imported files are not modified to be unique. For instance, if the struct DataType is imported (with qualifier) from two files, compilation will fail.

Suggestion

forge flatten should strip all qualifiers, and rename definitions as needed (recursively).

Example:

// contract.sol
pragma solidity ^0.8.13;
import "./aux1.sol" as A;
import "./aux2.sol" as B;

contract C {
  struct Data { A.DataType a; B.DataType b; }
}

// aux1.sol
struct DataType { string s; }

// aux2.sol
struct DataType { string s; }

forge flatten results in this (which fails to compile):

pragma solidity ^0.8.13;
struct DataType { string s; }

struct DataType { string s; }

contract C {
  struct Data { A.DataType a; B.DataType b; }
}

when it should result in something like

pragma solidity ^0.8.13;
struct DataType1 { string s; }

struct DataType2 { string s; }

contract C {
  struct Data { DataType1 a; DataType2 b; }
}

Note: this is kind of a followup to #1440.

[mattsse]

this is actually a rather hard problem because

the conflicting imports could appear multiple times in the source tree

// contract.sol
import "./aux1.sol" as A;
import "./aux2.sol" as B;

// contract2.sol
import "./aux1.sol" as A2;
import "./aux2.sol" as B2;

// contract3.sol
import "./aux1.sol" as Arbitrary0;
import "./aux2.sol" as Arbitrary;

at which point name replace becomes non-trivial

in order to support this we'd need to come up with some rules/assumptions to follow for this scenario.

I can think of:

• use the first occurrence (A,B)
• Combine all names
• derive name from file path

[adhusson]

I would go with "derive name from shortest unambiguous file path" (most likely from src/, or from the project root), and use $ as separator for ease of life.

So with

// src/util/aux.sol
function f() internal {}
// src/utils/c2.sol
import "./aux.sol" as B;
// src/c1.sol
import "./util/aux.sol" as A;
import "./util/c2.sol";

flattening c1.sol gives:

function utils_aux$f() internal {} // first $ separates path from original name

(As a note for later, I don't think collapsing multiple qualifiers for one import would break anything (imports are pure) and repeating the code would be super wasteful. The only issue is that I see those multiple qualifiers being used for type safety (say import "./math.sol" as Dollars and import "./math.sol" as Euros) and the flattened contract could be super hard to understand.)

[sambacha]

Can creating an entry file for flatten to act on not help in this regard?

basically creating a barrel contract file?