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

TS4.1.2 Regression: Incorrect arity inferred on function returned by util.callbackify if input function has optional or default argument #41607

Closed
whoisbenli opened this issue Nov 20, 2020 · 5 comments
Labels
Has Repro This issue has compiler-backed repros: https://aka.ms/ts-repros Working as Intended The behavior described is the intended behavior; this is not a bug

Comments

@whoisbenli
Copy link

whoisbenli commented Nov 20, 2020

TypeScript Version: v4.2.0-dev.20201119. Also impacts: 4.1.2. Does not impact: 4.1.0-beta, 4.0.5

Search Terms:
callbackify expected arguments optional default

Code

import util from 'util';

function functionWithDefaultArgReturnsPromise(arg1: number, arg2: string, arg3: boolean = false): Promise<number> {
   return new Promise<number>((resolve, reject) => {
      // do something
   });
}

const functionWithDefaultArgHasCallBack = util.callbackify(functionWithDefaultArgReturnsPromise)

functionWithDefaultArgHasCallBack(1, 'two', true, (err: any, result: any) => {
   // do something

   // this callback function triggers error 
   //
   //    "Expected 3 arguments, but got 4.(2554)"
   // 
   // in the v4.2.0-dev.20201119 (nightly at the time in the playground),
   // but not with 4.1.0-beta in the playground, but I do see it on 4.1.2 locally as well as the last release candidate.
});

Expected behavior:
No error, no red squiggly lines

Actual behavior:
Error "Expected 3 arguments, but got 4.(2554)" occurs with red squiggly underlines on the callbackified function.

Playground Link:
4.2.0-dev.20201119 - Error seen
https://www.typescriptlang.org/play?ts=4.2.0-dev.20201119#code/JYWwDg9gTgLgBAVxsANnAZlCI4HImq4DcAUCeggHYDGyElGVtw9A6sDABYAiApugEMEKGAEEoAcwBKvGAiiUAzgAUsIYIt4AKAZICMALjiUEIAEa8oAGji6JAJiOKYUYJQk27AZiNmIEFF4BBgBeDAEUTQBKI1VsDV4AHhNzSwA+OABvEjhcqFl5BkpeAHc4OPVNZNMLKDStLXzFAIA3Xht8gCteWii4EIzs3OG4AHpRuAATCDhmkFk3CRzcgF8o0hWyanpnRho6SnYuPkFhMUkACQFFAGEIlAAhAWoAa37EZBQAOmp7s2eXsB0ABPLQUfYsQ4cHj8IQicTSAoKFRqBJRMjg5hsaEnOHnCRXW73J6vLR6Gy4GAlCC4GwuBDtOBaSzWOBNM59AZZZZjCbTWbYWScRZkYbjOBcDRwX4oFD-V57LEMFzACQSSyKOA88baibDABEAFEAB5gHowXiTOBeWySUy8SgwRQ2MxIOASCDwAAsXy09gArP6vVF9bqtWKJm4JZxeHAWj77F8AAwAWkmvBaX3sSezejzAE4mZRVZwYChgbZ4FxY8h5lErGHXfBKJ64CVoXAfXpkymLDABC63QBJKYzTSxjhweidr7d+xwFAQGXlr4kNZEIA

4.1.2 - Error seen
https://www.typescriptlang.org/play?ts=4.1.2#code/JYWwDg9gTgLgBAVxsANnAZlCI4HImq4DcAUCeggHYDGyElGVtw9A6sDABYAiApugEMEKGAEEoAcwBKvGAiiUAzgAUsIYIt4AKAZICMALjiUEIAEa8oAGji6JAJiOKYUYJQk27AZiNmIEFF4BBgBeDAEUTQBKI1VsDV4AHhNzSwA+OABvEjhcqFl5BkpeAHc4OPVNZNMLKDStLXzFAIA3Xht8gCteWii4EIzs3OG4AHpRuAATCDhmkFk3CRzcgF8o0hWyanpnRho6SnYuPkFhMUkACQFFAGEIlAAhAWoAa37EZBQAOmp7s2eXsB0ABPLQUfYsQ4cHj8IQicTSAoKFRqBJRMjg5hsaEnOHnCRXW73J6vLR6Gy4GAlCC4GwuBDtOBaSzWOBNM59AZZZZjCbTWbYWScRZkYbjOBcDRwX4oFD-V57LEMFzACQSSyKOA88baibDABEAFEAB5gHowXiTOBeWySUy8SgwRQ2MxIOASCDwAAsXy09gArP6vVF9bqtWKJm4JZxeHAWj77F8AAwAWkmvBaX3sSezejzAE4mZRVZwYChgbZ4FxY8h5lErGHXfBKJ64CVoXAfXpkymLDABC63QBJKYzTSxjhweidr7d+xwFAQGXlr4kNZEIA

4.0.5 - Error not seen
https://www.typescriptlang.org/play?ts=4.0.5#code/JYWwDg9gTgLgBAVxsANnAZlCI4HImq4DcAUCeggHYDGyElGVtw9A6sDABYAiApugEMEKGAEEoAcwBKvGAiiUAzgAUsIYIt4AKAZICMALjiUEIAEa8oAGji6JAJiOKYUYJQk27AZiNmIEFF4BBgBeDAEUTQBKI1VsDV4AHhNzSwA+OABvEjhcqFl5BkpeAHc4OPVNZNMLKDStLXzFAIA3Xht8gCteWii4EIzs3OG4AHpRuAATCDhmkFk3CRzcgF8o0hWyanpnRho6SnYuPkFhMUkACQFFAGEIlAAhAWoAa37EZBQAOmp7s2eXsB0ABPLQUfYsQ4cHj8IQicTSAoKFRqBJRMjg5hsaEnOHnCRXW73J6vLR6Gy4GAlCC4GwuBDtOBaSzWOBNM59AZZZZjCbTWbYWScRZkYbjOBcDRwX4oFD-V57LEMFzACQSSyKOA88baibDABEAFEAB5gHowXiTOBeWySUy8SgwRQ2MxIOASCDwAAsXy09gArP6vVF9bqtWKJm4JZxeHAWj77F8AAwAWkmvBaX3sSezejzAE4mZRVZwYChgbZ4FxY8h5lErGHXfBKJ64CVoXAfXpkymLDABC63QBJKYzTSxjhweidr7d+xwFAQGXlr4kNZEIA

Related Issues:
No related issue found.

@whoisbenli whoisbenli changed the title Incorrect type inferred when calling util.callbackify on function that returns Promise Incorrect type inferred when calling util.callbackify on function with optional or default argument that returns Promise Nov 20, 2020
@whoisbenli whoisbenli changed the title Incorrect type inferred when calling util.callbackify on function with optional or default argument that returns Promise Regression: Incorrect function arity inferred on function returned by util.callbackify if input function has optional or default argument Nov 24, 2020
@whoisbenli whoisbenli changed the title Regression: Incorrect function arity inferred on function returned by util.callbackify if input function has optional or default argument Regression: Incorrect arity inferred on function returned by util.callbackify if input function has optional or default argument Nov 24, 2020
@whoisbenli whoisbenli changed the title Regression: Incorrect arity inferred on function returned by util.callbackify if input function has optional or default argument TS4.1.2 Regression: Incorrect arity inferred on function returned by util.callbackify if input function has optional or default argument Nov 24, 2020
@orta
Copy link
Contributor

orta commented Nov 25, 2020

Here's a compiler backed repro:

function functionWithDefaultArgReturnsPromise(arg1: number, arg2: string, arg3: boolean = false): Promise<number> {
    return new Promise<number>((resolve, reject) => {
        // do something
    });
}

const functionWithDefaultArgHasCallBack = callbackify(functionWithDefaultArgReturnsPromise)

functionWithDefaultArgHasCallBack(1, 'two', true, (err, result) => {
    // do something

    // this callback function triggers error 
    //
    //    "Expected 3 arguments, but got 4.(2554)"
    // 
    // in the v4.2.0-dev.20201119 (nightly at the time in the playground),
    // but not with 4.1.0-beta in the playground, but I do see it on 4.1.2 locally as well as the last release candidate.
});

// C&P'd infra from node's types below

interface ErrnoException extends Error {
    errno?: number;
    code?: string;
    path?: string;
    syscall?: string;
    stack?: string;
}

function callbackify(fn: () => Promise<void>): (callback: (err: ErrnoException) => void) => void;
function callbackify<TResult>(fn: () => Promise<TResult>): (callback: (err: ErrnoException, result: TResult) => void) => void;
function callbackify<T1>(fn: (arg1: T1) => Promise<void>): (arg1: T1, callback: (err: ErrnoException) => void) => void;
function callbackify<T1, TResult>(fn: (arg1: T1) => Promise<TResult>): (arg1: T1, callback: (err: ErrnoException, result: TResult) => void) => void;
function callbackify<T1, T2>(fn: (arg1: T1, arg2: T2) => Promise<void>): (arg1: T1, arg2: T2, callback: (err: ErrnoException) => void) => void;
function callbackify<T1, T2, TResult>(fn: (arg1: T1, arg2: T2) => Promise<TResult>): (arg1: T1, arg2: T2, callback: (err: ErrnoException | null, result: TResult) => void) => void;
function callbackify<T1, T2, T3>(fn: (arg1: T1, arg2: T2, arg3: T3) => Promise<void>): (arg1: T1, arg2: T2, arg3: T3, callback: (err: ErrnoException) => void) => void;
function callbackify<T1, T2, T3, TResult>(
    fn: (arg1: T1, arg2: T2, arg3: T3) => Promise<TResult>): (arg1: T1, arg2: T2, arg3: T3, callback: (err: ErrnoException | null, result: TResult) => void) => void;
function callbackify<T1, T2, T3, T4>(
    fn: (arg1: T1, arg2: T2, arg3: T3, arg4: T4) => Promise<void>): (arg1: T1, arg2: T2, arg3: T3, arg4: T4, callback: (err: ErrnoException) => void) => void;
function callbackify<T1, T2, T3, T4, TResult>(
    fn: (arg1: T1, arg2: T2, arg3: T3, arg4: T4) => Promise<TResult>): (arg1: T1, arg2: T2, arg3: T3, arg4: T4, callback: (err: ErrnoException | null, result: TResult) => void) => void;
function callbackify<T1, T2, T3, T4, T5>(
    fn: (arg1: T1, arg2: T2, arg3: T3, arg4: T4, arg5: T5) => Promise<void>): (arg1: T1, arg2: T2, arg3: T3, arg4: T4, arg5: T5, callback: (err: ErrnoException) => void) => void;
function callbackify<T1, T2, T3, T4, T5, TResult>(
    fn: (arg1: T1, arg2: T2, arg3: T3, arg4: T4, arg5: T5) => Promise<TResult>,
): (arg1: T1, arg2: T2, arg3: T3, arg4: T4, arg5: T5, callback: (err: ErrnoException | null, result: TResult) => void) => void;
// @ts-ignore
function callbackify<T1, T2, T3, T4, T5, T6>(
    fn: (arg1: T1, arg2: T2, arg3: T3, arg4: T4, arg5: T5, arg6: T6) => Promise<void>,
): (arg1: T1, arg2: T2, arg3: T3, arg4: T4, arg5: T5, arg6: T6, callback: (err: ErrnoException) => void) => void;
function callbackify<T1, T2, T3, T4, T5, T6, TResult>(
    fn: (arg1: T1, arg2: T2, arg3: T3, arg4: T4, arg5: T5, arg6: T6) => Promise<TResult>
): (arg1: T1, arg2: T2, arg3: T3, arg4: T4, arg5: T5, arg6: T6, callback: (err: ErrnoException | null, result: TResult) => void) => void {
    throw new Error("")
}

Workbench Repro

@typescript-bot typescript-bot added the Has Repro This issue has compiler-backed repros: https://aka.ms/ts-repros label Nov 25, 2020
@typescript-bot
Copy link
Collaborator

typescript-bot commented Nov 25, 2020

👋 Hi, I'm the Repro bot. I can help narrow down and track compiler bugs across releases! This comment reflects the current state of the repro in this issue running against the nightly TypeScript.


Comment by @orta

❌ Failed: -

  • Expected 3 arguments, but got 4.
  • Parameter 'err' implicitly has an 'any' type.
  • Parameter 'result' implicitly has an 'any' type.

Historical Information

Comment by @whoisbenli

Version Reproduction Outputs
4.1.2, Nightly

❌ Failed: -

  • Expected 3 arguments, but got 4.
  • Parameter 'err' implicitly has an 'any' type.
  • Parameter 'result' implicitly has an 'any' type.

3.7.5, 3.8.2, 3.9.2, 4.0.2

👍 Compiled

@ahejlsberg
Copy link
Member

This is an effect of #41308. With that PR we made our handling of optional parameters in overload ladders more consistent. For example:

declare function foo(cb: () => void): 0;
declare function foo(cb: (x: number) => void): 1;
declare function foo(cb: (x: number, y: number) => void): 2;

declare function f1(x: number): void;
declare function f2(x: number, y?: number): void;
declare function f3(x: number, y?: any): void;
declare function f4(x: number, y?: unknown): void;

let z1 = foo(f1);  // 1
let z2 = foo(f2);  // was 2 (pre-4.1), now 1
let z3 = foo(f3);  // 1
let z4 = foo(f4);  // 1

It has to do with some pretty strange behavior in the subtype relation which we use in the first pass of overload resolution. Previously we didn't consider (x: number, y?: number) => void to be a subtype of (x: number) => void, but strangely we did consider (x: number, y?: any) => void and (x: number, y?: unknown) => void to be a subtypes. We now consider all of them subtypes. It's definitely more consistent, but it ends up subtly changing which overloads we pick in overload ladders when the source has optional parameters.

To put it in context of the OP example above, pre-4.1 we report an error when the function signature is changed to

function functionWithDefaultArgReturnsPromise(arg1: number, arg2: string, arg3: unknown = false): Promise<number>;

i.e. if type unknown is used instead of boolean for the optional argument. That seems very inconsistent. In 4.1 we consistently pick the first overload that matches, which means we consider the optional argument to be omitted.

@RyanCavanaugh RyanCavanaugh added the Working as Intended The behavior described is the intended behavior; this is not a bug label Dec 2, 2020
@ahejlsberg
Copy link
Member

BTW, root cause of this issue is the same as #41563.

@typescript-bot
Copy link
Collaborator

This issue has been marked 'Working as Intended' and has seen no recent activity. It has been automatically closed for house-keeping purposes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Has Repro This issue has compiler-backed repros: https://aka.ms/ts-repros Working as Intended The behavior described is the intended behavior; this is not a bug
Projects
None yet
Development

No branches or pull requests

5 participants