sdk icon indicating copy to clipboard operation
sdk copied to clipboard

analyzer should show an error when casting to a non-overlapping type

Open DetachHead opened this issue 1 year ago • 5 comments

class A {}

class B extends A {}

class C {}

void main() {
  final a = A();
  final b = B();
  final c = C();
  a as B;
  a as C; // no error
}

there should be a warning on casts that are probably wrong, like in typescript

DetachHead avatar Jul 28 '22 10:07 DetachHead

This is working as intended, and it is certainly possible for such casts to succeed:

class A {}
class B {}

class AB implements A, B {}

void main() {
  A a = AB();
  a as B;
  print('Did not throw!');
}

eernstg avatar Jul 28 '22 11:07 eernstg

I made a proposal for a safer variant of casting (for instance, a cast which is supposed to be a downcast could be expressed as such, and then we'd have feedback for the case where it isn't), but I can't find it right now. It could also be a lint, based on the assumption that some organizations would prefer to use the same reasoning as in the TypeScript code here.

Edit: https://github.com/dart-lang/language/issues/1622 contains some example extension methods (getters, actually), expressing a few constrained version of as expressions. Note that they do not have to have a run-time cost (that is, they'd have the same cost as the built-in cast) because extension methods can be inlined, especially when the body is so small.

eernstg avatar Jul 28 '22 11:07 eernstg

Say there exists a class D implements B, C {} (and perhaps several other unrelated subtypes of both A and C), and you know, for some reason, that a is a C. What would you then need to do to avoid the warning, when the cast is actually correct?

You can do a double cast a as A as C (risking an unnecessary-cast warning for the first cast). But that's annoying.

Since you're only asking for a warning, it seems like it's a job for a lint. The linters does have a slew of "unrelated_something" lints. It could have an "unrelated_cast" as well. Then you could do a as C; // ignore: unrelated_cast.

So, I'm marking this as a request for the analyzer, because that's the closest thing in this repository, but it should more likely be a feature request for the linter.

lrhn avatar Jul 28 '22 11:07 lrhn

@eernstg

This is working as intended, and it is certainly possible for such casts to succeed

yeah i didn't say it's impossible for casts like that to succeed, but most of the time it's a mistake. hence the wording in the typescript error message and my OP

Conversion of type 'A' to type 'C' may be a mistake

there should be a warning on casts that are probably wrong


@lrhn

You can do a double cast a as A as C (risking an unnecessary-cast warning for the first cast). But that's annoying.

imo unnecessary-cast should be updated to support this usage

Since you're only asking for a warning, it seems like it's a job for a lint.

sounds good to me, i don't mind whether it's an analyzer error or a lint. i just didn't know where to raise this since some warnings are considered lints and others aren't? https://github.com/dart-lang/sdk/issues/48553#issuecomment-1067366673

DetachHead avatar Jul 28 '22 13:07 DetachHead

imo unnecessary-cast should be updated to support this usage

The standard way to make a cast acceptable to the linter is to use dynamic as the mid point: a as dynamic as C. Of course, you may then hit yet another lint saying that you shouldn't ever mention dynamic.

eernstg avatar Jul 28 '22 13:07 eernstg