Skip to content

Commit 84f312a

Browse files
hamirmahalhamirmahal
authored
feat: allow same-file const variable timeouts (#888)
for require-test-timeout rule
1 parent 5a408df commit 84f312a

2 files changed

Lines changed: 195 additions & 22 deletions

File tree

‎src/rules/require-test-timeout.ts‎

Lines changed: 171 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { createEslintRule, getAccessorValue } from '../utils'
22
import { parseVitestFnCall } from '../utils/parse-vitest-fn-call'
3+
import { getScope } from '../utils/scope'
34
import { AST_NODE_TYPES, TSESTree } from '@typescript-eslint/utils'
45
export const RULE_NAME = 'require-test-timeout'
56
export type MESSAGE_ID = 'missingTimeout'
@@ -18,6 +19,116 @@ export default createEslintRule<Options, MESSAGE_ID>({
1819
schema: [],
1920
},
2021
create(context) {
22+
function resolveConstTimeout(
23+
node: TSESTree.Node | undefined,
24+
propName = 'timeout',
25+
): number | undefined {
26+
if (!node) return undefined
27+
28+
if (
29+
node.type === AST_NODE_TYPES.Literal &&
30+
typeof node.value === 'number'
31+
)
32+
return node.value
33+
34+
// handle unary negative/positive numbers like -1
35+
if (
36+
node.type === AST_NODE_TYPES.UnaryExpression &&
37+
(node.operator === '-' || node.operator === '+') &&
38+
node.argument.type === AST_NODE_TYPES.Literal &&
39+
typeof node.argument.value === 'number'
40+
) {
41+
return node.operator === '-'
42+
? -node.argument.value
43+
: node.argument.value
44+
}
45+
46+
if (node.type === AST_NODE_TYPES.ObjectExpression) {
47+
for (const prop of node.properties) {
48+
if (prop.type !== AST_NODE_TYPES.Property) continue
49+
50+
const key = prop.key
51+
if (
52+
(key.type === AST_NODE_TYPES.Identifier && key.name === propName) ||
53+
(key.type === AST_NODE_TYPES.Literal && key.value === propName)
54+
) {
55+
if (
56+
prop.value.type === AST_NODE_TYPES.Literal &&
57+
typeof prop.value.value === 'number'
58+
) {
59+
return prop.value.value
60+
}
61+
62+
// if the value is an identifier, try to resolve that identifier to a const
63+
if (prop.value.type === AST_NODE_TYPES.Identifier) {
64+
const nested = resolveConstTimeout(prop.value, propName)
65+
if (nested !== undefined) return nested
66+
}
67+
68+
// explicitly present but non-numeric -> signal invalid
69+
return Number.NaN
70+
}
71+
}
72+
73+
return undefined
74+
}
75+
76+
if (node.type === AST_NODE_TYPES.Identifier) {
77+
const scope = getScope(context, node)
78+
const variable = scope.set.get(node.name)
79+
if (!variable || !variable.defs || variable.defs.length === 0)
80+
return undefined
81+
82+
for (const def of variable.defs) {
83+
if (def.type !== 'Variable') continue
84+
85+
// only accept `const` bindings
86+
const parent = def.parent
87+
if (!isVariableDeclaration(parent) || parent.kind !== 'const')
88+
continue
89+
90+
const declaratorNode = def.node
91+
if (!isVariableDeclarator(declaratorNode) || !declaratorNode.init)
92+
continue
93+
94+
const init = declaratorNode.init
95+
if (
96+
init.type === AST_NODE_TYPES.Literal &&
97+
typeof init.value === 'number'
98+
)
99+
return init.value
100+
101+
if (init.type === AST_NODE_TYPES.ObjectExpression) {
102+
for (const p of init.properties) {
103+
if (p.type !== AST_NODE_TYPES.Property) continue
104+
105+
const key = p.key
106+
if (
107+
(key.type === AST_NODE_TYPES.Identifier &&
108+
key.name === propName) ||
109+
(key.type === AST_NODE_TYPES.Literal && key.value === propName)
110+
) {
111+
if (
112+
p.value.type === AST_NODE_TYPES.Literal &&
113+
typeof p.value.value === 'number'
114+
)
115+
return p.value.value
116+
117+
if (p.value.type === AST_NODE_TYPES.Identifier) {
118+
const nested = resolveConstTimeout(p.value, propName)
119+
if (nested !== undefined) return nested
120+
}
121+
122+
return Number.NaN
123+
}
124+
}
125+
}
126+
}
127+
}
128+
129+
return undefined
130+
}
131+
21132
/**
22133
* Track positions (character offsets) of vi.setConfig({ testTimeout })
23134
* calls so we only exempt tests that appear _after_ the call. We use the
@@ -44,19 +155,25 @@ export default createEslintRule<Options, MESSAGE_ID>({
44155

45156
// Only accept a numeric literal >= 0 for testTimeout
46157
if (
47-
((key.type === AST_NODE_TYPES.Identifier &&
158+
(key.type === AST_NODE_TYPES.Identifier &&
48159
key.name === 'testTimeout') ||
49-
(key.type === AST_NODE_TYPES.Literal &&
50-
key.value === 'testTimeout')) &&
51-
prop.value.type === AST_NODE_TYPES.Literal &&
52-
typeof prop.value.value === 'number' &&
53-
prop.value.value >= 0
160+
(key.type === AST_NODE_TYPES.Literal &&
161+
key.value === 'testTimeout')
54162
) {
55-
const endOffset = node.range ? node.range[1] : 0
56-
57-
setConfigPositions.push(endOffset)
163+
const resolved = resolveConstTimeout(
164+
prop.value,
165+
'testTimeout',
166+
)
58167

59-
break
168+
if (
169+
resolved !== undefined &&
170+
!Number.isNaN(resolved) &&
171+
resolved >= 0
172+
) {
173+
const endOffset = node.range ? node.range[1] : 0
174+
setConfigPositions.push(endOffset)
175+
break
176+
}
60177
}
61178
}
62179
}
@@ -113,6 +230,24 @@ export default createEslintRule<Options, MESSAGE_ID>({
113230
}
114231
}
115232

233+
// identifier that resolves to a const numeric or const object with `timeout`
234+
if (a.type === AST_NODE_TYPES.Identifier) {
235+
const resolved = resolveConstTimeout(a, 'timeout')
236+
if (resolved !== undefined) {
237+
if (Number.isNaN(resolved)) {
238+
context.report({ node, messageId: 'missingTimeout' })
239+
return
240+
}
241+
242+
if (resolved >= 0) {
243+
foundNumericTimeout = true
244+
} else {
245+
context.report({ node, messageId: 'missingTimeout' })
246+
return
247+
}
248+
}
249+
}
250+
116251
// object literal with timeout property
117252
if (a.type === AST_NODE_TYPES.ObjectExpression) {
118253
for (const prop of a.properties) {
@@ -125,17 +260,20 @@ export default createEslintRule<Options, MESSAGE_ID>({
125260
key.name === 'timeout') ||
126261
(key.type === AST_NODE_TYPES.Literal && key.value === 'timeout')
127262
) {
128-
if (
129-
prop.value.type === AST_NODE_TYPES.Literal &&
130-
typeof prop.value.value === 'number' &&
131-
prop.value.value >= 0
132-
) {
133-
foundObjectTimeout = true
134-
} else {
135-
// any explicitly provided non-numeric or negative timeout is invalid
136-
context.report({ node, messageId: 'missingTimeout' })
137-
138-
return
263+
const resolved = resolveConstTimeout(prop.value, 'timeout')
264+
265+
if (resolved !== undefined) {
266+
if (Number.isNaN(resolved)) {
267+
context.report({ node, messageId: 'missingTimeout' })
268+
return
269+
}
270+
271+
if (resolved >= 0) {
272+
foundObjectTimeout = true
273+
} else {
274+
context.report({ node, messageId: 'missingTimeout' })
275+
return
276+
}
139277
}
140278
}
141279
}
@@ -149,3 +287,15 @@ export default createEslintRule<Options, MESSAGE_ID>({
149287
}
150288
},
151289
})
290+
291+
function isVariableDeclaration(
292+
node: TSESTree.Node | null | undefined,
293+
): node is TSESTree.VariableDeclaration {
294+
return !!node && node.type === AST_NODE_TYPES.VariableDeclaration
295+
}
296+
297+
function isVariableDeclarator(
298+
node: TSESTree.Node | null | undefined,
299+
): node is TSESTree.VariableDeclarator {
300+
return !!node && node.type === AST_NODE_TYPES.VariableDeclarator
301+
}

‎tests/require-test-timeout.test.ts‎

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,17 @@ ruleTester.run(RULE_NAME, rule, {
1717
'test.concurrent("a", () => {}, 400)',
1818
'test("a", () => {}, { timeout: 500 })',
1919
'test("a", { timeout: 500 }, () => {})',
20+
'const t = 500; test("a", { timeout: t }, () => {})',
21+
'const t = 500; test("a", () => {}, t)',
22+
'const opts = { timeout: 500 }; test("a", opts, () => {})',
23+
'const T = 1000; vi.setConfig({ testTimeout: T }); test("a", () => {})',
2024
'vi.setConfig({ testTimeout: 1000 }); test("a", () => {})',
25+
// mirrored `it` variants
26+
'const t = 500; it("a", { timeout: t }, () => {})',
27+
'const t = 500; it("a", () => {}, t)',
28+
'const opts = { timeout: 500 }; it("a", opts, () => {})',
29+
'const T = 1000; vi.setConfig({ testTimeout: T }); it("a", () => {})',
30+
'vi.setConfig({ testTimeout: 1000 }); it("a", () => {})',
2131
// multiple object args where one contains timeout
2232
'test("a", { foo: 1 }, { timeout: 500 }, () => {})',
2333
// both object and numeric timeout present
@@ -50,7 +60,20 @@ ruleTester.run(RULE_NAME, rule, {
5060
errors: [{ messageId: 'missingTimeout' }],
5161
},
5262
{
53-
code: 'const t = 500; test("a", { timeout: t }, () => {})',
63+
code: 'let t = 500; test("a", () => {}, t)',
64+
errors: [{ messageId: 'missingTimeout' }],
65+
},
66+
{
67+
code: 'const t = getTimeout(); test("a", () => {}, t)',
68+
errors: [{ messageId: 'missingTimeout' }],
69+
},
70+
// mirrored `it` invalid variants
71+
{
72+
code: 'let t = 500; it("a", () => {}, t)',
73+
errors: [{ messageId: 'missingTimeout' }],
74+
},
75+
{
76+
code: 'const t = getTimeout(); it("a", () => {}, t)',
5477
errors: [{ messageId: 'missingTimeout' }],
5578
},
5679
// null/undefined/identifier/negative cases

0 commit comments

Comments
 (0)