Confusing list type mismatch at runtime

[matthew-carroll]

I ran into a runtime type mismatch for lists that I expected to be of the same type. I'll provide a repro of the issue, and then I'll explain why this was confusing.

The following code sample reproduces the issue in Dart Pad.

void main() {
  print("Start");
  
  final editor = Editor(
    requestHandlers: defaultRequestHandlers,
  );
  
  editor.requestHandlers.insertAll(0, [
    (request) => Command2(),
  ]);
  
  print("End");
}

class Editor {
  Editor({
    List<EditRequestHandler>? requestHandlers,
  }) : requestHandlers = requestHandlers ?? [];
  
  final List<EditRequestHandler> requestHandlers;
}

class Request1 implements EditRequest {}

class Command1 implements EditCommand {}

class Request2 implements EditRequest {}

class Command2 implements EditCommand {}

class EditRequest {}

class EditCommand {}

typedef EditRequestHandler = EditCommand? Function(EditRequest);

final defaultRequestHandlers = [
  (request) => request is Request1
    ? Command1()
    : null,
];

Running this code results in the following error:

Start
Uncaught Error: TypeError: Instance of 'JSArray<(EditRequest) => EditCommand?>': type 'JSArray<(EditRequest) => EditCommand?>' is not a subtype of type 'Iterable<(dynamic) => Command1?>'

The fix for this issue is to make the following change:

// Change
final defaultRequestHandlers = [
  (request) => request is Request1
    ? Command1()
    : null,
];

// To
final defaultRequestHandlers = <EditRequestHandler>[
  (request) => request is Request1
    ? Command1()
    : null,
];

It looks like, in the absence of an explicit type declaration, the Dart runtime chose an implied list generic type from the first item that happened to be declared in the defaultRequestHandlers list, which seems to then be tracked into the Editor instance. By pushing the narrower implied type into the Editor, insertion operations on the editor's requestHandlers list fail when they're expected to succeed.

I can understand why Dart may have implied a List generic type based on the first item in the list, or based on a union of the items in the list, but this situation is a problem for at least a couple reasons.

First, I received no compiler complaint. This was a very confusing bug in my application and it took a lot of guessing to figure out why Dart was blowing up at runtime.

Second, the error message, while probably accurate, barely helped me comprehend the issue. All of my type contracts seemed correct and yet, mysteriously, I was losing the parameter type at runtime.

[lrhn]

It looks like, in the absence of an explicit type declaration, the Dart runtime chose an implied list generic type from the first item that happened to be declared in the defaultRequestHandlers list

Yes, that is precisely what happens. (Or rather, it tries to find a type for the list that matches all the elements, if there is more than one.)
And absent any "context type", the parameter to the function literal in that list becomes typed as dynamic, which makes it accept any argument. That's nice, but also means that the list type becomes List<EditCommand? Function(dynamic)> to hold that function.

Because function types are contravariant in their parameter types, EditCommand? Function(dynamic)> is a subtype of EditCommand? Function(EditRequest) (aka EditRequestHandler).

So defaultRequestHandlers, at type List<EditCommand? Function(dynamic)>, is a valid argument to a parameter of type List<EditRequestHandler>. (And that's fine, a function which takes dynamic can be used where an EditRequestHandler is needed.)

The error you then actually hit is that when you try to add a EditCommand? Function(EditRequist) function to that list, it complains because that function doesn't have dynamic as parameter. The list is only for functions which accepts all arguments!
That's the problem that Dart class generics are unsafely covariant, and List.add can throw without static warning if you've upcasted a list to a super-element-type. The same error you get if you do:

List<num> l = <int>[1];
l.add(3.14); // No warning, runtime error.

So, it's a well know issue, and not one that's easily fixed or detected statically.

The workarounds are to ensure that the list you store in the object is precisely a List<EditRequestHandler>, not a subtype of it.
That's actually rather hard to test (if all you have is an "is subtype" check, it's hard to ensure that something is not a proper subtype). The simplest approach is to create a new list of the correct type every time:

  }) : requestHandlers = [...?requestHandlers];

Alternatively, if you are the only one creating instances of the class, you can ensure that you pass in a list of the correct type everywhere, which means ensuring the type of the default handler list in some way:

final List<EditRequestHandler> defaultRequestHandlers = [ ... ];
// or 
final defaultRequestHandlers = <EditRequestHandler>[ ... ];
// or
final defaultRequestHandlers = [(EditRequest request) => request is Request1 ... ];

Without any hint, the compiler can't guess that you want a list of functions that can only be called with EditRequests, but that's the kind of function you later try to add.

[matthew-carroll]

Because function types are contravariant in their parameter types, EditCommand? Function(dynamic)> is a subtype of EditCommand? Function(EditRequest) (aka EditRequestHandler).

I might expect the change to dynamic if there were some variation in the list of function parameters, but given that every function in the list has a single parameter of the same type, why does Dart still expand the type definition from EditRequest to dynamic?

[lrhn]

The function literal in the list has a parameter type of dynamic. No other parameter type is specified, in any way, so that is what it defaults to.
EditRequest isn't mentioned in the list declaration, so that certainly won't be used.

That's why specifying a parameter type is a workaround for the issue.

[matthew-carroll]

Ah, I missed that in my own code. I was always thinking about the typedef for EditRequestHandler that I didn't pay attention to specifying the type in the anonymous function declaration. That makes sense.

Is there anything worth retaining in this issue? Anything here that would lead to added static checks, or any other Dart work? Or should I just close it?

[eernstg]

Statically checked declaration-site variance #524—if it gets enough support to be included into the language—is a mechanism that you could use to change these run-time errors into compile-time errors:

class A<inout X> {
  final theList = <X>[];
}

void main() {
  A<num> a = A<int>(); // Compile-time error.
  a.theList.add(1.5); // So we won't need to worry about this.
}

We don't have this feature at this time, but there is a coding pattern which has the same effect:

// Emulation of invariance:
// In the library that declares `A` today, we'd instead have this:

typedef _Inv<X> = X Function(X);
typedef A<X> = _A<X, _Inv<X>>;

class _A<X, Invariance> {
  final theList = <X>[];
}

// Client code, presumably in some other library, will still use `A`, and
// this type will make covariance a compile-time error (and it is checked
// at run time if we use `dynamic` to avoid the compile-time error).

void main() {
  A<num> a = A<int>(); // Compile-time error.
  a.theList.add(1.5); // So we won't need to worry about this.
}

However, even with that mechanism in place (as a language feature or emulated) it is not likely that we will be able to make platform-provided classes like List invariant, that's too massively breaking. So we can protect objects of type A<OfSomething> from covariant subtyping, but the list itself would still allow List<num> xs = a.theList;, which means that the static type of the list is now a covariant subtype of its run-time type (and that's the unsafe situation). So xs.add(1.5) would still be accepted at compile time, and it would still throw at run time.

So, unfortunately, you'll need to use a dynamic check in order to ensure that there is no covariant subtyping relation between the static and the dynamic type of the list. You could do something like the following:

void main() {
  print("Start");

  final editor = Editor(
    requestHandlers: defaultRequestHandlers,
  );

  print("Mid");

  editor.requestHandlers.insertAll(0, [
    (request) => Command2(),
  ]);

  print("End");
}

class Editor {
  Editor({
    List<EditRequestHandler>? requestHandlers,
  }) : requestHandlers = requestHandlers ?? [] {
    // Ensure that there is no covariant subtyping with `requestHandlers`.
    try {
      this.requestHandlers.addAll(const <EditRequestHandler>[]);
    } on TypeError catch (_) {
      throw "Some useful error message";
    } catch (_) {
      // You may or may not wish to allow a constant or unmodifiable list here.
    }
  }

  final List<EditRequestHandler> requestHandlers;
}

class Request1 implements EditRequest {}

class Command1 implements EditCommand {}

class Request2 implements EditRequest {}

class Command2 implements EditCommand {}

class EditRequest {}

class EditCommand {}

typedef EditRequestHandler = EditCommand? Function(EditRequest);

final defaultRequestHandlers = /*<EditRequestHandler>*/ [
  (request) => request is Request1 ? Command1() : null,
];

I think you can close the issue. The fact that the language uses dynamically checked covariance is well-known; the fact that statically checked variance would report unsafe code using compile-time errors and hence eliminate the run-time errors is the very purpose of having statically checked variance; and the fact that you can emulate invariance in any class that you can edit has also been mentioned in several different locations.