Omit type annotation for local variable lint warning is only shown in certain situation.

[dark-chocolate]

Screenshot:

It doesn't show any warning in first line, why is that so?

[davidmorgan]

The first line is a cast; the value is of type SizedBox, but you are declaring that you want to treat it as a Widget. This is allowed by the lint :)

The second line is redundant, 10 is already an int, so writing int does nothing. This is forbidden by the lint.

[bwilkerson]

Should the first line cause a lint?

SizedBox is a subclass of Widget, so it isn't a downcast, and given that the variable is marked const, it can't ever have any type other than SizedBox. I'm not seeing why we'd want/need to allow the type annotation on that line.

[davidmorgan]

It stops the type SizedBox from leaking through the API, so you can change it later without anyone relying on it--unless they're downcasting, but that's a clear warning that it's something dangerous :)

[bwilkerson]

Ok, that makes sense, thanks!

[dark-chocolate]

SizedBox is a subclass of Widget, so it isn't a downcast, and given that the variable is marked const, it can't ever have any type other than SizedBox. I'm not seeing why we'd want/need to allow the type annotation on that line.

@bwilkerson This was my initial thought too.

It stops the type SizedBox from leaking through the API, so you can change it later without anyone relying on it--unless they're downcasting, but that's a clear warning that it's something dangerous :)

@davidmorgan Can you also give an example on this, how can it be changed later?

Ok, that makes sense, thanks!

Only a Googler can understand a Googler :(

[dark-chocolate]

@davidmorgan

The first line is a cast; the value is of type SizedBox, but you are declaring that you want to treat it as a Widget. This is allowed by the lint :)

But this wasn't a cast, because if I use

final map = <dynamic, dynamic> {
  '0': 'zero',
  '1': 1,
};

final Map<String, dynamic> converted = map;

There are no lint warnings, but it isn't considered a cast, because above code throws an error. So, the solution would be to use

final converted = map.cast<String, dynamic>();

[davidmorgan]

const Widget widget = SizedBox();

is exactly equivalent to writing an explicit cast, which you can do using as:

const widget = SizedBox() as Widget;

and then:

final Map<String, dynamic> converted = map;

is also a cast, it's equivalent to

final converted = map as Map<String, dynamic> ;

and the difference between the two is that SafeBox is a Widget, so the cast works; but Map<dynamic, dynamic> is not a Map<String, dynamic, so the cast fails at runtime.

map.cast casts the contents of the collection instead of the collection, so depending on what values you have in there, it might or might not work :) ... I guess, the cast name is a little unfortunate, maybe it should have been castContents to make that clearer :)

I hope that helps :)

[dark-chocolate]

@davidmorgan Thank you so very much sir, I got it fully now, why some of you guys don't come to Stackoverflow and answers questions, we need people like you who have this kinda understanding, this is an unanswered question on SO

No one answered it just yet, and your this line cleared up all my basics.

map.cast casts the contents of the collection instead of the collection, so depending on what values you have in there, it might or might not work :) ... I guess, the cast name is a little unfortunate, maybe it should have been castContents to make that clearer :) Before I close this question I have one more doubt, as you said:

const Widget widget = SizedBox(); // no lint complaint

is actually

const widget = SizedBox() as Widget; // lint complains 

I only get lint warning in the 2nd cast and not the first one, is this intended behaviour?

[davidmorgan]

What lint warning do you get in the second one?

[dark-chocolate]

Screenshot attached:

[davidmorgan]

Oh. Reporting Unnecessary cast on that line looks like a bug, @bwilkerson what do you think please?

[dark-chocolate]

Well you guys know more than me, but I think that SizedBox (subclass) is already a Widget (superclass), so maybe that's why lint shows warning, that you don't need to cast it, it is already a widget.

This won't be an issue unless you want to allow const widget to only access Widget property and not the ones which are in SizedBox.

[eernstg]

True, the cast is marked as unnecessary because it's an upcast: It's allowed implicitly anyway.

[davidmorgan]

The key is whether there is a type annotation on the LHS or not:

const Widget widget = SizedBox() as Widget; // cast does nothing
const widget = SizedBox() as widget; // cast changes the type of `widget`

[eernstg]

It's worth noting that as disables static type checking, in the sense that e as T has type T no matter what e is. We could even have this, and there wouldn't be a compile-time error:

var widget = -1 as Widget;

So it's a dangerous practice to use as T at the end of a variable declaration in order to give that variable the type T.

[davidmorgan]

Good to know, thanks.

You wouldn't get the 'unnecessary cast' warning if it was going to fail at runtime, which seems precisely backwards to what is desirable.

Regardless, the 'unnecessary cast' message is wrong for this case; if we want to keep the checks as is, it should change to 'use type annotations for upcasts'?

[eernstg]

Yes, that would fit nicely in the example, but I suspect that it wouldn't be trivial to determine where that advice is applicable. For instance, var xs = [0, 1 as num]; makes xs a List<num> (which may be crucial because we want to do xs.add(1.0) below), but there is no direct connection from that list literal to the advice "xs should have type List<num>.

[bwilkerson]

(Just a side note, but we're now discussing a hint, which means that the discussion isno longer related to pedantic.)

The original design is that the unnecessary_cast diagnostic is produced when we can statically determine that the cast will have no effect at runtime. And from this perspective, the line

const widget = SizedBox() as Widget;

should have the diagnostic produced.

This made sense before type inference (when the check was added), but now that we're performing type inference, users need a way to control the static type of variables. Either of the following two lines would have the same effect on the static type of widget

const Widget widget = SizedBox();
const widget = SizedBox() as Widget;

The question is: which style to we want to promote.

If widget is a top-level variable (or were a static field in a class), then I believe we've decided to promote the first form because it is part of the public API of the library. If widget is a local variable, then I believe that we've decided to promote the second form.

I would argue that we shouldn't be producing a diagnostic when users are using the style we want them to follow. So, if I'm right about which forms we want users to use, then the production of the diagnostic is now a bug.

You wouldn't get the 'unnecessary cast' warning if it was going to fail at runtime, which seems precisely backwards to what is desirable.

I don't know about "desirable". The purpose of the diagnostic was to allow users to more easily find and remove code that has no runtime effect (similar to dead code). That seems like a good thing, though I would agree that it might be better to also flag code that is known to produce an exception at runtime.

... I suspect that it wouldn't be trivial to determine where that advice is applicable.

Worse than that, it isn't possible for analyzer to statically prove that a runtime exception will be thrown because analyzer can't assume that it has access to the whole program. Consider two unrelated classes and code that casts from one to the other

class A {}
class B {}

B castToB(A a) => a as B;

It is perfectly possible for this to succeed at runtime if someone has defined a class

class C extends A implements B {}
``
and then passes an instance of `C` to `castToB`.

[dark-chocolate]

@bwilkerson

const Widget widget = SizedBox(); const widget = SizedBox() as Widget;

If widget is a top-level variable (or were a static field in a class), then I believe we've decided to promote the first form because it is part of the public API of the library. If widget is a local variable, then I believe that we've decided to promote the second form.

Wouldn't it be cool if we can stick to one design pattern here, to me first make more sense

const Widget widget = SizedBox();

And as @eernstg mentioned, we can also misuse as:

final widget = SizedBox() as String;

Here is another question on Stackoverflow on this, I hope someone from Dart team answers it.

[natebosch]

If widget is a local variable, then I believe that we've decided to promote the second form.

This is not my understanding. Adding a type annotation to a local variable for the purposes of changing the static type to a super type of the inferred type is a supported and idiomatic usage.

One of the intentions of the lint is to improve the signal of intentional type annotations that have meaning like this, so that they aren't drowned out in the noise of redundant inferable types.