Deprecate complex module expressions in &mod.fun/arity captures

[sabiwara]

Close #14091

Separated the first commit for easier review: 5f5ef9c
We needed to make the decision after expansion, so we need to distinguish if we come from the arity notation and propagate this info.

Using the example from #14089:

Feedback welcome on the wording, this isn't my strong suit :)

[josevalim]

Add a TODO to convert to error on v2.0 :)

[josevalim]

For the warning, i would say:

expected the module in &module.fun/arity to expand to a variable or an atom, got: ~ts

You can either compute the module name outside of & or convert it to a regular anonymous function.

However, I know realize that this is only ambiguous for zero arity functions, there is no ambiguity here:

&expr().flatten/1

Should we abandon this? Perhaps only keep the first commit as a refactoring?

[sabiwara]

there is no ambiguity here: &expr().flatten/1

@josevalim I'm not sure I follow, what is the difference with arity 0?
Once has to know if it expands to:

fn x -> expr().flatten(x) end
# or
mod = expr()
fn x -> mod.flatten(x) end

So I think we could deprecate it in both cases?
Sorry if I misunderstood what you meant by ambiguity.

[josevalim]

You are right, it applies to all equally. And if we deprecate, we should definitely deprecate all arities.

The biggest question then is: do we expect &expr.fun/arity to always return a properly captured function? However, I don't think that's something we can guarantee right? What if fun is a macro? So what is the property we are expecting to get from this deprecation? Perhaps there is nothing to deprecate?

[sabiwara]

So what is the property we are expecting to get from this deprecation? Perhaps there is nothing to deprecate?

For me the biggest argument is that whatever is the current behavior, we have to maintain it for backward compatibility or might break user code accidentally. Whereas if we officially warn/raise, we tell users not to rely on this and keep the surface of the valid syntax we support smaller.

Regarding fun, I was wondering the same but it seems we're expanding the m.f/a capture only if fun is an atom, so perhaps we don't need to expect surprises here?

[sabiwara]

But I don't feel strongly about it, given it has been emitting warnings since 1.17 I don't expect the syntax to be widespread anyway. We can probably close?

[josevalim]

To expand a bit why we should close:

1. &mod.name/arity syntax does not guarantee we will return a captured MFA. It may return an anonymous functions if name is a macro. so this deprecation does not give us new guarantees
2. Rewriting &expr().name/arity requires doing work upfront OR rewriting to an anonymous function that manually forward arity arguments. But perhaps that’s not a good argument, the same amount of rewriting is required if name is dynamic

Arguments for deprecating:

1. someone may expect &expr().name/arity to execute expr() before the capture and return a module. That’s how I read it but maybe that’s only me?

[sabiwara]

someone may expect &expr().name/arity to execute expr() before the capture and return a module. That’s how I read it but maybe that’s only me?

It's not only you, I read it exactly the same.
I'm tempted to say that if the creator of the language himself can't tell the behavior of this code, it's a strong argument against keeping supporting it and for deprecating 😄

[josevalim]

So let’s deprecate it. The error message should say something like:

expected the module in &module.fun/arity to expand to a module or a variable, got: ~ts

you should either extract the expression to a variable or convert to the fn -> … end syntax

The fn/end syntax bit only applies to zero arity. For multiple arities, it should say to use the capture operator with &1, &2, …

WDYT?

[sabiwara]

@josevalim I applied your previous suggestion 8617fe6, I think it looked good. Are you worried "a regular anonymous function" would be unclear?

Also, are you suggesting we should spell out the actual proposed syntax, or just a hard-coded explanation?

[josevalim]

I think using fn -> is better than saying. I also didn’t consider non-zero arities :)