Skip to content

[unified-signatures] Rule is confused by a difference in signatures on an async function's return type #740

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
mdouglass opened this issue Jul 22, 2019 · 2 comments · Fixed by #1074
Closed

[unified-signatures] Rule is confused by a difference in signatures on an async function's return type #740

mdouglass opened this issue Jul 22, 2019 · 2 comments · Fixed by #1074
Labels
bug Something isn't working good first issue Good for newcomers has pr there is a PR raised to close this package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin

Comments

@mdouglass
Copy link

Repro

{
  "rules": {
    "@typescript-eslint/unified-signature": "error"
  }
}
function get1(key: string): string | undefined
function get1(key: string, defaultValue: string): string
function get1(key: string, defaultValue?: string): string | undefined
{
  const obj: Record<string, string> = { }
  return obj[key] || defaultValue
}

function get2(key: string): Promise<string | undefined>
function get2(key: string, defaultValue: string): Promise<string> // <-- warning here
async function get2(key: string, defaultValue?: string): Promise<string | undefined>
{
  const obj: Record<string, string> = { }
  return obj[key] || defaultValue
}

Expected Result
Neither get1 nor get2 have any warnings

Actual Result
get2 issues a unified signature warning on defaultValue: string in line 2 of get2's declaration. I don't see a way to combine the signatures without losing the information that the return type differs between the two overloads.

Versions

package version
@typescript-eslint/eslint-plugin 1.12.0
@typescript-eslint/parser 1.12.0
TypeScript 3.5.3
ESLint 6.1.0
node 12.6.0
npm 6.9.0
@mdouglass mdouglass added package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin triage Waiting for team members to take a look labels Jul 22, 2019
@bradzacher bradzacher added bug Something isn't working good first issue Good for newcomers and removed triage Waiting for team members to take a look labels Jul 22, 2019
@komkanit
Copy link
Contributor

Can I fix this pr? @bradzacher

@bradzacher
Copy link
Member

You don't need to ask, @komcal - anyone is welcome to submit PR for issues without the "has pr" tag.
These issues with "good first issue" are intended to be nice ones for new contributors to get stuck into, so feel free!

@komkanit komkanit mentioned this issue Oct 12, 2019
Merged
@bradzacher bradzacher added the has pr there is a PR raised to close this label Oct 15, 2019
bradzacher added a commit that referenced this issue Dec 3, 2019
@komkanit @bradzacher
test(eslint-plugin): [unified-signatures] add tests for #740 (#1074)
39f654b 
Co-authored-by: Brad Zacher <brad.zacher@gmail.com>
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 20, 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 good first issue Good for newcomers has pr there is a PR raised to close this 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.

test(eslint-plugin): [unified-signatures] add tests for #740 komkanit/typescript-eslint
3 participants
@mdouglass @bradzacher @komkanit