Skip to content

Runtime Error on Inferred Type For Closure In List with orElse #60009

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

Closed
caseycrogers opened this issue Jan 29, 2025 · 6 comments
Closed

Runtime Error on Inferred Type For Closure In List with orElse #60009

caseycrogers opened this issue Jan 29, 2025 · 6 comments
Labels
closed-as-intended Closed as the reported issue is expected behavior legacy-area-analyzer Use area-devexp instead. triage-automation See https://github.com/dart-lang/ecosystem/tree/main/pkgs/sdk_triage_bot. type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)

Comments

@caseycrogers
Copy link

caseycrogers commented Jan 29, 2025

Using orElse with a list of closures with inferred type produces a runtime type error:
TypeError: Instance of '() => (String?) => Null': type '() => (String?) => Null' is not a subtype of type '(() => (dynamic) => String?)?'

void main() {
  final badAppleValidator = (input) => input == 'apple' ? 'Apple is a banned word' : null;
  
  final List<String? Function(String?)> validatorsWithExplicitType = [
    badAppleValidator,
  ];

  final validatorsWithInferredType = [
    badAppleValidator,
  ];

  print('Testing with explicit type!');
  print('Runtime type: ${validatorsWithExplicitType.runtimeType}'); // Runtime type: List<(String?) => String?>
  print(validatorsWithExplicitType.validate('moose'));

  print('Testing with inferred type!');
  print('Runtime type: ${validatorsWithInferredType.runtimeType}'); // List<(Dynamic) => String?>
  // This line hits the `orElse` clause and produces the following runtime error:
  //   TypeError: Instance of '() => (String?) => Null': type '() => (String?) => Null'
  //   is not a subtype of type '(() => (dynamic) => String?)?'
  print(validatorsWithInferredType.validate('moose'));
}

extension RunValidations<T> on List<String? Function(String?)> { 
  String? validate(String input) {
    return firstWhere((vFunc) => vFunc.call(input) != null, orElse: () => (_) => null).call(input);
  }
}

Note that my construction of this bug is pretty awkward, the correct solution in my code was to refactor the validate method to be more readable and not to hit the bug, but I figured I'd file this issue anyway as this feels like a bug or limitation in the type inferrence.

  String? validate(String input) {
    return map((vFunc) => vFunc.call(input)).firsWhere((error) => error != null, orElse: (_) => null);
  }
@lrhn
Copy link
Member

lrhn commented Jan 29, 2025

I'd probably do: return map((validate) => validate(input)).nonNulls.firstOrNull;, to avoid calling the same function twice (and no need to use .call to call it). The <T> on RunValidations is also not used in the on type.

That doesn't change whether inference has a problem.

The receiver for firstWhere has static type List<String? Function(String?)>. The signature is of Iterable<E>.firstWhere is

  E firstWhere(bool Function(E element), {E Function()? orElse})

With E being String? Function(String?), and therefore a context type of String? Function(String?) Function()?, the (_) => null is indeed not a valid argument.
The function should return a String? Function(String?) (not nullable) and it returns null.

Try changing the argument to () => (_) => null, which is a valid and is inferred to () => (String?) => Null.

@lrhn lrhn closed this as completed Jan 29, 2025
@lrhn lrhn transferred this issue from dart-lang/language Jan 29, 2025
@lrhn lrhn added the closed-as-intended Closed as the reported issue is expected behavior label Jan 29, 2025
@dart-github-bot
Copy link
Collaborator

Summary: Type inference fails for lists of closures used with orElse. A runtime error occurs when the inferred type is not a subtype of the expected type.

@dart-github-bot dart-github-bot added legacy-area-analyzer Use area-devexp instead. triage-automation See https://github.com/dart-lang/ecosystem/tree/main/pkgs/sdk_triage_bot. type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) labels Jan 29, 2025
@caseycrogers
Copy link
Author

caseycrogers commented Jan 30, 2025

I'm a bit confused with your response, but I think my original post was confusingly framed and may have misled you. So to clarify:

The following scenario throws a runtime error if the list (this) is a variable with an inferred type and the orElse function is triggered:

return firstWhere((vFunc) => vFunc.call(input) != null, orElse: () => (_) => null).call(input);

The following is the rewrite I used to avoid the bug and improve code readability (as you pointed out, the original code is confusing and needlessly calls the function twice). This rewrite does not throw any exceptions and behaves as I expect:

return map((vFunc) => vFunc.call(input)).firsWhere((error) => error != null, orElse: (_) => null);

In the first scenario where call is used, I'm not really sure I understand why the type inference is failing, and, if there is a real issue, I especially don't understand why it's a runtime error and not static analysis error.

This error isn't blocking us in production as I just moved forward with the rewrite, but I still wonder if I misunderstood something in the original code or if there is in fact a bug/limitation in the type inference. Your suggested resolution (orElse: () => (_) => null) I think is already what the original repro code is doing. Note that, in the repro, everything works fine if the type declaration on the list is explicit. It only throws an exception if the type is inferred which suggests a legitimate problem/limitation with the type inference.

@eernstg
Copy link
Member

eernstg commented Jan 31, 2025

This is one of several issues where a run-time type check fails because of dynamically checked covariance. Here is a version of the original example which has been simplified to do fewer things (it just does the thing that fails now), and output some extra information:

void main() {
  final badAppleValidator =
      (input) => input == 'apple' ? 'Apple is a banned word' : null;
  final validatorsWithInferredType = [badAppleValidator];
  print(validatorsWithInferredType.validate('moose'));
}

extension RunValidations<T> on List<String? Function(String?)> {
  String? validate(String input) {
    this.checkCovariance();
    return firstWhere(
      (vFunc) => vFunc.call(input) != null,
      orElse: () => (String? _) => (null as String?),
    ).call(input);
  }
}

extension<X> on List<X> {
  bool get isCovariant {
    List<X> helper = this.take(0).toList();
    try {
      helper.addAll(<X>[]);
    } on TypeError catch (_) {
      return true;
    }
    return false;
  }

  void checkCovariance() {
    if (isCovariant) {
      print('Covariance detected: $runtimeType <: List<$X>');
    }
  }
}

This program runs and produces the following output:

Covariance detected: List<(dynamic) => String?> <: List<(String?) => String?>
Unhandled exception:
type '() => (String?) => String?' is not a subtype of type '(() => (dynamic) => String?)?' of 'orElse'
#0      ListBase.firstWhere (dart:collection/list.dart:122:53)
#1      RunValidations.validate (file:///usr/local/google/home/eernst/lang/dart/scratch/202501/n047.dart:11:12)
#2      main (file:///usr/local/google/home/eernst/lang/dart/scratch/202501/n047.dart:5:36)
#3      _delayEntrypointInvocation.<anonymous closure> (dart:isolate-patch/isolate_patch.dart:315:19)
#4      _RawReceivePort._handleMessage (dart:isolate-patch/isolate_patch.dart:194:12)

The important point is that the list this has static type List<String? Function(String?)> and run-time type List<String? Function(dynamic)> when validate is executed.

As @lrhn mentioned, the context type for the argument to orElse (which is a static type, based on the static type of this) is String? Function(String?) Function()?. This is used to infer the types of the function literals in the expression () => (_) => null in the original example, which again causes (_) => null to be inferred in the context String? Function(String?), which yields the return type String? (null has that type, no problem), and the parameter type String?.

I added those types to the function literals to make it explicit, but you can check that this is indeed the result of the type inference by capturing this function literal and printing its runtimeType.

The problem is that it is unsafe to pass an actual argument to orElse of firstWhere on a list whose type is covariant. In other words, having the statically known parameter type is not enough to ensure that the given function also has the actual parameter type.

As mentioned, the run-time type of this in validate is List<String? Function(dynamic)>, which means that the instantiated signature of firstWhere is firstWhere(bool test(String? Function(dynamic) element), {String? Function(dynamic) orElse()?}) → String? Function(dynamic), and the actual argument to orElse must be null or a function that returns String? Function(dynamic).

However, the return type of the actual argument is String? Function(String?), which is not a subtype of the required type, which means that the program stops with a run-time type error.

(The typing gets a little bit convoluted here, but we can use a substitutability argument to justify that String? Function(String?) is not a subtype if String? Function(dynamic). If it were then we could pass a String? Function(String?) where a String? Function(dynamic) is expected; they could then call it passing 42 as the argument (which is OK because 42 can be typed as dynamic just fine), but the actual function insists on getting a String or null, so it won't accept 42. So we can't substitute. So the required subtyping relation cannot exist.)

You can create a safer variant of firstWhere (I'd be surprised if it doesn't exist already, but it isn't difficult to write in any case):

extension<E> on List<E> {
  E safeFirstWhere(bool test(E element), {E orElse()?}) {
    for (E element in this) {
      if (test(element)) return element;
    }
    if (orElse != null) return orElse();
    throw "Some suitable exception";
  }
}

void main() {
  List<num> xs = <int>[1, 3];
  print(xs.safeFirstWhere((i) => i % 2 == 0, orElse: () => 2.5)); // OK.
  print(xs.firstWhere((i) => i % 2 == 0, orElse: () => 2.5)); // Throws.
}

The point is that safeFirstWhere is an extension method rather than a proper instance method. This eliminates the covariance issue because extension methods will always refer to the statically known type arguments of the receiver type, so the required type of the actual argument to orElse is exactly the same as the static type.

The fact that extension methods are working with compile-time types is a weakness in many cases, but when it comes to firstWhere and it's orElse argument, it's basically an improvement with no attached disadvantages (except that you need to import the extension).

Another way to avoid run-time type failures caused by dynamically checked covariance is to avoid the covariance in the first place. This topic is addressed by proposals like dart-lang/language#524 and dart-lang/language#229. With those approaches you're given support for a discipline where the covariant typing simply can't arise. For example (using dart-lang/language#229), if a list is declared to have type List<exactly num> rather than List<num> then the value cannot be a List<int> because List<int> is simply not a subtype of List<exactly num>. This means that you will give up on a certain amount of flexibility and gain more complete compile-time type checks.

@caseycrogers
Copy link
Author

Thanks for the extremely detailed response! It was helpful though I'll fully admit the complexity here has lost me.

I guess from my perspective as a user who doesn't understand the type system, I expect type errors to always be compile time errors and not runtime errors (assuming I'm not using any escape hatches like dynamic or as). It sounds like the answer is that this is an edge case where that's just fundamentally not possible with Dart?

@eernstg
Copy link
Member

eernstg commented Feb 14, 2025

Thanks for the kind words!

I expect type errors to always be compile time errors and not runtime errors (assuming I'm not using any escape hatches like dynamic or as).

Right, constructs using dynamic and as will directly turn off the regular static type checking.

However, the underlying static typing isn't designed in the same manner in all languages, and there will always be some phenomena that are deemed so unhealthy that a program has to stop (and, for instance, throw an exception), and the type system could have caught it at compile time.

For instance, some languages dealing with very limited resources can make memory management a compile-time decision. Almost all other languages will have 'Out of memory' as a possible run-time failure. Similarly, C# and Java can have dynamically checked covariance errors (ArrayStoreException and ArrayTypeMismatchException), and lots of languages can have null related errors, because they do not make nullability a part of the type system.

The unusual choice when it comes to Dart is that every type parameter is dynamically checked covariant.

This implies that mutating operations will be subject to run-time type checks when the actual type argument (say, of a List) might be a subtype of the value which is known at compile time.

It sounds like the answer is that this is an edge case where that's just fundamentally not possible with Dart?

Hmmm, dynamically checked covariance was a design decision which has turned out to work well in the vast, vast majority of cases (even though I'd very much like to be able to avoid it, per dart-lang/language#524 and dart-lang/language#229).

But it's not because it is difficult to make every type variable invariant (as in the traditional approach, which is fully type checked at compile time), it was an actual design decision.

The point is that invariance is really inconvenient in an OO language in many cases, and fully explicit variance control (e.g., wildcards in Java) gets complex very quickly.

So I'd join you in wishing that we can turn these run-time type errors into compile-time type errors (in particular, with something like those 524 and 229 proposals). But I wouldn't expect all Dart code to turn into statically checked variance only. It's simply not that much of a problem.

PS: You'll have absolutely no problems finding some math guys who think that's a scandal, and we just don't know better. ;-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed-as-intended Closed as the reported issue is expected behavior legacy-area-analyzer Use area-devexp instead. triage-automation See https://github.com/dart-lang/ecosystem/tree/main/pkgs/sdk_triage_bot. type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)
Projects
None yet
Development

No branches or pull requests

4 participants