Skip to content

Commit

Permalink
fix(no-unused-vars)!: enforce rule for unused arguments
Browse files Browse the repository at this point in the history
Previously we allowed unused arguments entirely, which can be a footgun
(particularly in Tap tests with `t`). This enforces the `no-unused-vars`
rule for arguments too, allowing unused arguments as long as later args
_are_ used.

BREAKING CHANGES: all named arguments and all positional arguments
_after the last used argument_ will now be checked.

Closes: #26
  • Loading branch information
mdeltito committed Dec 6, 2022
1 parent 648adb3 commit 697525e
Show file tree
Hide file tree
Showing 7 changed files with 49 additions and 7 deletions.
2 changes: 1 addition & 1 deletion .eslintrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@
"no-unused-labels": 2,
"no-unused-vars": [2, {
"vars": "all",
"args": "none",
"args": "after-used",
"ignoreRestSiblings": true
}],
"no-use-before-define": [2, {
Expand Down
7 changes: 7 additions & 0 deletions test/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,11 @@ test('valid config', async (t) => {

const [result] = await linter.lintText(code)
t.equal(result.errorCount, 0, 'error count')
// provide more context for failures by emitting a separate `fail`
// for each unexpected linting error
if (result.messages.length) {
for (const {message} of result.messages) {
t.fail(message)
}
}
}).catch(threw)
28 changes: 26 additions & 2 deletions test/failures.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,31 @@ test('Invalid linting for larger code blocks read from fixtures', async (t) => {
)
})

t.test('no-multlintFilesiple-empty-lines', async (t) => {
t.test('no-unused-vars for arguments', async (t) => {
const [result] = await linter.lintFiles(['no-unused-args-fixture'])
t.equal(result.errorCount, 2, 'error count')

const messages = result.messages
t.ok(
messages.every(({ruleId}) => {
return ruleId === 'no-unused-vars'
})
, 'errors from expected rule'
)

t.match(
messages[0].message
, "'c' is defined but never used."
, 'message expected for unused after-used'
)
t.match(
messages[1].message
, "'r' is defined but never used."
, 'message expected for unused'
)
})

t.test('no-multiple-empty-lines', async (t) => {
const [result] = await linter.lintFiles(['no-multiple-empty-lines-fixture'])
t.equal(result.errorCount, 1, 'error count')
const messages = result.messages
Expand All @@ -49,7 +73,7 @@ test('Invalid linting for larger code blocks read from fixtures', async (t) => {
t.equal(result.messages[0].ruleId, 'no-debugger', 'missing newline')
})

t.test('pluglintFilesin-logdna', async (t) => {
t.test('plugin-logdna', async (t) => {
const [result] = await linter.lintFiles(['logdna-plugin-fixture'])
const messages = result.messages
t.equal(result.errorCount, 6, 'error count')
Expand Down
2 changes: 1 addition & 1 deletion test/fixtures/function-call-argument-newline-fixture
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
module.exports = fooBar

function fooBar(a, b, c) {
return true
return b + c
}

fooBar('asdf',
Expand Down
4 changes: 2 additions & 2 deletions test/fixtures/function-paren-newline-fixture
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ function fooBar(a
, b
, c
) {
return true
return b + c
}

fooBar(
Expand All @@ -19,7 +19,7 @@ function twoBar(
a
, b, c
) {
return true
return c
}

twoBar(
Expand Down
11 changes: 11 additions & 0 deletions test/fixtures/no-unused-args-fixture
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
'use strict'

module.exports = fooBar

function fooBar(a, b, c) {
return a + b + baz('test')
}

function baz(r) {
return true
}
2 changes: 1 addition & 1 deletion test/fixtures/valid-code
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ async function doWork(opts) {
console.log('ready', 11 + 33)
})()

function plugin(opts) {
function plugin() {
if (this.is_new) {
this.id = Date.now()
} else {
Expand Down

0 comments on commit 697525e

Please sign in to comment.