Added a detailed description of `renameOverloadedMethods` (special thanks to @fabioberger). Updated Javascript styles in the Abi-Gen and Utils packages, around support for function overloading.

7 files changed, 67 insertions, 68 deletions

diff --git a/packages/abi-gen/src/index.ts b/packages/abi-gen/src/index.ts
index 9ceebe02b..ecef33b16 100644
--- a/packages/abi-gen/src/index.ts
+++ b/packages/abi-gen/src/index.ts
@@ -125,7 +125,7 @@ for (const abiFileName of abiFileNames) {
     }
 
     const methodAbis = ABI.filter((abi: AbiDefinition) => abi.type === ABI_TYPE_METHOD) as MethodAbi[];
-    const methodAbisSanitized = abiUtils.renameOverloadedMethods(methodAbis) as MethodAbi[];
+    const sanitizedMethodAbis = abiUtils.renameOverloadedMethods(methodAbis) as MethodAbi[];
     const methodsData = _.map(methodAbis, (methodAbi, methodAbiIndex: number) => {
         _.forEach(methodAbi.inputs, (input, inputIndex: number) => {
             if (_.isEmpty(input.name)) {
@@ -138,7 +138,7 @@ for (const abiFileName of abiFileNames) {
             ...methodAbi,
             singleReturnValue: methodAbi.outputs.length === 1,
             hasReturnValue: methodAbi.outputs.length !== 0,
-            tsName: methodAbisSanitized[methodAbiIndex].name,
+            tsName: sanitizedMethodAbis[methodAbiIndex].name,
             functionSignature: abiUtils.getFunctionSignature(methodAbi),
         };
         return methodData;
diff --git a/packages/base-contract/src/index.ts b/packages/base-contract/src/index.ts
index f6cea53fa..bfa99fac1 100644
--- a/packages/base-contract/src/index.ts
+++ b/packages/base-contract/src/index.ts
@@ -89,13 +89,10 @@ export class BaseContract {
         const methodAbis = this.abi.filter(
             (abiDefinition: AbiDefinition) => abiDefinition.type === AbiType.Function,
         ) as MethodAbi[];
-        this._ethersInterfacesByFunctionSignature = _.transform(
-            methodAbis,
-            (result: EthersInterfaceByFunctionSignature, methodAbi) => {
-                const functionSignature = abiUtils.getFunctionSignature(methodAbi);
-                result[functionSignature] = new ethersContracts.Interface([methodAbi]);
-            },
-            {},
-        );
+        this._ethersInterfacesByFunctionSignature = {};
+        _.each(methodAbis, methodAbi => {
+            const functionSignature = abiUtils.getFunctionSignature(methodAbi);
+            this._ethersInterfacesByFunctionSignature[functionSignature] = new ethersContracts.Interface([methodAbi]);
+        });
     }
 }
diff --git a/packages/deployer/src/cli.ts b/packages/deployer/src/cli.ts
index 3d69925a8..62afe0d4c 100644
--- a/packages/deployer/src/cli.ts
+++ b/packages/deployer/src/cli.ts
@@ -45,7 +45,7 @@ async function onDeployCommandAsync(argv: CliOptions): Promise<void> {
     const web3Wrapper = new Web3Wrapper(web3Provider);
     const networkId = await web3Wrapper.getNetworkIdAsync();
     const compilerOpts: CompilerOptions = {
-        contractDirs: getContractDirectoriesFromList(argv.contractsDir),
+        contractDirs: getContractDirectoriesFromList(argv.contractDirs),
         networkId,
         optimizerEnabled: argv.shouldOptimize,
         artifactsDir: argv.artifactsDir,
diff --git a/packages/deployer/src/compiler.ts b/packages/deployer/src/compiler.ts
index beaaab141..e3ecc6c72 100644
--- a/packages/deployer/src/compiler.ts
+++ b/packages/deployer/src/compiler.ts
@@ -26,7 +26,7 @@ import {
     CompilerOptions,
     ContractArtifact,
     ContractDirectory,
-    ContractIds,
+    ContractIdToSourceFileId,
     ContractNetworkData,
     ContractNetworks,
     ContractSourceDataByFileId,
@@ -75,6 +75,9 @@ export class Compiler {
                         encoding: 'utf8',
                     };
                     const source = await fsWrapper.readFileAsync(contentPath, opts);
+                    if (!_.startsWith(contentPath, contractBaseDir)) {
+                        throw new Error(`Expected content path '${contentPath}' to begin with '${contractBaseDir}'`);
+                    }
                     const sourceFilePath = contentPath.slice(contractBaseDir.length);
                     sources[sourceFilePath] = source;
                     logUtils.log(`Reading ${sourceFilePath} source...`);
@@ -114,7 +117,7 @@ export class Compiler {
         await createDirIfDoesNotExistAsync(this._artifactsDir);
         await createDirIfDoesNotExistAsync(SOLC_BIN_DIR);
         this._contractSources = {};
-        const contractIds: ContractIds = {};
+        const contractIdToSourceFileId: ContractIdToSourceFileId = {};
         const contractDirs = Array.from(this._contractDirs.values());
         for (const contractDir of contractDirs) {
             const sources = await Compiler._getContractSourcesAsync(contractDir.path, contractDir.path);
@@ -127,18 +130,20 @@ export class Compiler {
                 this._contractSources[sourceFileId] = source;
                 // Create a mapping between the contract id and its source file id
                 const contractId = constructContractId(contractDir.namespace, sourceFilePath);
-                if (!_.isUndefined(contractIds[contractId])) {
+                if (!_.isUndefined(contractIdToSourceFileId[contractId])) {
                     throw new Error(`Found duplicate contract with ID '${contractId}'`);
                 }
-                contractIds[contractId] = sourceFileId;
+                contractIdToSourceFileId[contractId] = sourceFileId;
             });
         }
         _.forIn(this._contractSources, this._setContractSpecificSourceData.bind(this));
         const specifiedContractIds = this._specifiedContracts.has(ALL_CONTRACTS_IDENTIFIER)
-            ? _.keys(contractIds)
+            ? _.keys(contractIdToSourceFileId)
             : Array.from(this._specifiedContracts.values());
         await Promise.all(
-            _.map(specifiedContractIds, async contractId => this._compileContractAsync(contractIds[contractId])),
+            _.map(specifiedContractIds, async contractId =>
+                this._compileContractAsync(contractIdToSourceFileId[contractId]),
+            ),
         );
     }
     /**
diff --git a/packages/deployer/src/utils/types.ts b/packages/deployer/src/utils/types.ts
index 08cab37b2..1a866b873 100644
--- a/packages/deployer/src/utils/types.ts
+++ b/packages/deployer/src/utils/types.ts
@@ -83,7 +83,7 @@ export interface ContractSources {
     [key: string]: string;
 }
 
-export interface ContractIds {
+export interface ContractIdToSourceFileId {
     [key: string]: string;
 }
 
diff --git a/packages/metacoin/test/metacoin_test.ts b/packages/metacoin/test/metacoin_test.ts
index 4a2307444..51830d1ef 100644
--- a/packages/metacoin/test/metacoin_test.ts
+++ b/packages/metacoin/test/metacoin_test.ts
@@ -36,12 +36,12 @@ describe('Metacoin', () => {
         });
     });
     describe('#transfer', () => {
-        it(`should successfully transfer tokens (via transfer_1)`, async () => {
+        it(`should successfully transfer tokens (via transfer1)`, async () => {
             const ZERO_ADDRESS = '0x0000000000000000000000000000000000000000';
             const amount = INITIAL_BALANCE.div(2);
             const oldBalance = await metacoin.balances.callAsync(ZERO_ADDRESS);
             expect(oldBalance).to.be.bignumber.equal(0);
-            const txHash = await metacoin.transfer_1.sendTransactionAsync(
+            const txHash = await metacoin.transfer1.sendTransactionAsync(
                 {
                     to: ZERO_ADDRESS,
                     amount,
@@ -59,13 +59,13 @@ describe('Metacoin', () => {
             expect(newBalance).to.be.bignumber.equal(amount);
         });
 
-        it(`should successfully transfer tokens (via transfer_2)`, async () => {
+        it(`should successfully transfer tokens (via transfer2)`, async () => {
             const ZERO_ADDRESS = '0x0000000000000000000000000000000000000000';
             const amount = INITIAL_BALANCE.div(2);
             const oldBalance = await metacoin.balances.callAsync(ZERO_ADDRESS);
             expect(oldBalance).to.be.bignumber.equal(0);
             const callback = 59;
-            const txHash = await metacoin.transfer_2.sendTransactionAsync(
+            const txHash = await metacoin.transfer2.sendTransactionAsync(
                 {
                     to: ZERO_ADDRESS,
                     amount,
@@ -84,13 +84,13 @@ describe('Metacoin', () => {
             expect(newBalance).to.be.bignumber.equal(amount);
         });
 
-        it(`should successfully transfer tokens (via transfer_3)`, async () => {
+        it(`should successfully transfer tokens (via transfer3)`, async () => {
             const ZERO_ADDRESS = '0x0000000000000000000000000000000000000000';
             const amount = INITIAL_BALANCE.div(2);
             const oldBalance = await metacoin.balances.callAsync(ZERO_ADDRESS);
             expect(oldBalance).to.be.bignumber.equal(0);
             const callback = 59;
-            const txHash = await metacoin.transfer_3.sendTransactionAsync(
+            const txHash = await metacoin.transfer3.sendTransactionAsync(
                 {
                     transferData: {
                         to: ZERO_ADDRESS,
diff --git a/packages/utils/src/abi_utils.ts b/packages/utils/src/abi_utils.ts
index 843b8589b..c4533d42e 100644
--- a/packages/utils/src/abi_utils.ts
+++ b/packages/utils/src/abi_utils.ts
@@ -12,63 +12,60 @@ export const abiUtils = {
         }
         return param.type;
     },
-    getFunctionSignature(abi: MethodAbi): string {
-        const functionName = abi.name;
-        const parameterTypeList = abi.inputs.map((param: DataItem) => this.parseFunctionParam(param));
+    getFunctionSignature(methodAbi: MethodAbi): string {
+        const functionName = methodAbi.name;
+        const parameterTypeList = _.map(methodAbi.inputs, (param: DataItem) => this.parseFunctionParam(param));
         const functionSignature = `${functionName}(${parameterTypeList})`;
         return functionSignature;
     },
+    /**
+     * Solidity supports function overloading whereas TypeScript does not.
+     * See: https://solidity.readthedocs.io/en/v0.4.21/contracts.html?highlight=overload#function-overloading
+     * In order to support overloaded functions, we suffix overloaded function names with an index.
+     * This index should be deterministic, regardless of function ordering within the smart contract. To do so,
+     * we assign indexes based on the alphabetical order of function signatures.
+     *
+     * E.g
+     * ['f(uint)', 'f(uint,byte32)']
+     * Should always be renamed to:
+     * ['f1(uint)', 'f2(uint,byte32)']
+     * Regardless of the order in which these these overloaded functions are declared within the contract ABI.
+     */
     renameOverloadedMethods(inputContractAbi: ContractAbi): ContractAbi {
         const contractAbi = _.cloneDeep(inputContractAbi);
         const methodAbis = contractAbi.filter((abi: AbiDefinition) => abi.type === AbiType.Function) as MethodAbi[];
-        const methodAbisByOriginalIndex = _.transform(
-            methodAbis,
-            (result: Array<{ index: number; methodAbi: MethodAbi }>, methodAbi, i: number) => {
-                result.push({ index: i, methodAbi });
-            },
-            [],
-        );
         // Sort method Abis into alphabetical order, by function signature
-        const methodAbisByOriginalIndexOrdered = _.sortBy(methodAbisByOriginalIndex, [
-            (entry: { index: number; methodAbi: MethodAbi }) => {
-                const functionSignature = this.getFunctionSignature(entry.methodAbi);
+        const methodAbisOrdered = _.sortBy(methodAbis, [
+            (methodAbi: MethodAbi) => {
+                const functionSignature = this.getFunctionSignature(methodAbi);
                 return functionSignature;
             },
         ]);
         // Group method Abis by name (overloaded methods will be grouped together, in alphabetical order)
-        const methodAbisByName = _.transform(
-            methodAbisByOriginalIndexOrdered,
-            (result: { [key: string]: Array<{ index: number; methodAbi: MethodAbi }> }, entry) => {
-                (result[entry.methodAbi.name] || (result[entry.methodAbi.name] = [])).push(entry);
-            },
-            {},
-        );
-        // Rename overloaded methods to overloadedMethoName_1, overloadedMethoName_2, ...
-        const methodAbisRenamed = _.transform(
-            methodAbisByName,
-            (result: MethodAbi[], methodAbisWithSameName: Array<{ index: number; methodAbi: MethodAbi }>) => {
-                _.forEach(methodAbisWithSameName, (entry, i: number) => {
-                    if (methodAbisWithSameName.length > 1) {
-                        const overloadedMethodId = i + 1;
-                        const sanitizedMethodName = `${entry.methodAbi.name}_${overloadedMethodId}`;
-                        const indexOfExistingAbiWithSanitizedMethodNameIfExists = _.findIndex(
-                            methodAbis,
-                            methodAbi => methodAbi.name === sanitizedMethodName,
+        const methodAbisByName: { [key: string]: MethodAbi[] } = {};
+        _.each(methodAbisOrdered, methodAbi => {
+            (methodAbisByName[methodAbi.name] || (methodAbisByName[methodAbi.name] = [])).push(methodAbi);
+        });
+        // Rename overloaded methods to overloadedMethodName1, overloadedMethodName2, ...
+        _.each(methodAbisByName, methodAbisWithSameName => {
+            _.each(methodAbisWithSameName, (methodAbi, i: number) => {
+                if (methodAbisWithSameName.length > 1) {
+                    const overloadedMethodId = i + 1;
+                    const sanitizedMethodName = `${methodAbi.name}${overloadedMethodId}`;
+                    const indexOfExistingAbiWithSanitizedMethodNameIfExists = _.findIndex(
+                        methodAbis,
+                        currentMethodAbi => currentMethodAbi.name === sanitizedMethodName,
+                    );
+                    if (indexOfExistingAbiWithSanitizedMethodNameIfExists >= 0) {
+                        const methodName = methodAbi.name;
+                        throw new Error(
+                            `Failed to rename overloaded method '${methodName}' to '${sanitizedMethodName}'. A method with this name already exists.`,
                         );
-                        if (indexOfExistingAbiWithSanitizedMethodNameIfExists >= 0) {
-                            const methodName = entry.methodAbi.name;
-                            throw new Error(
-                                `Failed to rename overloaded method '${methodName}' to '${sanitizedMethodName}'. A method with this name already exists.`,
-                            );
-                        }
-                        entry.methodAbi.name = sanitizedMethodName;
                     }
-                    // Add method to list of ABIs in its original position
-                    result.splice(entry.index, 0, entry.methodAbi);
-                });
-            },
-            [...Array(methodAbis.length)],
-        );
+                    methodAbi.name = sanitizedMethodName;
+                }
+            });
+        });
         return contractAbi;
     },
 };