Skip to content

Commit 7c2fcf6

Browse files
committed
Add no-nested-assertions rule
Fixes #150
1 parent b8c93c2 commit 7c2fcf6

5 files changed

Lines changed: 199 additions & 0 deletions

File tree

docs/rules/no-nested-assertions.md

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
# ava/no-nested-assertions
2+
3+
📝 Disallow nested assertions.
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+
Disallow nesting assertions, such as using an assertion as an argument to another assertion or putting assertions inside a `t.throws()` callback.
10+
11+
Nested assertions are confusing and error-prone. For example, assertions inside `t.throws()` will have their failures caught by `throws`, and assertions used as arguments make the code harder to read.
12+
13+
## Examples
14+
15+
```js
16+
import test from 'ava';
17+
18+
//
19+
test('main', t => {
20+
t.is(t.throws(() => foo()).message, 'expected');
21+
});
22+
23+
//
24+
test('main', t => {
25+
const error = t.throws(() => foo());
26+
t.is(error.message, 'expected');
27+
});
28+
29+
//
30+
test('main', t => {
31+
t.throws(() => {
32+
t.is(1, 2);
33+
});
34+
});
35+
36+
//
37+
test('main', t => {
38+
t.throws(() => foo(), {message: 'expected'});
39+
});
40+
```

index.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import noIgnoredTestFiles from './rules/no-ignored-test-files.js';
1313
import noImportTestFiles from './rules/no-import-test-files.js';
1414
import noIncorrectDeepEqual from './rules/no-incorrect-deep-equal.js';
1515
import noInlineAssertions from './rules/no-inline-assertions.js';
16+
import noNestedAssertions from './rules/no-nested-assertions.js';
1617
import noNestedTests from './rules/no-nested-tests.js';
1718
import noOnlyTest from './rules/no-only-test.js';
1819
import noSkipAssert from './rules/no-skip-assert.js';
@@ -48,6 +49,7 @@ const rules = {
4849
'no-import-test-files': noImportTestFiles,
4950
'no-incorrect-deep-equal': noIncorrectDeepEqual,
5051
'no-inline-assertions': noInlineAssertions,
52+
'no-nested-assertions': noNestedAssertions,
5153
'no-nested-tests': noNestedTests,
5254
'no-only-test': noOnlyTest,
5355
'no-skip-assert': noSkipAssert,
@@ -83,6 +85,7 @@ const recommendedRules = {
8385
'ava/no-import-test-files': 'error',
8486
'ava/no-incorrect-deep-equal': 'error',
8587
'ava/no-inline-assertions': 'error',
88+
'ava/no-nested-assertions': 'error',
8689
'ava/no-nested-tests': 'error',
8790
'ava/no-only-test': 'error',
8891
'ava/no-skip-assert': 'error',

readme.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ The rules will only activate in test files.
6464
| [no-import-test-files](docs/rules/no-import-test-files.md) | Disallow importing test files. || | | | |
6565
| [no-incorrect-deep-equal](docs/rules/no-incorrect-deep-equal.md) | Disallow using `deepEqual` with primitives. || | | 🔧 | |
6666
| [no-inline-assertions](docs/rules/no-inline-assertions.md) | Disallow inline assertions. || | | 🔧 | |
67+
| [no-nested-assertions](docs/rules/no-nested-assertions.md) | Disallow nested assertions. || | | | |
6768
| [no-nested-tests](docs/rules/no-nested-tests.md) | Disallow nested tests. || | | | |
6869
| [no-only-test](docs/rules/no-only-test.md) | Disallow `test.only()`. || | | | 💡 |
6970
| [no-skip-assert](docs/rules/no-skip-assert.md) | Disallow skipping assertions. || | | | 💡 |

rules/no-nested-assertions.js

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
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 = 'no-nested-assertions';
6+
7+
const create = context => {
8+
const ava = createAvaRule();
9+
const assertionCallStack = [];
10+
11+
return ava.merge({
12+
CallExpression: visitIf([
13+
ava.isInTestFile,
14+
ava.isInTestNode,
15+
])(node => {
16+
if (node.callee.type !== 'MemberExpression') {
17+
return;
18+
}
19+
20+
const rootName = util.getNameOfRootNodeObject(node.callee);
21+
if (!util.isTestObject(rootName)) {
22+
return;
23+
}
24+
25+
const methodName = util.getMembers(node.callee)[0];
26+
if (!util.assertionMethods.has(methodName)) {
27+
return;
28+
}
29+
30+
if (assertionCallStack.length > 0) {
31+
context.report({node, messageId: MESSAGE_ID});
32+
}
33+
34+
// Don't track `t.try()`, its callback is designed to contain assertions
35+
if (methodName !== 'try') {
36+
assertionCallStack.push(node);
37+
}
38+
}),
39+
'CallExpression:exit'(node) {
40+
if (assertionCallStack.length > 0 && assertionCallStack.at(-1) === node) {
41+
assertionCallStack.pop();
42+
}
43+
},
44+
});
45+
};
46+
47+
export default {
48+
create,
49+
meta: {
50+
type: 'problem',
51+
docs: {
52+
description: 'Disallow nested assertions.',
53+
recommended: true,
54+
url: util.getDocsUrl(import.meta.filename),
55+
},
56+
schema: [],
57+
messages: {
58+
[MESSAGE_ID]: 'Assertions should not be nested.',
59+
},
60+
},
61+
};

test/no-nested-assertions.js

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
import test from 'ava';
2+
import AvaRuleTester from 'eslint-ava-rule-tester';
3+
import rule from '../rules/no-nested-assertions.js';
4+
5+
const ruleTester = new AvaRuleTester(test, {
6+
languageOptions: {
7+
ecmaVersion: 'latest',
8+
},
9+
});
10+
11+
const header = 'const test = require(\'ava\');\n';
12+
const error = {
13+
messageId: 'no-nested-assertions',
14+
};
15+
16+
ruleTester.run('no-nested-assertions', rule, {
17+
assertionOptions: {
18+
requireMessage: true,
19+
},
20+
valid: [
21+
// Sequential assertions
22+
header + 'test(t => { t.is(1, 1); t.true(true); });',
23+
// Assertions in t.try() callback are fine
24+
header + 'test(t => { t.try(tt => { tt.is(1, 1); }); });',
25+
// Multiple assertions in t.try() callback are fine
26+
header + 'test(t => { t.try(tt => { tt.is(1, 1); tt.true(true); }); });',
27+
// Non-assertion methods
28+
header + 'test(t => { t.plan(1); t.is(1, 1); });',
29+
// Not a test object
30+
header + 'test(t => { foo.is(t.is(1, 1)); });',
31+
// Shouldn't be triggered since it's not a test file
32+
'test(t => { t.is(t.throws(fn).message, "expected"); });',
33+
],
34+
invalid: [
35+
// Assertion as argument to another assertion
36+
{
37+
code: header + 'test(t => { t.is(t.throws(fn).message, "expected"); });',
38+
errors: [error],
39+
},
40+
{
41+
code: header + 'test(t => { t.true(t.throws(fn).message); });',
42+
errors: [error],
43+
},
44+
{
45+
code: header + 'test(t => { t.deepEqual(t.throws(fn), expected); });',
46+
errors: [error],
47+
},
48+
// Assertion inside t.throws() callback
49+
{
50+
code: header + 'test(t => { t.throws(() => { t.is(1, 2); }); });',
51+
errors: [error],
52+
},
53+
// Assertion inside t.throwsAsync() callback
54+
{
55+
code: header + 'test(t => { t.throwsAsync(() => { t.true(false); }); });',
56+
errors: [error],
57+
},
58+
// Assertion inside t.notThrows() callback
59+
{
60+
code: header + 'test(t => { t.notThrows(() => { t.pass(); }); });',
61+
errors: [error],
62+
},
63+
// Assertion inside t.notThrowsAsync() callback
64+
{
65+
code: header + 'test(t => { t.notThrowsAsync(() => { t.true(true); }); });',
66+
errors: [error],
67+
},
68+
// Multiple assertions inside t.throws() callback
69+
{
70+
code: header + 'test(t => { t.throws(() => { t.is(1, 2); t.true(false); }); });',
71+
errors: [error, error],
72+
},
73+
// Deeply nested
74+
{
75+
code: header + 'test(t => { t.is(t.throws(() => { t.pass(); }).message, "x"); });',
76+
errors: [error, error],
77+
},
78+
// With .skip modifier
79+
{
80+
code: header + 'test(t => { t.is(t.throws.skip(fn).message, "expected"); });',
81+
errors: [error],
82+
},
83+
// Nesting within t.try() callback is still caught
84+
{
85+
code: header + 'test(t => { t.try(tt => { tt.is(tt.throws(fn).message, "expected"); }); });',
86+
errors: [error],
87+
},
88+
// Alternative test object names
89+
{
90+
code: header + 'test(t => { tt.is(tt.throws(fn).message, "expected"); });',
91+
errors: [error],
92+
},
93+
],
94+
});

0 commit comments

Comments
 (0)