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

Java: please emit covariant Map types #1517

Closed
2 tasks done
rix0rrr opened this issue Apr 9, 2020 · 2 comments · Fixed by #1653 or #1884
Closed
2 tasks done

Java: please emit covariant Map types #1517

rix0rrr opened this issue Apr 9, 2020 · 2 comments · Fixed by #1653 or #1884
Assignees
Labels
effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. language/dotnet Related to .NET bindings (C#, F#, ...) language/java Related to Java bindings p2

Comments

@rix0rrr
Copy link
Contributor

rix0rrr commented Apr 9, 2020

🚀 Feature Request

Affected Languages

  • Java
  • .NET (C#, F#, ...) (maybe! I don't know)

Right now, a {[key: K]: V} (or Record<K, V> which I prefer writing because all those curlies make me dizzy :) ), renders to the following type in Java:

Map<K, V>

Example:

TS:  {[key: string]: any}

# renders to

Java: Map<String, Object> 

I'd like to use this in the IAM library in addCondition(), where the user needs to actually pass a Record<string, string | string[]>, but since we don't do unions we take the common supertype, which is any.

The problem is, most people are going to want to use an ImmutableMap<> to pass as an argument. That's fine, but the effective type of that map in 9 times out of 10 is going to be:

ImmutableMap<String, String>

And it so happens that you cannot assign a Map<String, String> to a Map<String, Object>, as Java doesn't know about the subtyping relationship you intend (and prefers not to guess).

Proof:

    Map<String, Object> x = new HashMap<String, String>();

Main.java:6: error: incompatible types: HashMap<String,String> cannot be converted to Map<String,Object>
    Map<String, Object> x = new HashMap<String, String>();

https://repl.it/repls/TalkativeUnrulyTheories

Proposed solution

The actual type we should be emitting in Java, to mirror the subtyping relationship in TypeScript, is the following:

Map<String, ? extends Object>

(In fact it should actually be Map<? extends String, ? extends Object>, but since we never have anything other than a String in the first position (because JavaScript), we can simplify a little...)

And the same for List.

You would need to investigate yourself whether we need to explicitly annotate C# as well, or not.

@rix0rrr rix0rrr added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Apr 9, 2020
@SomayaB SomayaB added language/java Related to Java bindings and removed needs-triage This issue or PR still needs to be triaged. labels Apr 11, 2020
@SomayaB SomayaB added the language/dotnet Related to .NET bindings (C#, F#, ...) label Apr 11, 2020
@RomainMuller RomainMuller added the effort/small Small work item – less than a day of effort label Apr 14, 2020
@RomainMuller
Copy link
Contributor

I like this idea a lot!

I don't reckon C# needs something similar... But we should add a compliance test to confirm that.

RomainMuller added a commit that referenced this issue May 12, 2020
Using covariant expressions instead of just the type name enables a
better experience when using libraries such as Guava to build up
collections (`List` or `Map`) to use with the libraries. In particular,
collections of `any` are particularly annoying to use without covariant
typing because they'd force the user to manually specify generic
parameters in their code that add nothing to the safety of the program.

Fixes #1517
@mergify mergify bot closed this as completed in #1653 May 13, 2020
mergify bot pushed a commit that referenced this issue May 13, 2020
Using covariant expressions instead of just the type name enables a
better experience when using libraries such as Guava to build up
collections (`List` or `Map`) to use with the libraries. In particular,
collections of `any` are particularly annoying to use without covariant
typing because they'd force the user to manually specify generic
parameters in their code that add nothing to the safety of the program.

Fixes #1517
@NetaNir NetaNir reopened this May 14, 2020
@NetaNir
Copy link
Contributor

NetaNir commented May 14, 2020

Opening this cause we had to temporary revert the change

@RomainMuller RomainMuller added p1 p2 and removed p1 labels Aug 13, 2020
RomainMuller added a commit that referenced this issue Aug 13, 2020
Using covariant expressions instead of just the type name enables a
better experience when using libraries such as Guava to build up
collections (`List` or `Map`) to use with the libraries. In particular,
collections of `any` are particularly annoying to use without covariant
typing because they'd force the user to manually specify generic
parameters in their code that add nothing to the safety of the program.

Fixes #1517
RomainMuller added a commit that referenced this issue Aug 21, 2020
Using covariant expressions instead of just the type name enables a
better experience when using libraries such as Guava to build up
collections (`List` or `Map`) to use with the libraries. In particular,
collections of `any` are particularly annoying to use without covariant
typing because they'd force the user to manually specify generic
parameters in their code that add nothing to the safety of the program.

Fixes #1517
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. language/dotnet Related to .NET bindings (C#, F#, ...) language/java Related to Java bindings p2
Projects
None yet
4 participants