Address feedback

5 files changed, 54 insertions, 49 deletions

diff --git a/packages/deployer/CHANGELOG.md b/packages/deployer/CHANGELOG.md
index f1e9d38ee..a63d9cf3b 100644
--- a/packages/deployer/CHANGELOG.md
+++ b/packages/deployer/CHANGELOG.md
@@ -1,8 +1,8 @@
 # CHANGELOG
 
-## v0.2.0 - _??_
+## v0.2.0 - _TBD, 2018_
 
-    * Check dependencies when determining if contracts should be recompiled.
+    * Check dependencies when determining if contracts should be recompiled (#408).
 
 ## v0.1.0 - _February 16, 2018_
 
diff --git a/packages/deployer/src/cli.ts b/packages/deployer/src/cli.ts
index b093dd71b..ba156ac20 100644
--- a/packages/deployer/src/cli.ts
+++ b/packages/deployer/src/cli.ts
@@ -16,6 +16,7 @@ const DEFAULT_NETWORK_ID = 50;
 const DEFAULT_JSONRPC_PORT = 8545;
 const DEFAULT_GAS_PRICE = (10 ** 9 * 2).toString();
 const DEFAULT_CONTRACTS_LIST = '*';
+
 /**
  * Compiles all contracts with options passed in through CLI.
  * @param argv Instance of process.argv provided by yargs.
diff --git a/packages/deployer/src/compiler.ts b/packages/deployer/src/compiler.ts
index 783bc0ea3..149ca5d6d 100644
--- a/packages/deployer/src/compiler.ts
+++ b/packages/deployer/src/compiler.ts
@@ -20,17 +20,17 @@ import {
 import { utils } from './utils/utils';
 
 const ALL_CONTRACTS_IDENTIFIER = '*';
-const SOLIDITY_VERSION_REGEX = /(?:solidity\s\^?)([0-9]{1,2}[.][0-9]{1,2}[.][0-9]{1,2})/;
+const SOLIDITY_VERSION_REGEX = /(?:solidity\s\^?)(\d+\.\d+\.\d+)/;
 const SOLIDITY_FILE_EXTENSION_REGEX = /(.*\.sol)/;
 const IMPORT_REGEX = /(import\s)/;
-const DEPENDENCY_PATH_REGEX = /"([^"]+)"/;
+const DEPENDENCY_PATH_REGEX = /"([^"]+)"/; // Source: https://github.com/BlockChainCompany/soljitsu/blob/master/lib/shared.js
 
 export class Compiler {
     private _contractsDir: string;
     private _networkId: number;
     private _optimizerEnabled: number;
     private _artifactsDir: string;
-    private _contractSourcesIfExists?: ContractSources;
+    private _contractSources?: ContractSources;
     private _solcErrors: Set<string> = new Set();
     private _specifiedContracts: Set<string> = new Set();
     private _contractSourceData: ContractSourceData = {};
@@ -82,10 +82,10 @@ export class Compiler {
     private static _getContractSpecificSourceData(source: string): ContractSpecificSourceData {
         const dependencies: string[] = [];
         const sourceHash = ethUtil.sha3(source);
-        const solc_version = Compiler._parseSolidityVersion(source);
+        const solcVersion = Compiler._parseSolidityVersion(source);
         const contractSpecificSourceData: ContractSpecificSourceData = {
             dependencies,
-            solc_version,
+            solcVersion,
             sourceHash,
         };
         const lines = source.split('\n');
@@ -149,12 +149,12 @@ export class Compiler {
      */
     public async compileAllAsync(): Promise<void> {
         await this._createArtifactsDirIfDoesNotExistAsync();
-        this._contractSourcesIfExists = await Compiler._getContractSourcesAsync(this._contractsDir);
-        _.forIn(this._contractSourcesIfExists, (source, fileName) => {
+        this._contractSources = await Compiler._getContractSourcesAsync(this._contractsDir);
+        _.forIn(this._contractSources, (source, fileName) => {
             this._contractSourceData[fileName] = Compiler._getContractSpecificSourceData(source);
         });
         const fileNames = this._specifiedContracts.has(ALL_CONTRACTS_IDENTIFIER)
-            ? _.keys(this._contractSourcesIfExists)
+            ? _.keys(this._contractSources)
             : Array.from(this._specifiedContracts.values());
         _.forEach(fileNames, fileName => {
             this._setSourceTreeHash(fileName);
@@ -169,29 +169,29 @@ export class Compiler {
      * @param fileName Name of contract with '.sol' extension.
      */
     private async _compileContractAsync(fileName: string): Promise<void> {
-        if (_.isUndefined(this._contractSourcesIfExists)) {
+        if (_.isUndefined(this._contractSources)) {
             throw new Error('Contract sources not yet initialized');
         }
         const contractSpecificSourceData = this._contractSourceData[fileName];
-        const currentArtifact = (await this._getContractArtifactOrReturnAsync(fileName)) as ContractArtifact;
+        const currentArtifactIfExists = (await this._getContractArtifactIfExistsAsync(fileName)) as ContractArtifact;
         const sourceHash = `0x${contractSpecificSourceData.sourceHash.toString('hex')}`;
-        const sourceTreeHash = `0x${contractSpecificSourceData.sourceTreeHash.toString('hex')}`;
+        const sourceTreeHash = `0x${contractSpecificSourceData.sourceTreeHashIfExists.toString('hex')}`;
 
         const shouldCompile =
-            _.isUndefined(currentArtifact) ||
-            currentArtifact.networks[this._networkId].optimizer_enabled !== this._optimizerEnabled ||
-            currentArtifact.networks[this._networkId].source_tree_hash !== sourceTreeHash;
+            _.isUndefined(currentArtifactIfExists) ||
+            currentArtifactIfExists.networks[this._networkId].optimizer_enabled !== this._optimizerEnabled ||
+            currentArtifactIfExists.networks[this._networkId].source_tree_hash !== sourceTreeHash;
         if (!shouldCompile) {
             return;
         }
 
-        const fullSolcVersion = binPaths[contractSpecificSourceData.solc_version];
+        const fullSolcVersion = binPaths[contractSpecificSourceData.solcVersion];
         const solcBinPath = `./solc/solc_bin/${fullSolcVersion}`;
         const solcBin = require(solcBinPath);
         const solcInstance = solc.setupMethods(solcBin);
 
         utils.consoleLog(`Compiling ${fileName}...`);
-        const source = this._contractSourcesIfExists[fileName];
+        const source = this._contractSources[fileName];
         const input = {
             [fileName]: source,
         };
@@ -217,7 +217,7 @@ export class Compiler {
         const unlinked_binary = `0x${compiled.contracts[contractIdentifier].bytecode}`;
         const updated_at = Date.now();
         const contractNetworkData: ContractNetworkData = {
-            solc_version: contractSpecificSourceData.solc_version,
+            solc_version: contractSpecificSourceData.solcVersion,
             keccak256: sourceHash,
             source_tree_hash: sourceTreeHash,
             optimizer_enabled: this._optimizerEnabled,
@@ -227,11 +227,11 @@ export class Compiler {
         };
 
         let newArtifact: ContractArtifact;
-        if (!_.isUndefined(currentArtifact)) {
+        if (!_.isUndefined(currentArtifactIfExists)) {
             newArtifact = {
-                ...currentArtifact,
+                ...currentArtifactIfExists,
                 networks: {
-                    ...currentArtifact.networks,
+                    ...currentArtifactIfExists.networks,
                     [this._networkId]: contractNetworkData,
                 },
             };
@@ -253,28 +253,28 @@ export class Compiler {
      * Sets the source tree hash for a file and its dependencies.
      * @param fileName Name of contract file.
      */
-    private _setSourceTreeHash(fileName: string) {
+    private _setSourceTreeHash(fileName: string): void {
         const contractSpecificSourceData = this._contractSourceData[fileName];
         if (_.isUndefined(contractSpecificSourceData)) {
             throw new Error(`Contract data for ${fileName} not yet set`);
         }
-        if (_.isUndefined(contractSpecificSourceData.sourceTreeHash)) {
+        if (_.isUndefined(contractSpecificSourceData.sourceTreeHashIfExists)) {
             const dependencies = contractSpecificSourceData.dependencies;
             if (dependencies.length === 0) {
-                contractSpecificSourceData.sourceTreeHash = contractSpecificSourceData.sourceHash;
+                contractSpecificSourceData.sourceTreeHashIfExists = contractSpecificSourceData.sourceHash;
             } else {
                 _.forEach(dependencies, dependency => {
                     this._setSourceTreeHash(dependency);
                 });
                 const dependencySourceTreeHashes = _.map(
                     dependencies,
-                    dependency => this._contractSourceData[dependency].sourceTreeHash,
+                    dependency => this._contractSourceData[dependency].sourceTreeHashIfExists,
                 );
                 const sourceTreeHashesBuffer = Buffer.concat([
                     contractSpecificSourceData.sourceHash,
                     ...dependencySourceTreeHashes,
                 ]);
-                contractSpecificSourceData.sourceTreeHash = ethUtil.sha3(sourceTreeHashesBuffer);
+                contractSpecificSourceData.sourceTreeHashIfExists = ethUtil.sha3(sourceTreeHashesBuffer);
             }
         }
     }
@@ -285,11 +285,11 @@ export class Compiler {
      * @return Import contents object containing source code of dependency.
      */
     private _findImportsIfSourcesExist(importPath: string): ImportContents {
-        if (_.isUndefined(this._contractSourcesIfExists)) {
-            throw new Error('Contract sources not yet initialized');
-        }
         const fileName = path.basename(importPath);
-        const source = this._contractSourcesIfExists[fileName];
+        const source = this._contractSources[fileName];
+        if (_.isUndefined(source)) {
+            throw new Error(`Contract source not found for ${fileName}`);
+        }
         const importContents: ImportContents = {
             contents: source,
         };
@@ -309,7 +309,7 @@ export class Compiler {
      * @param fileName Name of contract file.
      * @return Contract data on network or undefined.
      */
-    private async _getContractArtifactOrReturnAsync(fileName: string): Promise<ContractArtifact | void> {
+    private async _getContractArtifactIfExistsAsync(fileName: string): Promise<ContractArtifact | void> {
         let contractArtifact;
         const contractName = path.basename(fileName, constants.SOLIDITY_FILE_EXTENSION);
         const currentArtifactPath = `${this._artifactsDir}/${contractName}.json`;
@@ -322,7 +322,7 @@ export class Compiler {
             return contractArtifact;
         } catch (err) {
             utils.consoleLog(`Artifact for ${fileName} does not exist`);
-            return contractArtifact;
+            return undefined;
         }
     }
 }
diff --git a/packages/deployer/src/deployer.ts b/packages/deployer/src/deployer.ts
index b14401050..021645fd1 100644
--- a/packages/deployer/src/deployer.ts
+++ b/packages/deployer/src/deployer.ts
@@ -35,9 +35,11 @@ export class Deployer {
      * @return Deployed contract instance.
      */
     public async deployAsync(contractName: string, args: any[] = []): Promise<Web3.ContractInstance> {
-        const contractArtifact: ContractArtifact = this._loadContractArtifactIfExists(contractName);
-        const contractData: ContractNetworkData = this._getContractDataFromArtifactIfExists(contractArtifact);
-        const data = contractData.unlinked_binary;
+        const contractArtifactIfExists: ContractArtifact = this._loadContractArtifactIfExists(contractName);
+        const contractNetworkDataIfExists: ContractNetworkData = this._getContractNetworkDataFromArtifactIfExists(
+            contractArtifactIfExists,
+        );
+        const data = contractNetworkDataIfExists.unlinked_binary;
         const from = await this._getFromAddressAsync();
         const gas = await this._getAllowableGasEstimateAsync(data);
         const txData = {
@@ -46,7 +48,7 @@ export class Deployer {
             data,
             gas,
         };
-        const abi = contractData.abi;
+        const abi = contractNetworkDataIfExists.abi;
         const web3ContractInstance = await this._deployFromAbiAsync(abi, args, txData);
         utils.consoleLog(`${contractName}.sol successfully deployed at ${web3ContractInstance.address}`);
         const contractInstance = new Contract(web3ContractInstance, this._defaults);
@@ -100,19 +102,21 @@ export class Deployer {
         contractAddress: string,
         args: any[],
     ): Promise<void> {
-        const contractArtifact: ContractArtifact = this._loadContractArtifactIfExists(contractName);
-        const contractData: ContractNetworkData = this._getContractDataFromArtifactIfExists(contractArtifact);
-        const abi = contractData.abi;
+        const contractArtifactIfExists: ContractArtifact = this._loadContractArtifactIfExists(contractName);
+        const contractNetworkDataIfExists: ContractNetworkData = this._getContractNetworkDataFromArtifactIfExists(
+            contractArtifactIfExists,
+        );
+        const abi = contractNetworkDataIfExists.abi;
         const encodedConstructorArgs = encoder.encodeConstructorArgsFromAbi(args, abi);
         const newContractData = {
-            ...contractData,
+            ...contractNetworkDataIfExists,
             address: contractAddress,
             constructor_args: encodedConstructorArgs,
         };
         const newArtifact = {
-            ...contractArtifact,
+            ...contractArtifactIfExists,
             networks: {
-                ...contractArtifact.networks,
+                ...contractArtifactIfExists.networks,
                 [this._networkId]: newContractData,
             },
         };
@@ -139,12 +143,12 @@ export class Deployer {
      * @param contractArtifact The contract artifact.
      * @return Network specific contract data.
      */
-    private _getContractDataFromArtifactIfExists(contractArtifact: ContractArtifact): ContractNetworkData {
-        const contractData = contractArtifact.networks[this._networkId];
-        if (_.isUndefined(contractData)) {
+    private _getContractNetworkDataFromArtifactIfExists(contractArtifact: ContractArtifact): ContractNetworkData {
+        const contractNetworkDataIfExists = contractArtifact.networks[this._networkId];
+        if (_.isUndefined(contractNetworkDataIfExists)) {
             throw new Error(`Data not found in artifact for contract: ${contractArtifact.contract_name}`);
         }
-        return contractData;
+        return contractNetworkDataIfExists;
     }
     /**
      * Gets the address to use for sending a transaction.
diff --git a/packages/deployer/src/utils/types.ts b/packages/deployer/src/utils/types.ts
index 166fc15dd..a3f722976 100644
--- a/packages/deployer/src/utils/types.ts
+++ b/packages/deployer/src/utils/types.ts
@@ -71,9 +71,9 @@ export interface ContractSourceData {
 
 export interface ContractSpecificSourceData {
     dependencies: string[];
-    solc_version: string;
+    solcVersion: string;
     sourceHash: Buffer;
-    sourceTreeHash?: Buffer;
+    sourceTreeHashIfExists?: Buffer;
 }
 
 export interface ImportContents {