Skip to content

no-floating-promises is not covered by eslint-plugin-promise #339

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
yang opened this issue Mar 7, 2019 · 6 comments
Closed

no-floating-promises is not covered by eslint-plugin-promise #339

yang opened this issue Mar 7, 2019 · 6 comments
Labels
documentation Documentation ("docs") that needs adding/updating package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin wontfix This will not be worked on

Comments

@yang
Copy link

yang commented Mar 7, 2019

The Roadmap lists no-floating-promises as "N/A" because of eslint-plugin-promise.

But that plugin does not and has no plans to support floating promise detection:

eslint-community/eslint-plugin-promise#151

FWIW, this check caught a large number of bugs for us in our async code - it's an easy mistake to make.

@yang yang added package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin triage Waiting for team members to take a look labels Mar 7, 2019
@yang yang changed the title no-floating-promises needs to be migrated no-floating-promises is not covered by eslint-plugin-promise Mar 7, 2019
@ThomasdenH
Copy link
Contributor

Note that it is marked as 'unimplemented', not as 'implemented by plugin'. I think this is still planned.

@bradzacher bradzacher added enhancement: new plugin rule New rule request for eslint-plugin and removed triage Waiting for team members to take a look labels Mar 8, 2019
@bradzacher
Copy link
Member

Thomas is correct, it is specifically this is marked as "🛑 = unimplemented", not "🔌 = in another plugin".
eslint-plugin-promise is listed purely as a plugin with many features related to promises. This is done primarily because we can't keep up with every single plugin we link to, so maybe they might have implement it from when the docs were written to today.

image

We're happy to accept PRs to add this rule considering it needs type information about the called function.

@bradzacher bradzacher added wontfix This will not be worked on documentation Documentation ("docs") that needs adding/updating and removed enhancement: new plugin rule New rule request for eslint-plugin labels Mar 8, 2019
@w0rp
Copy link

w0rp commented Apr 24, 2019

I think an issue should be opened for adding this rule to @typescript-eslint/eslint-plugin, as type information from TypeScript is required for implementing the rule, and eslint-plugin-promise doesn't have access to that type information. Perhaps @JoshuaKGoldberg might be able to help.

@JoshuaKGoldberg
Copy link
Member

@w0rp yes, as bradzacher posted, this repo is accepting PRs. I'll send one in Soon™️ if someone else doesn't get to it first. You're welcome to give it a shot 😉

@bradzacher
Copy link
Member

bradzacher commented Apr 24, 2019

@w0rp
If it's something that you really want done, and want it flagged specifically to be added, then feel free to open an issue.

Otherwise, the roadmap exists for a number of reasons, one of which being the tracking of things that could/should be added to the plugin, but are low priority (i.e. nobody has asked for them).

We love new contributors, so if you want to give it a go, please feel very free!
Otherwise it'll get done eventually by someone like @JoshuaKGoldberg or another core maintainer.

(as an aside - I closed this issue because it was about our docs being wrong [which they aren't], instead of being a feature request for the rule).

@w0rp
Copy link

w0rp commented Apr 24, 2019

Thank you, I have opened an issue for it now.

@typescript-eslint typescript-eslint locked as resolved and limited conversation to collaborators Feb 21, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
documentation Documentation ("docs") that needs adding/updating package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

5 participants