Skip to content
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

[promise-function-async] False positives with functions that return any #369

Closed
sindresorhus opened this issue Mar 21, 2019 · 9 comments
Closed
Labels
bug Something isn't working enhancement: plugin rule option New rule option for an existing eslint-plugin rule has pr there is a PR raised to close this package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin

Comments

@sindresorhus
Copy link

Repro

{
  "rules": {
    "typescript/promise-function-async": "error"
  }
}
function trim(query) {
	return query.trim();
}

Expected Result

I expected the rule to only report on functions that return a promise.

Actual Result

It reports on functions returning any.

Additional Info

Versions

package version
@typescript-eslint/eslint-plugin 1.5.0
@typescript-eslint/parser 1.5.0
TypeScript 3.3.3333
ESLint 5.15.3
node 10.15.1
npm 6.9.0
@sindresorhus sindresorhus added package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin triage Waiting for maintainers to take a look labels Mar 21, 2019
@j-f1
Copy link
Contributor

j-f1 commented Mar 21, 2019

This is technically correct since an object of type any could be a Promise. Perhaps there should be an option allowAny, on by default, that allows any to be used here?

@j-f1 j-f1 added bug Something isn't working enhancement: plugin rule option New rule option for an existing eslint-plugin rule and removed triage Waiting for maintainers to take a look labels Mar 21, 2019
@sindresorhus
Copy link
Author

While it might be technically correct, I don't see how it's ever wanted behavior for this rule. It would mean you would have to add async to anything that returns any, even originally synchronous functions.

@ThomasdenH
Copy link
Contributor

ThomasdenH commented Mar 21, 2019

Does the lint trigger on function a(): Promise<string> | string? I think the rule should only trigger when it definitely only returns promises, not when it could return a promise.

Edit: Never mind. This rule intends to solve functions 'either returning a Promise or something else' as well as 'returning a rejected Promise and throwing an error'.

If a function returns 'any', the user disables type checking in Typescript and relies on manually checked logic. I think the expected behaviour is thus that it doesn't lint in the specific case of any.

@j-f1
Copy link
Contributor

j-f1 commented Mar 21, 2019

That’s why i suggested adding an option that would allow any by default but let the user disallow it. However, if it’s very unlikely to be a useful option, there’s no need to add it.

@G-Rath
Copy link
Contributor

G-Rath commented Mar 28, 2019

I've got the same problem with axios & class getters:

class Mine {
  public get t() {
    return Axios;
  }
}

This gets flagged as should be marked with async (which is actually not allowed with get).

@scottohara
Copy link
Contributor

Also seeing this in a webpack.config.js that has functions that return various configurations of webpack plugins, e.g.

(note: this is from a project that has a mixed codebase of JS and TS files, and the webpack config is Javascript)

function extractCss(hashFilename) {
	return new MiniCssExtractPlugin({filename: hashFilename ? "[name]-[chunkhash:6].css" : "[name].css"});
}

function defineAppConfig(maxDataAgeDays) {
	return new webpack.DefinePlugin({MAX_DATA_AGE_DAYS: maxDataAgeDays});
}

Both of the above functions trigger @typescript-eslint/promise-function-async, but as far as I can tell neither of these plugin constructors return any or Promise types?

@burtek
Copy link
Contributor

burtek commented Jul 26, 2019

I'm getting similar error on v1.13.0

import { Reducer } from 'redux';
const reducer: Reducer<Record<string, string>> = (state = {}, _action) => state;

image

Versions

package version
@typescript-eslint/eslint-plugin 1.13.0
@typescript-eslint/parser 1.13.0
TypeScript 3.5.3
ESLint 6.1.0
node 10.14.2
npm 6.4.1

@bradzacher
Copy link
Member

have you tried the allowAny: false option?
Also if you try the 2.0 canary version, it should have the above option as the default value.

@bradzacher
Copy link
Member

(closing as this didn't get closed when #553 was merged)

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 21, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working enhancement: plugin rule option New rule option for an existing eslint-plugin rule has pr there is a PR raised to close this package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin
Projects
None yet
Development

No branches or pull requests

7 participants