Skip to content

Commit 50e719d

Browse files
committed
Add allow option to no-setup-in-describe
1 parent 1025c6f commit 50e719d

3 files changed

Lines changed: 174 additions & 6 deletions

File tree

documentation/rules/no-setup-in-describe.md

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,11 @@ Any setup directly in a `describe` is run before all tests execute. This is unde
2121
1. When doing TDD in a large codebase, all setup is run for tests that don't have `only` set. This can add a substantial amount of time per iteration.
2222
2. If global state is altered by the setup of another describe block, your test may be affected.
2323

24-
This rule reports all function calls and use of the dot operator (due to getters and setters) directly in describe blocks. An exception is made for Mocha's suite configuration methods, like `this.timeout();`, which do not represent setup logic.
24+
For this rule, "setup" means code that executes immediately while the suite callback itself is evaluated. That includes work done directly in the suite body before any tests run, even when the code looks harmless.
25+
26+
In practice, this rule reports direct function calls and direct property access in suite bodies. Property access is included because getters can execute code. Exceptions are made for Mocha's structural calls (`describe`, `it`, hooks, and their supported variants) and Mocha suite configuration calls such as `this.timeout();`, `it(...).timeout();`, or `before(...).timeout();`.
27+
28+
This rule does not make special exceptions for JavaScript builtins. For example, `Symbol()` is still a direct function call in the suite body and is reported by default.
2529

2630
If you're using [dynamically generated tests](https://mochajs.org/#dynamically-generating-tests), you should disable this rule.
2731

@@ -75,3 +79,40 @@ describe('something', function () {
7579
it('should take awhile', function () {});
7680
});
7781
```
82+
83+
## Options
84+
85+
This rule accepts one optional object:
86+
87+
- `allow`: an array of call names that should be allowed directly in suite bodies
88+
89+
Entries may be written with or without `()`. Dotted names are supported.
90+
91+
```json
92+
{
93+
"rules": {
94+
"mocha/no-setup-in-describe": ["error", {
95+
"allow": ["Symbol", "Object.freeze"]
96+
}]
97+
}
98+
}
99+
```
100+
101+
With this option, the following patterns would not be considered problems:
102+
103+
```js
104+
describe('something', function () {
105+
const token = Symbol('id');
106+
Object.freeze(sharedFixture);
107+
it('should work', function () {});
108+
});
109+
```
110+
111+
The `allow` option only applies to calls. It does not allow bare property access:
112+
113+
```js
114+
describe('something', function () {
115+
Object.freeze;
116+
it('should work', function () {});
117+
});
118+
```

source/rules/no-setup-in-describe.test.ts

Lines changed: 69 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1-
import { RuleTester } from 'eslint';
1+
import { type Rule, RuleTester } from 'eslint';
2+
import assert from 'node:assert';
23
import { noSetupInDescribeRule } from './no-setup-in-describe.js';
34

45
const ruleTester = new RuleTester({ languageOptions: { sourceType: 'script' } });
@@ -105,7 +106,23 @@ ruleTester.run('no-setup-in-describe', noSetupInDescribeRule, {
105106
code: 'describe("", function () { const bar = () => { a.b = "c"; }; it(); })',
106107
languageOptions: { ecmaVersion: 2015 }
107108
},
108-
'describe("", function () { var bar = function () { a.b = "c"; }; it(); })'
109+
'describe("", function () { var bar = function () { a.b = "c"; }; it(); })',
110+
{
111+
code: 'describe("", function () { const token = Symbol("bar"); it(); })',
112+
options: [{ allow: ['Symbol'] }]
113+
},
114+
{
115+
code: 'describe("", function () { const token = Symbol("bar"); it(); })',
116+
options: [{ allow: ['Symbol()'] }]
117+
},
118+
{
119+
code: 'describe("", function () { Object.freeze({}); it(); })',
120+
options: [{ allow: ['Object.freeze'] }]
121+
},
122+
{
123+
code: 'describe("", function () { Object.freeze({}); it(); })',
124+
options: [{ allow: ['Object.freeze()'] }]
125+
}
109126
],
110127

111128
invalid: [
@@ -282,6 +299,56 @@ ruleTester.run('no-setup-in-describe', noSetupInDescribeRule, {
282299
column: 28
283300
}
284301
]
302+
},
303+
{
304+
code: 'describe("", function () { const token = Symbol("bar"); it(); })',
305+
errors: [
306+
{
307+
message: 'Unexpected function call in describe block.',
308+
line: 1,
309+
column: 42
310+
}
311+
]
312+
},
313+
{
314+
code: 'describe("", function () { const token = Symbol("bar"); helper(); it(); })',
315+
options: [{ allow: ['Symbol'] }],
316+
errors: [
317+
{
318+
message: 'Unexpected function call in describe block.',
319+
line: 1,
320+
column: 57
321+
}
322+
]
323+
},
324+
{
325+
code: 'describe("", function () { Object.freeze; it(); })',
326+
options: [{ allow: ['Object.freeze'] }],
327+
errors: [
328+
{
329+
message: memberExpressionError,
330+
line: 1,
331+
column: 28
332+
}
333+
]
285334
}
286335
]
287336
});
337+
338+
describe('no-setup-in-describe create()', function () {
339+
it('normalizes non-string allow entries when invoked directly', function () {
340+
noSetupInDescribeRule.create({
341+
id: 'no-setup-in-describe',
342+
options: [{ allow: [42] }],
343+
settings: {},
344+
sourceCode: {
345+
ast: { body: [] },
346+
scopeManager: {
347+
globalScope: null
348+
}
349+
}
350+
} as unknown as Rule.RuleContext);
351+
352+
assert.ok(true);
353+
});
354+
});

source/rules/no-setup-in-describe.ts

Lines changed: 63 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,30 @@
11
import type { Rule } from 'eslint';
2+
import { extractMemberExpressionPath, isConstantPath } from '../ast/member-expression.js';
23
import { createMochaVisitors } from '../ast/mocha-visitors.js';
34
import type { CallExpression, MemberExpression } from '../ast/node-types.js';
45
import { isSuiteConfigCall } from '../mocha/config-call.js';
6+
import { reformatLastPathSegmentWithCallExpressions } from '../mocha/name-details.js';
7+
import { convertNameToPathArray, isSamePath } from '../mocha/path.js';
8+
import { getRuleOption, type InferSchemaOption, type RuleSchema } from '../rule-options.js';
59

610
const FUNCTION = 1;
711
const DESCRIBE = 2;
12+
const optionSchema = {
13+
type: 'object',
14+
properties: {
15+
allow: {
16+
type: 'array',
17+
items: {
18+
type: 'string'
19+
}
20+
}
21+
},
22+
additionalProperties: false
23+
} as const satisfies RuleSchema;
24+
25+
type Option = InferSchemaOption<typeof optionSchema>;
26+
type ResolvedOption = Option & { allow: string[]; };
27+
const defaultOption: ResolvedOption = { allow: [] };
828

929
function isNestedInDescribeBlock(nesting: readonly number[]): boolean {
1030
return (
@@ -30,6 +50,25 @@ function reportMemberExpression(
3050
});
3151
}
3252

53+
function ensureEndsWithParens(value: unknown): string {
54+
if (typeof value !== 'string') {
55+
return '';
56+
}
57+
58+
if (value.endsWith('()')) {
59+
return value;
60+
}
61+
62+
return `${value}()`;
63+
}
64+
65+
function normalizeAllowedCall(value: unknown): readonly string[] {
66+
const path = convertNameToPathArray(ensureEndsWithParens(value));
67+
const [lastPathSegment] = path.slice(-1);
68+
69+
return [...path.slice(0, -1), ensureEndsWithParens(lastPathSegment)];
70+
}
71+
3372
export const noSetupInDescribeRule: Readonly<Rule.RuleModule> = {
3473
meta: {
3574
type: 'suggestion',
@@ -38,19 +77,39 @@ export const noSetupInDescribeRule: Readonly<Rule.RuleModule> = {
3877
description: 'Disallow setup in describe blocks',
3978
url: 'https://github.com/lo1tuma/eslint-plugin-mocha/blob/main/documentation/rules/no-setup-in-describe.md'
4079
},
80+
defaultOptions: [defaultOption],
4181
messages: {
4282
unexpectedFunctionCall: 'Unexpected function call in describe block.',
4383
unexpectedMemberExpression:
4484
'Unexpected member expression in describe block. Member expressions may call functions via getters.'
4585
},
46-
schema: []
86+
schema: [optionSchema]
4787
},
4888
create(context) {
89+
const { allow } = getRuleOption<ResolvedOption>(context);
90+
const allowedCalls = allow.map(normalizeAllowedCall);
4991
const nesting: number[] = [];
5092
const suiteNodes = new WeakSet();
5193

94+
function isAllowedCall(node: Readonly<CallExpression>): boolean {
95+
const calleeWithParent = { ...node.callee, parent: node };
96+
const path = reformatLastPathSegmentWithCallExpressions(
97+
extractMemberExpressionPath(context.sourceCode, calleeWithParent),
98+
1
99+
);
100+
101+
return isConstantPath(path) &&
102+
allowedCalls.some((allowedCall) => {
103+
return isSamePath(path, allowedCall);
104+
});
105+
}
106+
107+
function isAllowedCallMemberExpression(node: Readonly<MemberExpression>): boolean {
108+
return node.parent.type === 'CallExpression' && node.parent.callee === node && isAllowedCall(node.parent);
109+
}
110+
52111
function handleCallExpressionInDescribe(node: Readonly<CallExpression>): void {
53-
if (isNestedInDescribeBlock(nesting) && !isSuiteConfigCall(node)) {
112+
if (isNestedInDescribeBlock(nesting) && !isSuiteConfigCall(node) && !isAllowedCall(node)) {
54113
reportCallExpression(context, node);
55114
}
56115
}
@@ -76,7 +135,8 @@ export const noSetupInDescribeRule: Readonly<Rule.RuleModule> = {
76135
if (
77136
!suiteNodes.has(node.parent) &&
78137
isNestedInDescribeBlock(nesting) &&
79-
!isSuiteConfigCall(node.parent)
138+
!isSuiteConfigCall(node.parent) &&
139+
!isAllowedCallMemberExpression(node)
80140
) {
81141
reportMemberExpression(context, node);
82142
}

0 commit comments

Comments
 (0)