Improve with/else error message on invalid form

[danhper]

When upgrading my code to Elixir 1.3, I forgot to use clauses in my else when replacing if/else by with/else,
and got an error message difficult to parse, so I added a clause to get something more readable.

Before:

== Compilation error on file web/models/app.ex ==
** (FunctionClauseError) no function clause matching in :elixir_with.expand_else/2
    (elixir) src/elixir_with.erl:58: :elixir_with.expand_else({:changeset, [line: 34]$
 nil}, %Macro.Env{aliases: [], context: nil, context_modules: [BuildyPush.App], expor$
_vars: nil, file: "/home/daniel/tmp/push-server/web/models/app.ex", function: {:valid$
te_settings, 1}, functions: [{Ecto.Changeset, [add_error: 3, add_error: 4, apply_chan$
es: 1, assoc_constraint: 2, assoc_constraint: 3, cast: 3, cast: 4, cast_assoc: 2, cas$
_assoc: 3, cast_embed: 2, cast_embed: 3, change: 1, change: 2, check_constraint: 2, c$
eck_constraint: 3, delete_change: 2, exclusion_constraint: 2, exclusion_constraint: 3$
 fetch_change: 2, fetch_field: 2, force_change: 3, foreign_key_constraint: 2, foreign$
key_constraint: 3, get_change: 2, get_change: 3, get_field: 2, get_field: 3, merge: 2$
 no_assoc_constraint: 2, no_assoc_constraint: 3, optimistic_lock: 2, optimistic_lock: 
3, prepare_changes: 2, put_assoc: 3, put_assoc: 4, put_change: 3, put_embed: 3, put_e$
bed: 4, traverse_errors: 2, unique_constraint: 2, unique_constraint: 3, ...]}, {Ecto, 
[assoc: 2, assoc_loaded?: 1, build_assoc: 2, build_assoc: 3, get_meta: 2, primary_key$
 1, primary_key!: 1, put_meta: 2]}, {Kernel, [!=: 2, !==: 2, *: 2, +: 1, +: 2, ++: 2, 
-: 1, -: 2, --: 2, /: 2, <: 2, <=: 2, ==: 2, ===: 2, =~: 2, >: 2, >=: 2, abs: 1, appl$
: 2, apply: 3, binary_part: 3, bit_size: 1, byte_size: 1, div: 2, elem: 2, exit: 1, f$
nction_exported?: 3, get_and_update_in: 3, get_in: 2, hd: 1, inspect: 1, inspect: 2, $
s_atom: 1, is_binary: 1, is_bitstring: 1, is_boolean: 1, is_float: 1, is_function: 1, 
is_function: 2, is_integer: 1, ...]}], lexical_tracker: #PID<0.284.0>, line: 28, macr$
_aliases: [], macros: [{Ecto.Query, [from: 1, from: 2]}, {Ecto.Schema, [embedded_sche$
a: 1, schema: 2]}, {Kernel, [!: 1, &&: 2, ..: 2, <>: 2, @: 1, alias!: 1, and: 2, bind$
ng: 0, binding: 1, def: 1, def: 2, defdelegate: 2, defexception: 1, defimpl: 2, defim$
l: 3, defmacro: 1, defmacro: 2, defmacrop: 1, defmacrop: 2, defmodule: 2, defoverrida$
le: 1, defp: 1, defp: 2, defprotocol: 2, defstruct: 1, destructure: 2, get_and_update_
in: 2, if: 2, in: 2, is_nil: 1, match?: 2, or: 2, pop_in: 1, put_in: 2, raise: 1, rais
e: 2, ...]}], module: BuildyPush.App, requires: [BuildyPush.Web, Ecto, Ecto.Changeset,
 Ecto.Query, Ecto.Schema, Kernel, Kernel.Typespec], vars: [changeset: nil]})
    (elixir) src/elixir_with.erl:48: :elixir_with.expand/3
    web/models/app.ex:28: (module)
    (stdlib) erl_eval.erl:670: :erl_eval.do_apply/6
    (elixir) lib/kernel/parallel_compiler.ex:116: anonymous fn/4 in Kernel.ParallelCom
piler.spawn_compilers/1

After:

== Compilation error on file web/models/app.ex ==
** (CompileError) web/models/app.ex:29: expected -> clauses in else block
    (elixir) src/elixir_with.erl:48: :elixir_with.expand/3
    web/models/app.ex:28: (module)
    (stdlib) erl_eval.erl:670: :erl_eval.do_apply/6
    (elixir) lib/kernel/parallel_compiler.ex:116: anonymous fn/4 in Kernel.ParallelCom
piler.spawn_compilers/1

[whatyouhide]

I feel this indentation is a bit off. Maybe we can have

Message = "expected -> clauses in else block",
elixir_errors:compile_error(Meta, ?m(E, file), Message, []).

Wdyt?

[whatyouhide]

Btw, I'm not sure about the error message either: maybe "expected one or more -> in this with's else block" or something like this?

[michalmuskala]

What is the message for case missing expressions? Shouldn't it be exactly the same here?

[whatyouhide]

Good point @michalmuskala, the message for cases is

expected -> clauses for do in case

so maybe we can drop the "one or more" but we should say "in with's else block" or something along those lines IMO.

[danhper]

Thank you for the feedback.
@whatyouhide Oh, you're right, my indentation is weird, I'll fix it with the message!
I agree with @michalmuskala, keeping the same message as case is more consistent.

The case message is

expected -> clauses for do in case

so what do you think about

expected -> clauses for else in with

[michalmuskala]

The message in case can be changed as well, if there's a better one, but IMHO they should match.

[whatyouhide]

Okay, let's go with "expected -> clauses for else in with", then we can think about changing both :)

[danhper]

I just updated the PR 😄

[whatyouhide]

This looks good to me, let's wait for a second thumbs up :)

[josevalim]

Second 👍 .

[whatyouhide]

Thanks @tuvistavie, great catch! :)