cincheo / jsweet

A Java to JavaScript transpiler.
http://www.jsweet.org
Other
1.46k stars 158 forks source link

Overloading of interface default methods not working #289

Open Ablu opened 7 years ago

Ablu commented 7 years ago

This code:

    interface A {
        public default void test(String s) {}
        public default void test(int i) {}
    }

    class B implements A {
    }

Generates something like:

        export interface A {
            test(s? : any) : any;
        }

        export class B implements API.A {
            public __parent: any;
            public test(i : number) {
            }
            public test(s : string) {
            }
            constructor(__parent: any) {
                this.__parent = __parent;
            }
        }

B then does not implement the interface, and also illegally overloads a function.

renaudpawlak commented 7 years ago

Thanks for reporting that bug. I need to fully re-implement default methods anyway. The way it is done is wrong (probably lack of sleep ;))

I'll try to plan it for v2...

joslarson commented 7 years ago

@renaudpawlak I see you have this planned for v2.1. I'm on 2.0-rc1 and I'm running into what I assume is the same issue, but I want to make sure.

Am I correct to assume that the the following Java interface...

public interface Foo {
    void bar();

    void bar(String name);

    void bar(String name, int size);

    void bar(int id, String name, int size, String mode);
}

...should transpile to the following typescript?

interface Foo {
    bar(): void;
    bar(name: string): void;
    bar(name: string, size: number): void;
    bar(id: number, name: string, size: number, mode: string): void;
}

It currently transpiles to this:

interface Foo {
    bar(id? : any, name? : any, size? : any, mode? : any) : any;
}
renaudpawlak commented 7 years ago

Nope, that a different issue, but it is not exactly a bug I'd say (unless it raises compilation errors). It would be more an improvement in the TypeScript generated code in case of overloading.

What you see is just the default behaviour when JSweet meets overloading. It takes the longest method and relaxes parameter typings, so that you can call any overloaded method.

Of course, this could be improved, especially for interfaces, were overloading is possible in TypeScript. Also, parameter naming should be improved... I have identified this issue before... It is part of a global improvement with method overloading, but it is not planned for v2, since most use cases I have met so far can work "okay" without it.

Thank you for reporting this problem anyway. I hope it's fine for you to wait until 2.1.

joslarson commented 7 years ago

@renaudpawlak thanks for the reply, and yes, of course I can wait :) I really appreciate your work on this. Would you like me to create a separate issue to track this?

Also you mentioned overloading being possible in TypeScript interfaces, and that syntax is also available in class method declarations like so:

class Foo {
    bar(): void;
    bar(name: string): void;
    bar(name: string, size: number): void;
    bar(id: number, name: string, size: number, mode: string): void;
    bar(...args: any[]) {  // this is just the implementation signature and is not exposed externally as a valid call signature
        // ...implementation
    }
}

You likely already know this, just wanted to make sure.

Also, the way it's transpiled currently means I'll lose strict type safety, since with the way these are transpiled currently I could call foo.bar(id, name) and the typescript compiler wouldn't complain, even though that's not a variation I've implemented. And people using my library get no hints has to how the method should be called other than the relaxed/merged signature, which they'll assume is the official signature, and then they'll end up calling it incorrectly, which is concerning to me.

Thanks for your time.

renaudpawlak commented 7 years ago

@joslarson Thank you for sharing. I don't have always time to catch-up with TypeScript features... I am totally sharing your concerns. I think better supporting overloading from an API perspective should be high priority for next release. Note that it should not be that hard... It is related to the issue of generating sound JSDoc comments in case of overloading...

Yes, since it is really not at all the same issue as default methods (which is a less crucial issue I believe), it would be good to have it in a separate issue. Don't hesitate to open one. Cheers.

joslarson commented 7 years ago

@renaudpawlak thanks, I've created a separate issue as requested: #328

@Ablu sorry for hijacking your issue