fix(@embark/solc): Import remapping update

Changes include:
- Add unit tests for recursively remapping imports
- Handle plugin contracts
- Allow `prepareForCompilation` to be called from `File`. Allows external compilers, like `embark-solc` to call this.
- Add flattened remappings to file getting prepared for compilation
- Return remapped contract content when file type is http
- Add check allowing files to be remapped/prepared multiple times

packages/embark/src/lib/core/file.ts

@@ -1,4 +1,5 @@
 import * as path from "path";
+import { ImportRemapping, prepareForCompilation } from "../utils/solidity/remapImports";
 
 const fs = require("./fs.js");
 const utils = require("../utils/utils");
@@ -10,11 +11,6 @@ export enum Types {
   http = "http",
 }
 
-interface ImportRemapping {
-  prefix: string;
-  target: string;
-}
-
 export class File {
   public type: Types;
   public externalUrl: string = "";
@@ -30,22 +26,34 @@ export class File {
   constructor(options: any) {
     this.type = options.type;
 
-    this.basedir = options.basedir;
+    this.basedir = options.basedir || "";
     this.resolver = options.resolver;
     this.pluginPath = options.pluginPath ? options.pluginPath : "";
     this.storageConfig = options.storageConfig;
     this.providerUrl = "";
     this.originalPath = options.originalPath || "";
 
-    if (this.type === Types.http) {
+    if (this.type === Types.custom && this.pluginPath) {
+      this.path = path.join(this.pluginPath, options.path).replace(fs.dappPath(), "");
+      if (this.path.startsWith("/")) {
+        this.path = this.path.substring(1);
+      }
+    } else if (this.type === Types.http) {
       const external = utils.getExternalContractUrl(options.externalUrl, this.providerUrl);
       this.externalUrl = external.url;
-      this.path = external.filePath;
+      this.path = fs.dappPath(external.filePath);
     } else {
       this.path = options.path.replace(/\\/g, "/");
     }
   }
 
+  public async prepareForCompilation(isCoverage = false) {
+    if (!this.path.endsWith(".sol")) {
+      return Promise.reject("this method is only supported for Solidity files");
+    }
+    return prepareForCompilation(this, isCoverage);
+  }
+
   public get content(): Promise<string> {
     return new Promise<string>((resolve) => {
       switch (this.type) {

packages/embark/src/lib/modules/pipeline/index.js

@@ -292,7 +292,7 @@ class Pipeline {
                 self.logger.trace("reading " + file.path);
                 file.content.then((fileContent) => {
                   self.runPlugins(file, fileContent, fileCb);
-                });
+                }).catch(fileCb);
               },
               function (err, contentFiles) {
                 if (err) {

packages/embark/src/lib/modules/solidity/index.js

@@ -1,6 +1,5 @@
 let async = require('../../utils/async_extend.js');
 let SolcW = require('./solcW.js');
-const remapImports = require('../../utils/solidity/remapImports');
 
 class Solidity {
 
@@ -177,7 +176,7 @@ class Solidity {
 
             originalFilepath[filename] = file.path;
             
-            remapImports.prepareForCompilation(file, options.isCoverage)
+            file.prepareForCompilation(options.isCoverage)
               .then(fileContent => {
                 input[file.path] = {content: fileContent.replace(/\r\n/g, '\n')};
                 fileCb();

packages/embark/src/lib/modules/solidity/solcP.js

@@ -1,8 +1,5 @@
 const fs = require('fs-extra');
-const path = require('path');
 const semver = require('semver');
-const constants = require('../../constants');
-const Utils = require('../../utils/utils');
 const ProcessWrapper = require('../../core/processes/processWrapper');
 const PluginManager = require('live-plugin-manager-git-fix').PluginManager;
 import LongRunningProcessTimer  from '../../utils/longRunningProcessTimer';
@@ -17,19 +14,9 @@ class SolcProcess extends ProcessWrapper {
   }
 
   findImports(filename) {
-    if (filename.startsWith('http') || filename.startsWith('git')) {
-      const fileObj = Utils.getExternalContractUrl(filename, this._providerUrl);
-      filename = fileObj.filePath;
-    }
     if (fs.existsSync(filename)) {
       return {contents: fs.readFileSync(filename).toString()};
     }
-    if (fs.existsSync(path.join('./node_modules/', filename))) {
-      return {contents: fs.readFileSync(path.join('./node_modules/', filename)).toString()};
-    }
-    if (fs.existsSync(path.join(constants.httpContractsDirectory, filename))) {
-      return {contents: fs.readFileSync(path.join(constants.httpContractsDirectory, filename)).toString()};
-    }
     return {error: 'File not found'};
   }
 

packages/embark/src/lib/utils/solidity/remapImports.ts

@@ -8,10 +8,14 @@ const fs = require("../../core/fs");
 const FIND_IMPORTS_REGEX = /^import[\s]*(['"])(.*)\1;/gm;
 const FIND_FILE_REGEX = /import[\s]*(['"])(.*)\1;/;
 
+export interface ImportRemapping {
+  prefix: string;
+  target: string;
+}
+
 interface RemapImport {
   path: string;
-  searchValue: string;
+  remapping: ImportRemapping;
-  replaceValue: string;
 }
 
 const getImports = (source: string) => {
@@ -33,14 +37,10 @@ const prepareInitialFile = async (file: File) => {
   }
 
   const destination = fs.dappPath(".embark", file.path);
-  if (file.type === Types.dappFile) {
+  if (file.type === Types.dappFile || file.type === Types.custom) {
     fs.copySync(fs.dappPath(file.path), destination);
   }
 
-  if (file.type === Types.custom) {
-    fs.writeFileSync(destination);
-  }
-
   file.path = destination;
 };
 
@@ -101,8 +101,9 @@ const rescursivelyFindRemapImports = async (file: File, filesProcessed: string[]
 
   for (const importPath of imports) {
     const newFile = buildNewFile(file, importPath);
-    file.importRemappings.push({prefix: importPath, target: newFile.path});
+    const remapping = { prefix: importPath, target: newFile.path };
-    remapImports.push({path: file.path, searchValue: importPath, replaceValue: newFile.path});
+    file.importRemappings.push(remapping);
+    remapImports.push({ path: file.path, remapping });
     remapImports = remapImports.concat(
       await rescursivelyFindRemapImports(newFile, filesProcessed),
     );
@@ -124,30 +125,55 @@ const isHttp = (input: string) => {
 };
 
 const replaceImports = (remapImports: RemapImport[]) => {
-  const byPath: {[path: string]: [{searchValue: string, replaceValue: string}]} = groupBy(remapImports, "path");
+  const byPath: { [path: string]: [{ remapping: ImportRemapping }] } = groupBy(remapImports, "path");
   Object.keys(byPath).forEach((p) => {
     let source = fs.readFileSync(p, "utf-8");
-    byPath[p].forEach(({searchValue, replaceValue}) => {
+    byPath[p].forEach(({ remapping }) => {
-      source = source.replace(`import "${searchValue}"`, `import "${replaceValue}"`);
+      source = source.replace(`import "${remapping.prefix}"`, `import "${remapping.target}"`);
     });
     fs.writeFileSync(p, source);
   });
 };
 
+const addRemappingsToFile = (file: File, remapImports: RemapImport[]) => {
+  const byPath: { [path: string]: [{ remapping: ImportRemapping }] } = groupBy(remapImports, "path");
+  const paths = Object.keys(byPath);
+  if (paths) {
+    file.importRemappings = []; // clear as we already have the first remapping added
+    paths.forEach((p) => {
+      const [...remappings] = byPath[p].map((importRemapping) => importRemapping.remapping);
+      file.importRemappings = file.importRemappings.concat(remappings);
+    });
+  }
+};
+
 const resolve = (input: string) => {
   try {
-    return require.resolve(input, {paths: [fs.dappPath("node_modules")]});
+    const result = require.resolve(input, { paths: [fs.dappPath("node_modules"), fs.embarkPath("node_modules")] });
+    return result;
   } catch (e) {
     return "";
   }
 };
 
 export const prepareForCompilation = async (file: File, isCoverage = false) => {
-  await prepareInitialFile(file);
+  if (!file.isPrepared) {
-  const remapImports = await rescursivelyFindRemapImports(file);
+    await prepareInitialFile(file);
-  replaceImports(remapImports);
+    const remapImports = await rescursivelyFindRemapImports(file);
+    replaceImports(remapImports);
+    // add all remappings to top-level file
+    addRemappingsToFile(file, remapImports);
+
+    // set flag to prevent copying, remapping, and changing of paths again
+    file.isPrepared = true;
+  }
+  let content;
+  if (file.type === Types.http) {
+    content = (await fs.readFile(file.path)).toString();
+  } else {
+    content = await file.content;
+  }
 
-  const content = await file.content;
   if (!isCoverage) {
     return content;
   }

packages/embark/src/test/config.js

@@ -4,6 +4,7 @@ const Plugins = require('../lib/core/plugins.js');
 const assert = require('assert');
 const TestLogger = require('../lib/utils/test_logger');
 const Events = require('../lib/core/events');
+const fs = require('../lib/core/fs');
 
 describe('embark.Config', function () {
   let config = new Config({
@@ -190,12 +191,13 @@ describe('embark.Config', function () {
       const expected = [
         {
           "type": "http",
-          "externalUrl": "https://raw.githubusercontent.com/embark-framework/embark/master/test_dapps/test_app/app/contracts/simple_storage.sol",
+          "externalUrl": "https://raw.githubusercontent.com/embark-framework/embark/master/test_dapps/packages/test_app/app/contracts/simple_storage.sol",
-          "path": ".embark/contracts/embark-framework/embark/master/test_dapps/test_app/app/contracts/simple_storage.sol",
+          "path": fs.dappPath(".embark/contracts/embark-framework/embark/master/test_dapps/packages/test_app/app/contracts/simple_storage.sol"),
-          "originalPath": ".embark/contracts/embark-framework/embark/master/test_dapps/test_app/app/contracts/simple_storage.sol",
+          "originalPath": ".embark/contracts/embark-framework/embark/master/test_dapps/packages/test_app/app/contracts/simple_storage.sol",
           "pluginPath": '',
           "basedir": "",
           "importRemappings": [],
+          "isPrepared": false,
           "resolver": undefined,
           "storageConfig": undefined,
           "providerUrl": ""
@@ -203,23 +205,25 @@ describe('embark.Config', function () {
         {
           "type": "http",
           "externalUrl": "https://raw.githubusercontent.com/status-im/contracts/master/contracts/identity/ERC725.sol",
-          "path": ".embark/contracts/status-im/contracts/master/contracts/identity/ERC725.sol",
+          "path": fs.dappPath(".embark/contracts/status-im/contracts/master/contracts/identity/ERC725.sol"),
           "originalPath": ".embark/contracts/status-im/contracts/master/contracts/identity/ERC725.sol",
           "pluginPath": '',
           "basedir": "",
           "importRemappings": [],
+          "isPrepared": false,
           "resolver": undefined,
           "storageConfig": undefined,
           "providerUrl": ""
         },
         {
           "externalUrl": "https://swarm-gateways.net/bzz:/1ffe993abc835f480f688d07ad75ad1dbdbd1ddb368a08b7ed4d3e400771dd63",
-          "path": ".embark/contracts/bzz:/1ffe993abc835f480f688d07ad75ad1dbdbd1ddb368a08b7ed4d3e400771dd63",
+          "path": fs.dappPath(".embark/contracts/bzz:/1ffe993abc835f480f688d07ad75ad1dbdbd1ddb368a08b7ed4d3e400771dd63"),
           "originalPath": ".embark/contracts/bzz:/1ffe993abc835f480f688d07ad75ad1dbdbd1ddb368a08b7ed4d3e400771dd63",
           "type": "http",
           "pluginPath": '',
           "basedir": "",
           "importRemappings": [],
+          "isPrepared": false,
           "resolver": undefined,
           "storageConfig": undefined,
           "providerUrl": ""

packages/embark/src/test/file.js

@@ -0,0 +1,42 @@
+/*globals describe, it, before*/
+const {File, Types} = require("../lib/core/file");
+const path = require("path");
+const {expect, assert} = require("chai");
+const fs = require("../lib/core/fs");
+const fsNode = require("fs");
+
+describe('embark.File', function () {
+  describe('Read file contents', function () {
+    it('should be able to download a file when type is "http"', async () => {
+      const file = new File({externalUrl: 'https://raw.githubusercontent.com/embark-framework/embark/master/test_apps/test_app/app/contracts/simple_storage.sol', type: Types.http});
+      const content = await file.content;
+
+      const contentFromFileSystem = fsNode.readFileSync(path.join(fs.embarkPath(), "../../", "test_dapps/packages/test_app/app/contracts/simple_storage.sol")).toString();
+      expect(content).to.equal(contentFromFileSystem);
+    });
+
+    it('should be able to read a file when type is "dappFile"', async () => {
+      const file = new File({path: fs.dappPath('contracts/recursive_test_0.sol'), type: Types.dappFile});
+      const content = await file.content;
+
+      const contentFromFileSystem = fs.readFileSync(fs.dappPath("contracts/recursive_test_0.sol")).toString();
+      expect(content).to.equal(contentFromFileSystem);
+    });
+
+    it('should be able to execute a resolver when type is "custom"', async () => {
+      const file = new File({path: fs.dappPath('contracts/recursive_test_0.sol'), type: Types.custom, resolver: (callback) => {
+        callback("test");
+      }});
+      expect(await file.content).to.equal("test");
+    });
+
+    it('should be able to read a file when type is "embarkInternal"', async () => {
+      const file = new File({path: 'test/contracts/recursive_test_0.sol', type: Types.embarkInternal});
+      const content = await file.content;
+
+      const contentFromFileSystem = fs.readFileSync(fs.dappPath("contracts/recursive_test_0.sol")).toString();
+      expect(content).to.equal(contentFromFileSystem);
+    });
+
+  });
+});

packages/embark/src/test/modules/solidity/remapImports.js

@@ -0,0 +1,105 @@
+/*globals describe, it, before*/
+const {File, Types} = require("../../../lib/core/file");
+const path = require("path");
+const remapImports = require("../../../lib/utils/solidity/remapImports");
+const {expect} = require("chai");
+const fs = require("../../../lib/core/fs");
+const fsNode = require("fs");
+
+let file, content;
+
+describe('embark.RemapImports', function () {
+  describe('Import remappings from local file', function () {
+    before('do the remappings', async () => {
+      file = new File({path: 'contracts/recursive_test_0.sol', type: Types.dappFile});
+      content = await remapImports.prepareForCompilation(file);
+    });
+
+    it("should find and add remappings for all recursive imports", (done) => {
+      expect(file.importRemappings[0]).to.deep.equal({
+        prefix: "./recursive_test_1.sol",
+        target: fs.dappPath(".embark/contracts/recursive_test_1.sol")
+      });
+      expect(file.importRemappings[1]).to.deep.equal({
+        prefix: "./recursive_test_2.sol",
+        target: fs.dappPath(".embark/contracts/recursive_test_2.sol")
+      });
+      expect(file.importRemappings[2]).to.deep.equal({
+        prefix: "embark-test-contract-0/recursive_test_3.sol",
+        target: fs.dappPath(".embark/node_modules/embark-test-contract-0/recursive_test_3.sol")
+      });
+      expect(file.importRemappings[3]).to.deep.equal({
+        prefix: "embark-test-contract-1/recursive_test_4.sol",
+        target: fs.dappPath(".embark/node_modules/embark-test-contract-1/recursive_test_4.sol")
+      });
+      done();
+    });
+
+    it("should update the contract content to use the remapped imports", function (done) {
+      expect(content).to.not.contain("./recursive_test_1.sol");
+      expect(content).to.contain(".embark/contracts/recursive_test_1.sol");
+
+      let contractFromFilesystem = fsNode.readFileSync(fs.dappPath(".embark/contracts/recursive_test_0.sol")).toString();
+      expect(contractFromFilesystem).to.not.contain("./recursive_test_1.sol");
+      expect(contractFromFilesystem).to.contain(".embark/contracts/recursive_test_1.sol");
+      
+      contractFromFilesystem = fsNode.readFileSync(fs.dappPath(".embark/contracts/recursive_test_1.sol")).toString();
+      expect(contractFromFilesystem).to.not.contain("./recursive_test_2.sol");
+      expect(contractFromFilesystem).to.contain(".embark/contracts/recursive_test_2.sol");
+
+      contractFromFilesystem = fsNode.readFileSync(fs.dappPath(".embark/contracts/recursive_test_2.sol")).toString();
+      expect(contractFromFilesystem).to.not.contain("import \"embark-test-contract-0/recursive_test_3.sol\"");
+      expect(contractFromFilesystem).to.contain(`import "${fs.dappPath(".embark/node_modules/embark-test-contract-0/recursive_test_3.sol")}"`);
+      
+      contractFromFilesystem = fsNode.readFileSync(fs.dappPath(".embark/node_modules/embark-test-contract-0/recursive_test_3.sol")).toString();
+      expect(contractFromFilesystem).to.not.contain("import \"embark-test-contract-1/recursive_test_4.sol\"");
+      expect(contractFromFilesystem).to.contain(`import "${fs.dappPath(".embark/node_modules/embark-test-contract-1/recursive_test_4.sol")}"`);
+      
+      done();
+    });
+
+  });
+
+  describe('Import remappings from external URL', function () {
+    before('do the external HTTP contract remappings', async () => {
+      file = new File({externalUrl: 'https://github.com/embark-framework/embark/blob/master/src/test/contracts/recursive_test_0.sol', type: Types.http});
+      content = await remapImports.prepareForCompilation(file);
+    });
+
+    it("should find and add remappings for all recursive imports", (done) => {
+      expect(file.importRemappings[0]).to.deep.equal({
+        prefix: "./recursive_test_1.sol",
+        target: fs.dappPath(".embark/contracts/embark-framework/embark/master/src/test/contracts/recursive_test_1.sol")
+      });
+      expect(file.importRemappings[1]).to.deep.equal({
+        prefix: "./recursive_test_2.sol",
+        target: fs.dappPath(".embark/contracts/embark-framework/embark/master/src/test/contracts/recursive_test_2.sol")
+      });
+      expect(file.importRemappings[2]).to.deep.equal({
+        prefix: "embark-test-contract-0/recursive_test_3.sol",
+        target: fs.dappPath(".embark/contracts/embark-framework/embark/master/src/test/contracts/embark-test-contract-0/recursive_test_3.sol")
+      });
+      done();
+    });
+
+    it("should update the contract content to use the remapped imports", function (done) {
+      expect(content).to.not.contain("./recursive_test_1.sol");
+      expect(content).to.contain(".embark/contracts/embark-framework/embark/master/src/test/contracts/recursive_test_1.sol");
+
+      let contractFromFilesystem = fsNode.readFileSync(fs.dappPath(".embark/contracts/embark-framework/embark/master/src/test/contracts/recursive_test_0.sol")).toString();
+      expect(contractFromFilesystem).to.not.contain("./recursive_test_1.sol");
+      expect(contractFromFilesystem).to.contain(".embark/contracts/embark-framework/embark/master/src/test/contracts/recursive_test_1.sol");
+      
+      contractFromFilesystem = fsNode.readFileSync(fs.dappPath(".embark/contracts/embark-framework/embark/master/src/test/contracts/recursive_test_1.sol")).toString();
+      expect(contractFromFilesystem).to.not.contain("./recursive_test_2.sol");
+      expect(contractFromFilesystem).to.contain(".embark/contracts/embark-framework/embark/master/src/test/contracts/recursive_test_2.sol");
+
+      contractFromFilesystem = fsNode.readFileSync(fs.dappPath(".embark/contracts/embark-framework/embark/master/src/test/contracts/recursive_test_2.sol")).toString();
+      expect(contractFromFilesystem).to.not.contain("import \"embark-test-contract-0/recursive_test_3.sol\"");
+      expect(contractFromFilesystem).to.contain(`import "${fs.dappPath(".embark/contracts/embark-framework/embark/master/src/test/contracts/embark-test-contract-0/recursive_test_3.sol")}"`);
+
+      done();
+    });
+  });
+
+});