Better subclass handling for argument_type_not_assignable

[FMorschel]

When we have a subclass (different type parameter) being used instead of the expected class on a parameter we have a bad error message compared to when we use the expected class.

Also, the current good error message is a bit redundant in the following example. I'm having the declaration path for Iterable twice on the error message (shortened it here to make the message more easy to see in the issue but in my case it is really big) and the actual problem is simply the type parameter for the elements.

Repro:

a.dart:

class A {
  const A();
}

main.dart:

import 'a.dart' as a;

void main() {
  final list = <a.A>{};
  list.addAll(A.list);
  list.addAll(A.iterable);
}

class A {
  const A();

  static List<A> get list => [
    for (int i = 0; i < 10; i++) const A(),
  ];

  static Iterable<A> get iterable => [
    for (int i = 0; i < 10; i++) const A(),
  ];
}

On main.dart line 5 list.addAll(A.list), we get:

The argument type 'List<A>' can't be assigned to the parameter type 'Iterable<A>'. dart(argument_type_not_assignable)

On line 6 list.addAll(A.iterable) we get a way better error message:

The argument type 'Iterable<A> (where A is defined in .\bin\bug.dart)' can't be assigned to the parameter type 'Iterable<A> (where A is defined in .\bin\a.dart)'. dart(argument_type_not_assignable)

iterable.dart(92, 22): Iterable is defined in ...\dart-sdk\lib\core\iterable.dart
bug.dart(9, 7): A is defined in .\bin\bug.dart

iterable.dart(92, 22): Iterable is defined in ...\dart-sdk\lib\core\iterable.dart
a.dart(1, 7): A is defined in .\bin\a.dart

[dart-github-bot]

Summary: The error message for argument_type_not_assignable is inconsistent when a subclass is used instead of the expected class. The error message for the subclass case is more informative, but both messages are redundant, repeating the declaration path for Iterable twice.

[srawlins]

@bwilkerson 's comment here I think applies to fixing this issue:

For anyone looking to fix this bug, there's already support for this kind of disambiguation when passing types in as the arguments to the reportError methods.

[FMorschel]

@srawlins, should I open a new issue concerning typedefs, or is this one the right place for this ask?

The repro:

typedef MyBadlyNamedTypedef = int Function(int);

void foo(MyBadlyNamedTypedef f) {}

void bar() {
  foo(baz);
}

String baz(int x) => x.toString();

The error message:

The argument type 'String Function(int)' can't be assigned to the parameter type 'MyBadlyNamedTypedef'. 

[bwilkerson]

What are you proposing as an alternate message?

[FMorschel]

To show the actual type somewhere, replace the typedef or show both with one inside () or something (preferred).

Take my example. If you have a badly named typedef or you are unfamiliar with what it means, this could be really weird.

Also, because of #57013, we still can't "Go to Type Definition" and see the typedef. So for Function types, to find out what it is you'd need to look at the definition for whatever you are trying to assign the Function, so you can see the typedef written down to find its definition (if it is a constructor you could also only see in the propriety definition some classes up the chain with lots of super and one this).

[srawlins]

I think good to track with a separate issue. Especially because there might be discussion on the messaging. I think it'd be great to show both type aliases and un-aliased actual types. But because of the lengthy message that creates, I think we essentially never do that, and default to the un-aliased types (or maybe it's a mixed bag).

[bwilkerson]

To show the actual type somewhere ...

That seems like a reasonable thing to do. I think we should show both.

But because of the lengthy message that creates ...

When we stick the URI of the type in messages to disambiguate them we end up with fairly long messages, but it's better than "Can't assign 'C' to 'C'." style messages.

The alternative that I could imaging is to put the extra information in a context message, something like "The typedef 'MyBadlyNamedTypedef' is equivalent to 'int Function(int)'."

Also, because of #57013, we still can't "Go to Type Definition" and see the typedef.

If the only reason to add this information to the message were because of the lack of navigation, I'd recommend that we fix the navigation and leave the message alone. But the message also gets displayed by the command-line analyzer, so I think that fixing that issue wouldn't impact this one.

[FMorschel]

Alright, #57056 created. Thanks!