Nullsafety warning

I notice that kotlin default lint allows, to do cast from nullable types to nonnullable without any warnings.

For kotlin compiler this code is completely fine and don’t cause any warnings

open class Parent(val value: String)
class Child(val secondValue: String, value: String): Parent(value = value)

fun checkCast(child: Child?) {
    val parent = (child as Parent) // if child == null, throws NullPointerException at runtime
    ...
}

At runtime, though, if function will be called with null, it will cause runtime NullPointerException. Kotlin already has UNCHECKED_CAST warning but it’s not appear in this case

UPD:

There is also no warning for simple cast nullable to nonnullable

fun example(nullableString: String?) {
    val nonNullableString = nullableString as String // No any warnings
    ...
}

I suppose you ought to cast it to Parent?

child as Parent?

I know that we can add question mark to cast it properly. But it’s easy to just miss this and get a runtime error

Maybe I am wrong but I believe the “as” operator will throw an error if the left operand cannot be cast into the type on the right. If you try to cast a String into an Parent with “as”, you get a runtime error because String is not a subtype of Parent. Similarly, if you try to cast null into Parent, you get a runtime error because Null is not a subtype of Parent.

If you try to do something like this

class Parent(val value: String)

val string = "5"
val parent = string as Parent

then lint highlights as with warnign This cast can never succeed

But in case with cast nullable to nonnullable there is no any warnings

Update the original question with additional example

We can have situations where we have no such warning (i.e. such cast never succeeds) while a runtime error is thrown.
For example, say we have class B inheriting class A.
We can downcast an A object into B with “as”, because some A are B. But we will get a runtime error when we cast an A that is not B.
Similarly, we have a child. Child inherits Parent. Some nullable Child is indeed a parent. Some aren’t (they are null). And since all children are parents, some nullable Child objects are Parent. Therefore, we can cast a Child? object into Parent because some Child? are Parent without said lint error (much like casting A into B). But if we ever run into a null object, which is not a subtype of Parent, we will get a runtime error.

This is basically what cast operator does and how it behaves in most languages, isn’t it? Casting means we inform the compiler that the object is of a subtype of the current type and usually we also expect the compiler to add a runtime check.

Why would it show a warning in this case? That would mean we have to show a warning in each place where we cast.

2 Likes

As I know nullsafety in kotlin provided by compile time checks and runtime checks with AssertNotNull or something, but if cast nullable object to nonullable it’s absolutely obvious, that it can cause runtime error because nullable != nonnullable no matter class did you use, and it will be good if compiler show warning for this case. If lint can show that cast Int to String This cast can never succeed, why we don’t expect that cast nullable to nonnullable produce similar kind of warning?

Swift for example would not allow this kind of cast without force it with as!

let string: String? = nil
let string2 = string as String // Cause compile time error asking for unwrap value or use as!

And as I think it would be good to have at least warning in kotlin when you cast nullable to nonullable type

Because this is entirely different case. If you cast Int to String then your code doesn’t make any sense, it just can’t work properly. If you cast Animal to Dog then implicitly, this cast may throw. Throwing CCE or NPE is pretty much the main feature of using as, so it is hard to miss this fact.

I don’t know Swift, but for me what you suggest here is just to rename as to as!. Because as can always throw, so it should always generate a warning, so to not see the warning we have to use as! which behaves exactly the same as current as. Do I read you correctly? :slight_smile:

1 Like

But cast nullable to nonnulable doesn’t make any sense either:) If you sure that you will receive only nonnull type, so you can declare parameter as nonnull, and complier will check it. But if you cast nullable to nonullable by mistake, the compiler is silent

Errr… this is entirely untrue. This way we can conclude that casting is not needed at all in programming languages. Well, if we can design the code in a way casting is not needed then this is always a safer option. But in practice, we need casting from time to time and then it may always throw at runtime.

But we can do it a little bit safer when we use it. Providing warning where we do cast nullable to nonullable, will increase our safety. The same as it already done in case with cast Int to String

I still think having a feature in the programming language that always shows a warning just because we use it, isn’t a good idea. Maybe except some very rare cases like “delicate APIs”.

But let’s just agree to disagree.

Then you will have to also agree to always show a warning when you downcast

this isnt really about null safety
its more about whether the cast can potentially succeed
the reason why the compiler lets you do casts like:

Parent as Child
Child? as Parent
Object as String
Int? as String?

is because they are potentially successfully, and if they fail, a runtime error is thrown
The reason you get a linter error:

String as Parent
Int as String

is because the cast will NEVER succeed.

It’s not about nullability, its about whether the cast may succeed. If it is potentially successful (though may fail at runtime) then no linter error; if it is statically proven to never succeed a linter error is shown

Stepping back for a moment, perhaps the real question is: when should the compiler show warnings? What are warnings for?

The intent with this type of warning seems to be that code compiling cleanly, without warnings, will not throw certain types of exception. However, the language never guarantees that completely; there are cases where it doesn’t apply.

In the case of null safety, warning-free code will not throw a NullPointerException — unless:

  • Platform types are used. (The docs don’t give an explicit motivation for this, but it seems likely that was done to avoid explicit null checks everywhere any value from Java was used.)
  • The not-null assertion operator !! is used. (Presumably the mere presence of that operator should act almost like a warning in itself.)

And in the case of type safety, warning-free code will not throw a ClassCastException — except when:

  • Casts are used. (Again, presumably the presence of a cast operator should act almost like a warning in itself.)

Right now, the compiler gives a warning when !! or as are unnecessary — when the compiler can prove that they always succeed. And, as detailed above, it warns if as can never succeed. But in other cases (because the operator can sometimes succeed, but the compiler can’t prove it always will), then there are no warnings.

Is that helpful? Consider the alternative: if the compiler warned whenever there was any possibility of failure (but continued to warn when the operator was redundant), then those operators would always give a warning. Would that be more helpful, highlighting the danger of those operators? Or would it generate ‘warning fatigue’, making it easier to overlook other, more serious, warnings?

Perhaps Kotlin has spoiled us: perhaps it provides so much safety (compared with many other languages) that we’ve come to expect any possible runtime failure to generate a compiler warning. Is that a reasonable (and practicable) expectation?

(I don’t have an answer… But I think it’s worth making the question explicit.)

For me warnings should be generated only for cases like:

  • It is possible to proof the code is incorrect, it always fails, e.g.: str as Int, 5 / 0, etc. Interestingly, x / 0 doesn’t show a warning, I think it should.
  • It is possible to proof the code is unnecessary or redundant, e.g. the if condition is always true/false, code is unreachable, local variable is unused, etc.
  • It is not 100% guaranteed one of above happens, but the compiler has good reasons to believe this is probable. For me code like: val divisor = if (cond) 2 else 0; return 5 / divisor could/should generate a warning. It won’t fail if cond is always true, but that second branch is guaranteed to fail, so there is no point in having it.
  • If we use features of the language that are generally fine, but we use them in a way that is not 100% safe or it may do a different thing than the user expects. Unsafe cast is a good example: animal as Dog makes a guarantee the result is a Dog, because otherwise it would not get to the next line of code. However, animalList as List<Dog> doesn’t provide such guarantees and it will succeed even for the list of cats, so it generates a warning to be very careful.
  • Things like: obsolete APIs, “delicate” APIs, low-level stuff - so something that should not be used in most cases, but we still need them as they could be helpful in some rare cases.

However, we should not generate warnings just because we use features that may potentially throw in the case we made a mistake. We should not generate warnings for cases like: animal as Dog, nullableAnimal!!, x / y, requireNotNull(x), etc., because we explicitly asked to perform an operation which naturally may throw. If I use as then I know it may throw - this is what this operator does. If we generate warnings in all cases like above, people would learn to just ignore warnings or suppress them at the file level.

1 Like

The cast should not allow to cast from nullable to nonnullable, the kotlin already has this kind operator !!. If you want to cast from nullable to nonullable you wont do

val string: String? = null
val string2 = string as String

but instead you’ll do

val string: String? = null
val string2 = string!!

So I Still think that linter should show warning in this case, because it’s not cast, it’s breaking of nullsafety

Cast itself is already dangerous
The !! operator is arguably a nonnull cast too
Please carefully read our replies again

Please carefully read what I am talking about

class Foo(val value: String)
class Bar(val value: String)

fun some(value: Foo?) {
    val foo = value as Foo // Silent
    val bar = value as Bar // Warning This cast can never succeed
    ...
}

The only thing that I am talking about is nullsafety should not be breaking by cast. Because cast don’t actualy change nulability of variable.

I can tell about val foo = value as Foo this cast the same as for val bar = value as Bar, because val foo = value as Foo can never succeed. If I will call my function with null argument, this function will always throw Exception. If you want to force it to silent the warning you can do something like val foo = (value as Foo?)!!. And everyone is happy, we have additional check and when you see the code like this you see potential issues with this code explicitly