[Style] Totally non-blocking, I just noticed that we can inline these two conditions. Feel free to ignore this, if you find the current implementation more readable.

thank you i agree this style

[Refactor] I see no reason why we would want to usetargetNodeAncestorHasVoidReturnType with some other set oftargets nodes. So I think we can just put those node types inside the function. WDYT?

I agreed that there was no possibility of reuse, so i moved the targets inside the function and changed the function name to be more clear.

[Bug] This function won't stop until it finds a matching parent, so the following case is false negative

functionfoo():void{constbar=()=>{returnconsole.log()}}

(playground)

I think that this function should look +- like this:

• if!node.parent -> return false
• ifnode.parent is one of theFunctionDeclaration,ArrowFunctionExpression,FunctionExpression:
  • ifnode.parent hasvoid in type annotation -> return true
  • ifnode.parent hasn'tvoid in type annotation -> return false
• returntargetNodeAncestorHasVoidReturnType(node.parent)

in playground, this case seems false positive. anyway, this is bug, so i add test code and fixtargetNodeAncestorHasVoidReturnType function

in playground, this case seems false positive

As far as I know:

false negative === the ruleshould report error, butit doesn't do so
false positive === the ruleshould not report error, butit does so

I'm not familiar with the testing, so I think I used the terminology the other way around. thank you for telling me!

[Style] Same as here:#8632 (comment)

WDYT about inlining this if statement?

same convention is make sense. thank you!

[Refactor] Why do we need thetargets tuple if we don't iterate over it? IMO if we directly comparenode.parent.type to AST node types, it will be more readable. What do you think?

Comparing variables directly seems much more readable!

[Bug] I just noticed that we determine whether function signature hasvoid return type or not by doing pure syntax check.

Thus, the following case is false positive (the rule shouldn't report error, since function returnsFoo - an alias ofvoid type):

typeFoo=voidfunctiontest():Foo{returnconsole.log()}

playground

I didn't even think about it. Thank you!

[Testing] Some more cases to add:

• the case mentionedhere -return console.log('foo') on the top-level
• function withoutvoid in signature contains in its body function withvoid in signature, and this nested function returnsreturn console.log('foo')
• Type aliases, like in the comment above ^ (in the signature of the parent function, in the signature of the returned function, in both signatures at the same time)

I added a test case, but I may have misunderstood the last comment, so please check my new test code. thank you!

I think it's fine.

Also, I think it would be nice to add the following cases:

• functiontest():void&void{returnconsole.log('foo')}

• typeFoo=voiddeclarefunctionfoo():Foofunctiontest():Foo{returnfoo()}

• // se UPD below. there is a typotypeFoo=voidconsttest:Foo=()=>console.log('err')

UPD: there was a typo in the last code block! We shouldn't consider case with such a weird typing (the function is being assigned to variable withvoid type). Instead, I meant the following code:

• typeFoo=voidconsttest=():Foo=>console.log('err')

thank you! add all test case and last case has ignoreArrowShorthand option to pass, so addignoreArrowShorthand option

i think last case situation is same tobelow comment

[Bug] Same as here#8632 (comment)...

typeFoo=voidconsttest=():Foo=>console.log()

The error shouldn't be reported, since arrow function has thevoid return type

playground

i think this error is because of ignoreArrowShorthand. so ignoreArrowShorthand option to true or change code like:

type Foo = voidconst test = (): Foo => { return console.log(); }

I add test case with ignoreArrowShorthand option true.

playground

i think this error is because of ignoreArrowShorthand. so ignoreArrowShorthand option to true or change code like:

The rule reports error because it does pure syntax check instead of type-level check.

typeFoo=voidconsttest1=():void=>console.log()// not reported - okconsttest2=():Foo=>console.log()// reported - ok

playground

ignoreArrowShorthand ignores all arrow functions, but rule users may want to ignore only void <-> void cases as mentioned in the original issue.

thank you i understand it! and since it seems to be valid even when declaring a type in a variable of a function expression, I modified the code and test case.

typeFoo=()=>void;consttest:Foo=function(){returnconsole.log('err');};

[Refactor] This code may be converted into a single return statement. What do you think?

[Bug] I somehow missed this in previous reviews: we can't rely on syntactic check here

():any=>console.log();// reported - nice!():unknown=>console.log();// reported - nice!typeFoo=any():Foo=>console.log();// not reported - bug!typeBar=unknown():Bar=>console.log();// not reported - bug!

playground

When specifying a type using a type alias, it is difficult to find a way to determine what type it is.

Could you please give me a hint?

type Bar = unknown(): Bar => console.log();  // how to know Bar is unknown type in create fn

@auvred Can you help with the issue left in the comment above?

I'm not 100% sure about this, but I'd try playing around with function signatures. There is nice utility function in thets-api-utils package -getCallSignaturesOfType.

See:https://github.com/JoshuaKGoldberg/ts-api-utils/blob/d808eba09baa3ba093cd31b85aad57bdcc539867/src/types/getters.ts#L23

Search with some examples of usage:https://github.com/search?q=repo%3Atypescript-eslint%2Ftypescript-eslint%20getCallSignaturesOfType&type=code

it works to me very well. thank you!

[Testing] I think it would be nice to add few tests after#8809 is merged. This PR fixes theancestorHasReturnType behavior.

We can add something like this:

functionfoo():void{()=>()=>console.log();}functionfoo():any{()=>()=>console.log();}

(both of the above cases should be reported)

[Refactor] (optional)hasValidReturnType function is declared in block wherecontext is visible. What do you think about callingconst services = getParserServices(context); insidehasValidReturnType (likecanFix function above does) instead of passingservices as an argument?

I too believe that using context is better than passing a service.

[Bug] Why do we get a contextual type for function expressions and arrow functions?

The rule you took this code from has a detailed explanation of why it callsgetContextualType for function expressions and arrow functions:

This means that the following case is reported:

consttest:()=>any=():void=>console.log()

(it should not be reported since(): void => console.log() is allowed withignoreVoidInVoid: true)

playground

[Bug] This code returns fromfor loop on the first iteration. So if a function has several signatures, the rule will check only the first signature:

functiontest1():voidfunctiontest1(arg:string):anyfunctiontest1(arg?:string):any|void{if(arg){returnarg}returnconsole.log()// not reported - ok}functiontest2(arg:string):anyfunctiontest2():voidfunctiontest2(arg?:string):any|void{if(arg){returnarg}returnconsole.log()// reported - bug}

playground

[Refactor] Let's do a bitwise OR of these flags instead of callingisTypeFlagSet twice?

I did not understand that the isTypeFlagSet function utilizes bitwise operators, so I called it twice. thank you for telling me!