language
language copied to clipboard
dart-lang
Dart treats a final nullable property as nullable even after checking that the property is not null
Considering this code:
void main() {
Person person = new Person(
name: "Leonardo",
);
if (person.jobTitle != null) {
showInConsole(person.jobTitle); // Error: The argument type 'String?' can't be assigned to the parameter type 'String'
}
}
class Person {
final String name;
final String? jobTitle;
const Person({required this.name, this.jobTitle});
}
void showInConsole(String text) {
print(text);
}
In the error line, person.jobTitle will not be null as it was checked for that before, but the analyzer keeps it with its original signature. Because of that, it is not usable inside the showInConsole function.
Some workarounds?
- Using a local variable for the property. In this case, I'm not sure if the analyzer keeps the local variable with its original signature after the null check, but it doesn't show an error anymore.
String? jobTitle= person.jobTitle;
if (jobTitle != null) {
showInConsole(jobTitle); // no errors
}
- Using the
!operator. But I'm not sure about this. Coming from TypeScript, this is a bad practice because if I remove the null check for some reason, it won't show the error and it will be throwing a runtime error.
if (person.jobTitle != null) {
showInConsole(person.jobTitle!); // no errors
}
The first workaround is the safest way, I think. But what if I change the type of cardBrand in the future? I would need to change it in every location I check for the property to not be null. Also, it could be annoying to create a local variable for every property you need to check.
If the analyzer can follow the flow of the first workaround for a local variable so it knows it's not null, shouldn't it do the same for the property of a local variable?
@tatumizer In that situation where you are getting the value from a function, I can understand that even if you check the return value to not be null, it doesn't mean the value won't be null when you run the function again (unless it is a pure function, but that might be more complex).
But what I am doing is not running a function so a non-ready-value. I am accessing a class property that is even marked as final so if I already checked for that property to not be null, it's impossible for that property to be null.
Also, I know person could be resigned so jobTitle could be null even after check. But if I mark person as final, the error is still present.
If I am checking the final property of a final variable to not be null, shouldn't it be enough for the analyzer to know it won't be null never?
I agree with you that, in your example, the error should be shown. As you said, jobTitle is overridden by a getter (so a function). And as a function, its return value might not be the same after every execution. That's why it should show an error.
But in my example, jobTitle is not a function. Is a final property. Yes, it can be overridden by another class, but I'm not using that other class. I'm using the class Person and its already final declared jobTitle. If I try to update the property or the final variable person, the analyzer won't let me because it will show an error telling me that it's not possible to set again those values.
void main() {
final Person person = new Person(name: "Leonardo");
person = new Person(name: "Eduardo"); // Error: The final variable 'person' can only be set once
person.jobTitle = "Developer"; // Error: 'jobTitle' can't be used as a setter because it's final
if (person.jobTitle != null) {
// Since person can not be reassigned either its property jobTitle,
// and I have already checked that person.jobTitle is not null,
// it is safe to say that person.jobTitle won't be null never.
showInConsole(person.jobTitle); // Error: The argument type 'String?' can't be assigned to the parameter type 'String'
}
}
class Person {
final String name;
final String? jobTitle;
const Person({required this.name, this.jobTitle});
}
void showInConsole(String text) {
print(text);
}
Since those values can not be reassigned and the analyzer knows it, shouldn't it also know that if I'm checking for those values to not be null, those values won't be null inside the if statement?
In case those values could be reassigned, how could you reassign this so it is reasonable to show an error?
final Person person = new Person(name: "Leonardo");
Well, you could argue that the compiler can see the declaration
Person person = new Person(name: "Leonardo");and thus, the Person is just a basic Person, in which the jobTitle is final.
In this situation, checking for the property to not be null should be enough since, as you said, the compiler can see the declarations.
But in the next version, you can modify it: Person person = giveMeAPerson()); And now compiler cannot assume anything about the real type of Person and hence about the finality of jobTitle.
In this situation, where the code has changed, since the compiler doesn't know because I'm using a function, it is reasonable to show the null-safety error.
Maybe there is something I'm still not getting, but I'm not talking about the second situation. I'm talking about the first situation, which is a different one because the compiler can see that the property won't be null.
but I'm not using that other class
A small change in your program will break your program in ways you don't expect (the reason for the breakage is subtle).
I think this is an important consideration. I think the language team has looked at the various cases in which a developer says "but I know the field isn't overridden," such as private fields (#1167), @sealed classes. Even in your case, the Person() constructor could be a factory constructor which returns a PersonToBeDemoted. Right now there are limited controls of variance in Dart; any time the static type of an object is Person, this means that the runtime type could be any subtype of Person.
One consolation is that the runtime behavior might take knowledge into consideration with optimizations. Looking at:
if (person.jobTitle != null) {
showInConsole(person.jobTitle!);
}
If the compiler has optimizations (based on visibility, exact type, etc), then it may not need to execute that ! null check. But for the sanity of the language, the ! must remain because of specification of the static types.
If fields are promoted in some case and aren't promoted in other case depending on global analysis, it is also confusing very much.
@tatumizer wrote:
Dart AFAIK always compiles the program from the sources - there's no partial compilation, precompiled libraries, etc.
That is not correct. Dart does, at least for some compilers, have modular compilation. I believe the development compiler (dartdevc) is one. Among other things, that's what enables hot reload. If you had to wait for the entire program to be compiled, you'd at most have luke-warm reload. 😁
Also, a package is a program part which is also compiled and run (for testing) independently of the program(s) it's part of. If having a subclass changes the behavior of the superclass, then a superclass method getting field promotion due to not having a subclass, would break in a program where a subclass is added.
It is vitally important to Dart's approach to object oriented programming (and ditto for most other languages with subclasses) that you can write a superclass without knowing its subclasses ahead of time. If you can write code in an instance method, which is then invalidated by a subclass, then you can't really use that feature safely anywhere. (Basically what @Cat-sushi said, if promotion inside a method depends on a global analysis of the entire final program, then it's incredibly fragile.)
Using global information for optimization is great, it enables more possible optimizations. Using global information to change semantics is not, it makes the semantics unpredictable and therefore unusable.
If you could declare that a class cannot have subclasses (or not subclasses outside of the declaring package), then you don't need global information (or seen differently, you have global information because the local declaration ensures global cooperation. And that only works with actual language features, not annotations that might or might not be checked).
It's about being able to see where things can fail. If you see person.jobTitle!, then you know that a null check occurs there. You can look to the surrounding code for hints to why the code feels safe to assume that the value won't be null.
And the author might consider whether there are better ways to do the same thing, one where you only make one type check.
That's why visibility is good, it allows you to notice things. It doesn't prevent you from doing something stupid if you really want to, but then again, nothing ever really does.
A ! is still a run-time cast. It's a cast from T? to T (where the type system can usually figure out which non-nullable type you mean). It's still able to fail at run-time, just like as T would. We're definitely not trying to encourage anybody to spread ! casts around their code. I hope that future code will more often be written in a way where it's not necessary to use !, or where it's used as sparingly as as should be now.
Existing code didn't have that possibility, and migrating existing code might (as the path of least rewrite) require adding ! in places where the current code would also fail (possibly later) if the value is actually null.
Clearly you guys know more than me in this subject, and that's probably why I still can't understand something that seems obvious for me and probably many of the people with this issue.
final Person person = Person(name: "Leonardo");
In this sentence, I'm saying that the type of person is Person, and yes, it could be any subtype of Person. But then, through using Person() I'm saying that actually is not a subtype, but is just a Person.
So now the analyzer have all the information it needs (the exact type that the variable is and the information about that type).
Given that, shouldn't it know that if I'm checking for person.jobTitle to not be null, it won't be null?
Sure, in a future the code can change. If instead of using Person() I use Developer(), the analyser should then treat that variable as that subtype, but nothing else. If Developer overrides jobTitle with a getter, it will start complaining where you use your person variable. If it doesn't override it, it won't complain because it's still a simple nullable property that I already checked to not be null.
I can even change the jobTitle of Person and making it a getter and sure, I will understand it would need some refactor in my code because that's usually what it means to change the structure of a class.
But right know, in that line of code I wrote at the beginning (and the structure of Person), the analyzer have enough information to know that if I'm checking for jobTitle to not be null, it won't be null.
@tatumizer Many thanks, finally I understand what's happening and why is the shown error "valid".
I generally create local variables if I have to check-and-use a nullable instance variable. That is a way to avoid the !, it's just a tad verbose.
I would very much like to have a feature which can check-and-bind in one operation, perhaps something like if (x is Foo foo) or if (x is! Null foo) which checks the value and binds it to a new variable only if it's not null. That allows me to introduce a new variable, check the value and promote the value along the successful check, which is the same thing I'm already doing, just with less verbosity.
I don't think we'll ever see sound promotion of virtual getters. We might be able to see it on non-virtual getters, but even there I'm somewhat against it because it would probably not work on getters, only fields, and that breaks the field/getter symmetry and locks classes into their current implementation.
Interfaces are promises about API and behavior, but only the API can really be checked by a computer. If we know that x.foo is a getter returning an int, that is all we know. We do not know that it's the same value each time you read it.
Dart has chosen to have fields and getters/setters be symmetric, so you can replace one with the other. That is, being a field is an implementation detail, not an API property. C# did something different, where they have getters and setters in the API, which are just methods called in special ways, but fields are completely different and (IIRC) not even virtual - so you have to write your getter/setter pair to access the field in a virtual way (like Java's getFoo/setFoo, just with nicer call syntax).
If we don't want to break that field/getter symmetry in Dart (and we still don't), we can't start assuming that even a final static field is promotable, because it would make it a breaking change to refactor that field into a getter.
Our problems here are largely self-inflicted. Because we can't look below the API to see that something really doesn't change (because either it's not guaranteed to be true because the getter is virtual, or because the promotion we'd do would then be fragile and depend on the getter staying a field), we can't promote anything except local variables. I do not believe that can change without dropping some very fundamental assumptions in the Dart design.
So, I want to focus on providing a different and better way to get a promoted variable from any existing value, rather than focusing on linking separate reads of a fundamentally unknowable getter.
Would it maybe make sense to introduce a syntax that prevents overriding a final variable with a getter?
The proposal here: field promotion with runtime checking looks like the best solution. We trade a little bit of safety to A LOT MORE convenience.
All the examples showing why fields can't promote don't look realistic. In real codebase, 99% of fields/getters don't change type between 2 consecutive calls.
I personally learn more toward sealed classes, preventing inheritance
If it is impossible to extends/implements a class, we know for sure that a final variable never changes, and the type-inference can work properly.
Even sealed classes wouldn't take away the getter/field symmetry problem. You'd still have to explicitly mark the field as unchanging, in a way where that can be strictly enforced by the compilers.
Say (obvious strawman): final final int x = 42;. This field cannot be overridden or implemented by anything but another final final declaration, so it's guaranteed that reading it twice will always give the same result. (And it's considered a breaking change to make a final final field into a mere final field, you're changing not just the implementation, but also the promise that the implementation won't change). Otherwise it's just a final field.
That would work even without sealed classes (you can implement the interface, but it better be with a final final x too!). It's an explicit promise to make a field always be unchanging. (Heck, we can even introduce final get x => ... getters that, somehow, promise to only ever have one value per instance, maybe by only returning the value of other "final" declarations. Starts to feel like const again, but just for the immutability).
Even sealed classes wouldn't take away the getter/field symmetry problem.
Why would that be a problem on a class that cannot be extended/implemented?
As it would not be possible to override a final with a getter. So there's never ever a case where a final property would change by reading it twice.
@tatumizer It is the problem that the current type system doesn't have double derived type which can't be 0, and it is not directly related to null safety.
@rrousselGit Getter/field symmetry works both ways: A client of a class cannot see whether a property is implemented with a user written getter or an implicitly induced field getter (it's always a getter, sometimes it comes from a field), and the author is free to change between the two. It's an abstraction layer - the interface only tells you that you can read something, it doesn't promise how it's implemented.
If you can get promotion from a final field on a sealed class, but not from a getter on a sealed class, then the author of the class is no longer free to change the field to a getter. It has started mattering to clients how the getter is implemented.
The author might not know about this, after all, he might never see the code. So he could think that changing a final field to a getter on his already sealed class is a perfectly good refactoring. Instead it's a breaking change.
If that applies to all final fields of sealed classes, without any way to opt out of it, then a sealed class becomes less modifiable than a non-sealed class, which is the opposite of the reason to seal (to allow safely adding members later, because we know that no-one is implementing the class).
That is why I'm opposed to any attempt to derive unmodifiability of a getter from its choice of implementation — that will immediately make that implementation choice a part of the public API and break the deliberately introduced abstraction layer. I want you to have to opt in to being "unmodifiable", so that it's a deliberate choice and promise that the implementation has certain properties.
I think this is still a far more complicated approach than giving a way to promote-and-bind, which can be used for arbitrary values, rather than adding even more features to the object interface model, and then maintaining and preserving "unmodifiabilty". The bang is just not worth the bucks, they're better spent on something with more bang.
@xvrh
If my understanding is correct,
in current Dart, even late variable have to be checked only once in every same context.
On the other hand, #1187 and #1188 are very expensive, because the compiler inserts null check everywhere referencing the fields.
So, I prefer #1191, #1201 or #1210.
@tatumizer
I understand that It is a design decision that x might returns different value every time referenced.
And it is not a issue of type system.
print(x) is valid regardless of if(x == 5).
showInConsole(x) is invalid regardless of if (x != null).
That is the difference.
@lrhn
I want you to have to opt in to being "unmodifiable", so that it's a deliberate choice and promise that the implementation has certain properties.
But don't sealed classes qualify as an "opt in"? As the enhanced type-inference would be only available if the class is sealed, which requires an extra keyword on the class definition
In the end, the final final you're describing could be a subset of the "sealed" feature where:
sealed class Example {
Example(this.a, this.b);
final int a;
final int b;
}
is equivalent to:
class Example {
sealed Example(this.a, this.b);
sealed final int a;
sealed final int b;
}
As for getters, I believe late is the solution, where instead of:
int get c => a + b;
we can do:
late final c = a + b;
@rrousselGit
But don't sealed classes qualify as an "opt in"?
I don't see how. You're opting in to not extending or implementing the interface. If that also means not being able to change a field to a getter, then you'd be opting in to two things at the same time, whether or not you only wanted one of them. You would have no way to define a sealed interface with a field (getter) that you might change to a getter in the future.
Linking the two would be counter-productive. You would only be able to have "final final" fields if you seal, and you won't be able to have mere "final" fields if you do seal.
Using sealed final ... instead of final final ... is probably easier on the eyes, but slightly different in meaning to me.
@tatumizer
It's definitely possible to make if (instanceVar != null) use(instanceVar) automatically insert a ! (an implicit downcast).
It's just not the way we have otherwise moved with Dart, now that we have otherwise removed implicit downcasts (from anything but dynamic). The people clamoring for no implicit downcasts would not be happy about re-introducing them just to avoid writing !.
And it won't change the meaning at all, compared to if you just write the !. It's still a potentially failing run-time check.
For now, we have chosen to make such potentially failing run-time checks explicit. That's why we introduced ! to begin with, so that it's still brief.
You would only be able to have "final final" fields if you seal, and you won't be able to have mere "final" fields if you do seal.
Getters wouldn't be "sealed"
So you can do:
sealed class Example {
Example(this._a);
final int _a;
int get a => _a;
}
This really needs a solution now that nnbd is rolling into flutter at large.
Without one, we're going to end up with a bunch of landmines like this:
void _handlePastePressed() async {
if (controller == null) return;
int start = controller!.selection.start;
removeTextRange(controller!.selection.start, controller!.selection.end);
String? value = (await Clipboard.getData("text/plain"))?.text;
if (value != null) {
addTextAtOffset(controller!.selection.start, value);
// Move cursor to end on paste, as one does on desktop :)
controller!.selection = TextSelection.fromPosition(TextPosition(offset: start + value.length));
}
}
This is not safe code at all, if a developer accidentally removes the null check at the top, the compiler would say nothing at all, and this is just a RTE waiting to happen.
The right way to code this would be to add somethng like 9 ? characters and several inline ?? 0, or convert controller to a local var, and do the nulll check on that. But both are clumsy and verbose to write, especially if you're checking 3 or 4 different vars.
My worry is that devs will opt for the ! here, since it's by far the least invasive and easiest to read, but they are really filling their methods with potential landmines. And all this to protect some language peculiarity that most aren't aware of?
It seems sad that such a code-smelly feature like overriding final fields with accessors, is forcing such a big weakness in something as core as sound null safety.
From a pragmatic standpoint, this will happen constantly as Flutters entire architecture is to bind us to class level fields, and then reference those fields in our build methods. We already have to define them twice generally, and now we will have to copy them a 3rd time any time we want to check if they are null? It's not really an ok solution tbh...an as a result no one outside the language team will bother to do it., ! operator will reign supreme, and really undo much of the wins here.
The only way I can see this happening Flutter is this bizarre concoction:
class Foo extends StatelessWidget {
const Foo({Key? key, this.value}) : super(key: key);
final String? value;
@override
Widget build(BuildContext context) {
return (value == null) ? Container() : Text(value);
}
}
class Bar extends Foo {
int _callCount = 0;
@override
String? get value => (_callCount++ < 1)? "" : null;
}
Intuitively, I'm sure most developers would be shocked to learn you can override a final field, with an accessor without a peep from the compiler, and this is a source of most of the confusion. No one knows you can do this, and no one does this, cause it's bizarre and probably an anti-pattern out of the gate. Devs expect a field to remain a field in general, which is why this is so hard to grok.
In C# the compiler would warn you to make this accessor as 'new' because it's replacing the base value and this should be called out to anyone reading (abnormal, and bordering on anti-pattern as well).
And if you tried to mark it as override, it would not allow it at all:
This is along the lines of what most developers would expect I think.