Pipe operator removes a function's import context if the import is outside a quote block

[polvalente]

Environment

• Elixir & Erlang/OTP versions (elixir --version): Elixir 1.13.0, Erlang 24.1.5
• Operating system: Ubuntu 20.04.3 LTS

Current behavior

The following code causes a compilation error related to operation/3 being undefined.

defmodule MyMath do
  def operation(fun, x, y), do: apply(fun, [x, y])
end

defmodule MyMacro do
  import MyMath

  defmacro __using__(_) do
    quote do
      def add(x, y) do
        add = &+/2

        add |> operation(x, y)
      end
    end
  end
end

defmodule T do
  use MyMacro
end

The code block below, however, has the import working as expected:

defmodule MyMath do
  def operation(fun, x, y), do: apply(fun, [x, y])
end

defmodule MyMacro do
  import MyMath

  defmacro __using__(_) do
    quote do
      def add(x, y) do
        add = &+/2

        operation(add, x, y)
      end
    end
  end
end

defmodule T do
  use MyMacro
end

Expected behavior

I would expect the first snippet to be equivalent to the second one. Upon inspection of the AST, it seems that the pipe operator is removing the import context from the {:operation, _, _} node, which makes it try to resolve to Ts namespace. This also seems to be why it works if the macro also forces the import upon the T module.

[sabiwara]

Hi, I tried to look into this issue, here is a minimal example:

iex> import Integer
Integer

iex> ast1 = quote do: is_odd(1)
{:is_odd, [context: Elixir, import: Integer], [1]}
iex> ast2 = quote do: 1 |> is_odd()
{:|>, [context: Elixir, import: Kernel], [1, {:is_odd, [], []}]}

iex> Code.eval_quoted(ast1)
{true, [{{:arg1, Elixir}, 1}]}
iex> Code.eval_quoted(ast2)
** (CompileError) nofile:1: undefined function is_odd/1 (there is no such import)
    (elixir 1.13.4) expanding macro: Kernel.|>/2
    nofile:1: (file)

This seems rather tricky to solve, since the pipe operator is just a macro and from the AST point of view this is just equivalent to Kernel.|>(1, is_odd()).
If my understanding is correct, adding a specific import resolution to the first parameter of the pipe operator would imply to make |> more than a simple macro, and the compiler would need to be aware of it.
Leaking the env of MyMacro in the module using it (T) is probably not a good approach either.

Actually, looking at the example, it would seem better to use add |> MyMath.operation(x, y) or have the import MyMath within the __using__ macro in the first place? But I agree the behavior should be consistent disregarding the fact we are using or not the pipe operator.

[Qqwy]

More than ast2 failing, it surprises me that @sabiwara's ast1 example works. I would have expected imports outside of a quote to not influence the hygiene inside the quote.

I think that this problem is not specific to |>.

Per the documentation quote -- 'Hygiene in Imports' Elixir delays function resolution to the last possible moment.
Maybe what we see here is an edge case where Elixir tries to bind too early?

[Ljzn]

This seems rather tricky to solve, since the pipe operator is just a macro and from the AST point of view this is just equivalent to Kernel.|>(1, is_odd()).
If my understanding is correct, adding a specific import resolution to the first parameter of the pipe operator would imply to make |> more than a simple macro, and the compiler would need to be aware of it.

Correct. The pipe operator should be treated specially inside quote.

[josevalim]

Actually, we should not treat any operator as special, because then it means people cannot implement pipe-like functionality using other operators.

I started working on this issue in this commit: 169595f

The idea is that we will keep imports as a list and we will annotate all arities and modules based on the function name. The open questions I have to address is:

1. Today we emit compilation tracer events and the imported event has a function and arity. And we don't know the arity at this point yet.

2. How to handle conflicts. Now module A and B can conflict on arity 2 but arity 3 is the one used. We can either raise early (with false negatives but better error message) or late (no false negatives, worse ux).