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

Warn about name conflicts in nested modules #12451

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 28 additions & 1 deletion lib/elixir/lib/kernel.ex
Original file line number Diff line number Diff line change
Expand Up @@ -4977,10 +4977,37 @@ defmodule Kernel do
do: {module, module, nil}

# defmodule Alias nested
defp alias_defmodule({:__aliases__, _, [h | t]}, _module, env) when is_atom(h) do
defp alias_defmodule({:__aliases__, _, [h | t] = parts}, _module, env) when is_atom(h) do
module = :elixir_aliases.concat([env.module, h])
alias = String.to_atom("Elixir." <> Atom.to_string(h))

case :lists.keyfind(alias, 1, env.aliases) do
# If the alias is the same as the module being defined, it means we're redefining
# the same nested module. In that case, let's not warn specifically about "shadowed"
# aliases here.
{^alias, ^module} ->
:ok

{^alias, other_parent} ->
defmod_name = Enum.join(parts, ".")
nested_mod_name = :elixir_aliases.concat([env.module | parts])

IO.warn(
"""
you are defining a module named #{defmod_name} inside module #{inspect(env.module)}, \
which will result in the full module name #{inspect(nested_mod_name)}. However, \
the implicit alias to #{h} will not be set up because there is already an existing \
\"alias #{inspect(other_parent)}, as: #{inspect(alias)}\". You must address this \
ambiguity either by removing the alias or by defining the nested module outside \
of the parent\
""",
env
)

false ->
:ok
end

case t do
[] -> {module, module, alias}
_ -> {String.to_atom(Enum.join([module | t], ".")), module, alias}
Expand Down
59 changes: 39 additions & 20 deletions lib/elixir/test/elixir/kernel/alias_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,13 @@ Code.require_file("../test_helper.exs", __DIR__)

alias Kernel.AliasTest.Nested, as: Nested

defmodule Nested do
def value, do: 1
end
Module.create(
Nested,
quote do
def value, do: 1
end,
file: __ENV__.file
)

defmodule Kernel.AliasTest do
use ExUnit.Case, async: true
Expand Down Expand Up @@ -80,20 +84,24 @@ end
defmodule Kernel.AliasNestingTest do
use ExUnit.Case, async: true

require Kernel.AliasNestingGenerator
Kernel.AliasNestingGenerator.create()
test "aliases nesting with previous alias" do
Code.eval_string("""
defmodule Testing do
import ExUnit.Assertions

test "aliases nesting" do
assert Parent.a() == :a
assert Parent.Child.b() == :a
end
require Kernel.AliasNestingGenerator
Kernel.AliasNestingGenerator.create()

defmodule Nested do
def value, do: 2
end
assert Parent.a() == :a
assert Parent.Child.b() == :a

test "aliases nesting with previous alias" do
assert Nested.value() == 2
defmodule Nested do
def value, do: 2
end

assert Nested.value() == 2
end
""")
end
end

Expand Down Expand Up @@ -129,14 +137,25 @@ defmodule Macro.AliasTest.Aliaser do
end
end

defmodule Macro.AliasTest.User do
defmodule Macro.AliasTest do
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is most likely no longer needs to be a separate module, we can fold into the previous one?

use ExUnit.Case, async: true

use Macro.AliasTest.Definer
use Macro.AliasTest.Aliaser

test "has a struct defined from after compile" do
assert is_map(struct(Macro.AliasTest.User.First, []))
assert is_map(struct(Macro.AliasTest.User.Second, []).baz)
ExUnit.CaptureIO.capture_io(:stderr, fn ->
Code.eval_string("""
defmodule Macro.AliasTest.User do
import ExUnit.Assertions
use Macro.AliasTest.Definer
use Macro.AliasTest.Aliaser

def run_test do
assert is_map(struct(Macro.AliasTest.User.First, []))
assert is_map(struct(Macro.AliasTest.User.Second, []).baz)
end
end
""")

apply(Macro.AliasTest.User, :run_test, [])
end)
end
end
51 changes: 51 additions & 0 deletions lib/elixir/test/elixir/kernel/warning_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -1954,6 +1954,57 @@ defmodule Kernel.WarningTest do
"extra parentheses on a remote function capture &System.pid()/0 have been deprecated. Please remove the parentheses: &System.pid/0"
end

test "nested defmodule when there is an alias with the same name" do
output =
ExUnit.CaptureIO.capture_io(:stderr, fn ->
Code.eval_string("""
defmodule Parent do
alias OtherParent.Child

defmodule Child do
end
end
""")
end)

assert output =~ """
you are defining a module named Child inside module Parent, which will result in the \
full module name Parent.Child. However, the implicit alias to Child will not be set \
up because there is already an existing "alias OtherParent.Child, as: Child". You \
must address this ambiguity either by removing the alias or by defining the nested \
module outside of the parent\
"""
after
purge([Parent, Child])
end

test "nested defmodule with multiple name components when there is an alias with the same name" do
output =
ExUnit.CaptureIO.capture_io(:stderr, fn ->
Code.eval_string("""
defmodule Parent do
alias OtherParent.Child

defmodule Child.One do
end

defmodule Child.Two do
end
end
""")
end)

assert output =~ """
you are defining a module named Child.One inside module Parent, which will result in \
the full module name Parent.Child.One. However, the implicit alias to Child will not \
be set up because there is already an existing \"alias OtherParent.Child, as: Child\". \
You must address this ambiguity either by removing the alias or by defining the \
nested module outside of the parent\
"""
after
purge([Parent, Child])
end

defp assert_compile_error(messages, string) do
captured =
capture_err(fn ->
Expand Down
7 changes: 3 additions & 4 deletions lib/elixir/test/elixir/typespec_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ Code.require_file("test_helper.exs", __DIR__)
# Holds tests for both Kernel.Typespec and Code.Typespec
defmodule TypespecTest do
use ExUnit.Case, async: true
alias TypespecTest.TypespecSample

defstruct [:hello]

Expand Down Expand Up @@ -120,7 +119,7 @@ defmodule TypespecTest do

test "redefined type" do
assert_raise Kernel.TypespecError,
~r"type foo/0 is already defined in .*test/elixir/typespec_test.exs:126",
~r"type foo/0 is already defined in .*test/elixir/typespec_test.exs:125",
fn ->
test_module do
@type foo :: atom
Expand All @@ -129,7 +128,7 @@ defmodule TypespecTest do
end

assert_raise Kernel.TypespecError,
~r"type foo/2 is already defined in .*test/elixir/typespec_test.exs:136",
~r"type foo/2 is already defined in .*test/elixir/typespec_test.exs:135",
fn ->
test_module do
@type foo :: atom
Expand All @@ -139,7 +138,7 @@ defmodule TypespecTest do
end

assert_raise Kernel.TypespecError,
~r"type foo/0 is already defined in .*test/elixir/typespec_test.exs:145",
~r"type foo/0 is already defined in .*test/elixir/typespec_test.exs:144",
fn ->
test_module do
@type foo :: atom
Expand Down