trufflesuite / truffle

:warning: The Truffle Suite is being sunset. For information on ongoing support, migration options and FAQs, visit the Consensys blog. Thank you for all the support over the years.
https://consensys.io/blog/consensys-announces-the-sunset-of-truffle-and-ganache-and-new-hardhat?utm_source=github&utm_medium=referral&utm_campaign=2023_Sep_truffle-sunset-2023_announcement_
MIT License
14.02k stars 2.32k forks source link

Relative imports with leading `../` segments going beyond compiler's VFS root are not resolved correctly #4163

Open cameel opened 3 years ago

cameel commented 3 years ago

Issue

When resolving relative imports with leading ../ segments the compiler stops at the root of its virtual filesystem and ignores any superfluous segments (see documentation for relative imports). For example import "../../../../d.sol" resolves into d.sol when found inside a/b/c.sol. Truffle instead follows the path outside of project directory resulting in such imports not working correctly. The way it works now it can result in the wrong file being imported which could go unnoticed by the user.

I think these imports should be reported as an error because this is just an obscure corner case that is not really worth supporting in practice. It's more likely to be a mistake on user's part and reporting it would be better. Even if Truffle used the correct path, they would not work anyway due to the recently added project:/ prefix (#4137).

Steps to Reproduce

cd /tmp/
mkdir truffle-test/
cd truffle-test/
npm install truffle
npx truffle init
echo 'module.exports.compilers.solc.version = "0.8.6";' >> truffle-config.js

echo 'contract C {}' > /tmp/C.sol
echo 'import "../../C.sol";' > contracts/C.sol
npx truffle compile

Actual Results

ParserError: Source "C.sol" not found: File import callback not supported
 --> project:/contracts/C.sol:1:1:
  |
1 | import "../../C.sol";
  | ^^^^^^^^^^^^^^^^^^^^^

Compilation failed. See above.

The sources dict in the JSON input contains these entries:

"project:/contracts/C.sol": { "content": "import \"../../C.sol\";\n" },
"project:/contracts/Migrations.sol": { "content": "// SPDX-License-Identifier: MIT\npragma solidity >=0.4.22 <0.9.0;\n\ncontract Migrations {\n  address public owner = msg.sender;\n  uint public last_completed_migration;\n\n  modifier restricted() {\n    require(\n      msg.sender == owner,\n      \"This function is restricted to the contract's owner\"\n    );\n    _;\n  }\n\n  function setCompleted(uint completed) public restricted {\n    last_completed_migration = completed;\n  }\n}\n" },
"/tmp/C.sol": { "content": "contract C {}\n" }

Expected Behavior

Something like this works fine as an input to the compiler because it resolves ../../C.sol into C.sol:

{
    "language": "Solidity",
    "sources": {
        "contracts/C.sol": { "content": "import \"../../C.sol\";\n" },
        "C.sol": { "content": "contract C {}\n" }
    },
    "settings": {"outputSelection": {"*": {"*": ["metadata", "evm.bytecode"]}}}
}

For Truffle this is not viable due to the project:/ prefix so I think this case should simply be disallowed.

Environment

eggplantzzz commented 3 years ago

Interesting, thanks for reporting this @cameel! Let me talk about it with the team!