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

alias overrriding even with Elixir. prefix #12456

Closed
josevalim opened this issue Mar 9, 2023 · 4 comments
Closed

alias overrriding even with Elixir. prefix #12456

josevalim opened this issue Mar 9, 2023 · 4 comments

Comments

@josevalim
Copy link
Member

I wanted to see how this alias overwriting work in case of external submodules and I've found something odd

defmodule Parent do
        alias OtherParent.Child
        def a1, do: IO.puts "Parent " <> inspect(__ENV__.aliases)
        defmodule Elixir.Child do
          def a2, do: IO.puts "Elixir.Child " <> inspect(__ENV__.aliases)
        end
        def a3, do: IO.puts "Parent " <> inspect(__ENV__.aliases)
end
iex(3)> Parent.a1
Parent [{Child, OtherParent.Child}]
:ok
iex(4)> Child.a2
Elixir.Child []
:ok
iex(5)> Parent.a3
Parent []
:ok

So it removes the alias. This does not happen for nested external submodule e.g. Elixir.Child.One

Originally posted by @lukaszsamson in #12451 (comment)

@sabiwara
Copy link
Contributor

sabiwara commented Oct 16, 2023

I think what's happening is:

  • defmodule Elixir.Child starts expanding an alias:
    unquote(with_alias)
    ({:alias, [defined: Child, context: Parent, line: 4], [Child, [as: nil, warn: false]]})
  • we call this clause which cleans the alias ([{'Elixir.Child','Elixir.OtherParent.Child'}] -> [])
  • this clause is what makes this alias removal test pass

I'm not sure what the right fix is, we need to be able to distinguish between the legit alias removal and the case where it is triggered by a nested defmodule.

@josevalim
Copy link
Member Author

@sabiwara I would argue that we should not do an alias if the module starts with Elixir., because the point of Elixir. is to bypass aliases altogether. :)

Do you agree @lukaszsamson?

@lukaszsamson
Copy link
Contributor

I would argue that we should not do an alias if the module starts with Elixir., because the point of Elixir. is to bypass aliases altogether. :)

It sounds reasonable.

@josevalim when we are at it let's take a look on another non trivial example. Does this behave correctly? It seems __MODULE__.Child expands to Elixir.Parent.Child and not Parent.Child and does not set an alias {Parent, Parent.Parent}.

defmodule Parent do
    alias OtherParent.Child
    def a1, do: IO.puts "Parent " <> inspect(__ENV__.aliases)
    defmodule __MODULE__.Child do
      def a2, do: IO.puts "__MODULE__.Child " <> inspect(__ENV__.aliases)
    end
    def a3, do: IO.puts "Parent " <> inspect(__ENV__.aliases)
end
iex(2)> Parent.a1
Parent [{Child, OtherParent.Child}]
:ok
iex(3)> Parent.Child.a2
__MODULE__.Child [{Child, OtherParent.Child}]
:ok
iex(4)> Parent.a3
Parent [{Child, OtherParent.Child}]
:ok

@josevalim
Copy link
Member Author

josevalim commented Oct 16, 2023

@lukaszsamson it is correct to me, because __MODULE__.Child is also fully qualified.

lukaszsamson added a commit to elixir-lsp/elixir_sense that referenced this issue Nov 23, 2023
lukaszsamson added a commit to elixir-lsp/elixir_sense that referenced this issue Nov 27, 2023
* fix warnings

* get line in predictable way

* position is :erl.anno since 1.16

* alias external submodule behaviour changed in 1.16

see elixir-lang/elixir#12456

* fix parser tests

this is a WIP solution atm

* handle anonymous_call in complete

* fix version check and another test

* remove not needed test

* pass cursor line to parser

fall back to container_cursor_to_quoted if unable to parse

* fix crash in parser

* fix warning

* improve parser

* return correct error on parse success via container_cursor_to_quoted

* do not fall back to container_cursor_to_quoted when fixing no env

* format on 1.15

* fix some dialyzer errors
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

4 participants