Thanks for the PR,@ronami!

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 onhttps://opencollective.com/typescript-eslint.

✅ Deploy Preview fortypescript-eslint ready!

Name	Link
🔨 Latest commit	8f790ba
🔍 Latest deploy log	https://app.netlify.com/sites/typescript-eslint/deploys/6747520cefbc0f0008ca94dc
😎 Deploy Preview	https://deploy-preview-10304--typescript-eslint.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.
Lighthouse	1 paths audited Performance: 94 (🔴 down 5 from production) Accessibility: 100 (no change from production) Best Practices: 92 (no change from production) SEO: 98 (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 yourNetlify site configuration.

☁️ Nx Cloud Report

CI is running/has finished running commands for commit8f790ba. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution

Sent with 💌 fromNxCloud.

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 86.76%. Comparing base(c13b6b4) to head(8f790ba).
Report is 53 commits behind head on main.

@@            Coverage Diff             @@##             main   #10304      +/-   ##==========================================+ Coverage   86.56%   86.76%   +0.19%==========================================  Files         431      443      +12       Lines       15188    15305     +117       Branches     4418     4455      +37     ==========================================+ Hits        13148    13279     +131+ Misses       1683     1672      -11+ Partials      357      354       -3

Codecov Report

Attention: Patch coverage is75.00000% with1 line in your changes missing coverage. Please review.

Project coverage is 86.55%. Comparing base(c13b6b4) to head(294711e).

Files with missing linesPatch %Lines
.../eslint-plugin/src/rules/promise-function-async.ts75.00%1 Missing⚠️
Additional details and impacted files

It seems like this line is not covered onno-unnecessary-type-parameters too, which usesgetSignatureFromDeclaration.

I'm not sure howgetSignatureFromDeclaration() can resolve toundefined in a function expression. I don't know if it's relevant, but TypeScript'schecker.ts defines the return type of the actual implementation as returningSignature.

I'm not sure howgetSignatureFromDeclaration() can resolve toundefined in a function expression. I don't know if it's relevant, but TypeScript'schecker.ts defines the return type of the actual implementation as returningSignature.

The actualgetSignatureFromDeclaration() exposed to the API appears to be herehttps://github.com/microsoft/TypeScript/blob/55f1248a2052eebdea6bc0e2eef754df89a44bf7/src/compiler/checker.ts#L1776-L1779.

Looks like it's validating that the node passed in isFunctionLike, which eventually drills down tohere, which should always returntrue.

So the| undefined does appear to be unreachable for the nodes that we are passing in here... For situations like this we'd typically usenullThrows to satisfy coverage

I'm not sure howgetSignatureFromDeclaration() can resolve toundefined in a function expression. I don't know if it's relevant, but TypeScript'schecker.ts defines the return type of the actual implementation as returningSignature.

The actualgetSignatureFromDeclaration() exposed to the API appears to be herehttps://github.com/microsoft/TypeScript/blob/55f1248a2052eebdea6bc0e2eef754df89a44bf7/src/compiler/checker.ts#L1776-L1779.

Looks like it's validating that the node passed in isFunctionLike, which eventually drills down tohere, which should always returntrue.

So the| undefined does appear to be unreachable for the nodes that we are passing in here... For situations like this we'd typically usenullThrows to satisfy coverage

Thanks! I've updated the PR to match this 👍

Currently, the rule flags the following as an issue and auto-fixes it withasync, even though the actual return type of the function isPromise<number> | number:

functionb():Promise<number>functionb(a:boolean):numberfunctionb(a?:boolean):Promise<number>|number{returnPromise.resolve(1);}

This PR changes the rule to treat the above similarly to how it treats the following:

functionb(a?:boolean):Promise<number>|number{returnPromise.resolve(1);}

I'm wondering if this should also be the case when theimplementation return type is not explicitly specified. I was playing around with this:

exportfunctiona():Promise<void>exportfunctiona(x:boolean):void// should not flag (right?), doesexportfunctiona(x?:boolean){if(x==null)returnPromise.reject(newError())thrownewError()}exportfunctionb():Promise<void>exportfunctionb(x:boolean):void// should not flag, doesn't (fixed in this PR)exportfunctionb(x?:boolean):Promise<void>|void{if(x==null)returnPromise.reject(newError())thrownewError()}// should not flag, doesn'texportfunctionc(x?:boolean):Promise<void>|void{if(x==null)returnPromise.reject(newError())thrownewError()}

Currently, the rule flags the following as an issue and auto-fixes it withasync, even though the actual return type of the function isPromise<number> | number:

functionb():Promise<number>functionb(a:boolean):numberfunctionb(a?:boolean):Promise<number>|number{returnPromise.resolve(1);}

This PR changes the rule to treat the above similarly to how it treats the following:

functionb(a?:boolean):Promise<number>|number{returnPromise.resolve(1);}

I'm wondering if this should also be the case when theimplementation return type is not explicitly specified. I was playing around with this:

exportfunctiona():Promise<void>exportfunctiona(x:boolean):void// should not flag (right?), doesexportfunctiona(x?:boolean){if(x==null)returnPromise.reject(newError())thrownewError()}exportfunctionb():Promise<void>exportfunctionb(x:boolean):void// should not flag, doesn't (fixed in this PR)exportfunctionb(x?:boolean):Promise<void>|void{if(x==null)returnPromise.reject(newError())thrownewError()}// should not flag, doesn'texportfunctionc(x?:boolean):Promise<void>|void{if(x==null)returnPromise.reject(newError())thrownewError()}

Interesting, I didn't take this into consideration. I was hoping I can get away with not having to checking every call signature.

I made some changes to the PR to address this, thanks 👍

Edit: There's no review so I can't ask for a re-review or remove theawaiting response label. I hope anyone sees this. 🙈