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

Need better error message about incompatible constructor signature #21253

Closed
fictitious opened this issue Jan 18, 2018 · 8 comments · Fixed by #40073
Closed

Need better error message about incompatible constructor signature #21253

fictitious opened this issue Jan 18, 2018 · 8 comments · Fixed by #40073
Labels
Bug A bug in TypeScript Domain: Error Messages The issue relates to error messaging Fix Available A PR has been opened for this issue Fixed A PR has been merged for this issue Good First Issue Well scoped, documented and has the green light Help Wanted You can do this

Comments

@fictitious
Copy link

TypeScript Version: 2.7.0-dev.201xxxxx

Code

class Foo { constructor(a: string) { } }

let FooConstructor: { new(): Foo } = Foo;

Expected behavior:

Error message should tell the reason why constructor signatures are incompatible.

Actual behavior:

t.ts(3,5): error TS2322: Type 'typeof Foo' is not assignable to type 'new () => Foo'.

This is not particularly helpful, see comments below this answer, especially in comparison to the error message about incompatible methods:

let foo: { foo(a: string): void };
let foo2: { foo(): void };

foo2 = foo;
Type '{ foo(a: string): void; }' is not assignable to type '{ foo(): void; }'.
  Types of property 'foo' are incompatible.
    Type '(a: string) => void' is not assignable to type '() => void'.

So for constructor signature, ideally, the message could be

Type 'typeof Foo' is not assignable to type 'new () => Foo'.
  Types of constructor signature are incompatible.
    Type 'new(a: string) => Foo' is not assignable to type 'new() => Foo'.
@DanielRosenwasser DanielRosenwasser added Help Wanted You can do this Good First Issue Well scoped, documented and has the green light Domain: Error Messages The issue relates to error messaging Bug A bug in TypeScript labels Jan 18, 2018
@DanielRosenwasser DanielRosenwasser added this to the Community milestone Jan 18, 2018
@xiongmao86
Copy link

Can I take a stab of this, is anyone working on it?

@DanielRosenwasser
Copy link
Member

Sure! Go for it. You'll need to modify diagnosticMessages.json and then run jake generate-diagnostics to create a new message. The logic is probably somewhere in signaturesRelatedTo.

To be more precise, it should be "Types of construct signature are incompatible.".

@xiongmao86
Copy link

Hi, @DanielRosenwasser. I'm looking into code and I have 2 question:

  1. Which number should I pick when I am adding a new diagnostic message? The number after 2322, from 2323 to 2566 is occupied, should I choose 2567, it is a bit far from 2322 so I am asking here.

  2. There are possibly 3 candidate place where I can place the call to report error message, the checker.ts is to big to provide a link so I am trying to put in here:

A:

if (getObjectFlags(source) & ObjectFlags.Instantiated && getObjectFlags(target) & ObjectFlags.Instantiated && source.symbol === target.symbol) {
    // We have instantiations of the same anonymous type (which typically will be the type of a
    // method). Simply do a pairwise comparison of the signatures in the two signature lists instead
    // of the much more expensive N * M comparison matrix we explore below. We erase type parameters
    // as they are known to always be the same.
    for (let i = 0; i < targetSignatures.length; i++) {
        const related = signatureRelatedTo(sourceSignatures[i], targetSignatures[i], /*erase*/ true, reportErrors);
        if (!related) {
            // A is here
            // ^^^^^^^^^
            return Ternary.False;
        }
        result &= related;
    }
}

B:

else {
    outer: for (const t of targetSignatures) {
        // Only elaborate errors from the first failure
        let shouldElaborateErrors = reportErrors;
        for (const s of sourceSignatures) {
            const related = signatureRelatedTo(s, t, /*erase*/ true, shouldElaborateErrors);
            if (related) {
                result &= related;
                errorInfo = saveErrorInfo;
                continue outer;
            }
            shouldElaborateErrors = false;
        }

        if (shouldElaborateErrors) {
            reportError(Diagnostics.Type_0_provides_no_match_for_the_signature_1,
                typeToString(source),
                signatureToString(t, /*enclosingDeclaration*/ undefined, /*flags*/ undefined, kind));
        }
        // B is here
        // ^^^^^^^^^
        return Ternary.False;
    }
}

And C is before the last return.

I can't figure out where to place calls by just looking at it, and would like put in console.log to figure it out, but found console is not recognized.

As a newbie I am always wondering how does an export reason through code, and any pointers would be appreciated. Thank you.

@xiongmao86
Copy link

I realized that I was coding the wrong way, and there is an alternative solution, which I will open a new pull request to.

@suhailsinghbains
Copy link

Hi @DanielRosenwasser
I think the PR has been closed and not merged, so does this mean that this issue is open? If it is open, would love to work on it!
Thanks

@amaksimovich2
Copy link
Contributor

Hi! Can I try to do this issue?

@RyanCavanaugh
Copy link
Member

See also #31046

@DanielRosenwasser
Copy link
Member

Thanks @a-tarasyuk!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript Domain: Error Messages The issue relates to error messaging Fix Available A PR has been opened for this issue Fixed A PR has been merged for this issue Good First Issue Well scoped, documented and has the green light Help Wanted You can do this
Projects
None yet
7 participants