Imperative Assignements are breaking the application in 1.7 update

[jfayad]

Environment

• Elixir & Erlang/OTP versions (elixir --version): Erlang/OTP 21 [erts-10.0] [ source] [64-bit]
  Elixir 1.7.0 (Compiled with Erlang/OTP 20)
• Operating system: Ubuntu 16.04

Current behavior

After updated Elixir from 1.6 to 1.7 the params variable will always be equal to %{} outside the if statements

params = %{}

        if Map.has_key?(entities, "email") do
          params = Map.put_new(params, :email, get_values(entities["email"]))
        end

Expected behavior

The params variable should be correctly assigned for now and updating to 1.7 shouldn't break such a code since nothing has been mentioned in the release not of 1.7

[fertapric]

I think that code will emit a warning similar to:

warning: variable "params" is unused

Note variables defined inside case, cond, fn, if and similar do not leak. If you want to conditionally override an existing variable "params", you will have to explicitly return the variable. For example:

    if some_condition? do
      atom = :one
    else
      atom = :two
    end

should be written as

    atom =
      if some_condition? do
        :one
      else
        :two
      end

Unused variable found at:
  your_file.ex:42

[jfayad]

Actually no @fertapric since the variable is defined outside the if statement.

[wojtekmach]

I can confirm the behaviour changed between 1.6 and 1.7:

~/foo% asdf local elixir 1.6.5 && elixir -e "x = 1; if(true, do: x = 2); IO.inspect x" 2> /dev/null
2
~/foo% asdf local elixir 1.7.0 && elixir -e "x = 1; if(true, do: x = 2); IO.inspect x" 2> /dev/null
1

[fertapric]

🤔 That's odd!

Before posting, I checked the warning was there using this small example:

params = %{}

if true do
  params = Map.put_new(params, :email, 1)
end

IO.inspect params

The warning was emitted for the assignment in the line 4 :)

[wojtekmach]

@fertapric I got the warning in both cases as expected, I removed it with | 2> /dev/null for brevity.

[josevalim]

Duplicate of #7403. Rationale is here: #7403 (comment) We should have a CHANGELOG entry too. But I will be glad to clarify it. -- *José Valim* www.plataformatec.com.br Skype: jv.ptec Founder and Director of R&D

[jfayad]

Thanks for the clarification @josevalim indeed updating the changelog would have saved us (and probably others) some precious time searching for the reason behind these errors.

[josevalim]

Julien, sorry for not being clear. I meant that this is already in the CHANGELOG but apparently not clear enough. Could you please give it a second pass and suggest how should we change the CHANGELOG to make this clearer? -- *José Valimwww.plataformatec.com.br <http://www.plataformatec.com.br/>Founder and Director of R&D*

[jfayad]

@josevalim I'll be glad to suggest but after I've made a CTRL+F of the following terms:

• Imperative
• Variable
• leak
• if
  on this page: https://github.com/elixir-lang/elixir/releases/tag/v1.7.0

I couldn't find anything relevant.
If a line is there, can you please point me to it?

[lackac]

@jfayad this line:

[Kernel] Raise on unsafe variables in order to allow us to better track unused variables

[josevalim]

I think adding “also known as imperative assignment” to that entry could be enough. Thoughts? -- *José Valimwww.plataformatec.com.br <http://www.plataformatec.com.br/>Founder and Director of R&D*

[jfayad]

I would definitely add the key expression Imperative assignment. So adding to the end of the sentence "and stop leaking Imperative assignment variables to the upper context"

[josevalim]

Updated, thanks everyone for the feedback!