Skip to content

[no-implied-eval] False positive with callback: Function #2358

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.

Sign up for GitHub

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

Jump to bottom
Closed
tosmolka opened this issue Aug 5, 2020 · 3 comments · Fixed by #2435
Closed

[no-implied-eval] False positive with callback: Function #2358

tosmolka opened this issue Aug 5, 2020 · 3 comments · Fixed by #2435
Labels
bug Something isn't working package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin

Comments

@tosmolka
Copy link
Contributor

tosmolka commented Aug 5, 2020

Repro

module.exports = {
    "env": {
        "browser": true,
        "es2020": true
    },
    "parser": "@typescript-eslint/parser",
    "parserOptions": {
        "ecmaVersion": 11,
        "sourceType": "module",
        "project": "tsconfig.json"
    },
    "plugins": [
        "@typescript-eslint"
    ],
    "rules": {
        "@typescript-eslint/no-implied-eval": "error"
    }
};
const foo = (callback: Function) => {
    setTimeout(callback, 0);
};

Expected Result
ESLint detects 0 problems

Actual Result
2:16 error Implied eval. Consider passing a function @typescript-eslint/no-implied-eval

✖ 1 problem (1 error, 0 warnings)

Additional Info

Handler argument is parsed as Identifier but isFunctionType() returns for it false.

Versions

package version
@typescript-eslint/eslint-plugin 3.8.0
@typescript-eslint/parser 3.8.0
TypeScript 3.9.7
ESLint 7.6.0
node 14.6.0
npm 6.14.5
@tosmolka tosmolka added package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin triage Waiting for team members to take a look labels Aug 5, 2020
@bradzacher
Copy link
Member

Why are you using the Function type here? It's generally recommended that you don't use this type ever, as it has a distinct lack of clarity as to what it means.

The bug here is that the Function symbol is not "function-like", and the Function type has no call-signatures.

If you use something like (...args: any[]) => void instead, then it will work fine.

@bradzacher bradzacher added bug Something isn't working and removed triage Waiting for team members to take a look labels Aug 6, 2020
@tosmolka
Copy link
Contributor Author

tosmolka commented Aug 6, 2020

Thanks @bradzacher , I don't own the code that is causing those false positives so can't comment on that. My assumption was that the rule should identify calls to setTimeout with string argument and this seemed like a bug.

@bradzacher
Copy link
Member

bradzacher commented Aug 6, 2020 •
edited
Loading

This is a bug! The Function type is unique in that in terms of the underlying type data it both is and isn't a function at the same time.

We have to add a special case for it.

Note that this rule doesn't ban string arguments, it instead bans non-functional arguments.

The reason it works that way is because under the hood, functions like setTimeout actually coerce any non-function input to string.

For example

setTimeout([5, 6, 7, 8, "console.log('hihihi')"], 5)

Will coerce the array to a string (5,6,7,8,console.log('hihihi')), and then execute it as JS.
This is actually valid JS (it's a sequence expression) so this will log the string to the console.

Yeah JS is wack sometimes.

@soobing soobing mentioned this issue Aug 29, 2020
Merged
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 15, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
bug Something isn't working package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(eslint-plugin): [no-implied-eval] handle the Function type soobing/typescript-eslint
2 participants
@bradzacher @tosmolka