[Null Safety] add a smart-cast to non-nullable inside condition

[subzero911]

Currently analyzer is kinda dumb:

Logically, we already checked that this value can't be null, so there's no need for exclamation mark after age.

It works in Kotlin nevertheless:

Very disappointed it's not such a case in Dart.

[lrhn]

Dart promotion is guaranteed to be sound. The language cannot guarantee that the value you tested in dog.age != null is the same value you read in the later dog.age. Other languages with other designs might be able to make that guarantee, but in Dart, all variables can be overridden in subclasses, all classes can be implemented and constructors can be changed to factory constructors that return such a subclass or implementation.

Looking just at your code

var dog = Dog(7);
if (dog.age != null) {
  ... dog.age ...
}

it's possible for that code to exist in a program where dog.age returns different values on different invocations.

It might be possible to detect that it won't happen in your program, by doing a whole-program analysis, but Dart bases promotion on local analysis only. That prevents you from introducing fragile promotions that will break when someone else does an otherwise perfectly valid refactoring (like changing a variable to a getter).

The analyzer is doing exactly what the language requires here. The language doesn't currently allow you to do promotion on anything but local variables. There are several requests to do more, and proposals on how to do it (like #1167).

[subzero911]

it's possible for that code to exist in a program where dog.age returns different values on different invocations.

Sorry, but I believe, it's not possible, because age is final. I couldn't come up with situation when final age would return different values.

The same, Kotlin won't smart-cast Int? to Int if it was var, not val

[lrhn]

That age is a final variable is not part of the interface, just the class declaration. It's a valid refactoring to change a final field to a getter. If we promoted final fields, but not getters, that would stop being a valid refactoring. A field and a getter would be different in the interface, which they currently aren't.

In other words, Dart doesn't only restrict promotion to what the current program does, but also what a reasonably refactored program could do, because if it promoted based on only the current program's behavior, it would effectively prohibit such refactorings.

[andreimesina]

Hi @lrhn, it's very nice to take dangerous refactoring results in consideration for Dart's language design.

However, at the same time, if smart nullability check was a thing in Dart, just as it is in Kotlin, wouldn't the refactoring just trigger a warning / error for the developer, like in any other refactoring situations?

How the example from above would work in my mind:

1. Smart nullability check

var dog = Dog(7);
if (dog.age != null) {
  ... dog.age ...
}

2. Somebody refactors the code (e.g. changing from variable to getter)
3. A warning / compile time error arises, saying that now the dog.age getter's nullability cannot be asserted anymore
4. The developer will have to update the call site in order to make sure the nullability check is still working

I'm not very experienced in Flutter but this is how it would happen in Kotlin, doing a similar change.

[lrhn]

That is precisely the difference between Dart and Java (maybe Kotlin).

In Java, changing a field to a getter method is a breaking change. Clients of the API will need to be rewritten to call getFoo() instead of testing foo. Your package version would need a major version increment to account for the breaking change.

In Kotlin, of it does allow the code you wrote, it still means that changing from a promotable field to a similarly accessed getter is a breaking change.

In Dart, changing a field to a getter is a completely safe and non-breaking refactoring. Clients of the API never need to know. You can do it on a patch release, since it doesn't change the API at all.

This was a deliberate design choice from the original designers of Dart, to avoid everybody doing the private-field/public-getter pattern that is the standard design in Java, because it allows you to change the getter implementation to something non-trivial without breaking the API.

That's what we would lose of we allowed any final variable, instance or static, to be promoted, but not a similarly accessed getter.

This symmetry does have the cost that we cannot distinguish privileged internal access to a public field from external API based access, which is why we can't just allow any direct final field access to be promoted.

Another consequence, with both benefits and costs, is that you can overrife a getter in a subclass, even the implicit getter of a field.

The one thing we maybe can do, is to allow promotion of access to fields from inside the same library where they are declared, considering those privileged accesses. If changing the field to a getter breaks promotion, they code being broken will be inside the same library, so the author will know immediately, and be able to fix it

It still requires the compiler to be able to determine that the field cannot be overridden to something non-final in another library, so right now it's limited to private variables and variables of final class hierarchies, where the compiler can be certain all implementations of the getter are in the same library.

That kind of promotion is being worked on. It's not here yet.