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

Editor changes overload resolution based on syntax #58300

Closed
mcriley821 opened this issue Apr 24, 2024 · 6 comments
Closed

Editor changes overload resolution based on syntax #58300

mcriley821 opened this issue Apr 24, 2024 · 6 comments
Labels
Not a Defect This behavior is one of several equally-correct options

Comments

@mcriley821
Copy link

mcriley821 commented Apr 24, 2024

πŸ”Ž Search Terms

editor overload syntax

πŸ•— Version & Regression Information

  • This is the behavior in every version I tried (all available in playground, including nightly)
  • I reviewed the FAQ for entries about overloads/editor.

⏯ Playground Link

https://www.typescriptlang.org/play?ts=5.4.5#code/CYUwxgNghgTiAEAzArgOzAFwJYHtVJxwAoAoeeWAcwAYAueAZwxi1UoBoKZKBGe1ZAFsARiBicqAJnrDCEEFFTsSASnoA3HFmABuEqEiwEKdNjwFiZLjX5DR4633iyc8xRO7TGzVh1UatXX1waDgkNExcfERCUnIqOmc5BSVHeiYWNg9KLwERMWU1eE1tPRIY4gBWIA

πŸ’» Code

declare function foo(
  arg0: string, arg1: number, arg2: boolean,
): void;
declare function foo(
  arg0: number, arg1: boolean, arg2: string,
): void;
declare function foo(
  arg0: boolean, arg1: string, arg2: number,
): void;

foo(5

πŸ™ Actual behavior

  1. Add a comma to the end of the argument list on L11 for the call to foo - note overload 2 is selected correctly by the editor
  2. Close the call to foo with a closing parenthesis.
  3. Erase and re-add comma back - note overload 2 is not selected.

Why does the presence of the closing parenthesis "break" the overload resolution?

πŸ™‚ Expected behavior

The correct overload is resolvable and should be chosen by the editor.

Additional information about the issue

No response

@mcriley821
Copy link
Author

I gave this a try with a different syntax for overloads:

declare function bar(
  ...args: [arg0: string, arg1: number, arg2: boolean] | [arg0: number, arg1: boolean, arg2: string] | [arg0: boolean, arg1: string, arg2: number]
): void;

This example is even worse. The editor correctly finds the 3 overloads, but bar(5, and bar(5, ) both fail to resolve to the second overload.

@Andarist
Copy link
Contributor

The difference lies in how callIsIncomplete gets computed in both scenarios:
https://github.dev/microsoft/TypeScript/blob/0b71b81d7d053a3acd379ff49550b3c67f9ce3cd/src/compiler/checker.ts#L33830-L33831
and how, based on that, the version without parenthesis returns true from hasCorrectArity:
https://github.dev/microsoft/TypeScript/blob/0b71b81d7d053a3acd379ff49550b3c67f9ce3cd/src/compiler/checker.ts#L33845-L33849

@RyanCavanaugh RyanCavanaugh added the Not a Defect This behavior is one of several equally-correct options label Apr 24, 2024
@RyanCavanaugh
Copy link
Member

I don't think it's really possible to define what the "correct" overload is for an argument set that doesn't match anything. When the parens are still open it makes sense to pretend the future params might be there, but when it's closed, well, who can say what kind of mistake you're making. For example, if there's an overload of one parameter of type "foo" and you wrote func("oo") it seems like we should show you "foo" rather than some other possible overload

@mcriley821
Copy link
Author

mcriley821 commented Apr 25, 2024

@RyanCavanaugh not really sure I agree with you. "oo" doesn't match "foo", so why would you show me "foo"?

Overload matching should be best effort matching, no?

In my example, the argument list isn't done, because the comma is indicating I intend to add more arguments.

Not adding another argument is a syntax error. Removing the comma is a compiler error. In either of these choices, it's an invalid call. Why complicate things by not even telling me what the next argument should be?

@mcriley821
Copy link
Author

mcriley821 commented Apr 25, 2024

In cases where you have trailing commas in an argument list, you'd have a compiler error if you couldn't find the right overload, so I'd agree here that it wouldn't matter which overload you show.

declare function foo(x: number): void;
declare function foo(x: "foo"): void;

foo(
  "oo",  // error: No overload matches this call.
)

But this is a specifically talking about functions with one parameter.

The main issue I'm trying to outline in my example is that there are multiple parameters, with a correct first argument, yet the overload is not resolved/narrowed.

@typescript-bot
Copy link
Collaborator

This issue has been marked as "Not a Defect" and has seen no recent activity. It has been automatically closed for house-keeping purposes.

@typescript-bot typescript-bot closed this as not planned Won't fix, can't repro, duplicate, stale Apr 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Not a Defect This behavior is one of several equally-correct options
Projects
None yet
Development

No branches or pull requests

4 participants