Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Oban.Worker 'use' macro can't compile with variables #65

Closed
ibarakaiev opened this issue Aug 10, 2023 · 10 comments
Closed

Oban.Worker 'use' macro can't compile with variables #65

ibarakaiev opened this issue Aug 10, 2023 · 10 comments
Labels
bug Something isn't working help wanted Extra attention is needed

Comments

@ibarakaiev
Copy link

ibarakaiev commented Aug 10, 2023

First of all, thanks for the amazing work on Styler!

Versions

  • Elixir: 1.15.2
  • Styler: 0.8.3

Example Input

As an example from Oban documentation,

defmodule MyApp.SnoozingWorker do
  @max_attempts 20

  use Oban.Worker, max_attempts: @max_attempts

  @impl Worker
  def backoff(%Job{} = job) do
    corrected_attempt = @max_attempts - (job.max_attempts - job.attempt)

    Worker.backoff(%{job | attempt: corrected_attempt})
  end

  @impl Worker
  def perform(job) do
    if MyApp.something?(job), do: :ok, else: {:snooze, 60}
  end
end

Stacktrace / Current Behaviour

Currently, the output of Styler is:

defmodule MyApp.SnoozingWorker do
  @moduledoc false
  use Oban.Worker, max_attempts: @max_attempts

  @max_attempts 20

  @impl Worker
  def backoff(%Job{} = job) do
    corrected_attempt = @max_attempts - (job.max_attempts - job.attempt)

    Worker.backoff(%{job | attempt: corrected_attempt})
  end

  @impl Worker
  def perform(job) do
    if MyApp.something?(job), do: :ok, else: {:snooze, 60}
  end
end

However, this breaks compilation, as @max_attempts is inaccessible when useing Oban.Worker (it's defined later). The @moduledoc example in the troubleshooting guide is similar to this, but the provided solution didn't work for me — and, frankly, this seems like it should be handled differently. In the case of @moduledoc, moving module attributes above it would not be acceptable, but with use macros it seems it would be a common use case.

@novaugust
Copy link
Contributor

novaugust commented Aug 10, 2023

Hi ibarakaiev, could you show me what went wrong when you tried the solution from the readme so I can update it to be more helpful for folks? Thank you, and thanks for taking styler for a spin :)

@ibarakaiev
Copy link
Author

ibarakaiev commented Aug 10, 2023

@novaugust appreciate your reply! Creating a bare Mix project and adding the following code compiles and works fine:

max_attempts = 20

defmodule MyApp.SnoozingWorker do
  @moduledoc "#{max_attempts}"

  use Oban.Worker, queue: :default, max_attempts: 20

  @max_attempts max_attempts

  def run do
    IO.puts(@max_attempts)
  end
end

However, using max_attempts as a keyword value when useing a macro fails:

max_attempts = 20

defmodule MyApp.SnoozingWorker do
  @moduledoc "#{max_attempts}"

  use Oban.Worker, queue: :default, max_attempts: max_attempts

  @max_attempts max_attempts

  def run do
    IO.puts(@max_attempts)
  end
end
error: undefined variable "max_attempts"
  deps/oban/lib/oban/worker.ex:6: MyApp.SnoozingWorker.__opts__/0


== Compilation error in file lib/worker.ex ==
** (CompileError) lib/worker.ex: cannot compile module MyApp.SnoozingWorker (errors have been logged)

@novaugust
Copy link
Contributor

hey @ibarakaiev, had a long weekend, sorry for the delay :)

thanks a lot for the repro. something interesting is happening here - i can repro the problem when using exactly your code (oban's use doesn't work), but using a different custom use + unquote + after_compile allows things to proceed no problem! 🤔

here's the code in oban:

https://github.com/sorentwo/oban/blob/43a653f63abda6a287841ea5e62bf44bf7e24d59/lib/oban/worker.ex#L301C22-L311

this more-or-less identical code compiles

defmodule TestUse do
  def __using__(opts) do
    quote location: :keep do
      @after_compile TestUse
      def __opts__ do
        Keyword.put(unquote(opts), :worker, to_string(__MODULE__))
      end
    end
  end

  defmacro __after_compile__(%{module: module}, _env) do
    Enum.each(module.__opts__(), &IO.inspect/1)
  end
end
a_variable = :woo

defmodule MyTestUse do
  @moduledoc "#{a_variable}"
  use TestUse, a_variable: a_variable
end

all of this compiles just fine. so, i'm wondering if something weird happens across dependency boundaries ¯\_(ツ)_/¯


tldr: huh. i'm going to sit on this for now until i understand what's special about oban's use vs use in other scenarios. at the least i'll update the title to help others find the bug. sorry for not having a clear solution for you just yet.

@novaugust novaugust changed the title Broken compilation after reordering the 'use' macro above the variables that it depends on Oban.Worker 'use' macro can't compile with variables Aug 14, 2023
@janpieper
Copy link

janpieper commented Dec 5, 2023

Any news on this? I just stumbled upon exactly that issue in my code 😕

Styler: 0.10.3
Elixir: 1.15.6
Erlang: 26.1.1

@novaugust
Copy link
Contributor

alas, i never got anywhere with it. tried tapping some friends to see if they could figure things out but no one knows what the difference is yet. sorry jan :(

@novaugust novaugust added the bug Something isn't working label Dec 8, 2023
@novaugust novaugust added the help wanted Extra attention is needed label Jan 5, 2024
@03juan
Copy link

03juan commented Jan 26, 2024

this more-or-less identical code compiles

defmodule TestUse do
  def __using__(opts) do

def __using__ will return the AST but it won't get compiled into the module. Calling MyTestUse.__info__(:functions) returns [].

When __using__ is defined as a macro then this does fail to compile:

Interactive Elixir (1.16.0) - press Ctrl+C to exit (type h() ENTER for help)
iex(1)> defmodule TestUse do
...(1)>   defmacro __using__(opts) do
...(1)>     quote location: :keep do
...(1)>       def opts do
...(1)>         unquote(opts)
...(1)>       end
...(1)>     end
...(1)>   end
...(1)> end
{:module, TestUse,
 <<70, 79, 82, 49, 0, 0, 6, 24, 66, 69, 65, 77, 65, 116, 85, 56, 0, 0, 0, 188,
   0, 0, 0, 19, 14, 69, 108, 105, 120, 105, 114, 46, 84, 101, 115, 116, 85, 115,
   101, 8, 95, 95, 105, 110, 102, 111, 95, ...>>, {:__using__, 1}}
iex(2)> a_variable = 1
1
iex(3)> defmodule MyTestUse do
...(3)>   use TestUse, an_opt: a_variable
...(3)> end
error: undefined variable "a_variable"
└─ iex:4: MyTestUse.opts/0

** (CompileError) iex: cannot compile module MyTestUse (errors have been logged)
    (elixir 1.16.0) src/elixir_module.erl:185: anonymous fn/9 in :elixir_module.compile/7

I think it's because the macro doesn't have access to the a_variable at compile-time expansion, as it's outside of the scope of the module definition, but if we first assign the variable to a module attribute @a a_variable; use TestUse, an_opt: @a then the compiler will expand the quoted macro and access the attribute's value in the module's context correctly.

That's as far as I can sus out before I get a meta-headache.

@novaugust
Copy link
Contributor

When __using__ is defined as a macro then this does fail to compile:

woww what a slipup. cheers juan. now i'm just surprised you can get away with doing either!

anyways, given that's the issue there's really no good workaround for it right now.

@03juan
Copy link

03juan commented Jan 27, 2024

Yes I agree. In honouring Styler's intention to strictly enforce Credo's StrictModuleLayout my opinion is that Oban is providing technically functional but stylistically incorrect sample code that ModuleDirectives specifically warns "This can break your code"!

So there no simpler way than to refactor as something like this 🤷

max_attempts = 20

defmodule MyApp.SnoozingWorker do
  @moduledoc "#{max_attempts}"

  use Oban.Worker, queue: :default, max_attempts: 20

  @max_attempts max_attempts

  def run do
    IO.puts(@max_attempts)
  end
end

@03juan
Copy link

03juan commented Jan 27, 2024

Ok, there is another way...

defmodule TestUse do
  defmacro __using__(opts) do
    quote location: :keep do
      @after_compile TestUse
      def opts, do: unquote(opts)
    end
  end

  defmacro __after_compile__(%{module: mod}, _env) do
    IO.inspect(mod.opts(), label: "after_compile #{mod}.opts()")
  end
end

a_var = :woo

defmodule A do
  use TestUse, my_var: unquote(a_var)
end

IO.inspect(A.opts(), label: "runtime A.opts()")

after_compile Elixir.A.opts(): [my_var: :woo]
runtime A.opts(): [my_var: :woo]

😆

@novaugust
Copy link
Contributor

eyyy so if folks just do use ... unquote(my_var) it works out eh? thanks for the sleuthing juan, i'll get the readme updated and close this issue out =)

novaugust added a commit that referenced this issue May 20, 2024
Fixes the longstanding compilation-breaking bug that occurs when module attributes are moved below directives that reference them.

Closes #156, Ref #65 and #43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

4 participants