Kotlin / dukat

Converter of <any kind of declarations> to Kotlin external declarations
552 stars 42 forks source link

Superfluous get()=definedExternally, set(value)=definedExternally #363

Open vlsi opened 4 years ago

vlsi commented 4 years ago

Could Dukat please strip get() = definedExternally set(value) = definedExternally from the generated code?

Sample from @octokit/types 5.4.0:

export declare type ResponseHeaders = {
    "cache-control"?: string;
    "content-length"?: number;
    "content-type"?: string;
    date?: string;
    etag?: string;
    "last-modified"?: string;
    link?: string;
    location?: string;
    server?: string;
    status?: string;
    vary?: string;
    "x-github-mediatype"?: string;
    "x-github-request-id"?: string;
    "x-oauth-scopes"?: string;
    "x-ratelimit-limit"?: string;
    "x-ratelimit-remaining"?: string;
    "x-ratelimit-reset"?: string;
    [header: string]: string | number | undefined;
};

Generated Kotlin code:

external interface ResponseHeaders {
    operator fun get(key: String): Any?
    operator fun set(key: String, value: String?)
    operator fun set(key: String, value: Number?)
    var date: String?
        get() = definedExternally
        set(value) = definedExternally
    var etag: String?
        get() = definedExternally
        set(value) = definedExternally
    var link: String?
        get() = definedExternally
        set(value) = definedExternally
    var location: String?
        get() = definedExternally
        set(value) = definedExternally
    var server: String?
        get() = definedExternally
        set(value) = definedExternally
    var status: String?
        get() = definedExternally
        set(value) = definedExternally
    var vary: String?
        get() = definedExternally
        set(value) = definedExternally
    @nativeGetter
    operator fun get(header: String): dynamic /* String? | Number? */
    @nativeSetter
    operator fun set(header: String, value: String?)
    @nativeSetter
    operator fun set(header: String, value: Number?)
}
Schahen commented 4 years ago

Let's discuss - why?

vlsi commented 4 years ago

Does

        get() = definedExternally
        set(value) = definedExternally

add anything significant?

It looks like it adds nothing, and it makes the code harder to follow. That is why I suggest to remove it.

joffrey-bion commented 4 years ago

I agree it may improve a bit the readability when we have to read the declaration files. However, since this code is generated, I don't believe this is really important. The rest of the ecosystem should be made so that we don't have to read generated files.

So I guess it may be worth making this change, but only after many other issues with high priority are solved.