[NNBD ] Static Analyzer incorrectly thinks casts will always succeed.

[esDotDev]

Consider this code:

RenderBox? rb = c.findRenderObject() as RenderBox;
return rb?.size ?? Size.zero;

findRenderObject returns RenderObject?, so clearly rb could be null here, but the compiler is giving a warning:

warning: The receiver can't be null, so the null-aware operator '?.' is unnecessary. (invalid_null_aware_operator at [flutter_folio] lib_utils\context_utils.dart:9)

Compiler seems to think the cast will always succeed, even if the source is a nullable.

Nullable or not, it should not be making this assumption?

[esDotDev]

Ah I see what is happening now, the cast needs to become as RenderBox?.

I guess this is working as designed? Sneaky one, but it makes sense.

[passsy]

The warning and your conclusion are correct.

Related: as? #399

[leafpetersen]

cc @bwilkerson @stereotype441 not sure if there's anything actionable here or not, but on the topic of error messages, this is a slightly tricky one to reason out as a user. It's almost the dual of the "failed promotion" problem, in that a promotion did occur, which caused a warning.

[bwilkerson]

The topic has come up before, but with no concrete action items.

I believe that the last time it came up one of the suggestions was a lint that would flag casts where the only effect is to remove nullability. While that might be an interesting lint in its own right, it seems somewhat tangential.

We could take a similar approach to the failed promotion work and track where promotion happened so that we could report something indicating where the promotion that caused the error occurred. That approach has the advantage that it's symmetric.

Another thought would be to attach enough information to the AST that we could display inference information in other ways, such as hover text. It might be useful if we could tell the user that rb has the type RenderObject rather than RenderObject? because of a promotion on line 8, even in the absence of a diagnostic. I don't know what the implementation cost of this option would be.

Those two possibilities are not mutually exclusive.

[stereotype441]

@leafpetersen

cc @bwilkerson @stereotype441 not sure if there's anything actionable here or not, but on the topic of error messages, this is a slightly tricky one to reason out as a user. It's almost the dual of the "failed promotion" problem, in that a promotion did occur, which caused a warning.

I like that way of thinking of it. In the same way that we're working on displaying "why not promoted" context for error messages, we could certainly add "why promoted" context for warnings like this one. So in the example:

RenderBox? rb = c.findRenderObject() as RenderBox;
return rb?.size ?? Size.zero;

The message would become something like:

return rb?.size ?? Size.zero;
         ^^
test.dart:2:9: warning: The receiver can't be null, so the null-aware operator '?.' is unnecessary
RenderBox? rb = c.findRenderObject() as RenderBox;
                                     ^^
test.dart:1:37: info: The variable `rb` was promoted to a non-nullable type here

@esDotDev Do you think that attaching this extra information to the warning message would have made it easier to figure out what was happening?

I filed dart-lang/sdk#45332 to track this idea.

[esDotDev]

Ya I think that would be very helpful. I think my confusion was two-fold

1. I always assume a cast can fail, so null is always an option on a cast, so it didn't occur to me to add ?
2. My mental model is still switching to consider int and int? as 2 different types

Once I understand that these are truly different types, and that the as is changing the type, then it all falls into place nicely.

[esDotDev]

Compiler is sending me mixed signals here!

I seem to be running into some tough corner case, where I have something like a list of non-null items, paired with a nullable field, and I need to search the list. ie:
List<T> items; and T? item;, I need to lookup whether T.id exists in the list, returning null if not.

This works but the cast is pretty verbose, and the compiler is giving me a unnecessary warning.

T? item = (items as List<T?>).firstWhereOrDefault((item) => item.documentId = oldItem?.documentId, defaultValue: null);

But if I remove the cast, I am not allowed to return null as the default.
error: The argument type 'Null' can't be assigned to the parameter type 'T'

I can't really figure out how to do this in one line without the cast. indexWhere is fairly readable but 2 lines, maybe this is the best approach now?

int index = items.indexWhere((item) => item.documentId = oldItem?.documentId);
T? item = index == -1? null : items[index];

Would be nice to be able to get back to the one-liner we had before.

T? item = items.firstWhere((item) => item.documentId = oldItem?.documentId, orElse: () => null);

[stereotype441]

@esDotDev yeah, that's a tricky corner case and you're not the first person to run into it. We added an extension called firstWhereOrNull to the collection package to handle it: https://pub.dev/documentation/collection/latest/collection/IterableExtension/firstWhereOrNull.html

The short story is that if you update your pubspec to depend on a recent enough version of the collection package (1.15.0-nullsafety.4 or newer), then you can add this import statement to your file:

import 'package:collection/collection.dart' show IterableExtension;

And then you should be able to do this:

T? item = items.firstWhereOrNull((item) => item.documentId = oldItem?.documentId);

Incidentally, this is one of the subtle changes that the migration tool is supposed to do automatically for you, but I completely understand if you haven't been able to use the migration tool due to dart-lang/sdk#45326. (Thank you for filing that bug, BTW. I'm working on a fix now).

[esDotDev]

Sweet, that's exactly what I was hoping for, ty!

[esDotDev]

Fwiw, ended up as this workaround for vanilla dart. Costs a line, but it avoids the cast warning, and reads/writes like we're used to:

List<T?> nullableItems = List.from(items);
T? item = nullableItems.firstWhere((item) => item?.documentId = oldItem?.documentId, orElse: () => null);

[a14n]

The lint cast_nullable_to_non_nullable can help.