dart-lang / sdk

The Dart SDK, including the VM, JS and Wasm compilers, analysis, core libraries, and more.
https://dart.dev
BSD 3-Clause "New" or "Revised" License
10.23k stars 1.58k forks source link

[extension types/switch] Incorrect `Add missing switch cases` completion? #56653

Open modulovalue opened 2 months ago

modulovalue commented 2 months ago

Consider:

void main() {
  Foo<S> f = FooCase(S("abc"));

  switch (f) {
  }
}

sealed class Foo<T> {}

final class FooCase<T> implements Foo<T> {
  final T t;

  const FooCase(this.t);
}

extension type S(String str) {}

In our editor, If we go to switch and try to repair this program via Add missing switch cases, then the switch statement gets completed to:

  switch (f) {
    case FooCase<String>():
      // TODO: Handle this case.
  }

That is, the type argument of FooCase gets autocompleted to the representation type and not the extension type itself.

This seems like a bug, or is this expected behavior? I would expect the type argument to be completed to the extension type and not the representation type.

dart-github-bot commented 2 months ago

Summary: The "Add missing switch cases" completion incorrectly suggests FooCase<String> instead of FooCase<S> for a switch statement on a Foo<S> object, where S is an extension type. This behavior is unexpected and may be a bug.

johnniwinther commented 1 month ago

cc @eernstg

eernstg commented 1 month ago

This seems like a bug, or is this expected behavior?

This seems to be concerned with the support for fixes in the analyzer, rather than anything involving 'area-front-end'. I'll adjust the labels accordingly.

It seems very reasonable to expect the generated cases to preserve the extension type as the actual type argument in the object pattern.

It might be difficult to do this in general because the information about the extension types may not be around at the time/place where those missing cases are being generated (because the exhaustiveness analysis uses the representation types everywhere, and 'generate missing cases' is probably sharing a lot of code with the exhaustiveness analysis).

Anyway, @dart-lang/analyzer-team, WDYT?

PS: I've chosen the label 'analyzer-bulk-fix'. Please correct that if it's wrong.

johnniwinther commented 1 month ago

It is correct that the "Add missing switch cases" fix is based directly on the exhaustiveness analysis, so a fix would require piping the original static type through the analysis.

bwilkerson commented 1 month ago

Because the fix would require changes to the exhaustiveness analysis, I'm switching the area label once again.