Type inference (Function type) doesn't fail compilation, TypeError thrown at runtime

[wujek-srujek]

Dart version: Dart SDK version: 2.16.2 (stable) (Tue Mar 22 13:15:13 2022 +0100) on "macos_x64"
Error reproducible with the below sample on DartPad.

The type inferer doesn't report a Function typing error and fails at runtime instead. Consider the sample below:

typedef Json = Map<String, dynamic>;

abstract class Entity {}

class EntityMapper<T extends Entity> {
  // Generated with json_serializable.
  final Json Function(T) toJson;
  final T Function(Json) fromJson;

  EntityMapper(this.toJson, this.fromJson);
}

class Repository<T extends Entity> {
  // Provides real data from Firestore.
  final Iterable<T> Function() _dataProvider;
  final EntityMapper<T> mapper;

  Repository(this._dataProvider, this.mapper);

  Iterable<T> findAll() => _dataProvider();
  
//   Json toJson(T entity) => mapper.toJson(entity);
}

class EntityA extends Entity {}

final entityAMapper = EntityMapper<EntityA>(
  (a) => {'name': 'A'},
  (json) => EntityA(),
);

class Processor {
  final Iterable<Repository<Entity>> _repositories;

  Processor(this._repositories);

  void process() {
    for (final repository in _repositories) {
      final entities = repository.findAll();
      final entityJsons = entities.map(repository.mapper.toJson); // <<<
//       final entityJsons = entities.map(repository.toJson);
      for (final entityJson in entityJsons) {
        print(entityJson);
      }
    }
  }
}

void main() {
  final entityARepository = Repository<EntityA>(
    () => [EntityA(), EntityA()],
    entityAMapper,
  );

  Processor([entityARepository]).process();
}

• There is an Entity superclass.
• There is a generic EntityMapper which takes functions mapping to/from JSON to instance.
• There is a mapper instance for each Entity subclass.
• A generic Repository is parameterized with a data provider and a mapper for a specific Entity subclass.
• A Processor gets a list of repositories, iterates over them, gets all entities and attempts to convert them to JSON.

The line marked with <<< causes the following failure:

Uncaught Error: TypeError: Closure 'entityAMapper_closure': type '(EntityA) => Map<String, dynamic>' is not a subtype of type '(Entity) => Map<String, dynamic>'

This happens only at runtime and the type inferer / analyzer aren't able to report this issue before that.

When the commented out method Repository.toJson is enabled, which basically just delegates a method call to the mapper, and the Processor is updated by now using the new Repository.toJson method, the code works fine.

1. Should type inference be able to find the issue with the original code?
2. Why doesn't the code work in the first place?
3. Why does simple delegation fix the error?

[wujek-srujek]

Also it is possible to change the code to create the mapper like this:

final entityAMapper = EntityMapper<EntityA>(
  (Entity a) => {'name': 'A'},
  (json) => EntityA(),
);

and the code works, but I think it is clearly wrong - the EntityMapper is parameterized with EntityA so why would the compiler allow the function parameter to be Entity? This actually makes the above code work without errors, but then the function doesn't have access to EntityA fields etc., all it knows is Entity, which is no ideal.

[eernstg]

The dynamic error arises because Dart uses dynamically checked covariance:

void main() {
  List<num> xs = <int>[1, 2]; // Accepted, because `List<num> <: List<int>`.
  xs.add(1.5); // Checked at run time, will throw because `1.5` does not have type `int`.
}

In this particular example the covariance is introduced at Processor([entityARepository]).process(), because a Repository<EntityA> is passed where a Repository<Entity> is expected, and the dynamic error arises when the Json Function(EntityA) when it is passed where a Json Function(Entity) is expected.

The statically safe solution is to introduce statically checked variance, dart-lang/language#524. With statically checked variance we'd need to make several adjustments, including class EntityMapper<inout T extends Entity>, and we would then avoid the potential run-time errors. I hope we'll get that into the language soon, but right now we don't have it.

The precise situation where the dynamic error occurs is actually when repository.mapper.toJson is evaluated. At that point repository has static type Repository<Entity> and dynamic type Repository<EntityA>, repository.mapper has static type EntityMapper<Entity> and dynamic type EntityMapper<EntityA>, and repository.mapper.toJson has static type Json Function(Entity) and dynamic type Json Function(EntityA). The latter is not a subtype of the former, and the compiler tracks the potential failure points and inserts a dynamic check; that check will actually fail in this case, so the evaluation ef repository.mapper.toJson throws.

It looks like the types are aligned, even though it is not statically safe (so we're passing an EntityA statically typed as Entity to a function expecting an EntityA, typed as having argument type Entity). So, at least with the example, you can cancel the intermediate type checks by changing the throwing expression to the following:

final entityJsons = (entities as dynamic).map((repository.mapper as dynamic).toJson);

The first cast to dynamic ensures that the map function will not throw even though it's argument isn't a Json Function(Entity), and the second one ensures that the evaluation of toJson will not throw.

The main reason why this particular example gives rise to multiple failures during dynamic covariance checks is that there is a class with an instance member whose type is not covariant in the type parameters of the enclosing class, namely EntityMapper.

This has been recognized from the very beginning as a very error-prone situation, but I guess it wasn't considered particularly frequent back then. We've had proposals about how to deal with that kind of type for a while (e.g., dart-lang/language#297), but the real solution is to support statically checked variance.

[wujek-srujek]

Thanks for the in-depth explanation and the links. I prefer not to use as dynamic workaround as I loose a lot of type safety (which is kind of funny as the whole issue is about runtime errors ;-)).

The main reason why this particular example gives rise to multiple failures during dynamic covariance checks is that there is a class with an instance member whose type is not covariant in the type parameters of the enclosing class, namely EntityMapper.

This has been recognized from the very beginning as a very error-prone situation

Have I committed some terrible error with my code? How should it be written in a type-safe way and also work at runtime?

[wujek-srujek]

There are also two questions: why the extra method fixes it, and why the compiler allows

final entityAMapper = EntityMapper<EntityA>(
  (Entity a) => {'name': 'A'},
  (json) => EntityA(),
);

Especially the snippet above seems to be wrong statically.

[eernstg]

How should it be written in a type-safe way and also work at runtime?

My assumption was that you do not really have a choice, because EntityMapper is generated by json_serializable, IIUC, and (most likely) the json representation is determined by some external constraints.

If you do have the freedom to avoid using "contravariant instance variables" then you could make the following adjustments:

typedef Json = Map<String, dynamic>;

abstract class Entity {}

abstract class EntityMapper<T extends Entity> {
  Json toJson(T t);
  T fromJson(Json json);
}

class Repository<T extends Entity> {
  final Iterable<T> Function() _dataProvider;
  final EntityMapper<T> mapper;
  Repository(this._dataProvider, this.mapper);
  Iterable<T> findAll() => _dataProvider();
  Json toJson(T entity) => mapper.toJson(entity);
}

class EntityA extends Entity {}

class EntityAMapper extends EntityMapper<EntityA> {
  toJson(a) => {'name': 'A'};
  fromJson(json) => EntityA();
}
final entityAMapper = EntityAMapper();

class Processor {
  final Iterable<Repository<Entity>> _repositories;

  Processor(this._repositories);

  void process() {
    for (final repository in _repositories) {
      final entities = repository.findAll();
      final entityJsons = entities.map(repository.mapper.toJson);
      for (final entityJson in entityJsons) {
        print(entityJson);
      }
    }
  }
}

void main() {
  final entityARepository = Repository<EntityA>(
    () => [EntityA(), EntityA()],
    entityAMapper,
  );

  Processor([entityARepository]).process();
}

About this:

final entityAMapper = EntityMapper<EntityA>(
  (Entity a) => {'name': 'A'},
  (json) => EntityA(),
);

The reason why the static analysis permits that in the original example is that Json Function(Entity) is a subtype of Json Function(EntityA) (because Entity is a supertype of EntityA, and a formal parameter type is in a contravariant position in the function type as a whole).

The extra method would be this one:

class Repository<T extends Entity> {
  ...
  Json toJson(T entity) => mapper.toJson(entity);
}

The reason why that method can be passed to map (I assume that's what it is used for) is that it is an instance method, and they do not give rise to dynamic errors at evaluation, even in the case where they use a type parameter in a contravariant position in the signature. And the reason why the evaluation of mapper.toJson does not throw is that this occurs inside the class that declares T, and the given instance of Repository has the actual type argument EntityA, and the mapper has the dynamic type EntityMapper<EntityA>. So there's no covariance in that particular case, and hence it is safe to access an instance variable like toJson.

[devoncarew]

It sounds like this is currently working as intended, though we may make changes here in the future (declaration-site variance; dart-lang/language#524).