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

cannot auto parse right type in these code #13844

Closed
ktangels opened this issue Feb 2, 2017 · 12 comments
Closed

cannot auto parse right type in these code #13844

ktangels opened this issue Feb 2, 2017 · 12 comments
Labels
Design Limitation Constraints of the existing architecture prevent this from being fixed

Comments

@ktangels
Copy link

ktangels commented Feb 2, 2017

TypeScript Version: 2.1.4

Code

function mergeObject<T extends any>(dst: T, src: T): T
{
    for (var x in src)
    {
        dst[x] = src[x];
    }
    return dst;
}
 
interface Hash<T> { [key: string]: T; }

type EventHandler = (s: BaseEventDealer) => void;
interface BaseEventDealer
{
    GetEventHandler(event: string): EventHandler;
}

class C implements BaseEventDealer
{
    GetEventHandler(event: string) { return C.h[event]; }

    static h : Hash<EventHandler> = {
        a:function(s){}
    }
}

class D extends C
{
    GetEventHandler(event: string) { return D.h[event]; }
    static h = mergeObject<Hash<EventHandler>>(
        {
            b:function(s:D){}
        },C.h);
}

Expected behavior:

type of D.h should be Hash<EventHandler>
type of return value of D.GetEventHandler should be EventHandler

Actual behavior:

type of D.h and return value of D.GetEventHandler both is ImplicitAny
but if you remove D.GetEventHandler or change its name, h will get right type
or you manually declare h:Hash<EventHandler> or GetEventHandler(event: string):EventHandler, they will both get right type

@sandersn
Copy link
Member

sandersn commented Feb 2, 2017

I tried to fix this with #6118 but it breaks existing code. I gave the reason in this comment.

@sandersn
Copy link
Member

sandersn commented Feb 2, 2017

Duplicate of #3667, except for statics.

@sandersn sandersn added the Duplicate An existing issue was already created label Feb 2, 2017
@sandersn sandersn closed this as completed Feb 2, 2017
@ktangels
Copy link
Author

ktangels commented Feb 2, 2017

Not Duplicate of #3667
I don't think we are talking about a same thing...
I mean, if without the code GetEventHandler(event: string) { return D.h[event]; }
type of D.h is already confirmed to right type and D.h[e] is right type
so why add a function returning it will change this fact and let it lose its type?

@sandersn
Copy link
Member

sandersn commented Feb 2, 2017

Sorry, I missed the second line of the "actual" description where you talked about removing GetEventHandler.

@sandersn sandersn reopened this Feb 2, 2017
@mhegazy
Copy link
Contributor

mhegazy commented Feb 2, 2017

This is actually the same issue caused by #10978.

The compiler is building a new type D and it needs to to figure out if D correctly extends C, so

  • compare all members of D to C
    • compare GetEventHandler in both D and C
      • compare return type of D.GetEventHandler to C.GetEventHandler
        • Return type of D.GetEventHandler is not defined, infer it from function declaration
          • so what is the type of D.h
            • that is not defined, infer it from initialization
              • so what is the type of that mergeObject call?
                • do overload resolution, and check the arguments
                  • compare Hash<EventHandler> to { b: (s:D)=> void }
                    • so what are the properties of D?
                    • There is one D.GetEventHandler, but what is its type?
                    • well, we were just trying to figure this out, since we do not know, call it any.
                    • and tell the user we just did that, i.e. no implicit any error

@mhegazy mhegazy added Design Limitation Constraints of the existing architecture prevent this from being fixed and removed Duplicate An existing issue was already created labels Feb 2, 2017
@ktangels
Copy link
Author

ktangels commented Feb 3, 2017

well let's consider what will happen when we rename D.GetEventHandlers to D.a like
class D extends C { a(event: string) { return D.h[event]; } static h = mergeObject<Hash<EventHandler>>( { b:function(s:D){} },C.h); }

now we want to know type of D.a
so what's the type of D.a?
so what is the type of D.h
that is not defined, infer it from initialization
so what is the type of that mergeObject call?
do overload resolution, and check the arguments
compare Hash to { b: (s:D)=> void }
so what are the properties of D?
There is one D.a, but what is its type?

it seems like it will meet the same problem, but IT"S NOT

why?

i guess, when we try to compare Hash to { b: (s:D)=> void }
what's type of D? extends BaseEventDealer, is it extends correctly? depends on type of D.GetEventHandler, so we check D.GetEventHandler and boom

but, consider this:

what's type of D? extends BaseEventDealer, is it extends correctly? depends on type of D.GetEventHandler, so we check D.GetEventHandler
we don't know, so we call it any
so is it extends correctly when D.GetEventHandler is any?
yes
so compare Hash to { b: (s:D)=> void }
yes
and so on, finally we got type of D.h
and finally we got type of D.GetEventHandler

I mean, here we got an error of recursive type parsing, but after we temporary consider it as any, everything will go just fine.Will it be a solution?

@ktangels
Copy link
Author

ktangels commented Feb 3, 2017

consider the same problem in #10978.

`class M {
constructor(fn: () => any) { }
}

const a = new M(() => a); /`

as RyanCavanaugh commented:
Let's figure out the type of a from its initializer
What is the type of new M ?
Resolve M to a constructor function
Which overload, if any, of the function is applicable?
There's one overload, is it applicable?
There's a function here, its argument list OK, is its return type OK?
That depends, is the type of a an OK return type for the overload under consideration?
PANIC, we are trying to see the type of a while resolving a's type!

then we don't know type of a, then we temporary treat is as any,
so when its type is any, is its return type OK?
yes
and finally can we get type of the temporary any?
yes
and resolved

@mhegazy
Copy link
Contributor

mhegazy commented Feb 4, 2017

I mean, here we got an error of recursive type parsing, but after we temporary consider it as any, everything will go just fine.Will it be a solution?

things can have only one type. having a type any some time and some other types a type C would lead to surprises and phantom errors.

@ktangels
Copy link
Author

ktangels commented Feb 4, 2017

maybe I should not call it any, we can call it unsolved(which treat as any)
then we cached all unsolved symbols and parse it again after all other symbols, if it still unsolved, then make it implicit any

@mhegazy
Copy link
Contributor

mhegazy commented Feb 4, 2017

It does not matter what you call it. What matters is the same expression has different types at different places.

@ktangels
Copy link
Author

I try to explain it in another way

when we meet a symbol which type is unable to parse but we know what it should be.
then we assume it is what it should be and we try to see if it will lead to any logical conflicts

@mhegazy
Copy link
Contributor

mhegazy commented Feb 14, 2017

this is not how the typescript compiler works.

@mhegazy mhegazy closed this as completed Apr 21, 2017
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Design Limitation Constraints of the existing architecture prevent this from being fixed
Projects
None yet
Development

No branches or pull requests

3 participants