Skip to content

Bug: [no-unsafe-argument] doesn't flag unsafe argument with a type constraint #10415

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

Open
4 tasks done
ronami opened this issue Nov 28, 2024 · 3 comments
Open
4 tasks done
Labels
accepting prs Go ahead, send a pull request that resolves this issue bug Something isn't working package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin

Comments

@ronami
Copy link
Member

ronami commented Nov 28, 2024

Before You File a Bug Report Please Confirm You Have Done The Following...

  • I have tried restarting my IDE and the issue persists.
  • I have updated to the latest version of the packages.
  • I have searched for related issues and found none that matched my issue.
  • I have read the FAQ and my problem is not listed.

Playground Link

https://typescript-eslint.io/play/#ts=4.8.4&fileType=.tsx&code=CYUwxgNghgTiAEYD2A7AzgF3hkmCMAXPABQZEoCuAtgEYgwCU8AvAHzwBuSAlsANwAoUJFgJk6LDkwAmIgB4AKvBAAPHCmBp4lWvVakiCpm049%2BAgQHpL8AGZRuELchhwwGCAE8BUjHmJ48FBaUCieDIJWNmgAFkgUEMB2DhA%2BuBjSAUEhYRECQA&eslintrc=N4KABGBEBOCuA2BTAzpAXGUEKQAIBcBPABxQGNoBLY-AWhXkoDt8B6Jge1tieQEMAZolp9oAc1gBbRC3RRE0aB2iRwYAL4h1QA&tsconfig=N4KABGBEDGD2C2AHAlgGwKYCcDyiAuysAdgM6QBcYoEEkJemy0eAcgK6qoDCAFutAGsylBm3TgwAXxCSgA&tokens=false

Repro Code

declare const test1: (t: number) => void;
declare const test2: <T extends number>(t: T) => void;

// fails correctly
test1(1 as any);

// should fail
test2(1 as any);

ESLint Config

module.exports = {
  parser: "@typescript-eslint/parser",
  rules: {
    "@typescript-eslint/no-unsafe-argument": "error"
  },
};

tsconfig

{
  "compilerOptions": {
    "strictNullChecks": true
  }
}

Expected Result

I expect all use-cases to be consistent and report this as an error.

Actual Result

The use-cases with a type constraint doesn't report this as an error.

Additional Info

This seems similar to #10314, and it looks like the fix should be similar.

I'll add that I see similar inconsistencies on no-unsafe-assignment and no-unsafe-return (false positives, in which the rule reports unnecessarily):

// no-unsafe-return
declare const test1: (callback: () => unknown) => void;
declare const test2: <T>(callback: () => T) => void;
declare const test3: <T extends unknown>(callback: () => T) => void;

// passes correctly
test1(() => 1 as any);

// should pass
test2(() => 1 as any);

// should pass
test3(() => 1 as any);

// no-unsafe-assignment
declare const test4: (t: { t: unknown }) => void;
declare const test5: <T extends unknown>(t: { t: T }) => void;

// pass correctly
test4({ t: 1 as any });

// should pass
test5({ t: 1 as any });

I'll be happy also to report them (separately or as part of this issue) if this makes sense.

@ronami ronami added bug Something isn't working package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin triage Waiting for team members to take a look labels Nov 28, 2024
@ronami ronami changed the title Bug: [no-unsafe-argument] don't flag values of an unconstrained or valid type parameter Bug: [no-unsafe-argument] doesn't flag unsafe arguments of a type constraint Nov 28, 2024
@ronami ronami changed the title Bug: [no-unsafe-argument] doesn't flag unsafe arguments of a type constraint Bug: [no-unsafe-argument] doesn't flag unsafe argument with a type constraint Nov 28, 2024
@kirkwaiblinger kirkwaiblinger added accepting prs Go ahead, send a pull request that resolves this issue and removed triage Waiting for team members to take a look labels Nov 28, 2024
@kirkwaiblinger
Copy link
Member

I'm starting to think that the root cause might be more akin to #6951, #9007 (and all the duplicates mentioned in #9007 (comment)) than to #10314.

@kirkwaiblinger
Copy link
Member

kirkwaiblinger commented Dec 14, 2024

that is to say, I think the rule is seeing it as an any -> any assignment, see the following examples:

// should flag, does
test2<number>(1 as any);

// doesn't flag.... and maybe shouldn't (??)
test2<any>(1 as any);

@ronami
Copy link
Member Author

ronami commented Dec 14, 2024

that is to say, I think the rule is seeing it as an any -> any assignment, see the following examples:

// should flag, does
test2<number>(1 as any);

// doesn't flag.... and maybe shouldn't (??)
test2<any>(1 as any);

Yes! I think it's very similar to what @bradzacher explained here: #6951 (comment).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepting prs Go ahead, send a pull request that resolves this issue bug Something isn't working package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin
Projects
None yet
Development

No branches or pull requests

2 participants