Elixir: warning: variable "me" is unused (if the variable is not meant to be used, prefix it with an underscore)

[RaymondLoranger]

Erlang/OTP 21 [erts-10.3] [64-bit] [smp:8:8] [ds:8:8:10] [async-threads:1]
Elixir 1.11.1 (compiled with Erlang/OTP 21)

test "returns %Game{} given valid args" do
  me = self()
  assert %Game{
               name: "Aveline",
               player1: %Player{name: "Jordan", gender: :m, pid: me}
             } = Game.new("Aveline", "Jordan", :m, me)
  end

See https://hex.pm/packages/islands_game

[maennchen]

@RaymondLoranger I believe this should be:

assert %Game{
               name: "Aveline",
               player1: %Player{name: "Jordan", gender: :m, pid: ^me}
             } = Game.new("Aveline", "Jordan", :m, me)

(See the ^ before the variable)

[maennchen]

https://elixir-lang.org/getting-started/pattern-matching.html#the-pin-operator

[josevalim]

Thanks @maennchen for the help! :heart:

[josevalim]

Actually, I think we can improve the warning here. If there is an existing me variable, we could say they can use the pin operator to match on it. :D

[maennchen]

@josevalim Sure, thanks for you doing it all the time :)

I agree, that would be super helpful (especially since it's not the first time that I've seen that question).

[RaymondLoranger]

Many thanks!

[josevalim]

Hi @RaymondLoranger , if you don't mind i want to keep this open because I want to improve the warning. :)

[maennchen]

@josevalim I'd love to dig a little deeper into elixir itself. Could I provide a PR and you give me a pointer where to look?

[josevalim]

@maennchen it will be my pleasure! I will give you a very loose pointer: the fix is going to be on the Erlang part of the codebase (lib/elixir/src). I am going to add some details below, but I will leave them hidden, in case you don't want too many spoilers.

Also, if you haven't yet, check out the README on how to compile the project and run tests. :)

The warning is emitted here: https://github.com/elixir-lang/elixir/blob/master/lib/elixir/src/elixir_env.erl#L94 and https://github.com/elixir-lang/elixir/blob/master/lib/elixir/src/elixir_env.erl#L99. One is called for the variables at the root of a definition and the second is called for variables inside if/case/etc. You will have to play with these data structures and figure out if it is possible to detect if there was a previous value for it or not. This may not actually be possible without refactoring other parts of the compiler, which may be too complex if that's your first dance with the compiler.

[josevalim]

Details added (and updated). ^

[josevalim]

It may be easier to actually tackle the problem here: https://github.com/elixir-lang/elixir/blob/master/lib/elixir/src/elixir_expand.erl#L338-L347. When you call var_unused, you can already pass a boolean saying if you are oveerriding or not. You will now store a tuple on UnusedVar with {Line, Overridden}. Then change the code on elixir_env to match on this tuple and emit the warning accordingly.

[maennchen]

@josevalim You are right and it can't be done only in elixir_env I think.

Test

  test "unused variable that could be pinned" do
    output =
      capture_err(fn ->
        # Note we use compile_string because eval_string does not emit unused vars warning
        Code.compile_string("""
        defmodule Sample do
          def test(arg) do
            compare_local = "hello"
            match?(compare_local, "hello")

            compare_nested = "hello"
            case "hello" do
              compare_nested -> true
              _other -> false
            end
          end
        end
        """)
      end)

    assert output =~
             "variable \"compare_local\" is unused (there is a variable with the same name in the context, did you mean to use the pin-operator?)"

    assert output =~
             "variable \"compare_nested\" is unused (there is a variable with the same name in the context, did you mean to use the pin-operator?)"
  after
    purge(Sample)
  end

Output when Dumping Unused Variable

#{{arg,0}=>2,{compare_local,1}=>3}
#{{arg,0}=>2,{compare_local,1}=>3}
#{{arg,0}=>2,{compare_local,1}=>3,{compare_nested,3}=>6}
#{{arg,0}=>2,{compare_local,1}=>3,{compare_nested,3}=>6}
#{{arg,0}=>2,{compare_local,1}=>3,{compare_nested,3}=>6}

[josevalim]

Yeah, I think the approach in my second details may be the way to go!

[maennchen]

@josevalim Worked, thanks for your help :heart:

[josevalim]

Closing in favor of PR!