Skip to content

Commit e798c78

Browse files
sindresorhussindresorhus
committed
Add prefer-t-throws rule
Fixes #156
1 parent b800067 commit e798c78

5 files changed

Lines changed: 347 additions & 0 deletions

File tree

‎docs/rules/prefer-t-throws.md‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
# ava/prefer-t-throws
2+
3+
📝 Prefer `t.throws()` or `t.throwsAsync()` over try/catch.
4+
5+
💼 This rule is enabled in the ✅ `recommended` [config](https://github.com/avajs/eslint-plugin-ava#recommended-config).
6+
7+
<!-- end auto-generated rule header -->
8+
9+
Using try/catch with `t.fail()` to test that a function throws is verbose and error-prone. AVA provides `t.throws()` and `t.throwsAsync()` which are more concise and produce better failure output.
10+
11+
This rule flags try/catch blocks inside test functions where the try block contains a direct `t.fail()` call, since this is a strong signal the pattern should be replaced with `t.throws()` or `t.throwsAsync()`.
12+
13+
## Examples
14+
15+
```js
16+
import test from 'ava';
17+
18+
// ❌
19+
test('main', t => {
20+
try {
21+
foo();
22+
t.fail();
23+
} catch (error) {
24+
t.true(error.message === 'expected');
25+
}
26+
});
27+
28+
// ✅
29+
test('main', t => {
30+
const error = t.throws(() => foo());
31+
t.true(error.message === 'expected');
32+
});
33+
34+
// ❌
35+
test('main', async t => {
36+
try {
37+
await fetchData();
38+
t.fail();
39+
} catch (error) {
40+
t.is(error.statusCode, 500);
41+
}
42+
});
43+
44+
// ✅
45+
test('main', async t => {
46+
const error = await t.throwsAsync(fetchData());
47+
t.is(error.statusCode, 500);
48+
});
49+
```

‎index.js‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import noUselessTPass from './rules/no-useless-t-pass.js';
2525
import preferAsyncAwait from './rules/prefer-async-await.js';
2626
import preferPowerAssert from './rules/prefer-power-assert.js';
2727
import preferTRegex from './rules/prefer-t-regex.js';
28+
import preferTThrows from './rules/prefer-t-throws.js';
2829
import testTitle from './rules/test-title.js';
2930
import testTitleFormat from './rules/test-title-format.js';
3031
import useT from './rules/use-t.js';
@@ -62,6 +63,7 @@ const rules = {
6263
'prefer-async-await': preferAsyncAwait,
6364
'prefer-power-assert': preferPowerAssert,
6465
'prefer-t-regex': preferTRegex,
66+
'prefer-t-throws': preferTThrows,
6567
'test-title': testTitle,
6668
'test-title-format': testTitleFormat,
6769
'use-t': useT,
@@ -99,6 +101,7 @@ const recommendedRules = {
99101
'ava/prefer-async-await': 'error',
100102
'ava/prefer-power-assert': 'off',
101103
'ava/prefer-t-regex': 'error',
104+
'ava/prefer-t-throws': 'error',
102105
'ava/test-title': 'error',
103106
'ava/test-title-format': 'off',
104107
'ava/use-t': 'error',

‎readme.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ The rules will only activate in test files.
7676
| [prefer-async-await](docs/rules/prefer-async-await.md) | Prefer async/await over returning a Promise. | ✅ | | | | |
7777
| [prefer-power-assert](docs/rules/prefer-power-assert.md) | Enforce using only assertions compatible with [power-assert](https://github.com/power-assert-js/power-assert). | | | ✅ | | |
7878
| [prefer-t-regex](docs/rules/prefer-t-regex.md) | Prefer `t.regex()` over `RegExp#test()` and `String#match()`. | ✅ | | | 🔧 | |
79+
| [prefer-t-throws](docs/rules/prefer-t-throws.md) | Prefer `t.throws()` or `t.throwsAsync()` over try/catch. | ✅ | | | | |
7980
| [test-title](docs/rules/test-title.md) | Require tests to have a title. | ✅ | | | | |
8081
| [test-title-format](docs/rules/test-title-format.md) | Require test titles to match a pattern. | | | ✅ | | |
8182
| [use-t](docs/rules/use-t.md) | Require test functions to use `t` as their parameter. | ✅ | | | | |

‎rules/prefer-t-throws.js‎

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
1+
import {visitIf} from 'enhance-visitors';
2+
import createAvaRule from '../create-ava-rule.js';
3+
import util from '../util.js';
4+
5+
const MESSAGE_ID_SYNC = 'prefer-t-throws';
6+
const MESSAGE_ID_ASYNC = 'prefer-t-throws-async';
7+
8+
function findTFailIndex(body) {
9+
for (const [index, statement] of body.entries()) {
10+
if (
11+
statement.type === 'ExpressionStatement'
12+
&& statement.expression.type === 'CallExpression'
13+
&& statement.expression.callee.type === 'MemberExpression'
14+
) {
15+
const {callee} = statement.expression;
16+
const rootName = util.getNameOfRootNodeObject(callee);
17+
18+
if (
19+
util.isTestObject(rootName)
20+
&& !util.isPropertyUnderContext(callee)
21+
) {
22+
const members = util.getMembers(callee);
23+
const firstNonSkipMember = members.find(name => name !== 'skip');
24+
if (firstNonSkipMember === 'fail') {
25+
return index;
26+
}
27+
}
28+
}
29+
}
30+
31+
return -1;
32+
}
33+
34+
function hasDirectAwait(node) {
35+
if (!node || typeof node !== 'object') {
36+
return false;
37+
}
38+
39+
if (node.type === 'AwaitExpression') {
40+
return true;
41+
}
42+
43+
// `for await...of` is a ForOfStatement with `await: true`, not an AwaitExpression
44+
if (node.type === 'ForOfStatement' && node.await) {
45+
return true;
46+
}
47+
48+
// Don't descend into nested function scopes
49+
if (
50+
node.type === 'FunctionExpression'
51+
|| node.type === 'ArrowFunctionExpression'
52+
|| node.type === 'FunctionDeclaration'
53+
) {
54+
return false;
55+
}
56+
57+
for (const key of Object.keys(node)) {
58+
if (key === 'parent') {
59+
continue;
60+
}
61+
62+
const child = node[key];
63+
64+
if (Array.isArray(child)) {
65+
if (child.some(element => hasDirectAwait(element))) {
66+
return true;
67+
}
68+
} else if (child && typeof child === 'object' && child.type && hasDirectAwait(child)) {
69+
return true;
70+
}
71+
}
72+
73+
return false;
74+
}
75+
76+
const create = context => {
77+
const ava = createAvaRule();
78+
79+
return ava.merge({
80+
TryStatement: visitIf([
81+
ava.isInTestFile,
82+
ava.isInTestNode,
83+
])(node => {
84+
if (!node.handler) {
85+
return;
86+
}
87+
88+
const tFailIndex = findTFailIndex(node.block.body);
89+
90+
// No t.fail() found, or it's the first statement (no throwing code before it)
91+
if (tFailIndex < 1) {
92+
return;
93+
}
94+
95+
const statementsBeforeFail = node.block.body.slice(0, tFailIndex);
96+
const isAsync = statementsBeforeFail.some(statement => hasDirectAwait(statement));
97+
98+
context.report({
99+
node,
100+
messageId: isAsync ? MESSAGE_ID_ASYNC : MESSAGE_ID_SYNC,
101+
});
102+
}),
103+
});
104+
};
105+
106+
export default {
107+
create,
108+
meta: {
109+
type: 'suggestion',
110+
docs: {
111+
description: 'Prefer `t.throws()` or `t.throwsAsync()` over try/catch.',
112+
recommended: true,
113+
url: util.getDocsUrl(import.meta.filename),
114+
},
115+
schema: [],
116+
messages: {
117+
[MESSAGE_ID_SYNC]: 'Use `t.throws()` instead of try/catch.',
118+
[MESSAGE_ID_ASYNC]: 'Use `t.throwsAsync()` instead of try/catch.',
119+
},
120+
},
121+
};

‎test/prefer-t-throws.js‎

Lines changed: 173 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,173 @@
1+
import RuleTester from './helpers/rule-tester.js';
2+
import rule from '../rules/prefer-t-throws.js';
3+
4+
const ruleTester = new RuleTester();
5+
6+
const syncError = {messageId: 'prefer-t-throws'};
7+
const asyncError = {messageId: 'prefer-t-throws-async'};
8+
9+
ruleTester.run('prefer-t-throws', rule, {
10+
valid: [
11+
// Try/catch without `t.fail()`
12+
'test(t => { try { foo(); } catch (error) { t.is(error.message, "expected"); } });',
13+
// Try/finally without catch
14+
'test(t => { try { foo(); } finally { cleanup(); } });',
15+
// `t.fail()` inside a nested arrow function, not a direct statement
16+
'test(t => { try { foo(() => { t.fail(); }); } catch (error) { t.pass(); } });',
17+
// `t.fail()` inside a nested function expression, not a direct statement
18+
'test(t => { try { foo(function() { t.fail(); }); } catch (error) { t.pass(); } });',
19+
// `t.fail()` only in catch block, not in try block
20+
'test(t => { try { foo(); } catch (error) { t.fail(); } });',
21+
// Not a test file
22+
{code: 'test(t => { try { foo(); t.fail(); } catch (error) { t.pass(); } });', noHeader: true},
23+
// Empty try block
24+
'test(t => { try {} catch (error) { t.pass(); } });',
25+
// `t.fail()` inside nested if, not a direct statement
26+
'test(t => { try { if (true) { t.fail(); } } catch (error) { t.pass(); } });',
27+
// `t.context.fail()` is not `t.fail()`
28+
'test(t => { try { foo(); t.context.fail(); } catch (error) { t.pass(); } });',
29+
// `t.fail()` is the only statement in try block (no throwing code before it)
30+
'test(t => { try { t.fail(); } catch (error) { t.pass(); } });',
31+
// `t.fail()` is the first statement, code follows it
32+
'test(t => { try { t.fail(); foo(); } catch (error) { t.pass(); } });',
33+
// Not a test object (`foo.fail()`)
34+
'test(t => { try { bar(); foo.fail(); } catch (error) { t.pass(); } });',
35+
// `t.fail()` inside for loop in try block, not a direct statement
36+
'test(t => { try { for (const x of items) { t.fail(); } } catch (error) { t.pass(); } });',
37+
// `t.fail()` inside switch case in try block, not a direct statement
38+
'test(t => { try { switch (x) { case 1: t.fail(); } } catch (error) { t.pass(); } });',
39+
// `t.fail()` inside while loop
40+
'test(t => { try { while (true) { t.fail(); } } catch (error) { t.pass(); } });',
41+
// `t.fail()` inside block statement (bare braces)
42+
'test(t => { try { { t.fail(); } } catch (error) { t.pass(); } });',
43+
// `t.fail()` inside nested function declaration
44+
'test(t => { try { function f() { t.fail(); } f(); } catch (error) { t.pass(); } });',
45+
],
46+
invalid: [
47+
// Basic sync
48+
{
49+
code: 'test(t => { try { foo(); t.fail(); } catch (error) { t.is(error.message, "expected"); } });',
50+
errors: [syncError],
51+
},
52+
// Basic async with `await` in try block
53+
{
54+
code: 'test(async t => { try { await foo(); t.fail(); } catch (error) { t.is(error.message, "expected"); } });',
55+
errors: [asyncError],
56+
},
57+
// With message argument to `t.fail()`
58+
{
59+
code: 'test(t => { try { foo(); t.fail("should have thrown"); } catch (error) { t.true(error instanceof Error); } });',
60+
errors: [syncError],
61+
},
62+
// In a hook
63+
{
64+
code: 'test.beforeEach(t => { try { setup(); t.fail(); } catch (error) { t.pass(); } });',
65+
errors: [syncError],
66+
},
67+
// With serial modifier
68+
{
69+
code: 'test.serial(t => { try { foo(); t.fail(); } catch (error) { t.pass(); } });',
70+
errors: [syncError],
71+
},
72+
// Alternative test object name (`tt`)
73+
{
74+
code: 'test(t => { try { foo(); tt.fail(); } catch (error) { tt.pass(); } });',
75+
errors: [syncError],
76+
},
77+
// Alternative test object name (`t1`)
78+
{
79+
code: 'test(t => { try { foo(); t1.fail(); } catch (error) { t1.pass(); } });',
80+
errors: [syncError],
81+
},
82+
// `t.skip.fail()`
83+
{
84+
code: 'test(t => { try { foo(); t.skip.fail(); } catch (error) { t.pass(); } });',
85+
errors: [syncError],
86+
},
87+
// With `t.plan()`
88+
{
89+
code: 'test(t => { t.plan(1); try { foo(); t.fail(); } catch (error) { t.is(error.code, 1); } });',
90+
errors: [syncError],
91+
},
92+
// Async hook with `await` in try
93+
{
94+
code: 'test.afterEach(async t => { try { await cleanup(); t.fail(); } catch (error) { t.pass(); } });',
95+
errors: [asyncError],
96+
},
97+
// Try/catch/finally - still flags if try has `t.fail()`
98+
{
99+
code: 'test(t => { try { foo(); t.fail(); } catch (error) { t.pass(); } finally { cleanup(); } });',
100+
errors: [syncError],
101+
},
102+
// `t.fail()` with dead code after it in try block
103+
{
104+
code: 'test(t => { try { foo(); t.fail(); bar(); } catch (error) { t.pass(); } });',
105+
errors: [syncError],
106+
},
107+
// Empty catch body
108+
{
109+
code: 'test(t => { try { foo(); t.fail(); } catch (error) {} });',
110+
errors: [syncError],
111+
},
112+
// Catch without binding
113+
{
114+
code: 'test(t => { try { foo(); t.fail(); } catch {} });',
115+
errors: [syncError],
116+
},
117+
// Async test but sync try block (no `await`) - should suggest `t.throws()`, not `t.throwsAsync()`
118+
{
119+
code: 'test(async t => { try { foo(); t.fail(); } catch (error) { t.pass(); } });',
120+
errors: [syncError],
121+
},
122+
// `await` only inside nested function in try - direct code is sync, so `t.throws()`
123+
{
124+
code: 'test(async t => { try { foo(async () => { await bar(); }); t.fail(); } catch (error) { t.pass(); } });',
125+
errors: [syncError],
126+
},
127+
// Variable declaration with `await` before `t.fail()`
128+
{
129+
code: 'test(async t => { try { const result = await fetch(url); t.fail(); } catch (error) { t.is(error.status, 404); } });',
130+
errors: [asyncError],
131+
},
132+
// Multiple statements before `t.fail()`
133+
{
134+
code: 'test(t => { try { setup(); foo(); bar(); t.fail(); } catch (error) { t.pass(); } });',
135+
errors: [syncError],
136+
},
137+
// Multiple statements with `await` before `t.fail()`
138+
{
139+
code: 'test(async t => { try { setup(); await foo(); t.fail(); } catch (error) { t.pass(); } });',
140+
errors: [asyncError],
141+
},
142+
// Nested try/catch both with `t.fail()` - two errors
143+
{
144+
code: 'test(async t => { try { try { await foo(); t.fail(); } catch (e) { t.pass(); } await bar(); t.fail(); } catch (e) { t.pass(); } });',
145+
errors: [asyncError, asyncError],
146+
},
147+
// `test.after` hook
148+
{
149+
code: 'test.after(t => { try { cleanup(); t.fail(); } catch (error) { t.pass(); } });',
150+
errors: [syncError],
151+
},
152+
// `test.after.always` hook
153+
{
154+
code: 'test.after.always(t => { try { cleanup(); t.fail(); } catch (error) { t.pass(); } });',
155+
errors: [syncError],
156+
},
157+
// `for await...of` in try block should suggest `t.throwsAsync()`
158+
{
159+
code: 'test(async t => { try { for await (const x of stream) { process(x); } t.fail(); } catch (error) { t.pass(); } });',
160+
errors: [asyncError],
161+
},
162+
// `await` inside regular `for...of` body (not `for await...of`) should suggest `t.throwsAsync()`
163+
{
164+
code: 'test(async t => { try { for (const x of items) { await process(x); } t.fail(); } catch (error) { t.pass(); } });',
165+
errors: [asyncError],
166+
},
167+
// Multiple independent try/catch blocks - each flagged separately
168+
{
169+
code: 'test(t => { try { foo(); t.fail(); } catch (error) { t.pass(); } try { bar(); t.fail(); } catch (error) { t.pass(); } });',
170+
errors: [syncError, syncError],
171+
},
172+
],
173+
});

0 commit comments

Comments
 (0)