I’m not nitpicking, I would just like to understand the reasoning behind this. Imagine the following code:
public class JavaFoo {
@Nullable
public final String foo;
...
}
And, in another Kotlin file:
val j = JavaFoo(null)
if (j.foo != null) {
val foo: String = j.foo
}
This compiles okay and IDE is happy. Yet, if I move the JavaFoo class to another jar file and depend on it, suddenly IDE shows an error that Smart Cast is impossible, because j.foo is a public API property declared in different module (this is on up-to-date IDEA 2016.3.2 with newest Kotlin plugin 1.0.6). There is a bug because Gradle Kotlin cmdline compiles this flawlessly, but that’s not currently the point.
My question is what’s the reasoning behind this error appearing when the class is moved to a different jar? Is the IDE plugin worried that the class in the different jar may be changed?
A smart cast is only valid when multiple accesses of the same property are guaranteed to return the same value. If the property being accessed is defined in a different module from the access location, the module containing the property can be recompiled separately from the module where it’s accessed, breaking the key requirement of the smart cast. Therefore, cross-module smart casts are not allowed.
Understood, thank you for explaining. In that case the CLI kotlin compiler should complain as well - currently it actually does the opposite and prints a warning regarding unnecessary !! operator. Should I open a bug in this regard?
It is such a common use case to put your Data classes in a separate module/project/library that it feels like we should find some way to allow this.
I understand the general case not being safe, but maybe a compiler directive saying “I know what I’m doing, I’m not going to change the contract on my self” or… I don’t know, but smart casting is so essential to being able to write nice clean code this seems like a use case there should be some way of supporting.
The problem is not that the type is changed; the problem is that the property implementation can be changed so that it does not return the same value on every invocation:
class ObjectInOtherModule {
val something get() = if (Math.random() < 0.5) "" else null
}
This is really annoying, it should at least be possible on immutable properties. Can the compiler create a variable and limit the invocation to one? Using the !! syntax outside the module but not within the module looks funky.
Do you mean that the compiler should create a different getter implementation in the module where the property is being used and call it instead? And then if you change the library implementation, the compiler will ignore the changed implementation and keep using the one it has created so that the smart casts don’t break?
I mean at the call site. I usually write the code like this inside a module where smart cast works:
if (data.id != null) handle(data.id)
and outside the module where smart cast doesn’t work:
val id: Int? = data.id
if (id != null) handle(id)
Could the compiler generate the second code when consuming Kotlin immutable properties, and then break (like other code would) if the API changes runtime to something that is not an immutable property?
Once again: the problem here is separate compilation. First, you compile the module containing the call site. Then, you recompile the library and change the property implementation. Then you run the resulting combination of classfiles. The call site does not get recompiled, so there is no chance for anything to break at compile time; what you’ll get is incorrect behavior at runtime.
How is this different than changing the getter to return a String instead of Int if you’re not recompiling the module containing the call site?
Bottom line: I think it is awkward that if I copy paste some working code from one library to another it refuses to compile and I have to rewrite it. All my code is together in a Gradle project - a change in the library implementation triggers a recompilation of the calling module.
An annotation/compiler switch to change the error to a warning would alleviate the awkwardness.
I think there is confusion about val here. val does not mean immutable. It means the value is read-only for you. The class is allowed to return different values at different times.
And the value may change between if (data.id != null) and handle(data.id). That is why a smart cast is not possible, and you have to use a local variable.
Kotlin is forcing you to do the right thing: Explicitly state the behavior that you want. Instead of Kotlin making assumptions about what you want, and then getting incorrect (and hard to reproduce and hard to debug) behavior at runtime, like yole said.
If you change a String to an Int, you’ll get an exception from the JVM when you try to load a class that accesses a method that no longer exists. The breakage will not be silent. In this case, it will be.
I completely understand that you don’t like current experience. I’m just explaining (over and over again) that the problem is real and there is no trick that could be used to avoid it while still maintaining the static typing guarantees of the language. You can of course pretend that the problem does not exist, but it does, and we are not going to make any changes to the compiler to hide it.
By immutable properties I mean properties that wrap around a final field in the bytecode.
data class Data(val id: Int)
Creates the following bytecode:
// access flags 0x12
private final I id
// access flags 0x11
public final getId()I
If the compiler generated a getIdSmartCast() method and compiled against that, wouldn’t the JVM throw a NoSuchMethodError the same way as changing the Int to a String would?
Maybe I should just do some gradle magic and copy my data class sources everywhere
Well, now you’re suggesting that the library authors must be forbidden from changing how a property is implemented, because if it was immutable at some point and gets changed into a custom getter later, then the client code will break. This will indeed preserve the behavior of smart casts, but will impose unacceptable penalties on library evolution.
Will IntelliJ in the future be able to refactor-move data classes to separate modules without having a ton of code breaking? I’m not sure how, but it seems to me like a really common use case.
In Android projects is a really common case to decrease the compile time.
It would be really great to have something to be able to split the app in modules without losing Kotlin features
@yole This problem can be solved pretty nice by using Arrow.Option instead of nullable type. Maybe wise to consider introduction of an Option type in core Kotlin as well?
@yole Maybe it would be great to introduce a flag for the compiler which tells, which modules compile together, the same time (maven projects consisting of many modules are often compiled and shipped together). This would solve the headache for many of us. My teammate had an other brilliant idea, he’ll add it soon.
