Skip to content

explicit-timer-delay: Add rule#2814

Open
azat-io wants to merge 8 commits intosindresorhus:mainfrom
azat-io:feat/explicit-timer-delay
Open

explicit-timer-delay: Add rule#2814
azat-io wants to merge 8 commits intosindresorhus:mainfrom
azat-io:feat/explicit-timer-delay

Conversation

@azat-io
Copy link
Copy Markdown
Contributor

@azat-io azat-io commented Nov 3, 2025

Close #2809

@azat-io azat-io force-pushed the feat/explicit-timer-delay branch from 8a05b7f to e1c0b10 Compare November 3, 2025 18:12
@azat-io
Copy link
Copy Markdown
Contributor Author

azat-io commented Jan 28, 2026

Ping

@sindresorhus
Copy link
Copy Markdown
Owner

  1. Parenthesized first argument breaks the "always" fix
    Repro:
setTimeout((callback));

Expected fix:

setTimeout((callback), 0);

Actual fix result:

setTimeout((callback, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0));

The fixer inserts inside the parentheses, so the call still has one argument and ESLint keeps re-fixing it until the pass limit. This is definitley not safe for --fix.

  1. Parenthesized zero breaks the "never" fix
    Repro:
setTimeout(callback, (0));

Expected fix:

setTimeout(callback);

Actual fix result:

setTimeout(callback));

This leaves invalid syntax (extra )), so --fix can break parsing.


```js
// ❌
setTimeout(() => console.log('Hello'));
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please see other rules for formatting here. Whenever possible, show fail/pass cases together.

@sindresorhus
Copy link
Copy Markdown
Owner

Match the common options style used in other docs.

  • Use Type + Default together (for example: Type: 'string'\ Default: 'always').
  • Use single-quoted option values ('always', 'never') to match repo convention.
  • Prefer JS config examples over JSON for consistency with most rule docs.

@azat-io azat-io force-pushed the feat/explicit-timer-delay branch from e1c0b10 to 704256a Compare February 11, 2026 04:50
@azat-io
Copy link
Copy Markdown
Contributor Author

azat-io commented Feb 11, 2026

@sindresorhus Thanks for the review! I've fixed your comments!

@sindresorhus
Copy link
Copy Markdown
Owner

A few more things:

  1. In never mode, the fixer rewrites setTimeout(callback, 0, arg1, arg2) to setTimeout(callback, arg1, arg2).
    That changes runtime behavior because arg1 becomes the delay value. This is definitley not safe for --fix.
    Expected: either skip fixing when extra callback args exist, or preserve semantics.

  2. The matcher currently treats any global object member named setTimeout/setInterval as a timer call.
    Example: Math.setTimeout(callback) gets flagged and autofixed to add , 0.
    Expected: limit matches to real timer globals (setTimeout, setInterval, and window/globalThis/global members).

@azat-io azat-io force-pushed the feat/explicit-timer-delay branch from a99744d to 2c5fd38 Compare February 24, 2026 14:03
@azat-io
Copy link
Copy Markdown
Contributor Author

azat-io commented Feb 24, 2026

@sindresorhus Fixed ✅

@sindresorhus
Copy link
Copy Markdown
Owner

sindresorhus commented Mar 25, 2026

  1. The isZeroDelay doc comment has extra leading spaces on each line (2-space indent), while the checkTimerCall comment right above it doesn't. Should match.

  2. context.options[0] || MODE_ALWAYS — since defaultOptions: [MODE_ALWAYS] is set, context.options[0] is already populated. The || fallback is redundant. Should just be context.options[0].

  3. self is in the globalObjects set but has no test coverage at all. Should have at least one valid + one invalid case.

  4. In always mode, setTimeout(...args) gets flagged (no fix). This is a false postiive when args already contains a delay. Consider skipping when the only argument is a spread element.

@azat-io
Copy link
Copy Markdown
Contributor Author

azat-io commented Mar 25, 2026

@sindresorhus Fixed ✅

@sindresorhus
Copy link
Copy Markdown
Owner

@azat-io
Copy link
Copy Markdown
Contributor Author

azat-io commented Mar 25, 2026

@sindresorhus Fixed ✅

@sindresorhus
Copy link
Copy Markdown
Owner

sindresorhus commented Mar 25, 2026

Found 2 issues:

  1. never mode reports an unfixable error for setTimeout(callback, 0, arg1, arg2) — The rule flags the 0 delay but correctly skips the auto-fix (since removing , 0 would shift arg1 into the delay slot, breaking the call). This leaves users with a lint error they cannot safely fix without restructuring the call. The rule should skip reporting entirely for this case rather than surfacing an unfixable error.

if (isZeroDelay(delayArgument)) {
const problem = {
node: delayArgument,
messageId: MESSAGE_ID_REDUNDANT_DELAY,
data: {name},
};
if (arguments_.length === 2) {
problem.fix = function (fixer) {
const [firstArgument] = arguments_;
const [, firstArgumentEnd] = getParenthesizedRange(firstArgument, context);
const [, delayArgumentEnd] = getParenthesizedRange(delayArgument, context);
return fixer.removeRange([firstArgumentEnd, delayArgumentEnd]);
};
}
return problem;
}
}

  1. Dead guard in always mode — The if (firstArgument && firstArgument.type !== 'SpreadElement') condition is always true at that point. firstArgument is guaranteed non-nullish (the arguments_.length === 0 early return above ensures arguments_.length >= 1), and the SpreadElement case was already handled by the preceding early return. The condition can be simplified.

@azat-io azat-io force-pushed the feat/explicit-timer-delay branch from b284281 to 85768e6 Compare March 25, 2026 18:14
@azat-io
Copy link
Copy Markdown
Contributor Author

azat-io commented Mar 25, 2026

Thanks. I've fixed both issues!

@sindresorhus
Copy link
Copy Markdown
Owner

  1. In never mode, +0 still slips through. setTimeout(callback, +0) and setInterval(callback, +(0)) are explicit zero delays, but the current zero check only handles 0 and -0, so these are not reported. I double checked this manually from the parsed AST and it definitley does not match the current condition. There should also be a regression test for this form.

  2. The 'never' docs still use inline /* eslint ... */ comments in the examples. Earlier review asked for JS config examples to match the rest of the docs, so that comment is not fully addressed yet.

@sindresorhus
Copy link
Copy Markdown
Owner

Some tests to add:

  • +0 in never mode.
    setTimeout(callback, +0);
    setInterval(callback, +(0));
    This is the main missed semantic equivalent of zero.

  • self in never mode.
    There is coverage for self.setTimeout(fn, 0) in always, but not for removing 0 in never.
    self.setTimeout(fn, 0);
    self.setInterval(fn, 0);

  • Parenthesized -0 in never mode.
    There is -0, and there is (0), but not (-0).
    setTimeout(callback, (-0));

  • Explicit global shadowing negatives.
    Make sure member calls are ignored when the global object name is shadowed.
    const window = foo; window.setTimeout(callback);
    const self = foo; self.setInterval(callback);

  • Computed member access should stay ignored.
    globalThis['setTimeout'](callback);
    That documents the current supported boundary.

@azat-io
Copy link
Copy Markdown
Contributor Author

azat-io commented Mar 27, 2026

Done

@sindresorhus sindresorhus force-pushed the main branch 2 times, most recently from ee4565f to 9141823 Compare March 27, 2026 19:31
@sindresorhus sindresorhus force-pushed the main branch 2 times, most recently from 814b622 to 0e023e3 Compare April 2, 2026 12:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Rule proposal: explicit-timer-delay

2 participants