Use solidity fuzzy import parser

packages/cli/package.json

@@ -56,6 +56,7 @@
   },
   "homepage": "https://github.com/OpenZeppelin/openzeppelin-sdk/tree/master/packages/cli#readme",
   "dependencies": {
+    "@openzeppelin/fuzzy-solidity-import-parser": "^0.1.0",
     "@openzeppelin/resolver-engine-core": "^0.3.3",
     "@openzeppelin/resolver-engine-imports-fs": "^0.3.3",
     "@openzeppelin/test-environment": "^0.1.0",
@@ -119,7 +120,6 @@
     "semver": "^5.5.0",
     "simple-git": "^1.110.0",
     "solc-wrapper": "^0.5.8",
-    "solidity-parser-antlr": "^0.4.2",
     "spinnies": "^0.3.0",
     "toposort": "^2.0.2",
     "truffle-config": "1.1.16",

packages/cli/src/models/compiler/solidity/ResolverEngineGatherer.ts

@@ -1,4 +1,5 @@
 import { ResolverEngine } from '@openzeppelin/resolver-engine-core';
+import { Loggy } from '@openzeppelin/upgrades';
 import pathSys from 'path';
 import urlSys from 'url';
 import { getImports } from '../../../utils/solidity';
@@ -13,72 +14,6 @@ interface ImportFile {
   provider: string;
 }
 
-interface ImportTreeNode extends ImportFile {
-  uri: string;
-  // uri and url the same as in rest of resolver-engine
-  // it might mean github:user/repo/path.sol and raw link
-  // or it might mean relative vs absolute file path
-  imports: { uri: string; url: string }[];
-}
-
-/**
- * This function accepts root files to be searched for and resolves the sources, finds the imports in each source and traverses the whole dependency tree gathering absolute and uri paths
- * @param roots
- * @param workingDir
- * @param resolver
- */
-async function gatherDependencyTree(
-  roots: string[],
-  workingDir: string,
-  resolver: ResolverEngine<ImportFile>,
-): Promise<ImportTreeNode[]> {
-  const result: ImportTreeNode[] = [];
-  const alreadyImported = new Set();
-
-  /**
-   * This function traverses the depedency tree and calculates absolute paths for each import on the way storing each file in in a global array
-   * @param file File in a depedency that should now be traversed
-   * @returns An absolute path for the requested file
-   */
-  async function dfs(file: { searchCwd: string; uri: string }): Promise<string> {
-    const url = await resolver.resolve(file.uri, file.searchCwd);
-    if (alreadyImported.has(url)) {
-      return url;
-    }
-
-    const resolvedFile = await resolver.require(file.uri, file.searchCwd);
-
-    alreadyImported.add(url);
-
-    const foundImportURIs = getImports(resolvedFile.source);
-
-    const fileNode: ImportTreeNode = {
-      uri: file.uri,
-      imports: [],
-      ...resolvedFile,
-    };
-
-    const resolvedCwd = pathSys.dirname(url);
-    for (const importUri of foundImportURIs) {
-      const importUrl = await dfs({ searchCwd: resolvedCwd, uri: importUri });
-      fileNode.imports.push({ uri: importUri, url: importUrl });
-    }
-
-    result.push(fileNode);
-    return resolvedFile.url;
-  }
-
-  await Promise.all(roots.map(what => dfs({ searchCwd: workingDir, uri: what })));
-
-  return result;
-}
-
-function stripNodes(nodes: ImportTreeNode[]): ImportFile[] {
-  return nodes.map(node => {
-    return { url: node.url, source: node.source, provider: node.provider };
-  });
-}
-
 /**
  * Starts with roots and traverses the whole depedency tree of imports, returning an array of sources
  * @param roots
@@ -106,8 +41,18 @@ export async function gatherSources(
   while (queue.length > 0) {
     // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
     const fileData = queue.shift()!;
-    const resolvedFile: ImportFile = await resolveImportFile(resolver, fileData);
-    const foundImports = getImports(resolvedFile.source);
+
+    let resolvedFile: ImportFile;
+    try {
+      resolvedFile = await resolveImportFile(resolver, fileData);
+    } catch (err) {
+      // Our custom fuzzy parser may yield false positives, potentially asking for imports that don't exist. This can
+      // happen both due to parser bugs and to invalid Solidity sources the parser accepts. Because of this, we carry on
+      // to the compiler instead of erroring out: if an import is indeed missing or the code is incorrect, the compiler
+      // will complain about this with a more accurate error message.
+      Loggy.noSpin.warn(__filename, 'gatherSources', 'compile-warnings', `${err.message}`);
+      continue;
+    }
 
     // if imported path starts with '.' we assume it's relative and return it's
     // path relative to resolved name of the file that imported it
@@ -129,7 +74,27 @@ export async function gatherSources(
       });
     }
 
+    let foundImports: string[];
+    try {
+      foundImports = getImports(resolvedFile.source);
+    } catch {
+      // The are two reasons why the parser may crash:
+      //  - the source is not valid Solidity code
+      //  - the parser has a bug
+      // Invalid source will be better diagnosed by the compiler, meaning we shouldn't halt execution so that it gets a
+      // chance to inspect the source. A buggy parser will produce false negatives, but since we're not able to detect
+      // that here, it makes more sense to fail loudly, hopefully leading to a bug report by a user.
+      Loggy.noSpin.warn(
+        __filename,
+        'gatherSources',
+        'solidity-parser-warnings',
+        `Error while parsing ${trimURL(resolvedFile.url)}`,
+      );
+      foundImports = [];
+    }
+
     const fileParentDir = pathSys.dirname(resolvedFile.url);
+
     for (const foundImport of foundImports) {
       let importName: string;
       // If it's relative, resolve it; otherwise, pass through
@@ -188,33 +153,17 @@ async function resolveImportFile(
           cwd: process.cwd(),
         });
       }
+
       const cwd = pathSys.relative(process.cwd(), fileData.cwd);
       const cwdDesc = cwd.length === 0 ? 'the project' : `folder ${cwd}`;
       const relativeTo = pathSys.relative(process.cwd(), fileData.relativeTo);
-      err.message = `Could not find file ${fileData.file} in ${cwdDesc} (imported from ${relativeTo})`;
+      err.message = `Could not find file ${trimURL(fileData.file)} in ${cwdDesc} (imported from ${relativeTo})`;
     }
+
     throw err;
   }
 }
 
-/**
- * This function gathers sources and **REWRITES IMPORTS** inside the source files into resolved, absolute paths instead of using shortcut forms
- * Because the remapping api in solc is not compatible with multiple existing projects and frameworks, changing relative paths to absolute paths
- * makes us avoid any need for finding imports after starting the solc compilation
- * @param roots
- * @param workingDir What's the starting working dir for resolving relative imports in roots
- * @param resolver
- */
-export async function gatherSourcesAndCanonizeImports(
-  roots: string[],
-  workingDir: string,
-  resolver: ResolverEngine<ImportFile>,
-): Promise<ImportFile[]> {
-  function canonizeFile(file: ImportTreeNode) {
-    file.imports.forEach(i => (file.source = file.source.replace(i.uri, i.url)));
-  }
-
-  const sources = await gatherDependencyTree(roots, workingDir, resolver);
-  sources.forEach(canonizeFile);
-  return stripNodes(sources);
+function trimURL(url: string): string {
+  return url.length < 40 ? url : url.substring(0, 40) + '...';
 }

packages/cli/src/models/compiler/solidity/SolidityProjectCompiler.ts

@@ -159,7 +159,7 @@ class SolidityProjectCompiler {
     return (
       !maxArtifactsMtimes ||
       !maxSourcesMtimes ||
-      maxArtifactsMtimes < maxSourcesMtimes ||
+      maxArtifactsMtimes <= maxSourcesMtimes ||
       !artifactCompiledVersion ||
       !compilerVersionsMatch(artifactCompiledVersion, this.compilerVersion.longVersion) ||
       !compilerSettingsMatch(currentSettings, artifactSettings)

packages/cli/src/utils/solidity.ts

@@ -1,5 +1,6 @@
 import { eq as semverEq, parse as parseSemver } from 'semver';
 import { CompilerVersionOptions } from '../models/compiler/solidity/SolidityContractsCompiler';
+import { getMetadata } from '@openzeppelin/fuzzy-solidity-import-parser';
 
 export function getPragma(source: string): string {
   if (!source) return null;
@@ -35,15 +36,5 @@ 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;
+  return getMetadata(source).imports;
 }

packages/cli/test/models/compiler/solidity/SolidityProjectCompiler.test.js

@@ -1,9 +1,10 @@
 require('../../../setup');
 
-import { FileSystem } from '@openzeppelin/upgrades';
+import { Loggy, FileSystem } from '@openzeppelin/upgrades';
 import { compileProject } from '../../../../src/models/compiler/solidity/SolidityProjectCompiler';
 import { unlinkSync, existsSync, statSync, utimesSync, writeFileSync } from 'fs';
 import path from 'path';
+import sinon from 'sinon';
 import { writeJSONSync, readJSONSync } from 'fs-extra';
 
 describe('SolidityProjectCompiler', function() {
@@ -93,9 +94,18 @@ describe('SolidityProjectCompiler', function() {
       schema.compiler.optimizer.should.be.deep.equal(optimizer);
     });
 
-    it('outputs friendly error on invalid import', async function() {
+    it('outputs friendly warning and solc error on invalid import', async function() {
+      const logWarnStub = sinon.stub(Loggy.noSpin, 'warn');
+
       writeFileSync(`${inputDir}/Invalid.sol`, 'pragma solidity ^0.5.0; import "./NotExists.sol";');
-      await compileProject({ inputDir, outputDir }).should.be.rejectedWith(/could not find file \.\/NotExists\.sol/i);
+      await compileProject({ inputDir, outputDir }).should.be.rejectedWith(/file import callback not supported/i);
+
+      logWarnStub.calledOnce.should.equal(true);
+      logWarnStub.firstCall.args.should.include(
+        'Could not find file ./NotExists.sol in folder test/mocks/mock-stdlib/contracts (imported from test/mocks/mock-stdlib/contracts/Invalid.sol)',
+      );
+
+      sinon.restore();
     });
 
     // For more info, see: https://github.com/zeppelinos/zos/issues/1071

packages/lib/src/utils/Solidity.ts

@@ -1,5 +1,3 @@
-import flatten from 'truffle-flattener';
-
 export function flattenSourceCode(contractPaths: string[], root = process.cwd()): Promise<any> {
-  return flatten(contractPaths, root);
+  return require('truffle-flattener')(contractPaths, root);
 }