Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

forge flatten does not support qualified imports #2545

Closed
2 tasks done
adhusson opened this issue Aug 1, 2022 · 3 comments · Fixed by #6936
Closed
2 tasks done

forge flatten does not support qualified imports #2545

adhusson opened this issue Aug 1, 2022 · 3 comments · Fixed by #6936
Assignees
Labels
C-forge Command: forge Cmd-forge-flatten Command: forge flatten T-bug Type: bug

Comments

@adhusson
Copy link
Contributor

adhusson commented Aug 1, 2022

Component

Forge

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

  • Foundry
  • 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.

@adhusson adhusson added the T-bug Type: bug label Aug 1, 2022
@onbjerg onbjerg added C-forge Command: forge Cmd-forge-flatten Command: forge flatten labels Aug 1, 2022
@mattsse
Copy link
Member

mattsse commented Aug 2, 2022

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
Copy link
Contributor Author

adhusson commented Aug 2, 2022

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
Copy link
Contributor

sambacha commented Aug 5, 2022

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

basically creating a barrel contract file?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-forge Command: forge Cmd-forge-flatten Command: forge flatten T-bug Type: bug
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

5 participants