Skip to content

Commit

Permalink
Enhance the padding-line lint rules (#1146)
Browse files Browse the repository at this point in the history
DEFRA/water-abstraction-team#115

We enhanced our linting rules to enforce [spaces around blocks and expressions](#1143). This is a great step to helping folks 'fall into the pit of success' and avoid reviewers having to nitpick PRs.

But having enabled it, we've spotted that it is highlighting some things that are perfectly acceptable.

We also think there are a few other options we could add.

So, this change is about building on the [padding-line-between-statements](https://eslint.style/rules/js/padding-line-between-statements) we've already added.

**Notes**

- Remove the config that is causing the violation
- Ensure blanks after 'use strict' declaration
- Ensure blanks after variable declarations

That last change ensures we put a blank line between variable declarations and the logic.

Running the linter across all files highlighted a number of violations. But the first 3 we fixed in this change give a good example of what to expect from this rule.

- `app/presenters/base.presenter.js` - simple example of separating variables from logic to help readability
- `app/presenters/bill-runs/two-part-tariff/review-bill-run.presenter.js` - demonstrating that comments are handled!
- `app/presenters/licences/set-up.presenter.js` - this is an example of compromise. It could be rightly argued it is better to have the var declaration next to the logic. But for the sake of consistency and having a tool to automate this stuff, we'll compromise and make the change
  • Loading branch information
Cruikshanks authored Jun 28, 2024
1 parent 0cfd7ef commit e5dab58
Show file tree
Hide file tree
Showing 5 changed files with 12 additions and 3 deletions.
10 changes: 7 additions & 3 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,16 @@ module.exports = {
'@stylistic/js/padding-line-between-statements': [
'error',
{ blankLine: 'always', prev: '*', next: 'block' },
{ blankLine: 'always', prev: '*', next: 'expression' },
{ blankLine: 'always', prev: '*', next: 'return' },
{ blankLine: 'always', prev: 'block', next: '*' },
{ blankLine: 'always', prev: 'block', next: 'function' },
{ blankLine: 'always', prev: 'expression', next: '*' },
{ blankLine: 'always', prev: 'function', next: '*' }
{ blankLine: 'always', prev: 'function', next: '*' },
// blank lines after every sequence of variable declarations
{ blankLine: 'always', prev: ['const', 'let', 'var'], next: '*' },
{ blankLine: 'any', prev: ['const', 'let', 'var'], next: ['const', 'let', 'var'] },
// blank lines after all directive prologues e.g. 'use strict'
{ blankLine: 'always', prev: 'directive', next: '*' },
{ blankLine: 'any', prev: 'directive', next: 'directive' }
],
'arrow-body-style': ['error', 'always'],
'import/extensions': ['error', 'always'],
Expand Down
1 change: 1 addition & 0 deletions app/controllers/root.controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
function index (_request, _h) {
return { status: 'alive' }
}

module.exports = {
index
}
1 change: 1 addition & 0 deletions app/presenters/base.presenter.js
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,7 @@ function titleCase (value) {

words.forEach((word) => {
const titleCasedWord = word.charAt(0).toUpperCase() + word.slice(1)

titleCasedWords.push(titleCasedWord)
})

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ function go (billRun, filterIssues, filterLicenceHolderNumber, filterLicenceStat
const issues = filterIssues ? _prepareIssues(filterIssues) : filterIssues

const filter = { issues, licenceHolderNumber: filterLicenceHolderNumber, licenceStatus: filterLicenceStatus }

// this opens the filter on the page if any filter data has been received so the user can see the applied filters
filter.openFilter = (filterIssues || filterLicenceHolderNumber || filterLicenceStatus) !== undefined

Expand Down
2 changes: 2 additions & 0 deletions app/presenters/licences/set-up.presenter.js
Original file line number Diff line number Diff line change
Expand Up @@ -157,10 +157,12 @@ function _endsSixYearsAgo (endDate) {
const timeStamp = { hour: 23, minutes: 59, seconds: 59, ms: 999 }

const yesterday = new Date()

yesterday.setDate(yesterday.getDate() - 1)
yesterday.setHours(timeStamp.hour, timeStamp.minutes, timeStamp.seconds, timeStamp.ms)

const sixYearsFromYesterday = new Date(yesterday.getTime())

sixYearsFromYesterday.setFullYear(yesterday.getFullYear() - sixYears)

return endDate.date < sixYearsFromYesterday
Expand Down

0 comments on commit e5dab58

Please sign in to comment.