Merge pull request #354 from 0xProject/feature/tslint-config/underscore-protected-members

Modify lint rules to enforce underscore for protected members

21 files changed, 65 insertions, 58 deletions

diff --git a/packages/0x.js/contract_templates/contract.handlebars b/packages/0x.js/contract_templates/contract.handlebars
index 3e501cce6..d3fe1b8cc 100644
--- a/packages/0x.js/contract_templates/contract.handlebars
+++ b/packages/0x.js/contract_templates/contract.handlebars
@@ -20,6 +20,6 @@ export class {{contractName}}Contract extends BaseContract {
 {{/each}}
     constructor(web3ContractInstance: Web3.ContractInstance, defaults: Partial<TxData>) {
         super(web3ContractInstance, defaults);
-        classUtils.bindAll(this, ['web3ContractInstance', 'defaults']);
+        classUtils.bindAll(this, ['_web3ContractInstance', '_defaults']);
     }
 } // tslint:disable:max-file-line-count
diff --git a/packages/0x.js/contract_templates/partials/call.handlebars b/packages/0x.js/contract_templates/partials/call.handlebars
index ef4bda724..0475136f0 100644
--- a/packages/0x.js/contract_templates/partials/call.handlebars
+++ b/packages/0x.js/contract_templates/partials/call.handlebars
@@ -5,8 +5,8 @@ public {{this.name}} = {
     ): Promise<{{> return_type outputs=outputs}}> {
         const self = this as {{contractName}}Contract;
         const result = await promisify<{{> return_type outputs=outputs}}>(
-            self.web3ContractInstance.{{this.name}}.call,
-            self.web3ContractInstance,
+            self._web3ContractInstance.{{this.name}}.call,
+            self._web3ContractInstance,
         )(
             {{> params inputs=inputs}}
         );
diff --git a/packages/0x.js/contract_templates/partials/tx.handlebars b/packages/0x.js/contract_templates/partials/tx.handlebars
index 8a43e5319..9df83266a 100644
--- a/packages/0x.js/contract_templates/partials/tx.handlebars
+++ b/packages/0x.js/contract_templates/partials/tx.handlebars
@@ -9,7 +9,7 @@ public {{this.name}} = {
     {{/this.payable}}
     ): Promise<string> {
         const self = this as {{contractName}}Contract;
-        const txDataWithDefaults = await self.applyDefaultsToTxDataAsync(
+        const txDataWithDefaults = await self._applyDefaultsToTxDataAsync(
             txData,
             self.{{this.name}}.estimateGasAsync.bind(
                 self,
@@ -17,7 +17,7 @@ public {{this.name}} = {
             ),
         );
         const txHash = await promisify<string>(
-            self.web3ContractInstance.{{this.name}}, self.web3ContractInstance,
+            self._web3ContractInstance.{{this.name}}, self._web3ContractInstance,
         )(
             {{> params inputs=inputs}}
             txDataWithDefaults,
@@ -29,11 +29,11 @@ public {{this.name}} = {
         txData: TxData = {},
     ): Promise<number> {
         const self = this as {{contractName}}Contract;
-        const txDataWithDefaults = await self.applyDefaultsToTxDataAsync(
+        const txDataWithDefaults = await self._applyDefaultsToTxDataAsync(
             txData,
         );
         const gas = await promisify<number>(
-            self.web3ContractInstance.{{this.name}}.estimateGas, self.web3ContractInstance,
+            self._web3ContractInstance.{{this.name}}.estimateGas, self._web3ContractInstance,
         )(
             {{> params inputs=inputs}}
             txDataWithDefaults,
@@ -45,7 +45,7 @@ public {{this.name}} = {
         txData: TxData = {},
     ): string {
         const self = this as {{contractName}}Contract;
-        const abiEncodedTransactionData = self.web3ContractInstance.{{this.name}}.getData();
+        const abiEncodedTransactionData = self._web3ContractInstance.{{this.name}}.getData();
         return abiEncodedTransactionData;
     },
 };
diff --git a/packages/0x.js/src/contract_wrappers/contract_wrapper.ts b/packages/0x.js/src/contract_wrappers/contract_wrapper.ts
index 5f11d810a..873489dc9 100644
--- a/packages/0x.js/src/contract_wrappers/contract_wrapper.ts
+++ b/packages/0x.js/src/contract_wrappers/contract_wrapper.ts
@@ -52,7 +52,7 @@ export class ContractWrapper {
         this._onLogAddedSubscriptionToken = undefined;
         this._onLogRemovedSubscriptionToken = undefined;
     }
-    protected unsubscribeAll(): void {
+    protected _unsubscribeAll(): void {
         const filterTokens = _.keys(this._filterCallbacks);
         _.each(filterTokens, filterToken => {
             this._unsubscribe(filterToken);
diff --git a/packages/0x.js/src/contract_wrappers/ether_token_wrapper.ts b/packages/0x.js/src/contract_wrappers/ether_token_wrapper.ts
index 8fa7aa78b..cbafcfe94 100644
--- a/packages/0x.js/src/contract_wrappers/ether_token_wrapper.ts
+++ b/packages/0x.js/src/contract_wrappers/ether_token_wrapper.ts
@@ -159,11 +159,11 @@ export class EtherTokenWrapper extends ContractWrapper {
     /**
      * Cancels all existing subscriptions
      */
-    public unsubscribeAll(): void {
-        super.unsubscribeAll();
+    public _unsubscribeAll(): void {
+        super._unsubscribeAll();
     }
     private _invalidateContractInstance(): void {
-        this.unsubscribeAll();
+        this._unsubscribeAll();
         this._etherTokenContractsByAddress = {};
     }
     private async _getEtherTokenContractAsync(etherTokenAddress: string): Promise<EtherTokenContract> {
diff --git a/packages/0x.js/src/contract_wrappers/exchange_wrapper.ts b/packages/0x.js/src/contract_wrappers/exchange_wrapper.ts
index fceab851a..63c0d073a 100644
--- a/packages/0x.js/src/contract_wrappers/exchange_wrapper.ts
+++ b/packages/0x.js/src/contract_wrappers/exchange_wrapper.ts
@@ -678,8 +678,8 @@ export class ExchangeWrapper extends ContractWrapper {
     /**
      * Cancels all existing subscriptions
      */
-    public unsubscribeAll(): void {
-        super.unsubscribeAll();
+    public _unsubscribeAll(): void {
+        super._unsubscribeAll();
     }
     /**
      * Gets historical logs without creating a subscription
@@ -861,7 +861,7 @@ export class ExchangeWrapper extends ContractWrapper {
         return contractAddress;
     }
     private _invalidateContractInstances(): void {
-        this.unsubscribeAll();
+        this._unsubscribeAll();
         delete this._exchangeContractIfExists;
     }
     private async _isValidSignatureUsingContractCallAsync(
diff --git a/packages/0x.js/src/contract_wrappers/generated/base_contract.ts b/packages/0x.js/src/contract_wrappers/generated/base_contract.ts
index 28a7e2f52..d8fac7eea 100644
--- a/packages/0x.js/src/contract_wrappers/generated/base_contract.ts
+++ b/packages/0x.js/src/contract_wrappers/generated/base_contract.ts
@@ -3,9 +3,9 @@ import * as _ from 'lodash';
 import * as Web3 from 'web3';
 
 export class BaseContract {
-    protected web3ContractInstance: Web3.ContractInstance;
-    protected defaults: Partial<TxData>;
-    protected async applyDefaultsToTxDataAsync<T extends TxData|TxDataPayable>(
+    protected _web3ContractInstance: Web3.ContractInstance;
+    protected _defaults: Partial<TxData>;
+    protected async _applyDefaultsToTxDataAsync<T extends TxData|TxDataPayable>(
         txData: T,
         estimateGasAsync?: (txData: T) => Promise<number>,
     ): Promise<TxData> {
@@ -15,7 +15,7 @@ export class BaseContract {
         // 3. Gas estimate calculation + safety margin
         const removeUndefinedProperties = _.pickBy;
         const txDataWithDefaults = {
-            ...removeUndefinedProperties(this.defaults),
+            ...removeUndefinedProperties(this._defaults),
             ...removeUndefinedProperties(txData as any),
             // HACK: TS can't prove that T is spreadable.
             // Awaiting https://github.com/Microsoft/TypeScript/pull/13288 to be merged
@@ -27,7 +27,7 @@ export class BaseContract {
         return txDataWithDefaults;
     }
     constructor(web3ContractInstance: Web3.ContractInstance, defaults: Partial<TxData>) {
-        this.web3ContractInstance = web3ContractInstance;
-        this.defaults = defaults;
+        this._web3ContractInstance = web3ContractInstance;
+        this._defaults = defaults;
     }
 }
diff --git a/packages/0x.js/src/contract_wrappers/token_wrapper.ts b/packages/0x.js/src/contract_wrappers/token_wrapper.ts
index 4216ff462..98c24d059 100644
--- a/packages/0x.js/src/contract_wrappers/token_wrapper.ts
+++ b/packages/0x.js/src/contract_wrappers/token_wrapper.ts
@@ -342,8 +342,8 @@ export class TokenWrapper extends ContractWrapper {
     /**
      * Cancels all existing subscriptions
      */
-    public unsubscribeAll(): void {
-        super.unsubscribeAll();
+    public _unsubscribeAll(): void {
+        super._unsubscribeAll();
     }
     /**
      * Gets historical logs without creating a subscription
@@ -374,7 +374,7 @@ export class TokenWrapper extends ContractWrapper {
         return logs;
     }
     private _invalidateContractInstances(): void {
-        this.unsubscribeAll();
+        this._unsubscribeAll();
         this._tokenContractsByAddress = {};
     }
     private async _getTokenContractAsync(tokenAddress: string): Promise<TokenContract> {
diff --git a/packages/0x.js/test/ether_token_wrapper_test.ts b/packages/0x.js/test/ether_token_wrapper_test.ts
index 6069b42bf..9716abab8 100644
--- a/packages/0x.js/test/ether_token_wrapper_test.ts
+++ b/packages/0x.js/test/ether_token_wrapper_test.ts
@@ -144,7 +144,7 @@ describe('EtherTokenWrapper', () => {
             etherTokenAddress = etherToken.address;
         });
         afterEach(() => {
-            zeroEx.etherToken.unsubscribeAll();
+            zeroEx.etherToken._unsubscribeAll();
         });
         // Hack: Mocha does not allow a test to be both async and have a `done` callback
         // Since we need to await the receipt of the event in the `subscribe` callback,
diff --git a/packages/0x.js/test/exchange_wrapper_test.ts b/packages/0x.js/test/exchange_wrapper_test.ts
index c15cd65a9..044298601 100644
--- a/packages/0x.js/test/exchange_wrapper_test.ts
+++ b/packages/0x.js/test/exchange_wrapper_test.ts
@@ -921,7 +921,7 @@ describe('ExchangeWrapper', () => {
             );
         });
         afterEach(async () => {
-            zeroEx.exchange.unsubscribeAll();
+            zeroEx.exchange._unsubscribeAll();
         });
         // Hack: Mocha does not allow a test to be both async and have a `done` callback
         // Since we need to await the receipt of the event in the `subscribe` callback,
diff --git a/packages/0x.js/test/subscription_test.ts b/packages/0x.js/test/subscription_test.ts
index f485bf84b..337e2effa 100644
--- a/packages/0x.js/test/subscription_test.ts
+++ b/packages/0x.js/test/subscription_test.ts
@@ -49,7 +49,7 @@ describe('SubscriptionTest', () => {
             tokenAddress = token.address;
         });
         afterEach(() => {
-            zeroEx.token.unsubscribeAll();
+            zeroEx.token._unsubscribeAll();
             _.each(stubs, s => s.restore());
             stubs = [];
         });
@@ -76,7 +76,7 @@ describe('SubscriptionTest', () => {
                 const callback = (err: Error | null, logEvent?: DecodedLogEvent<ApprovalContractEventArgs>) => _.noop;
                 zeroEx.token.subscribe(tokenAddress, TokenEvents.Approval, indexFilterValues, callback);
                 stubs = [Sinon.stub((zeroEx as any)._web3Wrapper, 'getBlockAsync').throws(new Error('JSON RPC error'))];
-                zeroEx.token.unsubscribeAll();
+                zeroEx.token._unsubscribeAll();
                 done();
             })().catch(done);
         });
diff --git a/packages/0x.js/test/token_wrapper_test.ts b/packages/0x.js/test/token_wrapper_test.ts
index 070d6ec47..34ebe30c2 100644
--- a/packages/0x.js/test/token_wrapper_test.ts
+++ b/packages/0x.js/test/token_wrapper_test.ts
@@ -377,7 +377,7 @@ describe('TokenWrapper', () => {
             tokenAddress = token.address;
         });
         afterEach(() => {
-            zeroEx.token.unsubscribeAll();
+            zeroEx.token._unsubscribeAll();
         });
         // Hack: Mocha does not allow a test to be both async and have a `done` callback
         // Since we need to await the receipt of the event in the `subscribe` callback,
diff --git a/packages/abi-gen/CHANGELOG.md b/packages/abi-gen/CHANGELOG.md
index 9247b315a..ffa8a7a35 100644
--- a/packages/abi-gen/CHANGELOG.md
+++ b/packages/abi-gen/CHANGELOG.md
@@ -1,6 +1,7 @@
 # CHANGELOG
 
 ## v0.2.0 - _???_
+
 * Added CLI options for explicit specifying location of partials and main template (#346)
 
 ## v0.1.0 - _January 11, 2018_
diff --git a/packages/abi-gen/src/index.ts b/packages/abi-gen/src/index.ts
index 5412d11ce..fe2b56524 100644
--- a/packages/abi-gen/src/index.ts
+++ b/packages/abi-gen/src/index.ts
@@ -42,8 +42,10 @@ const args = yargs
         demandOption: true,
         normalize: true,
     })
-    .example("$0 --abis 'src/artifacts/**/*.json' --out 'src/contracts/generated/' --partials 'src/templates/partials/**/*.handlebars' --template 'src/templates/contract.handlebars'", 'Full usage example')
-    .argv;
+    .example(
+        "$0 --abis 'src/artifacts/**/*.json' --out 'src/contracts/generated/' --partials 'src/templates/partials/**/*.handlebars' --template 'src/templates/contract.handlebars'",
+        'Full usage example',
+    ).argv;
 
 function registerPartials(partialsGlob: string) {
     const partialTemplateFileNames = globSync(partialsGlob);
diff --git a/packages/testnet-faucets/src/ts/ether_request_queue.ts b/packages/testnet-faucets/src/ts/ether_request_queue.ts
index 1c4b19ab9..710d49f39 100644
--- a/packages/testnet-faucets/src/ts/ether_request_queue.ts
+++ b/packages/testnet-faucets/src/ts/ether_request_queue.ts
@@ -9,14 +9,14 @@ import { utils } from './utils';
 const DISPENSE_AMOUNT_ETHER = 0.1;
 
 export class EtherRequestQueue extends RequestQueue {
-    protected async processNextRequestFireAndForgetAsync(recipientAddress: string) {
+    protected async _processNextRequestFireAndForgetAsync(recipientAddress: string) {
         utils.consoleLog(`Processing ETH ${recipientAddress}`);
-        const sendTransactionAsync = promisify(this.web3.eth.sendTransaction);
+        const sendTransactionAsync = promisify(this._web3.eth.sendTransaction);
         try {
             const txHash = await sendTransactionAsync({
                 from: configs.DISPENSER_ADDRESS,
                 to: recipientAddress,
-                value: this.web3.toWei(DISPENSE_AMOUNT_ETHER, 'ether'),
+                value: this._web3.toWei(DISPENSE_AMOUNT_ETHER, 'ether'),
             });
             utils.consoleLog(`Sent ${DISPENSE_AMOUNT_ETHER} ETH to ${recipientAddress} tx: ${txHash}`);
         } catch (err) {
diff --git a/packages/testnet-faucets/src/ts/request_queue.ts b/packages/testnet-faucets/src/ts/request_queue.ts
index 20f2833a1..7d685522a 100644
--- a/packages/testnet-faucets/src/ts/request_queue.ts
+++ b/packages/testnet-faucets/src/ts/request_queue.ts
@@ -11,46 +11,46 @@ const MAX_QUEUE_SIZE = 500;
 const DEFAULT_QUEUE_INTERVAL_MS = 1000;
 
 export class RequestQueue {
-    protected queueIntervalMs: number;
-    protected queue: string[];
-    protected queueIntervalId: NodeJS.Timer;
-    protected web3: Web3;
+    protected _queueIntervalMs: number;
+    protected _queue: string[];
+    protected _queueIntervalId: NodeJS.Timer;
+    protected _web3: Web3;
     constructor(web3: any) {
-        this.queueIntervalMs = DEFAULT_QUEUE_INTERVAL_MS;
-        this.queue = [];
+        this._queueIntervalMs = DEFAULT_QUEUE_INTERVAL_MS;
+        this._queue = [];
 
-        this.web3 = web3;
+        this._web3 = web3;
 
-        this.start();
+        this._start();
     }
     public add(recipientAddress: string): boolean {
         if (this.isFull()) {
             return false;
         }
-        this.queue.push(recipientAddress);
+        this._queue.push(recipientAddress);
         return true;
     }
     public size(): number {
-        return this.queue.length;
+        return this._queue.length;
     }
     public isFull(): boolean {
         return this.size() >= MAX_QUEUE_SIZE;
     }
-    protected start() {
-        this.queueIntervalId = timers.setInterval(() => {
-            const recipientAddress = this.queue.shift();
+    protected _start() {
+        this._queueIntervalId = timers.setInterval(() => {
+            const recipientAddress = this._queue.shift();
             if (_.isUndefined(recipientAddress)) {
                 return;
             }
             // tslint:disable-next-line:no-floating-promises
-            this.processNextRequestFireAndForgetAsync(recipientAddress);
-        }, this.queueIntervalMs);
+            this._processNextRequestFireAndForgetAsync(recipientAddress);
+        }, this._queueIntervalMs);
     }
-    protected stop() {
-        clearInterval(this.queueIntervalId);
+    protected _stop() {
+        clearInterval(this._queueIntervalId);
     }
     // tslint:disable-next-line:prefer-function-over-method
-    protected async processNextRequestFireAndForgetAsync(recipientAddress: string) {
+    protected async _processNextRequestFireAndForgetAsync(recipientAddress: string) {
         throw new Error('Expected processNextRequestFireAndForgetAsync to be implemented by a subclass');
     }
 }
diff --git a/packages/testnet-faucets/src/ts/zrx_request_queue.ts b/packages/testnet-faucets/src/ts/zrx_request_queue.ts
index 3d73f9dd2..db1b619a8 100644
--- a/packages/testnet-faucets/src/ts/zrx_request_queue.ts
+++ b/packages/testnet-faucets/src/ts/zrx_request_queue.ts
@@ -20,13 +20,13 @@ export class ZRXRequestQueue extends RequestQueue {
     private _zeroEx: ZeroEx;
     constructor(web3: Web3, networkId: number) {
         super(web3);
-        this.queueIntervalMs = QUEUE_INTERVAL_MS;
+        this._queueIntervalMs = QUEUE_INTERVAL_MS;
         const zeroExConfig = {
             networkId,
         };
         this._zeroEx = new ZeroEx(web3.currentProvider, zeroExConfig);
     }
-    protected async processNextRequestFireAndForgetAsync(recipientAddress: string) {
+    protected async _processNextRequestFireAndForgetAsync(recipientAddress: string) {
         utils.consoleLog(`Processing ZRX ${recipientAddress}`);
         const baseUnitAmount = ZeroEx.toBaseUnitAmount(DISPENSE_AMOUNT_ZRX, 18);
         try {
diff --git a/packages/tslint-config/CHANGELOG.md b/packages/tslint-config/CHANGELOG.md
index 1d56bca5b..04d705941 100644
--- a/packages/tslint-config/CHANGELOG.md
+++ b/packages/tslint-config/CHANGELOG.md
@@ -1,5 +1,9 @@
 # CHANGELOG
 
+## v0.5.0 - _TBD, 2018_
+
+    * Modified custom 'underscore-privates' rule, changing it to 'underscore-private-and-protected' requiring underscores to be prepended to both private and protected variable names (#354)
+
 ## v0.4.0 - _December 28, 2017_
 
     * Added custom 'underscore-privates' rule, requiring underscores to be prepended to private variable names
diff --git a/packages/tslint-config/rules/underscorePrivatesRule.ts b/packages/tslint-config/rules/underscorePrivateAndProtectedRule.ts
index 472ea09ff..2f05b0c18 100644
--- a/packages/tslint-config/rules/underscorePrivatesRule.ts
+++ b/packages/tslint-config/rules/underscorePrivateAndProtectedRule.ts
@@ -12,7 +12,7 @@ type RelevantClassMember =
 // Copied from: https://github.com/DanielRosenwasser/underscore-privates-tslint-rule
 // The version on github is not published on npm
 export class Rule extends Lint.Rules.AbstractRule {
-    public static FAILURE_STRING = 'private members must be prefixed with an underscore';
+    public static FAILURE_STRING = 'private and protected members must be prefixed with an underscore';
 
     public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {
         return this.applyWithFunction(sourceFile, walk);
@@ -54,7 +54,7 @@ function nameStartsWithUnderscore(text: string) {
     return text.charCodeAt(0) === UNDERSCORE.charCodeAt(0);
 }
 function memberIsPrivate(node: ts.Declaration) {
-    return Lint.hasModifier(node.modifiers, ts.SyntaxKind.PrivateKeyword);
+    return Lint.hasModifier(node.modifiers, ts.SyntaxKind.PrivateKeyword, ts.SyntaxKind.ProtectedKeyword);
 }
 function nameIsIdentifier(node: ts.Node): node is ts.Identifier {
     return node.kind === ts.SyntaxKind.Identifier;
diff --git a/packages/tslint-config/tslint.json b/packages/tslint-config/tslint.json
index 44a8517dc..3266b022f 100644
--- a/packages/tslint-config/tslint.json
+++ b/packages/tslint-config/tslint.json
@@ -73,7 +73,7 @@
         ],
         "space-within-parens": false,
         "type-literal-delimiter": true,
-        "underscore-privates": true,
+        "underscore-private-and-protected": true,
         "variable-name": [true, "ban-keywords", "allow-pascal-case"],
         "whitespace": [
             true,
diff --git a/packages/website/ts/blockchain.ts b/packages/website/ts/blockchain.ts
index d53994c0c..71995e2cd 100644
--- a/packages/website/ts/blockchain.ts
+++ b/packages/website/ts/blockchain.ts
@@ -689,7 +689,7 @@ export class Blockchain {
         }
     }
     private _stopWatchingExchangeLogFillEvents(): void {
-        this._zeroEx.exchange.unsubscribeAll();
+        this._zeroEx.exchange._unsubscribeAll();
     }
     private async _getTokenRegistryTokensByAddressAsync(): Promise<TokenByAddress> {
         utils.assert(!_.isUndefined(this._zeroEx), 'ZeroEx must be instantiated.');