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

Multi alias use is not detected #54

Closed
aweiker opened this issue Mar 17, 2016 · 5 comments
Closed

Multi alias use is not detected #54

aweiker opened this issue Mar 17, 2016 · 5 comments

Comments

@aweiker
Copy link
Contributor

aweiker commented Mar 17, 2016

Elixir 1.2 added support for multi use aliases. Credo should pick this up and not warn on missing aliases.

Here is an example that should be allowed and not have any warnings.

defmodule Sample.App do
  @moduledoc false

  alias Sample.App.{One, Two}

  def foo do
    {One.one, Two.two}
  end
end

defmodule Sample.App.One do
  @moduledoc false
  def one, do: "One"
end

defmodule Sample.App.Two do
  @moduledoc false
  def two, do: "Two"
end

However, this will give the following messages


  Software Design
┃
┃ [D] ↓ Nested modules could be aliased at the top of the invoking module.
┃       lib/sample.ex:7:22 (Sample.foo)
┃ [D] ↓ Nested modules could be aliased at the top of the invoking module.
┃       lib/sample.ex:7:6 (Sample.foo)

Please report incorrect results: https://github.com/rrrene/credo/issues

Analysis took 0.07 seconds (0.00s to load, 0.07s running checks)
6 mods/funs, found 2 software design suggestions.
@rrrene
Copy link
Owner

rrrene commented Mar 18, 2016

Hi Aaron,

maybe you mistyped your example, but that is not how you use aliasing. An alias is used to reference a given module by another (shorter) name, which is by default the last part of the module atom. Your Sample module should therefore read:

defmodule Sample do
  @moduledoc false

  alias Sample.{One, Two}

  def foo do
    {One.one, Two.two}      # <----
  end
end

Was this a typo in your example or did I misinterpret the report? In any case, thanks for reporting! 👍

@aweiker
Copy link
Contributor Author

aweiker commented Mar 18, 2016

Yes, you are correct. I messed up the sample.

I corrected the sample that way it's obvious.

Also, what I just discovered while updating it is that this works Sample.One and Sample.Two but when it is Sample.App.One and Sample.App.Two it does not.

@aweiker
Copy link
Contributor Author

aweiker commented Mar 18, 2016

Doing some more digging into it and it appears that this may be the preferred syntax and it currently does get through the current checks.

defmodule Sample.App do
  @moduledoc false

  alias Sample.{App.One, App.Two}  # <----

  def foo do
    {One.one, Two.two}
  end
end

@rrrene rrrene closed this as completed in d9329ea Mar 18, 2016
@rrrene
Copy link
Owner

rrrene commented Mar 18, 2016

v0.3.8 contains a fix for this. Would you mind test-driving it? 😁

@aweiker
Copy link
Contributor Author

aweiker commented Mar 18, 2016

Looks good.

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