Elixir: Should the compiler warn about an unmatched `else`?

[nathanl]

Environment

• Elixir & Erlang/OTP versions (elixir --version): Elixir 1.7.1 (compiled with Erlang/OTP 21)
• Operating system: MacOS High Sierra (10.13.6)

Problem

I was just in a debugging session with a colleague where the bug came down to an incorrectly-nested else. A minimal reproduction is:

defmodule Else do
  def weirdness do
    with true <- true do
      IO.puts("yay")
    end
  else
    match -> IO.inspect(["wat", match])
  end
end

Else.weirdness() # => prints both `"yay"` and `["wat", :ok]`

We were very confused to have both the do clause of the with executed and also the else. Eventually we realized that the problem is that the else is not nested in the with. (Side note: the value of _match here is :ok, but I don't know why.)

Perhaps an even more minimal reproduction is:

defmodule LonelyElse do
  def here do
  else
    match -> IO.inspect(["wat", match])
  end
end
LonelyElse.here() # => prints `["wat", nil]`

I don't think either of these else clauses makes sense. Could the compiler issue a warning in cases where an else does not have a matching if or with or catch?

[josevalim]

Looks good to me. If there is no expression on try or the try is a literal, the else is irrelevant. :+1:

[josevalim]

This also could be added to the Erlang compiler. /cc @michalmuskala

[whatyouhide]

Mh, do we document anywhere that else can be used for functions? The docs for def/2 only mention rescue, catch, and after. I think that we should add it there anyways, or stop supporting it (which I don't think is what we want since everything else works).

[OvermindDL1]

Huh, I just tried it, else in a def acts like a new case body that the previous main do body gets piped in to. Interesting functionality, I kind of like it, but it's still easy enough to pipe into a new case anyway to do the same thing so probably worth removing anyway.

[josevalim]

All of the keywords in a definition body are treated equivalent to a try.
The question is if a try with only “else” is useful, and I don’t think it
is. It is the same as not having a try at all. Alternatively, do we even

support a try without any of after/catch/else?

José Valimwww.plataformatec.com.br
http://www.plataformatec.com.br/Founder and Director of R&D

[whatyouhide]

@josevalim we don't.

iex(1)> try do
...(1)> :ok
...(1)> end
** (CompileError) iex:1: missing :catch/:rescue/:after/:else option in "try"

[josevalim]

@whatyouhide so i think we should raise if the try only has else too. I can't see any practical usage for it.

[nathanl]

I originally asked that the compiler warn about this, but on further thought, I think erroring out is better. Since there is no logical use for an unmatched else, it seems reasonable to me to consider it a mistake every time. A warning might go unnoticed and let bugs like the one we had slip through. When I mix format, I'd like an error just as if I forgot a closing paren.

(I think that's what you're saying, too, @josevalim; I just wasn't sure if you meant all cases of a "lone else".)

For comparison, an else with no if in Ruby produces a syntax error. Same thing in JavaScript.

[umamaistempo]

@nathanl there is a potential use that I have seen in the wild

def bla do
  case something() do
    1 -> :something
     _ -> :bla
  end
else
  anything -> {:ok, anything}
end

But naturally this is a bad pratice and should be discouraged, so I am all in for raising an error just like the situation @whatyouhide described

[nathanl]

@umamaistempo Interesting - so anything there is whatever case returns. It's much less clear than capturing a variable or moving the case to another function, though.
Thanks for showing an example usage!

[umamaistempo]

Yup, this could be made in a cleaner way by binding the result of case and then working over it, so I would discourage using def/else for this. But it is a fact that there are people doing this on the wild