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

Is it possible to rewrite modules using alias Foo, as: Bar? #24

Open
kieraneglin opened this issue Jun 12, 2024 · 1 comment
Open

Is it possible to rewrite modules using alias Foo, as: Bar? #24

kieraneglin opened this issue Jun 12, 2024 · 1 comment

Comments

@kieraneglin
Copy link
Contributor

Hey there! I found this library via this blog post and wanted to include it in my project.

One thing I've found is that it doesn't seem to support rewiring modules where the modules must us as:. Here's an example based on a real-world issue that I'm running up against during a refactor:

defmodule Foo do
  # See they have the same final name part so I _must_ use `as:`
  alias Foo.YtDlp.CommandRunner, as: YtDlpRunner
  alias Foo.Apprise.CommandRunner, as: AppriseRunner

  def yt_dlp_bar, do: YtDlpRunner.bar()
  def apprise_baz, do: AppriseRunner.baz()
  # ...
end

These are both modules that interface with CLI programs. Currently I'm using the standard Mox-recommended method of DI, but I dislike that it puts testing concerns in my main app code. I was hoping I could do something like this using rewire in my tests:

rewire Foo, YtDlpRunner: YtDlpRunnerMock, AppriseRunner: AppriseRunnerMock

# ...

But that doesn't seem to be possible. I could be missing something, but in my testing it seems like the AST is functionally rewritten to this:

defmodule Foo do
  alias Foo.YtDlp.CommandRunner, as: YtDlpRunnerMock
  alias Foo.Apprise.CommandRunner, as: AppriseRunnerMock

  def yt_dlp_bar, do: YtDlpRunnerMock.bar()
  def apprise_baz, do: AppriseRunnerMock.baz()
  # ...
end

Which only overwrites the as: alias, but it's still ultimately pointing to the original modules.


Is there a way to handle this and I just missed it? If not, would you be open to a PR that handles this case? This would be a breaking change but I think it's a valuable one

@stephanos
Copy link
Owner

stephanos commented Jul 1, 2024

I must preface this by saying I haven't looked at this library in years. From your description and looking at the test cases again, I fully agree that it would be better to rewrite the alias import, not just the as: part. Actually, it seems to me like an oversight/a bug rather than a design choice. So I'd welcome a PR here!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants