Tightening Map.get(key) and others


#1

In a few places the Java collections framework has some annoying type safety holes, for example Map<K, V>.get(Object) rather as you would expect Map<K, V>.get(K). There are some other examples of odd things like this where Object is still in use even post-generics.

Actually I just got bit on the ass by this - I refactored some code and didn’t notice that I was now passing in the wrong type to Map.get, so a certain code path would never be taken :frowning: This is the kind of thing type systems are meant to save me from, so … as Kotlin already modifies the type system of java collections using eg Map vs MutableMap and via extension methods, and it has better generics in general, would it be possible for Kotlin to fix this by tightening the type bounds thus saving me from myself?


#2

In short, it would break the Java interop: the transparency of collections is a big win for Kotlin, and we are not ready to sacrifice it, although the problem you bring up is very valid :(


#3

Could you elaborate on the interop problem it would create? A Java collection already has its types manipulated in the Kotlin world via List/MutableList etc and that doesn't seem to cause problems.

If Kotlin code saw the get method as tighter than Java, I can see that this would be a problem if someone actually relied on map.get(some other type that happens to compare equal) working as part of an API, but is that really common? I’d hope not! Perhaps there could be an unsafeGet() equivalent that has the original looseness.


#4

The problems arise in inheritance scenarios: e.g., what do we do if a Kolin class extends java.util.HashMap, whose get() takes Object, or if two hierarchies: a pure-Kotlin one and a pure-Java one merge at some point with a Kotlin trait extending both?

Also, I’m not entirely sure we can make Javac swallow the bridge-like methods we’d have to generate to emulate the typed get(): Javac is rather picky about having unexpected bridges…


#5

There's also an issue which you can vote / subscribe to:

https://youtrack.jetbrains.com/issue/KT-4336


#6

What about giving a warning at compile time ?

What about having alternative methods with the right signature
get(key:K)
unsafeGet(key:Any?)

Eventually, if it is possible,
mapping java’s method to unsafeGet in kotlin
mapping kotlin’s method to get in java   ?

The point would be to encourage the use of the safe kotlin method against the unsafe/misleading java one…


#7

Warnings at compile time: it's on the roadmap.

About unsafeGet: We can only have safeGet, because get(Object) is virtual and must be overridable (and no magic with name mapping works consistently there, we’ve checked, sad, but true).


#8

Java 8 adds getOrDefault which is strictly typed. It's a bit of a mouthful of course. The Kotlin stdlib could polyfill it with an extension method that disappears when the runtime target version is 8+.

However, I wonder if there is a simple hack that would resolve this nicely. Kotlin provides [] which maps to .get(X). Kotlin doesn’t care if you index a map with the wrong type as a result of the underlying transform. But what if [] mapped to .safeGet if it exists in the indexed type, or .get() otherwise? I’m not sure if this complication of the overload rules would have any unexpected side effects (perhaps safeGet should have a less likely to pre-exist name). Or perhaps the mapping between operator and method name could be controllable on a per-type basis to allow map[k] to become map.getOrDefault(k, null) specifically for the Map interface.


#9

Alternatively to your proposal we can require an annotation on methods that are bound by convention, then one would have to provide an extension "get" for Map, and give it the right signature and annotations. There are downsides to this approach, e.g. nothing will be bound by convention by default...


#10

There is also another tricky thing about HashMap.get(lookup: Any?) function. The parameter is not quite a "key", but rather a "lookup value". The search is actually performed by lookup.hashCode() function and lookup.equals(storedKey), so you can have an arbitrary object, e.g. wrapper over actual key, or some kind of a key signature, which generates same hashcode and can compare for equality, and you will be able to find a value. So there are possibly valid use cases, when lookup object is not of key type. IDE (and probably compiler) warning is more than enough, and doesn't require tricky games with members and types.


#11

D'oh, just got bitten by this again, this time with Set.contains() which is also Any.

If there’s any way I can vote for an IDE warning or Inspector flag on this, well, that’d probably have helped.

Going back to language level solutions:

operator [] = safeGet
fun Map<T>.safeGet(key: T) = this.get(key)

seems like a smallest, simplest language level fix for maps. The use of get() rather than [] could then be what triggers the IDE warning - anyway, it is more readable IMO to avoid using the explicit get method and rely on the square brackets notation.

I do not see any obvious solution to fix Set.contains() given there is no operator for it and the simple tightening does not work :frowning: If only there was a solution to the inheritance issues.