Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
DetachHead opened this issue Jul 28, 2022 · 5 comments
Open
Labels
analyzer-linter Issues with the analyzer's support for the linter package area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. linter-lint-request P3 A lower priority bug or feature request type-enhancement A request for a change that isn't a bug

Comments

@DetachHead
Copy link

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

@eernstg
Copy link
Member

eernstg commented Jul 28, 2022

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
Copy link
Member

eernstg commented Jul 28, 2022

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: dart-lang/language#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.

@lrhn
Copy link
Member

lrhn commented Jul 28, 2022

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.

@DetachHead
Copy link
Author

@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? #48553 (comment)

@eernstg
Copy link
Member

eernstg commented Jul 28, 2022

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.

@keertip keertip added type-enhancement A request for a change that isn't a bug P3 A lower priority bug or feature request labels Aug 1, 2022
@srawlins srawlins transferred this issue from dart-lang/sdk Apr 5, 2024
@devoncarew devoncarew added analyzer-linter Issues with the analyzer's support for the linter package area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. labels Nov 19, 2024
@devoncarew devoncarew transferred this issue from dart-lang/linter Nov 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
analyzer-linter Issues with the analyzer's support for the linter package area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. linter-lint-request P3 A lower priority bug or feature request type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

6 participants