Use solidity fuzzy import parser

[spalladino]

Increases performance of nothing-to-compile runs by using a custom fuzzy parser that only extracts imports from the text, instead of the full antlr4 parser.

Running both versions in the contracts repository, this yields a 40% speed increase when having nothing to compile.

This commit also lazy-loads truffle-flattener, which was causing significant delays to the CLI startup.

[spalladino]

We should wait until OpenZeppelin/fuzzy-import-parser#2 is implemented before merging

[nventuro]

Neat! 🙌

I took a look at the whole compiler execution flow looking for differences that may arise from switching to this new parser (which is somewhat lax and may yield false positives) and found two. One is a simple change to this PR's changeset, while the other is not as straightforward.

On ResolverEngineGatherer, gatherSources will call resolvePath on each imported path. and resolveImportFile will later try to find the file, erroring out if it doesn't find it (Could not find file X imported from Y). Because the fuzzy parser may yield false positives, this behaviour is no longer correct.

A reasonable course of action might be to emit a warning instead of an error and attempt to carry on, assuming the missing dependency is a false positive detection. If it is indeed a false positive Solidity will correctly identify the syntax error, leading to a happy user. On true positives and missing imports, we will emit a warning ourselves about a possible missing import, and Solidity will complain about an unresolved dependency: these two together should be enough for the user to figure out what's wrong.

For reference, this is the message emitted by Solidity when an import is not resolved.

contracts/mocks/Boolean.sol:3:1: ParserError: Source "Foo.sol" not found: File import callback not supported
import "Foo.sol";
^---------------^

We may want to reference the 'file import callback not supported' bit in our warning, since it is a red herring.

[spalladino]

Indeed. That's why I want to be able to report it and not silently fail.

…

On Thu, Nov 28, 2019 at 4:08 PM Nicolás Venturo ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In packages/cli/src/utils/solidity.ts <#1291 (comment)> : > @@ -35,15 +36,9 @@ export function compilerSettingsMatch(s1: CompilerVersionOptions, s2: CompilerVe } export function getImports(source: string): string[] { - // Copied from https://github.com/nomiclabs/buidler/blob/1cd52f91d7f8b6756c5ac33b78f93b151b072ea4/packages/buidler-core/src/internal/solidity/imports.ts - // eslint-disable-next-line @typescript-eslint/no-var-requires - const parser = require('solidity-parser-antlr'); - const ast = parser.parse(source, { tolerant: true }); - - const importedFiles: string[] = []; - parser.visit(ast, { - ImportDirective: (node: { path: string }) => importedFiles.push(node.path), - }); - - return importedFiles; + try { + return getMetadata(source).imports; + } catch { + return []; Wouldn't that be a bug in the parser? It shouldn't crash when fed valid Solidity source — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#1291?email_source=notifications&email_token=AADI4JHU6RYNH5F6N3QVZ6TQWAJLZA5CNFSM4JQWTOXKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCNLHYFQ#discussion_r351908225>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AADI4JEJTHCA3ISHYXCYVYDQWAJLZANCNFSM4JQWTOXA> .

[nventuro]

@spalladino Once CI passes, this should be ready to merge. I extracted the error handling out of getImports to make it more explicit, and added calls to trimURL whenever we report a problematic file location that may have been the result of buggy parsing.

[spalladino]

@nventuro given I'm the author, I cannot approve the PR. Please approve it and merge, I think it's ok!