Skip to content

Commit

Permalink
Ensure blank line b4 function return statements (#1007)
Browse files Browse the repository at this point in the history
DEFRA/water-abstraction-team#115

This has been an unwritten convention we often pick up in the PRs of those new to the team. We like to see a blank line before the `return` statement in a function. We believe it helps make the code easier to follow and understand.

```javascript
// Bad
function rubbishNameGenerator () {
  const firstName = 'John'
  const lastName = 'Doe'
  return `${firstName} ${lastName}`
}

// Good
function rubbishNameGenerator () {
  const firstName = 'John'
  const lastName = 'Doe'

  return `${firstName} ${lastName}`
}
```

But we don't want to see it all the time!

```javascript
// Bad
function rubbishNameGeneratorV2 (useAlternate = false) {
  if (useAlternate) {

    return 'Jane Doe'
  }

  return 'John Doe'
}

// Good
function rubbishNameGeneratorV2 (useAlternate = false) {
  if (useAlternate) {
    return 'Jane Doe'
  }

  return 'John Doe'
}
```

We weren't sure we could make ESLint see the difference but it turns out we can! So, this change enables and adds an initial configuration for the rule [padding-line-between-statements](https://eslint.style/rules/js/padding-line-between-statements).

> We expect to expand the config to cover some more unwritten conventions regarding white space in our code. But those will come separately!
  • Loading branch information
Cruikshanks authored May 13, 2024
1 parent e88d651 commit 71727aa
Show file tree
Hide file tree
Showing 16 changed files with 21 additions and 1 deletion.
6 changes: 5 additions & 1 deletion .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,11 @@ module.exports = {
ignoreStrings: true,
ignoreTemplateLiterals: true,
ignoreUrls: true
}]
}],
'@stylistic/js/padding-line-between-statements': [
'error',
{ blankLine: 'always', prev: '*', next: 'return' }
]
}
}

Expand Down
1 change: 1 addition & 0 deletions app/controllers/return-requirements.controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,7 @@ async function startDate (request, h) {
const { sessionId } = request.params

const pageData = await StartDateService.go(sessionId)

return h.view('return-requirements/start-date.njk', {
...pageData
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ function go (billRun) {
status,
toFinancialYearEnding
} = billRun

return {
batch: {
id,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ function _prepareDate (startDate, endDate) {
function _totalBillableReturns (reviewChargeElements) {
return reviewChargeElements.reduce((total, reviewChargeElement) => {
total += reviewChargeElement.amendedAllocated

return total
}, 0)
}
Expand Down
1 change: 1 addition & 0 deletions app/presenters/licences/view-licence-summary.presenter.js
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,7 @@ function _generateAbstractionPeriods (licenceVersions) {
const formattedAbstactionPeriods = licenceVersions[0].licenceVersionPurposes.map((purpose) => {
const startDate = formatAbstractionDate(purpose.abstractionPeriodStartDay, purpose.abstractionPeriodStartMonth)
const endDate = formatAbstractionDate(purpose.abstractionPeriodEndDay, purpose.abstractionPeriodEndMonth)

return `${startDate} to ${endDate}`
})

Expand Down
1 change: 1 addition & 0 deletions app/services/bill-runs/annual/process-bill-run.service.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ async function _finaliseBillRun (billRun, billRunPopulated) {
// this in the UI
if (!billRunPopulated) {
await _updateStatus(billRun.id, 'empty')

return
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@ function _consolidateAndCalculate (referencePeriod, abstractionsPeriods) {

const totalDays = consolidatedAbstractionPeriods.reduce((acc, abstractionPeriod) => {
const abstractionOverlapPeriod = _calculateAbstractionOverlapPeriod(referencePeriod, abstractionPeriod)

return acc + _calculateDays(abstractionOverlapPeriod)
}, 0)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ function _billLicenceKey (billId, licenceId) {
function _preGenerateBills (billingAccounts, billRunId, billingPeriod) {
const keyedBills = billingAccounts.reduce((acc, billingAccount) => {
const { id: billingAccountId, accountNumber } = billingAccount

// Note that the array of billing accounts will already have been deduped so we don't need to check whether a
// bill licence already exists in the object before generating one
return {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ async function _finaliseBillRun (billRun, accumulatedLicenceIds, resultsOfProces
// this in the UI
if (!isPopulated) {
await _updateStatus(billRun.id, 'empty')

return
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,13 @@ function _bannerMessage (yar, parsedPayload) {

if (status) {
yar.flash('banner', `Licence changed to ${status}.`)

return
}

if (progress) {
yar.flash('banner', 'This licence has been marked.')

return
}

Expand Down
1 change: 1 addition & 0 deletions app/services/health/fetch-system-info.service.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ async function go () {
async function _getCommitHash () {
try {
const { stdout, stderr } = await exec('git rev-parse HEAD')

return stderr ? `ERROR: ${stderr}` : stdout.replace('\n', '')
} catch (error) {
return `ERROR: ${error.message}`
Expand Down
1 change: 1 addition & 0 deletions app/services/health/info.service.js
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@ async function _getRedisConnectivityData () {
async function _getVirusScannerData () {
try {
const { stdout, stderr } = await exec('clamdscan --version')

return stderr ? `ERROR: ${stderr}` : stdout
} catch (error) {
return `ERROR: ${error.message}`
Expand Down
1 change: 1 addition & 0 deletions app/services/idm/fetch-user-roles-and-groups.service.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ function _extractRolesFromGroups (groups) {
return groups.flatMap((group) => {
const { roles } = group
delete group.roles

return roles
})
}
Expand Down
1 change: 1 addition & 0 deletions app/services/licences/fetch-licence.service.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ async function _fetchLicence (id) {
])
})
.modify('registeredToAndLicenceName')

return result
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ const LicenceModel = require('../../models/licence.model.js')
async function go (id) {
const licence = await _fetchLicence(id)
const licenceEnded = _licenceEnded(licence)

return licenceEnded
}

Expand Down
1 change: 1 addition & 0 deletions app/validators/return-requirements/start-date.validator.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ function go (payload, licenceStartDate, licenceEndDate) {

if (selectedOption === 'anotherStartDate') {
payload.fullDate = _fullDate(payload)

return _validateAnotherStartDate(payload, licenceStartDate, licenceEndDate)
}

Expand Down

0 comments on commit 71727aa

Please sign in to comment.