An annotation to deprecate a function's signature and declare the upcoming signature

[srawlins]

I'd like a way to mark a signature (function, method, ...) as deprecated, with an annotation which includes the new signature. For example, perhaps for Dart 3.0, we want to change the signature of Future.doWhile from

Future<dynamic> Function(FutureOr<bool> Function())

to

Future<bool?> Function(FutureOr<bool> Function())

We might annotate Future.doWhile with an annotation. Some ideas:

@UpcomingSignature<Future<bool?> Function(FutureOr<bool>)>()
Future<dynamic> Function(FutureOr<bool> action()) { ... }

// If function type literals are supported:
@DeprecatedSignature(newSignature: Future<bool?> Function(FutureOr<bool>))
Future<dynamic> Function(FutureOr<bool> action()) { ... }

The annotation does not really have teeth by itself, unless static analysis tools or front-ends like the analyzer can use them to report when usages of the current types would result in errors. For example:

void f() async {
  int a = await Future.doWhile(...);
}

This code has no errors today. But analyzer could use the new signature to report that this code will be broken in an upcoming release of dart:async. This includes type checks that happen anywhere: improper override, invalid assignment, etc.

I think this only works when the new type at each position is a subtype of (or assignable to) the existing type. For example, there is not much we can do with this:

@UpcomingSignature<String Function()>
int f() => 7;

There is no incremental migration path for this signature change. The analyzer could report that this is not a proper use of UpcomingSignature.

If this feature, interpreting a function's signature as something other than the actual function signature, feels extremely Wild West and makes your skin crawl, I'd suggest we only support the annotation behind an opt-in analysis option like use-upcoming-signatures: true.

This would allow us to change the signature (perhaps removing dynamic) of several core Dart package functions:

• Future<dynamic> doWhile(FutureOr<bool> action())
• Future<dynamic> forEach<T>(Iterable<T> elements, FutureOr<dynamic> action(T element))
• Future<dynamic> addStream(Stream<T> source, {bool? cancelOnError})
• Future<dynamic> close()
• Future<dynamic> addStream(Stream<S> stream)
• Future<dynamic> get done
• ...

It also allows package authors to mark an upcoming signature change to a function without deprecating the actual function for a new one.

[lrhn]

This can't be simply seen as supertype/subtype. Any change to a signature can be breaking in one direction or another. If you change a parameter type from num to int (or, say, from Function to int Function(Object, StackTrace)), the resulting signature is a supertype of the current function signature. If you change the return type from dynamic to something else, the new signature is a subtype of the original. They restrict behavior in different ways - changing a parameter to a subtype restricts which arguments you can pass. Changing a return type to a subtype restricts how you can override it (and if it's from dynamic, what you can do, but dynamic is special). Generally, what this would help with are changing to function signature to a supertype, which means that not all existing invocations will remain valid, but all invocations that would be valid with the new type, also are at the original type. Otherwise you can't migrate to the new signature until it's actually there.

I like it. I'd definitely go for changing Future.then/Future.catchError to take error handlers of type FutureOr<R> Function(Object, StackTrace) instead of Function.

It's slightly similar to what we did for Object.operator== where the type changed in null-safe code, but legacy code would still see the old signature, but actually changing the type (which is required for subclasses to be able to implement only the new type) requires it to be a language feature, not just an annotation. (It really changes the declared type depending on who's looking, which is kind-of magical.) That itself is a variant of the golden grail of API migration: Versioned platform library APIs, where new/migrated code sees the new API and legacy code keeps working on the old API (until we remove it in 3.0). That is non-trivial in a language with dynamic invocations, though.

[bwilkerson]

So, would this feature carry its weight? That is, would the value of having this kind of support be high enough to justify the implementation and maintenance efforts?

Or would it be better to just change the signature and have enough support in dart fix to allow it to migrate user's code semi-automatically?

[srawlins]

Thanks for all the feedback, @lrhn !

@bwilkerson:

So, would this feature carry its weight? That is, would the value of having this kind of support be high enough to justify the implementation and maintenance efforts?

I think this is the question, ☹️ . I suspect it could be wildly complicated to implement in analyzer, or perhaps wildly simple? Perhaps making two smaller annotations (@UpcomingReturnType(), @UpcomingParameterType()) may make it simpler?

An alternative could be to hard-code upcoming signatures in analyzer, but I don't think this would be much simpler... the wiring still needs to be very deep and wide-spread.

Or would it be better to just change the signature and have enough support in dart fix to allow it to migrate user's code semi-automatically?

This may be where the feature carries its weight; pub packages can upgrade to Dart 3.0 (or a major version bump of a dependency) individually. But in google3, there will almost certainly be a big bang migration. Dart 3.0 will land across the whole system, and I don't think we can't land all of the dart fixes atomically, coupled with 3.0.

But now that I think about it... what would a dart fix do for a changing signature?

[bwilkerson]

Perhaps making two smaller annotations ...

That might simplify the checking, although I have to wonder whether we couldn't just pull the two pieces of information from a single annotation and check both directions independently.

An alternative could be to hard-code upcoming signatures in analyzer ...

I agree. Hard-coding information should usually be a last resort because it's hard to maintain and hard to know when it's safe to remove it.

... what would a dart fix do for a changing signature?

Good question. Presumably nothing in cases where the invocation is consistent with the new signature. In other cases, I don't know that there's any automatable fix. Perhaps for a change in the return type we could add an explicit cast back to the previous type? For overrides we could update the declaration to be consistent, but then users might have to update the body of the method.

So, yeah, if the remediation is largely required to be manual, then an automated tool won't be of much help.

[davidmorgan]

This came up for discussion as we have a couple of real cases of signature changes where we want to tighten up nullability being expensive, due to the associated diagnostics and need to clean up dead code / null checks / casts.

This is a particularly irksome case because you know the user code will work--they could handle nulls before, now they don't have to--but it's still expensive to land.

Thinking through the various options I'm not sure I have anything good to suggest, though. Full analyzer support seems expensive, I think we essentially have to run analysis twice for the two different versions of the API ... and I did not come up with any appealing heuristic which would let us temporarily ignore the diagnostics we care about in bulk.

[mkustermann]

As asked offline, here's a common example from flutter's code base: TextStyle has nullable fields but doesn't have to, that leads to many ! null-assertions in code using these fields:

const double _kDefaultFontSize = 14.0;

class TextStyle {
  TextStyle({this.fontSize, ...});
  final double? fontSize;
  ...
}

could be converted to

class TextStyle {
  TextStyle({double? fontSize, ...})
      : fontSize = fontSize ?? _defaultFontSize, ...;
  final double fontSize;
  ...
}

[davidmorgan]

The recent example I saw was with an RPC API incorrectly returning Future<T?> instead of Future<T>.

Usually the migration tool could tell the result must be not null, but it incorrectly tried to do as Future<T> instead of await followed by !. So migration of each use of the API needed manual intervention. Possibly this could have been improved in the migration tool--but the actual issue was the RPC API being wrong. I was able to fix it along with all uses in a single very large CL.

To fix incrementally it would have been useful to have a way to change the result type from Future<T?> to Future<T> while disabling hints/warnings caused by that change.