Rework compiler imports #1411
Rework compiler imports #1411
Conversation
- Move mock project to compile to test/mocks - Remove stdlib-with-deps as it is already covered in mock-project-to-compile - Add package.json where missing - Remove invalid import statements
All cases are now manually handled without a dependency to resolver-engine. Support for relative imports not starting with '.' is removed, unless they are relative to the project root, like 'contracts/foo/bar.sol'. Note that these are not supported for dependencies, only for project contracts. A new compiler setting, workingDir, is added. All relative imports not starting with '.' are resolved on this directory.
|
@ylv-io I'd appreciate a review! The PR is still in draft since I want to make a few more manual tests to see if everything is working fine. |
| public async call(): Promise<void> { | ||
| await this._loadSoliditySourcesFromDir(); | ||
| await this._loadDependencies(); | ||
| const roots = await this.loadProjectSoliditySources(); |
There was a problem hiding this comment.
Love removing _ and more explicit flow of data.
| private async loadProjectSoliditySources(): Promise<string[]> { | ||
| const dir = this.inputDir; | ||
| if (!existsSync(dir) || !lstatSync(dir).isDirectory) return []; | ||
| return promisify(glob)('**/*.sol', { cwd: dir, absolute: true }); |
|
|
||
| async function fileExists(file: string): Promise<boolean> { | ||
| try { | ||
| return (await promisify(fs.stat)(file)).isFile(); |
There was a problem hiding this comment.
Why we are so concerned about it being a file rather than just checking with pathExists?
There was a problem hiding this comment.
It could be a directory instead of a file
| ` | ||
| pragma solidity ^0.5.0; | ||
| import "./subfolder/GreeterLib.sol"; | ||
| import "contracts/subfolder/GreeterLib2.sol"; |
There was a problem hiding this comment.
Why is it GreeterLib2.sol? Shouldn't it be GreeterLib.sol?
There was a problem hiding this comment.
I wanted to test importing each with a different format from root
There was a problem hiding this comment.
Test name says compiles including more than one import to the **same file**. I don't see here two imports of the same file. What I am missing?
There was a problem hiding this comment.
The other ones were on contracts/subfolder/Greeter.sol:
Co-Authored-By: Igor Yalovoy <to@ylv.io>
This PR reworks how Solidity imports are resolved. It removes the dependency on resolver-engine for simplicity, and moves all logic to the
SourcesGatherermodule, and includes a few refactors along the way. I suggest reviewing commit by commit.The most important change is that only a subset of import formats are now supported:
/foo/bar/baz.sol../foo/bar.solcontracts/foo/bar.sol@openzeppelin/contracts/foo/bar.solNote that a path relative to the current project root is only supported for project contracts, not in a dependency. This is because we reserve the
contracts/foo/bar.solname for abar.solwithin the project, and we could have clashes if we also use it for a dependency. Dependencies should only use relative paths.This PR also ensures that there are no repeated artifact names. If there are, a warning is emitted, and only one of them is written to disk, to prevent corruption of the generated JSON files by concurrent writes.
Fixes #1404