From e803807dd9f93ce23dda7120be525ea0e9435ad3 Mon Sep 17 00:00:00 2001 From: Whymarrh Whitby Date: Sat, 15 Aug 2020 09:28:11 -0230 Subject: [PATCH] Fix no-param-reassign issues (#9235) See [`no-param-reassign`](https://eslint.org/docs/rules/no-param-reassign) for more information. This change enables `no-param-reassign` and fixes the issues raised by the rule. --- .eslintrc.js | 1 + app/scripts/controllers/network/network.js | 3 +-- app/scripts/controllers/preferences.js | 1 + app/scripts/controllers/transactions/index.js | 1 + .../transactions/tx-state-manager.js | 1 + app/scripts/lib/buy-eth-url.js | 1 + app/scripts/lib/migrator/index.js | 1 + app/scripts/lib/setupSentry.js | 8 +++---- app/scripts/migrations/038.js | 23 ++++++++++--------- test/e2e/benchmark.js | 9 ++++---- test/e2e/ganache.js | 5 ++-- test/e2e/webdriver/driver.js | 6 ++--- .../unit/app/controllers/permissions/mocks.js | 1 + .../signature-request-original.component.js | 1 + .../ui/check-box/check-box.component.js | 1 + .../confirm-transaction.duck.js | 6 ++--- ui/app/ducks/gas/gas.duck.js | 8 ++++--- ui/app/helpers/utils/switch-direction.js | 3 ++- ui/app/helpers/utils/util.js | 1 + .../add-recipient/ens-input.component.js | 4 ++-- ui/app/pages/send/send.utils.js | 1 + ui/app/pages/send/tests/send-utils.test.js | 7 +++--- ui/app/store/actions.js | 3 +-- 23 files changed, 54 insertions(+), 42 deletions(-) diff --git a/.eslintrc.js b/.eslintrc.js index 6c5058390..4e74e1209 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -61,6 +61,7 @@ module.exports = { 'no-loop-func': 'error', 'no-negated-condition': 'error', 'no-nested-ternary': 'error', + 'no-param-reassign': 'error', 'no-plusplus': ['error', { 'allowForLoopAfterthoughts': true }], 'no-process-exit': 'error', 'no-prototype-builtins': 'error', diff --git a/app/scripts/controllers/network/network.js b/app/scripts/controllers/network/network.js index f22dbc323..90a60f8cd 100644 --- a/app/scripts/controllers/network/network.js +++ b/app/scripts/controllers/network/network.js @@ -101,8 +101,7 @@ export default class NetworkController extends EventEmitter { if (!type) { return } - network = networks.networkList[type]?.chainId || network - this.networkStore.putState(network) + this.networkStore.putState(networks.networkList[type]?.chainId || network) } isNetworkLoading () { diff --git a/app/scripts/controllers/preferences.js b/app/scripts/controllers/preferences.js index 4c0e97ceb..dee126a8b 100644 --- a/app/scripts/controllers/preferences.js +++ b/app/scripts/controllers/preferences.js @@ -689,6 +689,7 @@ export default class PreferencesController { _getTokenRelatedStates (selectedAddress) { const accountTokens = this.store.getState().accountTokens if (!selectedAddress) { + // eslint-disable-next-line no-param-reassign selectedAddress = this.store.getState().selectedAddress } const providerType = this.network.providerStore.getState().type diff --git a/app/scripts/controllers/transactions/index.js b/app/scripts/controllers/transactions/index.js index 795c7c6ea..9f8da729d 100644 --- a/app/scripts/controllers/transactions/index.js +++ b/app/scripts/controllers/transactions/index.js @@ -271,6 +271,7 @@ export default class TransactionController extends EventEmitter { const defaultGasPrice = await this._getDefaultGasPrice(txMeta) const { gasLimit: defaultGasLimit, simulationFails } = await this._getDefaultGasLimit(txMeta, getCodeResponse) + // eslint-disable-next-line no-param-reassign txMeta = this.txStateManager.getTx(txMeta.id) if (simulationFails) { txMeta.simulationFails = simulationFails diff --git a/app/scripts/controllers/transactions/tx-state-manager.js b/app/scripts/controllers/transactions/tx-state-manager.js index 7de54fb9c..89eaf2464 100644 --- a/app/scripts/controllers/transactions/tx-state-manager.js +++ b/app/scripts/controllers/transactions/tx-state-manager.js @@ -262,6 +262,7 @@ export default class TransactionStateManager extends EventEmitter { if (typeof txParams.data === 'undefined') { delete txParams.data } + // eslint-disable-next-line no-param-reassign txParams = normalizeTxParams(txParams, false) this.validateTxParams(txParams) return txParams diff --git a/app/scripts/lib/buy-eth-url.js b/app/scripts/lib/buy-eth-url.js index 46d8909da..a82d3f2b0 100644 --- a/app/scripts/lib/buy-eth-url.js +++ b/app/scripts/lib/buy-eth-url.js @@ -11,6 +11,7 @@ export default function getBuyEthUrl ({ network, address, service }) { // default service by network if not specified if (!service) { + // eslint-disable-next-line no-param-reassign service = getDefaultServiceForNetwork(network) } diff --git a/app/scripts/lib/migrator/index.js b/app/scripts/lib/migrator/index.js index a82b93294..5c2f6185f 100644 --- a/app/scripts/lib/migrator/index.js +++ b/app/scripts/lib/migrator/index.js @@ -46,6 +46,7 @@ export default class Migrator extends EventEmitter { throw new Error('Migrator - Migration did not update version number correctly') } // accept the migration as good + // eslint-disable-next-line no-param-reassign versionedData = migratedData } catch (err) { // rewrite error message to add context without clobbering stack diff --git a/app/scripts/lib/setupSentry.js b/app/scripts/lib/setupSentry.js index c974aafd8..02ad02d6b 100644 --- a/app/scripts/lib/setupSentry.js +++ b/app/scripts/lib/setupSentry.js @@ -122,13 +122,13 @@ export default function setupSentry ({ release, getState }) { function simplifyErrorMessages (report) { rewriteErrorMessages(report, (errorMessage) => { // simplify ethjs error messages - errorMessage = extractEthjsErrorMessage(errorMessage) + let simplifiedErrorMessage = extractEthjsErrorMessage(errorMessage) // simplify 'Transaction Failed: known transaction' - if (errorMessage.indexOf('Transaction Failed: known transaction') === 0) { + if (simplifiedErrorMessage.indexOf('Transaction Failed: known transaction') === 0) { // cut the hash from the error message - errorMessage = 'Transaction Failed: known transaction' + simplifiedErrorMessage = 'Transaction Failed: known transaction' } - return errorMessage + return simplifiedErrorMessage }) } diff --git a/app/scripts/migrations/038.js b/app/scripts/migrations/038.js index fdbaa1d3b..e5386f7a9 100644 --- a/app/scripts/migrations/038.js +++ b/app/scripts/migrations/038.js @@ -19,17 +19,18 @@ function transformState (state) { const { ABTestController: ABTestControllerState = {} } = state const { abTests = {} } = ABTestControllerState - if (!abTests.fullScreenVsPopup) { - state = { - ...state, - ABTestController: { - ...ABTestControllerState, - abTests: { - ...abTests, - fullScreenVsPopup: 'control', - }, + if (abTests.fullScreenVsPopup) { + return state + } + + return { + ...state, + ABTestController: { + ...ABTestControllerState, + abTests: { + ...abTests, + fullScreenVsPopup: 'control', }, - } + }, } - return state } diff --git a/test/e2e/benchmark.js b/test/e2e/benchmark.js index f34436328..a70bf6805 100644 --- a/test/e2e/benchmark.js +++ b/test/e2e/benchmark.js @@ -92,17 +92,18 @@ async function isWritable (directory) { } async function getFirstParentDirectoryThatExists (directory) { + let nextDirectory = directory for (;;) { try { - await fs.access(directory, fsConstants.F_OK) - return directory + await fs.access(nextDirectory, fsConstants.F_OK) + return nextDirectory } catch (error) { if (error.code !== 'ENOENT') { throw error - } else if (directory === path.dirname(directory)) { + } else if (nextDirectory === path.dirname(nextDirectory)) { throw new Error('Failed to find parent directory that exists') } - directory = path.dirname(directory) + nextDirectory = path.dirname(nextDirectory) } } } diff --git a/test/e2e/ganache.js b/test/e2e/ganache.js index cd15990c8..29cc085bc 100644 --- a/test/e2e/ganache.js +++ b/test/e2e/ganache.js @@ -10,9 +10,8 @@ const defaultOptions = { } class Ganache { - async start (options) { - options = Object.assign({}, defaultOptions, options) - + async start (opts) { + const options = { ...defaultOptions, ...opts } const port = options.port this._server = ganache.server(options) diff --git a/test/e2e/webdriver/driver.js b/test/e2e/webdriver/driver.js index 6ae17bdcf..8f1bc6744 100644 --- a/test/e2e/webdriver/driver.js +++ b/test/e2e/webdriver/driver.js @@ -136,9 +136,8 @@ class Driver { } async switchToWindowWithTitle (title, windowHandles) { - if (!windowHandles) { - windowHandles = await this.driver.getAllWindowHandles() - } + // eslint-disable-next-line no-param-reassign + windowHandles = windowHandles || await this.driver.getAllWindowHandles() for (const handle of windowHandles) { await this.driver.switchTo().window(handle) @@ -158,6 +157,7 @@ class Driver { * @returns {Promise} */ async closeAllWindowHandlesExcept (exceptions, windowHandles) { + // eslint-disable-next-line no-param-reassign windowHandles = windowHandles || await this.driver.getAllWindowHandles() for (const handle of windowHandles) { diff --git a/test/unit/app/controllers/permissions/mocks.js b/test/unit/app/controllers/permissions/mocks.js index 65b311182..54f47e5d8 100644 --- a/test/unit/app/controllers/permissions/mocks.js +++ b/test/unit/app/controllers/permissions/mocks.js @@ -108,6 +108,7 @@ export function getPermissionsMiddleware (permController, origin, extensionId) { return (req, res = {}, next = noop, end) => { return new Promise((resolve, reject) => { + // eslint-disable-next-line no-param-reassign end = end || _end middleware(req, res, next, end) diff --git a/ui/app/components/app/signature-request-original/signature-request-original.component.js b/ui/app/components/app/signature-request-original/signature-request-original.component.js index 29e06bab6..31c214e38 100644 --- a/ui/app/components/app/signature-request-original/signature-request-original.component.js +++ b/ui/app/components/app/signature-request-original/signature-request-original.component.js @@ -247,6 +247,7 @@ export default class SignatureRequestOriginal extends Component { { rows.map(({ name, value }, index) => { if (typeof value === 'boolean') { + // eslint-disable-next-line no-param-reassign value = value.toString() } return ( diff --git a/ui/app/components/ui/check-box/check-box.component.js b/ui/app/components/ui/check-box/check-box.component.js index b1c62433a..4a7457140 100644 --- a/ui/app/components/ui/check-box/check-box.component.js +++ b/ui/app/components/ui/check-box/check-box.component.js @@ -12,6 +12,7 @@ export const { CHECKED, INDETERMINATE, UNCHECKED } = CHECKBOX_STATE const CheckBox = ({ className, disabled, id, onClick, checked, title }) => { if (typeof checked === 'boolean') { + // eslint-disable-next-line no-param-reassign checked = checked ? CHECKBOX_STATE.CHECKED : CHECKBOX_STATE.UNCHECKED diff --git a/ui/app/ducks/confirm-transaction/confirm-transaction.duck.js b/ui/app/ducks/confirm-transaction/confirm-transaction.duck.js index 536307ac7..441b4e161 100644 --- a/ui/app/ducks/confirm-transaction/confirm-transaction.duck.js +++ b/ui/app/ducks/confirm-transaction/confirm-transaction.duck.js @@ -258,16 +258,14 @@ export function setFetchingData (isFetching) { } export function updateGasAndCalculate ({ gasLimit, gasPrice }) { - gasLimit = addHexPrefix(gasLimit) - gasPrice = addHexPrefix(gasPrice) return (dispatch, getState) => { const { confirmTransaction: { txData } } = getState() const newTxData = { ...txData, txParams: { ...txData.txParams, - gas: gasLimit, - gasPrice, + gas: addHexPrefix(gasLimit), + gasPrice: addHexPrefix(gasPrice), }, } diff --git a/ui/app/ducks/gas/gas.duck.js b/ui/app/ducks/gas/gas.duck.js index 78b9ef8ba..8451c800a 100644 --- a/ui/app/ducks/gas/gas.duck.js +++ b/ui/app/ducks/gas/gas.duck.js @@ -323,20 +323,22 @@ async function fetchExternalBasicGasAndTimeEstimates (dispatch) { } function extrapolateY ({ higherY, lowerY, higherX, lowerX, xForExtrapolation }) { + /* eslint-disable no-param-reassign */ higherY = new BigNumber(higherY, 10) lowerY = new BigNumber(lowerY, 10) higherX = new BigNumber(higherX, 10) lowerX = new BigNumber(lowerX, 10) xForExtrapolation = new BigNumber(xForExtrapolation, 10) + /* eslint-enable no-param-reassign */ const slope = (higherY.minus(lowerY)).div(higherX.minus(lowerX)) const newTimeEstimate = slope.times(higherX.minus(xForExtrapolation)).minus(higherY).negated() return Number(newTimeEstimate.toPrecision(10)) } -function getRandomArbitrary (min, max) { - min = new BigNumber(min, 10) - max = new BigNumber(max, 10) +function getRandomArbitrary (minStr, maxStr) { + const min = new BigNumber(minStr, 10) + const max = new BigNumber(maxStr, 10) const random = new BigNumber(String(Math.random()), 10) return new BigNumber(random.times(max.minus(min)).plus(min)).toPrecision(10) } diff --git a/ui/app/helpers/utils/switch-direction.js b/ui/app/helpers/utils/switch-direction.js index f0ed42bf1..81170f43c 100644 --- a/ui/app/helpers/utils/switch-direction.js +++ b/ui/app/helpers/utils/switch-direction.js @@ -1,10 +1,11 @@ /** * Switch the CSS stylesheet used between 'rtl' and 'ltr' - * @param {('ltr' | 'rtl')} direction - Text direction, either left-to-right (ltr) or right-to-left (rtl) + * @param {('ltr' | 'rtl' | 'auto')} direction - Text direction, either left-to-right (ltr) or right-to-left (rtl) * @return {Promise} */ const switchDirection = async (direction) => { if (direction === 'auto') { + // eslint-disable-next-line no-param-reassign direction = 'ltr' } let updatedLink diff --git a/ui/app/helpers/utils/util.js b/ui/app/helpers/utils/util.js index 28f3dd6f4..ffd8aa412 100644 --- a/ui/app/helpers/utils/util.js +++ b/ui/app/helpers/utils/util.js @@ -241,6 +241,7 @@ export function getRandomFileName () { } export function exportAsFile (filename, data, type = 'text/csv') { + // eslint-disable-next-line no-param-reassign filename = filename || getRandomFileName() // source: https://stackoverflow.com/a/33542499 by Ludovic Feltz const blob = new window.Blob([data], { type }) diff --git a/ui/app/pages/send/send-content/add-recipient/ens-input.component.js b/ui/app/pages/send/send-content/add-recipient/ens-input.component.js index df8bf87a2..392b9af77 100644 --- a/ui/app/pages/send/send-content/add-recipient/ens-input.component.js +++ b/ui/app/pages/send/send-content/add-recipient/ens-input.component.js @@ -78,8 +78,8 @@ export default class EnsInput extends Component { updateEnsResolutionError('') } - lookupEnsName = (recipient) => { - recipient = recipient.trim() + lookupEnsName = (ensName) => { + const recipient = ensName.trim() log.info(`ENS attempting to resolve name: ${recipient}`) this.ens.lookup(recipient) diff --git a/ui/app/pages/send/send.utils.js b/ui/app/pages/send/send.utils.js index ca0fdf044..ed01da74c 100644 --- a/ui/app/pages/send/send.utils.js +++ b/ui/app/pages/send/send.utils.js @@ -238,6 +238,7 @@ async function estimateGas ({ // if not, fall back to block gasLimit if (!blockGasLimit) { + // eslint-disable-next-line no-param-reassign blockGasLimit = MIN_GAS_LIMIT_HEX } diff --git a/ui/app/pages/send/tests/send-utils.test.js b/ui/app/pages/send/tests/send-utils.test.js index b1f57e49a..1640c8046 100644 --- a/ui/app/pages/send/tests/send-utils.test.js +++ b/ui/app/pages/send/tests/send-utils.test.js @@ -10,13 +10,14 @@ import { const stubs = { addCurrencies: sinon.stub().callsFake((a, b) => { + let [a1, b1] = [a, b] if (String(a).match(/^0x.+/u)) { - a = Number(String(a).slice(2)) + a1 = Number(String(a).slice(2)) } if (String(b).match(/^0x.+/u)) { - b = Number(String(b).slice(2)) + b1 = Number(String(b).slice(2)) } - return a + b + return a1 + b1 }), conversionUtil: sinon.stub().callsFake((val) => parseInt(val, 16)), conversionGTE: sinon.stub().callsFake((obj1, obj2) => obj1.value >= obj2.value), diff --git a/ui/app/store/actions.js b/ui/app/store/actions.js index c9c4a63dd..f2b774ada 100644 --- a/ui/app/store/actions.js +++ b/ui/app/store/actions.js @@ -511,8 +511,7 @@ export function decryptMsgInline (decryptedMsgData) { } dispatch(updateMetamaskState(newState)) - decryptedMsgData = newState.unapprovedDecryptMsgs[decryptedMsgData.metamaskId] - return decryptedMsgData + return newState.unapprovedDecryptMsgs[decryptedMsgData.metamaskId] } }