feat(eslint-plugin): add rule [strict-void-return] #9707

phaux · 2024-08-02T16:47:45Z

PR Checklist

Addresses an existing open issue:
- fixes Rule proposal: warn against passing a non-void returning callback in a position accepting a void returning callback #2988
- fixes Rule proposal check void-covariant return types of implemented interfaces and extended classes #1744
That issue was marked as accepting prs
Steps in Contributing were taken

Overview

So basically I implemented the void checking as requested by #2988. For every checked node (arguments, assignments, returns, etc) I take the actual function type and the contextual function type and compare return types. Only object method shorthand required slightly different logic.

That already worked pretty well, but I also found #1744 and decided to include it in this rule as well, since I already had a similar thing implemented for object shorthand methods. ~~I added this as an option considerBaseClass and considerImplementedInterfaces, enabled by default.~~

Then I noticed that callback for addEventListener is not detected as void context. That's because it has another signature where the callback can return any. I was stuck on this for a long time. Ultimately I looked at how no-misused-promises does this and implemented something similar ~~as considerOtherSignatures option, enabled by default.~~

At this point this rule already did everything no-misused-promises's checkVoidReturn did, but better. It doesn't have problems like #8054 or #8739. Maybe it's worth splitting no-misused-promises into 3 separate rules in the future? (this one being one of them)

EDIT: autofixes removed for now

I also added many autofixes and suggestions. They are possible when the provided function is a function literal and we can inspect its body. Some of them are the same as in no-confusing-void-expression so I moved them into utils. It might make sense to change some autofixes into suggestions instead so they don't accidentally remove a big chunk of code. Let me know if that's a good idea.

The biggest feature is automatic suggestions which I and probably others had to type manually a thousand of times:

takesCallback(async () => {
  block;
});

into

takesCallback(() => {
  (async () => {
    block;
  })().catch(err => {});
});

or

// eslint-disable-next-line @typescript-eslint/no-misused-promises
takesCallback(async () => {
  try {
    block;
  } catch {}
});

To allow the second suggestion without the need of ignoring the line I added the option allowReturnPromiseIfTryCatch. It's just a simple extra if near the end of the long routine that checks everything that could be wrong in the function body. I hope it can stay.

typescript-eslint · 2024-08-02T16:47:48Z

Thanks for the PR, @phaux!

typescript-eslint is a 100% community driven project, and we are incredibly grateful that you are contributing to that community.

The core maintainers work on this in their personal time, so please understand that it may not be possible for them to review your work immediately.

Thanks again!

🙏 Please, if you or your company is finding typescript-eslint valuable, help us sustain the project by sponsoring it transparently on https://opencollective.com/typescript-eslint.

netlify · 2024-08-02T16:48:03Z

✅ Deploy Preview for typescript-eslint ready!

Name	Link
🔨 Latest commit	`ede0145`
🔍 Latest deploy log	https://app.netlify.com/projects/typescript-eslint/deploys/6862ee99c4ec1b0009ab51f4
😎 Deploy Preview	https://deploy-preview-9707--typescript-eslint.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.
Lighthouse	1 paths audited Performance: 99 (🟢 up 1 from production) Accessibility: 100 (no change from production) Best Practices: 100 (no change from production) SEO: 100 (no change from production) PWA: 80 (no change from production) View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify project configuration.

nx-cloud · 2024-08-02T16:48:42Z

View your CI Pipeline Execution ↗ for commit ede0145.

Command	Status	Duration	Result
`nx run integration-tests:test`	❌ Failed	45s	View ↗
`nx test eslint-plugin --coverage=false`	✅ Succeeded	5m 6s	View ↗
`nx run-many -t typecheck`	✅ Succeeded	2m 18s	View ↗
`nx run-many -t lint`	✅ Succeeded	11s	View ↗
`nx test typescript-estree --coverage=false`	✅ Succeeded	<1s	View ↗
`nx run types:build`	✅ Succeeded	1s	View ↗
`nx test eslint-plugin-internal --coverage=false`	✅ Succeeded	<1s	View ↗
`nx run generate-configs`	✅ Succeeded	6s	View ↗
`Additional runs (27)`	✅ Succeeded	...	View ↗

☁️ Nx Cloud last updated this comment at 2025-06-30 20:19:20 UTC

codecov · 2024-08-05T14:22:42Z

Codecov Report

Attention: Patch coverage is 98.31325% with 7 lines in your changes missing coverage. Please review.

Project coverage is 90.90%. Comparing base (f9bd7d8) to head (ede0145).

Files with missing lines	Patch %	Lines
...slint-plugin/src/util/getBaseTypesOfClassMember.ts	78.78%	7 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #9707      +/-   ##
==========================================
+ Coverage   90.84%   90.90%   +0.06%     
==========================================
  Files         501      504       +3     
  Lines       50919    51334     +415     
  Branches     8387     8490     +103     
==========================================
+ Hits        46256    46664     +408     
- Misses       4648     4655       +7     
  Partials       15       15

Flag	Coverage Δ
unittest	`90.90% <98.31%> (+0.06%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
packages/eslint-plugin/src/configs/eslintrc/all.ts	`100.00% <100.00%> (ø)`
...lugin/src/configs/eslintrc/disable-type-checked.ts	`100.00% <100.00%> (ø)`
packages/eslint-plugin/src/configs/flat/all.ts	`100.00% <100.00%> (ø)`
...nt-plugin/src/configs/flat/disable-type-checked.ts	`100.00% <100.00%> (ø)`
...ages/eslint-plugin/src/rules/strict-void-return.ts	`100.00% <100.00%> (ø)`
packages/eslint-plugin/src/util/walkStatements.ts	`100.00% <100.00%> (ø)`
...slint-plugin/src/util/getBaseTypesOfClassMember.ts	`78.78% <78.78%> (ø)`

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

JoshuaKGoldberg · 2024-08-10T02:49:33Z

👋 Exciting PR, really looking forward to the rule! Just marking as a draft because there are unit test failures. This keeps getting me excited when it pops up in my notifications 😄. Let us know if you want to talk or ask questions about any of them.

Edit: ACK on the questions in the OP, I don't have the bandwidth to answer just now, but hopefully someone else does. Please ping us if those are blocking progress!

phaux · 2024-08-10T15:48:22Z

Fixed and I'm not planning any more changes so I'm undrafting it I guess.

JoshuaKGoldberg

OK! Very happy to have finally made it to this PR: it's a great piece of work. As you said, it covers a heck of a lot of functionality, and does so in ways that are really solid compared to previous approaches. Fantastic! 👏

It's also a lot of code that was hard to read through. For an initial version of the rule, I think nuance around suggestions aren't necessary. And the fixers would need to be suggestions given they change code behavior.

I left requests for simplification through the code: for messages, options, and the suggestions.

But, my advice would be to hold off applying that large set of removals until the conversation in https://github.com/typescript-eslint/typescript-eslint/pull/9707/files#r1741336663 is resolved. The consensus might end up being that the options are good and useful after all.

packages/eslint-plugin/src/rules/strict-void-return.ts

packages/eslint-plugin/docs/rules/strict-void-return.mdx

packages/eslint-plugin/src/rules/strict-void-return.ts

JoshuaKGoldberg · 2024-09-03T01:18:34Z

packages/eslint-plugin/src/rules/strict-void-return.ts

+        }
+      },
+      AssignmentExpression: (node): void => {
+        if (['=', '||=', '&&=', '??='].includes(node.operator)) {


[Testing] If I remove this if and just leave its body, all unit tests still pass. That means there's at least a missing test case.

This is still the case.

Added a test but it doesn't matter:

declare let foo: () => void; foo += () => 1;

still doesn't fail with condition removed.

Sounds to me like the if can be removed, then! 🔪

Change request: remove the if?

Just noting for posterity, node.operator in an AssignmentExpression can be any of:

"=" | "+=" | "-=" | "*=" | "**=" | "/=" | "%=" | "<<=" | ">>=" | ">>>=" | "&=" | "|=" | "||=" | "&&=" | "??=" | "^="

The operators not mentioned in the if all produce TypeScript errors when given functions. So I agree with adding the valid test case containing foo += () => 1. That makes sure the removal of the if doesn't break those cases.

packages/eslint-plugin/src/util/getBaseTypesOfClassMember.ts

packages/eslint-plugin/src/rules/strict-void-return.ts

JoshuaKGoldberg · 2024-09-03T01:43:38Z

packages/eslint-plugin/docs/rules/strict-void-return.mdx

At this point this rule already did everything no-misused-promises's checkVoidReturn did, but better. It doesn't have problems #8054 or #8739. Maybe it's worth splitting no-misused-promises into 3 separate rules in the future? (this one being one of them)

This is a great question. no-misused-promises was already largely overlapped by no-unnecessary-condition. This new strict-void-return pretty much takes on the rest of no-misused-promises, making no-misused-promises redundant if you have both no-unnecessary-condition and strict-void-return...

I'd be in favor of deprecated no-misused-promises in favor of using no-unnecessary-condition + strict-void-return. The only benefit I can think of for no-misused-promises would be projects that want to only apply the checks for Promises... Maybe these two rules could each be given some kind of "only check Promises" option?

Also of note is that no-misused-promises's checkVoidReturns is pretty configurable. Maybe, if this rule is to replace no-misused-promises, it'd be useful to have each of those configurable options? Or, on the other hand, maybe those options are holdovers that real-world don't generally use? Investigation needed. I think those options can be a followup & shouldn't block this PR.

What do you think?

Also cc: @typescript-eslint/triage-team in general, and @kirkwaiblinger + @alythobani from #8765.

no-misused-promises was already largely overlapped by no-unnecessary-condition. This new strict-void-return pretty much takes on the rest of no-misused-promises, making no-misused-promises redundant if you have both no-unnecessary-condition and strict-void-return...

Yeah the only thing left I think would be the checksSpreads option:

const myPromise = Promise.resolve({num: 2, str: "2"}); const myObject = {...myPromise}; // Expected a non-Promise value to be spreaded in an object. eslint(@typescript-eslint/no-misused-promises)

I do agree it could make sense to replace checksVoidReturn with strict-void-return. Although there may be tradeoffs in terms of eng effort and/or UX complexity if we wanted to retain all the configurability on top of having an onlyChecksPromises option.

As for checksConditionals, I actually just found microsoft/TypeScript#34717 and microsoft/TypeScript#39175—looks like checkConditionals has been covered by TypeScript for a couple years now :)

The only benefit I can think of for no-misused-promises would be projects that want to only apply the checks for Promises

Yeah e.g. one example I've seen when looking into this topic (void function assignability), is using push with forEach:

declare function forEach<T>(arr: T[], callback: (el: T) => void): void; let target: number[] = []; forEach([1, 2, 3], el => target.push(el)); // OK

It's possible some users would prefer to only check Promises so they can still use shorthands like the above without linter errors (and/or just mainly care about forgetting to await Promises), in which case an onlyChecksPromises option would be useful if we did replace checksVoidReturn with strict-void-return.

Maybe, if this rule is to replace no-misused-promises, it'd be useful to have each of those configurable options? Or, on the other hand, maybe those options are holdovers that real-world don't generally use?

It looks like #4619 was originally the impetus for adding the options (#4623); and based on the thread it looks like there are at least a few people who find the configurability you added very helpful!

Maybe a lame response, but is there a compelling reason not to land this first, then consider the no-misused-promises deprecation, and which options we might need to port or create in order to do so, afterwards?

Just thinking, deprecating no-misused-promises might have some strings attached, such as some nontrivial updating of the docs in no-floating-promises that explain how to lint against promise antipatterns outside of ExpressionStatements.

Agreed with landing this first, then considering a deprecation as a followup.

In fact, this rule is pretty big and scary. We don't really have a process for declaring rules as "canary" or "experimental". #8676 is the closest we have to a feature request. Maybe we should set a precedent?

(I don't think this PR should be blocked on that)

alythobani

Very cool PR, great job sticking it out and a lot of thought put into things!

Left some thoughts/questions, hope they're helpful 🌅

packages/eslint-plugin/src/rules/strict-void-return.ts

alythobani · 2024-09-06T05:33:47Z

packages/eslint-plugin/docs/rules/strict-void-return.mdx

no-misused-promises was already largely overlapped by no-unnecessary-condition. This new strict-void-return pretty much takes on the rest of no-misused-promises, making no-misused-promises redundant if you have both no-unnecessary-condition and strict-void-return...

Yeah the only thing left I think would be the checksSpreads option:

const myPromise = Promise.resolve({num: 2, str: "2"}); const myObject = {...myPromise}; // Expected a non-Promise value to be spreaded in an object. eslint(@typescript-eslint/no-misused-promises)

I do agree it could make sense to replace checksVoidReturn with strict-void-return. Although there may be tradeoffs in terms of eng effort and/or UX complexity if we wanted to retain all the configurability on top of having an onlyChecksPromises option.

As for checksConditionals, I actually just found microsoft/TypeScript#34717 and microsoft/TypeScript#39175—looks like checkConditionals has been covered by TypeScript for a couple years now :)

The only benefit I can think of for no-misused-promises would be projects that want to only apply the checks for Promises

Yeah e.g. one example I've seen when looking into this topic (void function assignability), is using push with forEach:

declare function forEach<T>(arr: T[], callback: (el: T) => void): void; let target: number[] = []; forEach([1, 2, 3], el => target.push(el)); // OK

It's possible some users would prefer to only check Promises so they can still use shorthands like the above without linter errors (and/or just mainly care about forgetting to await Promises), in which case an onlyChecksPromises option would be useful if we did replace checksVoidReturn with strict-void-return.

Maybe, if this rule is to replace no-misused-promises, it'd be useful to have each of those configurable options? Or, on the other hand, maybe those options are holdovers that real-world don't generally use?

It looks like #4619 was originally the impetus for adding the options (#4623); and based on the thread it looks like there are at least a few people who find the configurability you added very helpful!

packages/eslint-plugin/src/rules/strict-void-return.ts

packages/eslint-plugin/docs/rules/strict-void-return.mdx

kirkwaiblinger · 2024-09-12T01:17:25Z

packages/eslint-plugin/docs/rules/strict-void-return.mdx

Maybe a lame response, but is there a compelling reason not to land this first, then consider the no-misused-promises deprecation, and which options we might need to port or create in order to do so, afterwards?

Just thinking, deprecating no-misused-promises might have some strings attached, such as some nontrivial updating of the docs in no-floating-promises that explain how to lint against promise antipatterns outside of ExpressionStatements.

JoshuaKGoldberg · 2024-12-31T21:06:44Z

👋 Just checking in @phaux - is this ready for review? Anything we can help with?

phaux · 2025-01-07T18:42:53Z

I addressed most of the issues.

I left some unrelated refactors in another rule because they were needed for this rule's auto fixes, which are now removed and will be sent as separate PR. If that's important I can try to remove the unrelated changes too for now. I have to do it manually because they were part of the big initial commit and I was lazy.

The messages still use dynamic IDs, but I reduced their number by a lot. If that's still too many messages, let me know and I will just replace all of them with a single generic message like "value returned in void context" and move them to a separate PR so it can be discussed later too.

JoshuaKGoldberg

Progress! 🚀

Requesting some more simplification changes that'll help us review please.

JoshuaKGoldberg · 2025-02-24T15:09:10Z

packages/eslint-plugin/src/util/discardReturnValueFix.ts

+  if (newReturnText[0] === '{') {
+    // The value would be interpreted as a block statement,
+    // so we need to wrap it in parentheses.
+    newReturnText = `(${newReturnText})`;


[Testing] This isn't covered by any tests - either it's a valid case that should be tested, or it's not and we can remove it.

JoshuaKGoldberg · 2025-02-24T15:12:33Z

packages/utils/src/eslint-utils/assert.ts

[Refactor] -1 from me in general on this extra set of assertions. There's a reason we haven't leaned into this pattern more: values should be their expected type before being used.

E.g. instead of:

memberTsNode: ts.MethodDeclaration | ts.PropertyDeclaration, // ... ESLintUtils.assert(ts.isClassLike(memberTsNode.parent), '...');

...we'd want either:

If ts.MethodDeclaration | ts.PropertyDeclaration's .parent is always a ts.ClassLikeDeclaration, the AST's types should reflect that

If ts.MethodDeclaration | ts.PropertyDeclaration's .parent isn't always a ts.ClassLikeDeclaration, then memberTsNode's type should have something like { parent: ts.ClassLikeDeclaration } added

In other words: I think these assertions are surface-level fixes for type issues that should be fixed at a deeper level.

I'll also note that we generally try to work with the TSESTree AST rather than TypeScript's, when possible. Will mention this on getBaseTypesOfClassMember itself.

replaced with as

JoshuaKGoldberg · 2025-02-24T15:15:47Z

packages/eslint-plugin/src/util/getBaseTypesOfClassMember.ts

+ */
+export function* getBaseTypesOfClassMember(
+  checker: ts.TypeChecker,
+  memberTsNode: ts.MethodDeclaration | ts.PropertyDeclaration,


Continuing from the comment on assertions: I'll also note that we generally try to work with the TSESTree AST rather than TypeScript's, when possible. We have more flexibility and power to make the AST more specific because we're the ones who define it (and we define it as a discriminated union).

In this case it'd be more typical for a typed lint rule to take in the TSESTree node. You can also pass in the services: ParserServicesWithTypeInformation to use APIs like services.getSymbolAtLocation.

JoshuaKGoldberg · 2025-02-24T15:27:11Z

packages/eslint-plugin/src/rules/strict-void-return.ts

+      asyncFuncInAttr:
+        'Async event handler `{{attrName}}` passed as a prop to `{{elemName}}`, which expects a void event handler.',
+      asyncFuncInExtMember:
+        'Async function provided as `{{memberName}}` method of `{{className}}`, whose base class `{{baseName}}` declares it as a void method.',


[Typo] Grammar fix:

Suggested change

'Async function provided as `{{memberName}}` method of `{{className}}`, whose base class `{{baseName}}` declares it as a void method.',

'Async function provided as `{{memberName}}` method of `{{className}}`, whose base class `{{baseName}}` declares as a void method.',

packages/eslint-plugin/tests/rules/strict-void-return.test.ts

JoshuaKGoldberg · 2025-02-24T15:39:46Z

packages/eslint-plugin/src/rules/strict-void-return.ts

+  defaultOptions: [
+    {
+      allowReturnNull: false,
+      allowReturnPromiseIfTryCatch: true,


[Feature] In this PR's OP, allowReturnPromiseIfTryCatch is only mentioned in the collapsed note around fixers & suggestions. Seems to me it's no longer necessary, right?

ACK that it's not much added code but I don't think we have consensus that it's a strategy we'd want to add to the rule. Could you please split it out? We can always talk about it more in a followup issue - either its own and/or the one around fixers & suggestions.

JoshuaKGoldberg · 2025-02-24T15:40:10Z

packages/eslint-plugin/src/rules/strict-void-return.ts

+        }
+      },
+      AssignmentExpression: (node): void => {
+        if (['=', '||=', '&&=', '??='].includes(node.operator)) {


This is still the case.

packages/eslint-plugin/src/rules/strict-void-return.ts

JoshuaKGoldberg · 2025-02-24T15:45:40Z

packages/eslint-plugin/src/rules/strict-void-return.ts

+    fixable: 'code',
+    hasSuggestions: false,


Suggested change

fixable: 'code',

hasSuggestions: false,

JoshuaKGoldberg · 2025-02-24T15:46:01Z

packages/eslint-plugin/src/rules/strict-void-return.ts

+    {
+      allowReturnNull: false,
+      allowReturnPromiseIfTryCatch: true,
+      allowReturnUndefined: true,


[Question] Separate from the promise/try-catch options, I don't know that we really need the remaining allowReturn* options. They're added complexity for the first version of a rule we haven't tried out in the real world yet. Do you see a strong need for them? (if not, I'm -1 on including them)

JoshuaKGoldberg · 2025-04-07T13:17:03Z

👋 Checking in @phaux, do you think you'll have the time to drive this to completion over the next month or two? I really like the idea of the rule but it's been sitting for a while - it'd be nice to either really push it forward or open it up to get worked on.

phaux · 2025-04-07T13:40:43Z

@JoshuaKGoldberg Sorry for the wait. I recreated this branch from scratch and the next version will be as minimal as possible so hopefully it finally passes review :) I will send it in few days probably.

phaux · 2025-04-25T20:27:36Z

Ready

JoshuaKGoldberg

OK! Sorry this took so long to get a review - it's a really wonderful new rule and has a lot of nuance that I wanted to give the attention it deserves. The functionality generally looks great to me, I think you nailed a lot of the hard-to-get-right cases. Awesome! 👏

I'm mostly requesting changes to address the logical branches that don't seem to impact unit tests when removed. For each, some code change should be done: either adding tests for a necessary runtime check or removing code (possibly also fiddling with types) for an unnecessary-at-runtime check.

I also took a pass at docs. They don't have to be perfect for merge, we can always touch them up as we go.

Thanks for bringing this back into review, it's a really exciting rule!

JoshuaKGoldberg · 2025-06-30T20:13:03Z

packages/eslint-plugin/src/rules/strict-void-return.ts

+        }
+      },
+      AssignmentExpression: (node): void => {
+        if (['=', '||=', '&&=', '??='].includes(node.operator)) {


Sounds to me like the if can be removed, then! 🔪

Change request: remove the if?

Just noting for posterity, node.operator in an AssignmentExpression can be any of:

"=" | "+=" | "-=" | "*=" | "**=" | "/=" | "%=" | "<<=" | ">>=" | ">>>=" | "&=" | "|=" | "||=" | "&&=" | "??=" | "^="

The operators not mentioned in the if all produce TypeScript errors when given functions. So I agree with adding the valid test case containing foo += () => 1. That makes sure the removal of the if doesn't break those cases.

JoshuaKGoldberg · 2025-06-30T20:15:35Z

packages/eslint-plugin/src/rules/strict-void-return.ts

+      },
+    };
+
+    /** Checks whether the type is a void-returning function type. */


[Docs] Nit: I think the "is void returning function type" title says the same thing as this comment.

Suggested change

/** Checks whether the type is a void-returning function type. */

JoshuaKGoldberg · 2025-06-30T20:17:12Z

packages/eslint-plugin/src/rules/strict-void-return.ts

+      return (
+        // At least one return type is void
+        returnTypes.some(type =>
+          tsutils.isTypeFlagSet(type, ts.TypeFlags.Void),


[Question] This uses Void but the next check uses VoidLike. Swapping to any other combination of the type flags doesn't produce any test failures. Was there a reason to choose one vs. the other?

I'm genuinely asking, I don't know 😄

JoshuaKGoldberg · 2025-06-30T20:22:52Z

packages/eslint-plugin/docs/rules/strict-void-return.mdx

+If a promise is returned from a callback that should return void,
+it probably won't be awaited and its rejection will be silently ignored
+or crash the process depending on runtime.


[Docs] Some users bristle at definitive, declarative statements like "if you do X, then Y will happen" when their design allows X without Y. Even if X and Y are not good.

Suggested change

If a promise is returned from a callback that should return void,

it probably won't be awaited and its rejection will be silently ignored

or crash the process depending on runtime.

If a callback is meant to return void, values returned from functions are likely ignored.

Ignoring a returned Promise means any Promise rejection will be silently ignored

or crash the process depending on runtime.

JoshuaKGoldberg · 2025-06-30T20:25:08Z

packages/eslint-plugin/docs/rules/strict-void-return.mdx

+Returning a value from a void function is likely a mistake on part of the programmer.
+This rule will often warn you early when using a function in a wrong way.


Suggested change

Returning a value from a void function is likely a mistake on part of the programmer.

This rule will often warn you early when using a function in a wrong way.

Returning a value from a void function often is an indication of incorrect assumptions about APIs.

Those incorrect assumptions can often lead to bugs.

The following `forEach` loop is a common mistake: its author likely either meant to add `console.log` or meant to use `.map` instead.

JoshuaKGoldberg · 2025-06-30T20:44:29Z

packages/eslint-plugin/src/rules/strict-void-return.ts

+      if (
+        propNode.value.type === AST_NODE_TYPES.AssignmentPattern ||
+        propNode.value.type === AST_NODE_TYPES.TSEmptyBodyFunctionExpression
+      ) {
+        return;
+      }


[Testing] Removing this if check doesn't fail any unit tests. Either this check should exist and tests are missing, or this is dead code that can be removed.

Similar note to earlier on the type narrowing.

JoshuaKGoldberg · 2025-06-30T20:45:57Z

packages/eslint-plugin/src/rules/strict-void-return.ts

+      if (propNode.value == null) {
+        return;
+      }


[Testing] Removing this if check doesn't fail any unit tests. Either this check should exist and tests are missing, or this is dead code that can be removed.

JoshuaKGoldberg · 2025-06-30T20:47:01Z

packages/eslint-plugin/src/rules/strict-void-return.ts

+      if (
+        methodNode.value.type === AST_NODE_TYPES.TSEmptyBodyFunctionExpression
+      ) {
+        return;
+      }
+      if (methodNode.kind !== 'method') {
+        return;
+      }


[Testing] Removing this if checks doesn't fail any unit tests. Either they should exist and tests are missing, or this is dead code that can be removed.

JoshuaKGoldberg · 2025-06-30T20:50:10Z

packages/eslint-plugin/src/rules/strict-void-return.ts

+      JSXAttribute: (node): void => {
+        if (
+          node.value?.type === AST_NODE_TYPES.JSXExpressionContainer &&
+          node.value.expression.type !== AST_NODE_TYPES.JSXEmptyExpression


[Testing] Removing the && node.value... !== JSXEmptyExpression doesn't fail any unit tests. Either this check should exist and tests are missing, or this is dead code that can be removed.

If the reason it's here is to narrow node.value.expression's type down to TSESTree.Expression then maybe either the types are suggesting a legit case to handle, or are wrong?

JoshuaKGoldberg · 2025-06-30T20:57:47Z

packages/eslint-plugin/docs/rules/strict-void-return.mdx

Agreed with landing this first, then considering a deprecation as a followup.

In fact, this rule is pretty big and scary. We don't really have a process for declaring rules as "canary" or "experimental". #8676 is the closest we have to a feature request. Maybe we should set a precedent?

(I don't think this PR should be blocked on that)

phaux added 5 commits August 2, 2024 03:13

feat(eslint-plugin): add rule [strict-void-return]

76c295b

chore: generate configs

5454727

better handling of base class/interfaces

38e2d18

update docs

cb58ccc

restructure some code

75da13d

Merge branch 'main' into strict-void-return

db1bc42

kirkwaiblinger added the enhancement: new plugin rule New rule request for eslint-plugin label Aug 5, 2024

phaux added 2 commits August 5, 2024 15:30

Merge branch 'main' into strict-void-return

e0ef02d

update to new RuleTester

2d82282

phaux added 3 commits August 9, 2024 14:01

more detailed messages

8cea97b

Merge branch 'main' into strict-void-return

c16c874

more coverage

d8f2b03

JoshuaKGoldberg marked this pull request as draft August 10, 2024 02:49

phaux added 2 commits August 10, 2024 12:50

lint fix

b353dc9

fix addEventListener test

3de72ca

phaux marked this pull request as ready for review August 10, 2024 15:48

phaux added 2 commits August 15, 2024 13:16

Merge branch 'main' into strict-void-return

ae1fd10

fix node imports

cc7a23e

JoshuaKGoldberg requested changes Sep 3, 2024

View reviewed changes

JoshuaKGoldberg added the awaiting response Issues waiting for a reply from the OP or another party label Sep 3, 2024

alythobani reviewed Sep 9, 2024

View reviewed changes

kirkwaiblinger reviewed Sep 12, 2024

View reviewed changes

phaux added 2 commits September 14, 2024 13:00

Merge branch 'main' into strict-void-return

fe7a62e

simplify options

a335f3c

phaux added 2 commits January 7, 2025 17:58

remove unused function

bb4c90b

Merge branch 'main' into strict-void-return

5d76db0

phaux marked this pull request as ready for review January 7, 2025 18:43

phaux requested a review from JoshuaKGoldberg January 7, 2025 18:43

github-actions bot removed the awaiting response Issues waiting for a reply from the OP or another party label Jan 7, 2025

JoshuaKGoldberg requested changes Feb 24, 2025

View reviewed changes

JoshuaKGoldberg added the awaiting response Issues waiting for a reply from the OP or another party label Feb 24, 2025

Merge branch 'main' into strict-void-return

5c56d72

JoshuaKGoldberg added the stale PRs or Issues that are at risk of being or have been closed due to inactivity for a prolonged period label Apr 7, 2025

recreate from main

55b6378

phaux force-pushed the strict-void-return branch from eeb8ae4 to 55b6378 Compare April 24, 2025 13:29

phaux added 8 commits April 24, 2025 17:00

Merge branch 'main' into strict-void-return

600422e

take ESTree nodes in utils

f72849a

add tests

b43a125

add test

76c3b30

remove dynamic messages

75ac458

remove unnecessary code

2f0326d

remove allowReturnPromiseIfTryCatch

8b20c4b

update docs

74dfcba

phaux requested a review from JoshuaKGoldberg April 25, 2025 14:11

github-actions bot removed the awaiting response Issues waiting for a reply from the OP or another party label Apr 25, 2025

JoshuaKGoldberg removed the stale PRs or Issues that are at risk of being or have been closed due to inactivity for a prolonged period label Jun 18, 2025

Merge branch 'main' into strict-void-return

ede0145

JoshuaKGoldberg requested changes Jun 30, 2025

View reviewed changes

JoshuaKGoldberg added the awaiting response Issues waiting for a reply from the OP or another party label Jun 30, 2025

	'Async function provided as `{{memberName}}` method of `{{className}}`, whose base class `{{baseName}}` declares it as a void method.',
	'Async function provided as `{{memberName}}` method of `{{className}}`, whose base class `{{baseName}}` declares as a void method.',

		Returning a value from a void function is likely a mistake on part of the programmer.
		This rule will often warn you early when using a function in a wrong way.

-Returning a value from a void function is likely a mistake on part of the programmer.
-This rule will often warn you early when using a function in a wrong way.
+Returning a value from a void function often is an indication of incorrect assumptions about APIs.
+Those incorrect assumptions can often lead to bugs.
+The following `forEach` loop is a common mistake: its author likely either meant to add `console.log` or meant to use `.map` instead.

Uh oh!

feat(eslint-plugin): add rule [strict-void-return] #9707

Are you sure you want to change the base?

feat(eslint-plugin): add rule [strict-void-return] #9707

Conversation

phaux commented Aug 2, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Checklist

Overview

Uh oh!

typescript-eslint bot commented Aug 2, 2024

Uh oh!

netlify bot commented Aug 2, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for typescript-eslint ready!

Uh oh!

nx-cloud bot commented Aug 2, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Aug 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

JoshuaKGoldberg commented Aug 10, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

phaux commented Aug 10, 2024

Uh oh!

JoshuaKGoldberg left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alythobani Sep 6, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alythobani left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

alythobani Sep 6, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JoshuaKGoldberg commented Dec 31, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

phaux commented Jan 7, 2025

Uh oh!

JoshuaKGoldberg left a comment

Choose a reason for hiding this comment

Uh oh!

phaux commented Aug 2, 2024 •

edited

Loading

netlify bot commented Aug 2, 2024 •

edited

Loading

nx-cloud bot commented Aug 2, 2024 •

edited

Loading

codecov bot commented Aug 5, 2024 •

edited

Loading

JoshuaKGoldberg commented Aug 10, 2024 •

edited

Loading

JoshuaKGoldberg left a comment •

edited

Loading

alythobani Sep 6, 2024 •

edited

Loading

alythobani left a comment •

edited

Loading

alythobani Sep 6, 2024 •

edited

Loading

JoshuaKGoldberg commented Dec 31, 2024 •

edited

Loading