Implicitly add missing parameters to anonymous functions

[rrousselGit]

TL;DR the idea is to allow creating an anonymous function with fewer arguments than the function type requires.

Consider:

void Function(A a, B b, {required Named named, required Another another}) Callback;

The idea is to allow:

Callback cb = ()  {};
Callback cb= (A a) {};
Callback cb= (A a, B b) {};
Callback cb= ({named}) {};
Callback cb= (a, {another}) {};
...

On the other hand, this would not allow:

Callback cb= (B b) {}; // Error, the first postional argument should be of type "A"

Note that this is specific to the creation of anonymous functions, when we are assigning them to a known type.

As such, the following is not possible:

var fn = () {};
Callback cb = fn; // Error, cannot assign () => void to Callback

The way this would work is, when defining a closure, the compiler would add the missing parameters.

So:

Callback cb = ()  {};

would in fact be equivalent to writing:

Callback cb = (_, __, {named, another})  {};

---

The benefits are:

• reduced boilerplate for developers when defining anonymous functions where they don't care about some parameters

• significant increase in the usefulness of named parameters for anonymous functions. For example instead of:

  Widget(
    builder: (_, __, value) {
      return Text(value);
    }
  )

  we could change the API to use named parameters and have:

  Widget(
    builder: ({value}) {
      return Text(value);
    }
  )

• improved migration when libraries add parameters to a function For example Riverpod has a PR changing:

  AsyncValue<int> value;
  value.when(
    loading: () {},
    error: (Object err, StackTrace stack) {},
    data: (int data) {}
  );

  to:

  AsyncValue<int> value;
  value.when(
    loading: (AsyncValue<int>? previous) {},
    error: (Object err, StackTrace stack, AsyncValue<int>? previous) {},
    data: (int data) {}
  );

  This will break a lot of code, 99% of which are using anonymous functions since that is how it is meant to be used. With this feature, the change would not break existing code

Similarly, combined with a linter, this could allow package authors to add parameters to a function without it being a breaking change.

This would be possible by marking a parameter with an annotation that says "no function tear-off allowed" like:

class Example {
  @noTearOff
  void Function(A a, B b) callback;

such that we have:

Example(
  callback: () {} // OK
)

void Function(A a, B b) fn = ...

Example(
  callback: fn, // "callback" does not accept function tear-off
)

[Levi-Lesches]

How do you resolve where the unused parameter goes if two positional arguments are of the same type? For example, say we have the following:

void main() {
  test((String msg) => print(msg));
}

void test(void printer(String msg)) => printer("Test");

Then the author of test changes it to:

void test(void printer(String msg, String errorMsg)) => printer("Success", "No errors");

What gets printed? In other words, which of the following "translations" is correct?

(String msg, _) => print(msg);  // "Success"
(_, String errorMsg) => print(errorMsg);  // "No errors"

Whatever the answer is, it's not necessarily obvious.

[TimWhiting]

Using named arguments on callbacks would be really nice with this. The caller can pass more arguments than the receiver wants to use. However I would suggest that there be a lint / compile time error for parameters annotated with something like @noIgnore, so the API author can enforce using a parameter.

[rrousselGit]

How do you resolve where the unused parameter goes if two positional arguments are of the same type? For example, say we have the following:

Based on the order. The matching isn't done on type, but index for positional parameters.

Positional parameters would not allow skipping a parameter. Only named parameters would, where the matching is done based on the parameter name.

So with:

Function(String a, String b) Callback;

then:

Callback cb = (something) {}

will always resolve to:

Callback cb = (something, _) {}

[rrousselGit]

Using named arguments on callbacks would be really nice with this. The caller can pass more arguments than the receiver wants to use. However I would suggest that there be a lint / compile time error for parameters annotated with something like @noIgnore, so the API author can enforce using a parameter.

What case do you have in mind? I don't see a scenario where it would be necessary for a function define a parameter.

[Levi-Lesches]

Based on the order.

That's still prone to confusing behavior:

void main() async {
  await getNetworkMessage((msg) => print(msg));
}

Future<void> getNetworkMessage(void printer(String response)) async {
  final Response response = await http.get(uri);
  if (response.statusCode == 200) printer(response.body);
}

Now, getNetworkMessage is changed:

Future<void> getNetworkMessage(void callback(int code, String response)) async {
  final Response response = await http.get(uri);
  callback(statusCode, response.body);  // allow the caller to handle HTTP codes themselves
}

Instead of actually handling a 200 or 404, the call in main simply prints the status code, without even printing the message like the user intended.

Functions are integrated into the type system, which makes them safe to use, and lets the analyzer figure out when to warn you. Implicitly altering the type of a function that is already declared in user-code changes it entirely (different than subtyping, such as accepting a num when an int is provided). That is unsafe, and would IMO lead to confusing or at least unintuitive behavior.

[rrousselGit]

The example you gave would be considered as a bad practice/anti-pattern. When editing the prototype of a callback, new positional parameters should always be the last parameter.

So the code you gave would not pass code-reviews.

[Levi-Lesches]

Breaking changes do happen. But again, there's still plenty of ambiguity:

// network.dart
Future<void> getNetworkMessage(void callback(int code, String response)) async {
  final Response response = await http.get(uri);
  callback(statusCode, response.body);  // allow the caller to handle HTTP codes themselves
}

// main.dart
void main() async {
  // this will only print the status code, not the message
  await getNetworkMessage((msg) => print(msg));
}

Even with no change made to network.dart, now the analyzer can't warn the user of their error. That kinda defeats the point of type safety and analysis, since what was once a compile-time error is now a hidden bug (not even a runtime error). It might be the user's fault, but it's still unsafe. It's very different than being able to omit types for brevity because type inference is safe.

[rrousselGit]

Types aren't removed. Your example is quite rare, since "print" accepts just about anything. For real code, you'd have a compilation error.

And you wouldn't have to use positional parameters. The ambiguity is an issue with positional parameters in general, and is already an existing issue.

If you are worried about it you can make your parameters named, which leads to:

({message}) => print(message)

Removing all ambiguity in the process.

With this feature, I would expect the community to slowly move toward using named parameters instead of positional parameters.

[Levi-Lesches]

Types aren't removed. Your example is quite rare, since "print" accepts just about anything. For real code, you'd have a compilation error.

Again, I refer to my original example of the two or more parameters with the same type:

void main() {
 test((String msg) => print(msg));
}
void test(void printer(String msg, String errorMsg)) => printer("Success", "No errors");

And you wouldn't have to use positional parameters.

If you are worried about it you can make your parameters named, which leads to:

({message}) => print(message)

Removing all ambiguity in the process.

With this feature, I would expect the community to slowly move toward using named parameters instead of positional parameters.

Problem is, this syntax is for the user of an API, not the author of the API. The user doesn't get to choose whether the API uses callbacks with named parameters or positional parameters, so your solution would be benefitting the wrong party.

[rrousselGit]

I refer to my original example of the two or more parameters with the same type:

This would he no different without this feature.

That's the reason named parameters exists to begin with.

Problem is, this syntax is for the user of an API, not the author of the API.

Both parties work for the sake goal. If users find it preferable to use named parameters, package authors will use named parameters.

Widgets use almost exclusively named parameters. Nothing prevents people from doing the same with callbacks. Keeping positional parameters for simple cases, like ".map"

I'd expect anything that has a list of parameters which can potentially grow over time to use named parameters.

That'll make both package authors and library users happy. The former have a way to reduce breaking changes, and the later get to write less code.

[Levi-Lesches]

This would he no different without this feature.

It would be a compile-time error today. With this feature, it would simply do the wrong thing at runtime.

So let's say this only works for named parameters, for now. Then this feature essentially becomes "allow the caller to pass named parameters that I'll ignore". From your example:

Widget(
   builder: ({value}) {
     // context and child are passed, but ignored
     return Text(value);
   }
)

Then this would be a duplicate of another issue (I searched for a long time and couldn't find it, but it's out there). And I believe this is anyway a duplicate of #1673

[rrousselGit]

1673 is a problem description

This issue is a solution proposal.

This would he no different without this feature.

It would be a compile-time error today. With this feature, it would simply do the wrong thing at runtime.

To take your example, nothing prevents you today from writing:

(msg, code) => print(msg)

when the function is defined as:

(code, msg) => void

So the problem is existing today.

The only thing this issue does is removing the unused parameter.

[ykmnkmi]

Can this acceptable to void argument for futures and streams callbacks?

void callback() {
 ...;
}

Stream<void>().listen(() { ...; });
Stream<void>().listen(callback);
Future<void>().then(() { ...; });
Future<void>().then(callback);

[Levi-Lesches]

The only thing this issue does is removing the unused parameter.

...and implicitly add them. It's turning (msg) => print(msg) into (msg, _) => print(msg). That's the issue

[eernstg]

This is an interesting idea. However, wouldn't it fit just as well as a tool feature (that is, a quick fix in IDEs)? Cf. https://github.com/dart-lang/sdk/issues/46972.

As a quick fix, this implies that the resulting code would be more verbose, because we'd actually add the missing parameters in the source code, we just wouldn't have to type them manually, and we wouldn't have to remember the parameter list. (As far as I can see, IntelliJ with Dart doesn't offer much help as I'm writing a function literal which is passed as an argument to a formal parameter whose function type requires more parameters than the ones I already wrote).

On the other side, we would avoid some added complexity for readers of the code. If we make this a language feature then the function literal would remain abbreviated (and it is probably fair to say that the missing parameters are added as syntactic sugar during compilation), and this might make the code harder to read. (OK, it could also be easier to read, but then, on average: a bit harder to understand. ;-) Another benefit that we'd have if this is turned into a quick fix is that it would work equally well for parameters that we don't care about and parameters that we do care about: We just quick-fix the parameters into existence, and then we can ignore any or all of them.

[rrousselGit]

I don't think that quick fixes are a solution.

While it reduces the burden of upgrading a package, it doesn't fix the versioning issue.
Folks wouldn't be able to have a dependency as ^1.0.0 where version 1.1.0 adds a parameter to a callback, because existing code would stop compiling.
Whereas this proposal would allow that – assuming that we can warn against tear-offs and we can prevent users from calling the function themselves (the former doable using a meta annotation, and the later is possible with private variables)

Similarly, the visual noise is still a problem. Some functions may want to pass 5+ parameters (for legitimate reasons that it). In which case the unused parameters starts to really be problematic.Ï

The closest solution we have today would be to have a function that take a single class instance as parameter like:

class Props {
  final A a;
  final B b;
}

typedef Callback = void Function(Props props);

It solves both readability and versioning concerns.
But having to define a class to represent the function parameters feels like a step backward, since the language did a great job as avoiding having to rely on those java-ish "Delegate" classes so far.

Also, without object destructuring, the syntax wouldn't be perfect, as we'd end-up with:

(props) => print(props.a)

This "props" variable would typically have a name that has no real meaning, since it's just a container.
And we'd lose valuable compiler inference like:

(int? a) {
  if (a != null) {
    // "a" is smartcasted to "int" within the if block
  }
}

In a way this feature is akin to implementing both destructuring and anonymous records, specific to closures.

[eernstg]

True, the language feature would allow for a more smooth migration.

However, the current approach would be to consider the addition of extra parameters in the type of a formal parameter as a breaking change. Your proposal is still going to break all the call sites where the function object isn't directly obtained from a function literal, so it's going to be a breaking change in any case, and we might then be able to apply the quick fix globally.

So the "less noise" argument is the strongest argument in favor of a language mechanism at this point, as far as I can see.

[rrousselGit]

However, the current approach would be to consider the addition of extra parameters in the type of a formal parameter as a breaking change. Your proposal is still going to break all the call sites where the function object isn't directly obtained from a function literal, so it's going to be a breaking change in any case, and we might then be able to apply the quick fix globally.

I don't quite agree with this conclusion.

Assuming with have the necessary lints built in the official toolings, I see this as no different than other variants like @nonVirtual/@sealed/@protected/...

While these annotations technically don't prevents users from ignoring the warning, if somehow a package defined:

@sealed
class Example {
  Example(int a);
}

then I wouldn't consider refactoring to the following as breaking change:

@sealed
class Example {
  factory Example(int a) {...}
}

If we are pedantic, technically this change would be breaking since the @sealed doesn't prevent users to extend Example. But the warning says that extending Example is illegal. As such, if users chose to ignore it and then their application break, it would be their fault not the package author's fault.

[rrousselGit]

A concrete example would be an API exposing:

class Example {
  Example({
    @noTearOff void Function()? onChange
  }): _onChange = onChange;

  final void Function()? _onChange;

  void doSomething() {
    _onChange();
  }
}

With this code, an instance of the class Example would not be able to call the onChange parameter. The function call is private to the library.

As such IMO refactoring it to:

class Example {
  Example({
    @noTearOff void Function(int value)? onChange
  }): _onChange = onChange;

  void doSomething() {
    _onChange(42);
  }
}

would not be a breaking change.

Because the only thing which could break that a user can write would be:

void Function() fn;
Example(onChange: onChange)

but this case would be covered by the @noTearOff warning, marking this as illegal.

[lrhn]

I think a @noTearoff annotation is an admission of failure. It means that map.forEach(print) won't work to print the keys, you have to do map.forEach((key) => print(key)). I can't explain that to users, not after all these years of saying that you shouldn't eta-expand your function values.

If we allow function closures to be rewritten to take extra arguments that are not in the source code, it seems like a smaller change to allow tear-offs to be torn off with more parameters than the method needs. We are already creating a new function object when doing a tear-off, so creating a slightly different function object in order to match the context type isn't event new, we already do that with instantiated tear-offs of generic functions.

The one place where that doesn't apply is where you already have a function value of a different type. Wrapping that in something which takes extra arguments is changing the existing value. (But if we can do tear-offs, you can always tear off the call method, so it shouldn't be a big issue to do it elsewhere).

The big problem is not limits on which functions we change, but the fact that we change functions to begin with. Passing a function with two few parameters can be an error. Silently expanding the parameter list to something matching the call-site can hide an error. I'd at least prefer to have some kind of syntax for it, even if it's just a !! prefix. (The "make it so" operator which does its darndest to make the value match the context type).

[alexeyinkin]

I agree that tear-off is a useful case for this feature.

class A {
  int a;
  A({required this.a});
}

class AB {
  int a;
  int b;
  AB({required this.a, required this.b});
}

void addFactory(Type type, Object Function({required int a, required int b}) fn) {
  // ..
}

void main() {
  addFactory(AB, AB.new); // OK
  addFactory(A, A.new);   // Error, would be OK with this feature.
  // ..
}

Also, if silencing missing positional arguments can introduce errors in user code, this can be first implemented for named arguments only. This is because they are more often thought of as being optional.

[rubenferreira97]

I am revisiting this issue because it also confused me, coming from a TypeScript background where similar functionality is allowed:

function performAction(callback: (param1: number, param2: string) => void): void {
  console.log('Performing action...');
  callback(1, 'param2');
}

performAction(() => {
  console.log('Performing action');
});

performAction((param1 /*:number*/) => { // param1 infers number
  console.log(param1); 
});

performAction((param1 /*:number*/, param2 /*:string*/) => { // param1 infers number and param2 infers string
  console.log(param1, param2); 
});

TypeScript developers rely on this feature to minimize the "noise" of unused parameters.

Based on the order.

That can still lead to confusing behavior:

void main() async {
  await getNetworkMessage((msg) => print(msg));
}

Future<void> getNetworkMessage(void printer(String response)) async {
  final Response response = await http.get(uri);
  if (response.statusCode == 200) printer(response.body);
}

Now, getNetworkMessage is changed:

Future<void> getNetworkMessage(void callback(int code, String response)) async {
  final Response response = await http.get(uri);
  callback(statusCode, response.body);  // allow the caller to handle HTTP codes themselves
}

@Levi-Lesches, in my opinion, this example is not entirely valid because it is analogous to this:

void main() async {
  getNetworkMessageOld((msg) => print(msg));
  getNetworkMessage((msg) => print(msg));
}

void getNetworkMessageOld(void printer(String response)) {
  printer('Response');
}

void getNetworkMessage(void printer(int response)) {
  printer(1);
}

This is about changing the function signature. You also broke it here.

You can alleviate the problem by explicitly setting the types:

void main() async {
  getNetworkMessage((String msg) => print(msg));
}

void getNetworkMessage(void printer(int response)) {
  printer(1);
}

With this proposal, we could say this is an issue:

void main() async {
  getNetworkMessage((String msg) => print(msg));
}

void getNetworkMessageOld(void printer(String msg)) {
  printer(msg);
}

void getNetworkMessage(void printer(String response, String msg)) {
  printer(response, msg);
}

But I still think this is the library developer's fault for introducing a hidden breaking change. The analogous example would be changing void printer(int statusCode) to void printer(int counter) and the caller expecting getNetworkMessage((int statusCode) => print(statusCode)).