[breaking change] Remove implicit tearoff of `call` methods

[leafpetersen]

Currently an assignment from a "callable class type" (an interface type where the interface has a call method) to a function type or the type Function will perform an implicit call method tearoff.

That is:

class MyClass {
  int call() => 42;
}

means that Function f = MyClass(); will be allowed, even if MyClass is not a subtype of Function, and will be implicitly converted to/treated as Function f = MyClass.call;. Whether to do the implicit tear-off depends on the context type where the expression occurs, which means that we have an expression which changes its type, invisibly, depending on an, also invisible, context type. It's hard to read, and if something goes wrong, it's very hard to see where.

This breaking change removes this implicit tear-off behavior, and requires you to write the .call explicitly to get the current behavior.

Historically, prior to Dart 2.0, such callable class types were subtypes of the function type of their call method. The Dart 1 type system was unsound in many ways, and this particular subtyping could not be included soundly in the Dart 2 type system. Instead an implicit coercion was introduced, which made some of the old code keep working, but not all of it. Code that relied on the actual callable class object being stored in the variable would no longer work. The torn-off call method was stored instead.

The current code still looks like it stores the object, which is deceptive and potentially error-prone. And it does so based on a potentially invisible context type, which makes it intractable to verify the code by visual inspection. There is no syntax suggesting that something special is going on in this assignment. There is no warning if it stops happening for some reason, perhaps because the context type changes to dynamic.

Because of that, it's considered better for readability and safety to force the tear-off to be explicit.

Also, with the patterns proposal, there will be more ways to do assignment, with more distance between the variable and the initializing expression, which makes it even easier to make mistakes and overlook the implicit tear-off.

There are no plans to change the behavior when calling such an object. You can still do:

var myFunctionLike = MyClass();
var myInt = myFunctionLike();

without having to write myFunctionLike.call(). There is syntax here suggesting exactly what's happening (something is getting called), and no expression which changes type because of an invisible coercion, and almost no dependency on the context type (unless the call method is generic, then type inference may use the context type to infer type arguments as usual).

[leafpetersen]

@natebosch will investigate the feasibility of this.

[kevmoo]

IIRC this would be a potential (big?) win for dart2js (@sigmundch @rakudrama ?)

[lrhn]

I don't think it will do much for dart2js since it just makes an implicit inferred .call into an explicit .call. When dart2js sees the code, inference should already have happened in the front end, so there won't be any difference.

The front-end inference might get slightly faster.

(I'd be happy to be wrong, though!)

[sigmundch]

Does this issue relate to supportsExplicitGetterCalls (https://github.com/dart-lang/sdk/blob/94d797ebee87e7bd02521c465e52056fdc9b4d01/pkg/kernel/lib/target/targets.dart#L503)?

Dart2js currently can't use the CFE lowering to encode getter calls as an explicit .call because of JS interop. For example, in code like this:

class A {
  Function get f;
}

main() => A().f();

The CFE lowering changes main to be equivalent to (A().f)(). This is enabled for the VM, but we have this disabled in dart2js. For historical reasons, the code above is implemented as a direct call, and changing it to load the function and invoke it separately unfortunately changes semantics: it drops the receiver if the property was referring to a JavaScript function.

[natebosch]

Does this issue relate to supportsExplicitGetterCalls

No. This issue only relates to how non-Function types are implicitly coerced to a Function type by tearing off .call. There is no tearoff or non-Function type involved in the getter call scenario.

[natebosch]

I have an implementation of a fix for this usage which unconditionally wraps the expression in parenthesis to make it safe to tear off .call. https://dart-review.googlesource.com/c/sdk/+/256163

@bwilkerson - I am trying to figure out if there might be a way to avoid introducing these parens when they are not necessary. Do you have any suggestions for reliable ways to detect this? Would it make sense to try to enumerate the types of expressions that do or don't need wrapping?

[eernstg]

https://dart-lang.github.io/linter/lints/unnecessary_parenthesis.html.

[mit-mit]

@natebosch any updates about this?

[natebosch]

The lint is working. I need to take a look at the unnecessary_parenthesis implementation to see if it is simple enough to reproduce in the fix.

[btrautmann]

I'm having a bit of trouble understanding the intent of this ticket, could somebody help me understand if the following would still be possible:

class MyClass {
  void call({required String name}) {
    ...
  }
}

void main() {
  final myClass = MyClass();
  myClass(name: 'Brandon');
}

I'm hoping so, since this is one of my favorite language features coming from Kotlin.

Edit: Additionally, if that would still be possible, I'm wondering what this proposal's intent is and how it would impact the usage of a class as written above.

[lrhn]

Calling the callable object's call method implicitly will still be possible. What would not be possible is:

  void Function({required String name}) f = MyClass();

You would have to do (the equivalent):

  void Function({required String name}) f = MyClass().call;

You won't get an implicit tear-off of the call method in a function-typed context.

In Dart 1, the MyClass() object was directly assignable to the function type. Users may still expect that f will contain the MyClass instance. It won't, and that's concerning from a readability stand-point.

It's actually the context-dependency that worries me, more than the tear-off readability. If you care about the readability, you can always write .call yourself. In a context like FutureOr<Function> f = o;, where o has a type which both implements Future<Function> and has a call method, we should still do "something reasonable". It's not always that the user actually agree with the compiler what the reasonable thing is. Currently we do something ... I'm not sure I even remember exactly what.

Because it sometimes get tricky, especially around union types like FutureOr and X?, removing the responsibility for figuring out the right thing from the compiler, and putting it on the author, seems like a worth-while change. Even though it does make authors write more code, and makes "callable objects" act less like functions.

[btrautmann]

That was extremely helpful @lrhn, thank you for that explanation!

Even though it does make authors write more code, and makes "callable objects" act less like functions.

Totally agree that predictability/reasonableness in this case is more valuable than readability/minimizing code written.

Thanks again!

[eernstg]

Note that the ground is being prepared for this change with a lint: avoid-implicit-call-tearoffs.

[natebosch]

We will want to include the lint in our core lint rules so that the widest audience sees it surfaced.

[mit-mit]

Hi @natebosch any news about the fix for this?

[natebosch]

any news about the fix for this?

Fix is implemented and waiting for review. https://dart-review.git.corp.google.com/c/sdk/+/256163

The crbot failures in that CL are related to the package:linter roll and should be resolved by https://dart-review.git.corp.google.com/c/sdk/+/264961

[spkersten]

We have classes like:

class Function1Mock<T0, T1> extends Mock {
  T0 call([T1? arg1]) => ...
}

that are used in tests like:

late Function2Mock<void, String> onNameChanged;

setUp(() {
  onNameChanged = Function2Mock();
});

// code being tested
// void sut(Function<void, String> onNameChanged) => ...

test("...", () {
  sut(onNameChanged);
  ...
  verify(onNameChanged("expected name")).called(1);
});

Would sut(onNameChanged); need to be changed to sut(onNameChanged.call);?

[lrhn]

@spkersten Yes. That's exactly the situation where the change applies, an implicit tear-off of the call method due to a function-typed context type.

You could also just change the function name to, say, function, and do:

test("...", () {
  sut(onNameChanged.function);
  ...
  verify(onNameChanged.function("expected name")).called(1);
});

That's being very explicit that you have a mock that you only use the one function of.

[srawlins]

It's so great that a lot was done in 2.19 for this change, with a new lint rule and a fix. Is this change planned for Dart 3.0?

[natebosch]

Is this change planned for Dart 3.0?

No, we are going to put the lint in our core set now, and then put this change behind a language version.

There are still some open questions about the UX after this change. Once the language doesn't have implicit tearoffs, we will either need to

• Re-implement the lint to find places where a tear-off would have been possible, instead of where they are happening. @bwilkerson - do you have any impression on how hard this might be? The current implementation is very simple because the AST has a node for the implicit tear-off. If the change is language versioned - would we keep that node in the AST?
• Attach a fix to the type error that will surface. @bwilkerson - is it feasible to implement a quick fix that only works for a small subset of a given diagnostic's occurrences?
• Don't offer specific help to the user. @mit-mit - how critical is it that we have quick fixes to add the .call tearoff which work in the language version without implicit tearoffs?

[itsjustkevin]

I am going to remove this breaking change from the Dart 3 project as it is not planned for Dart 3.

[escamoteur]

I know I'm late for this discussion but for my package flutter_command this change would make all code look much uglier as the beauty of a callable class is that it can be assigned to event handlers which is a central use case for my command package.

[rrousselGit]

I have yet to find a case where the associated lint was helpful to me. On the flip side, having to add an extra .call everywhere I voluntarily used a tear-off was a degradation of the syntax IMO

I don't care that Function foo = Callbable() doesn't enable foo is Callbable. I did try using this feature before, but I wasn't too surprised by it not working either.

But I do use callable classes quite often for spying on event-handlers:

class OnChangeListener<T> extends Mock {
  void cal(T value);
}

test('example', () {
  final listener = OnChangeListener<int>();

  someObservableObject.listen(listener); 

  verify(listener(0));
});

[rrousselGit]

Food for thought: Couldn't we instead remove the implicit tear-off and rather have callable classes implement Function?

So we could still do Function foo = Callable();. But we could also do (foo as Callable).field

I assume that would remove the original concern?

[escamoteur]

It just looks so ugly:

[lrhn]

@rrousselGit As it is, every object which implements Function is a function value, which are a separate kind of objects from class instances. Which is not a big optimization, because the only thing you can do with Function-typed values is to do dynamic invocations on them. You can get the same effect by casting to dynamic before doing the dynamic invocation.

But that also means that Function is not very useful. (You can just use dynamic.) Most likely, just implementing Function is not enough, and people will want the object to implement the actual function type of the call method. Which is just not compatible with Dart's current type system, when generics are covariant, but call method arguments would be contravariant. There's a reason we stopped doing that when moving to the sounder Dart 2 type system.

So, making callable classes implement Function is likely too little to be useful, yet too invasive for the type system to be practical.

For @escamoteur, if you call your function run instead of call, then:

      onRetry: feedSource.updateDataCommand.run

doesn't look as bad. It looks weird because call isn't a good name for that function, it's only there to not be visible when calling.

[escamoteur]

It might look better but I don't know why this change is necessary at all. I never heard about anyone having problems with the way it is now and the idea that a command object can be used like a function just make sense as you can call it directly too. I really hope you don't plan to abandon callable classes as a whole. My other package get_it lives from them too. Am 9. Jan. 2024, 18:41 +0100 schrieb Lasse R.H. Nielsen @.***>:

@rrousselGit As it is, every object which implements Function is a function value, which are a separate kind of objects from class instances. Which is not a big optimization, because the only thing you can do with Function-typed values is to do dynamic invocations on them. You can get the same effect by casting to dynamic before doing the dynamic invocation. But that also means that Function is not very useful. (You can just use dynamic.) Most likely, just implementing Function is not enough, and people will want the object to implement the actual function type of the call method. Which is just not compatible with Dart's current type system, when generics are covariant, but call method arguments would be contravariant. There's a reason we stopped doing that when moving to the sounder Dart 2 type system. So, making callable classes implement Function is likely too little to be useful, yet too invasive for the type system to be practical. For @escamoteur, if you call your function run instead of call, then: onRetry: feedSource.updateDataCommand.run doesn't look as bad. It looks weird because call isn't a good name for that function, it's only there to not be visible when calling. — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.Message ID: @.***>