Merge pull request #1928 from ericmj/typespec-kw-guard

Change typespec guards to keyword list

lib/elixir/lib/kernel/typespec.ex

@@ -243,8 +243,19 @@ defmodule Kernel.Typespec do
   Converts a spec clause back to Elixir AST.
   """
   def spec_to_ast(name, { :type, line, :fun, [{:type, _, :product, args}, result] }) do
-    args = lc arg inlist args, do: typespec_to_ast(arg)
-    { :::, [line: line], [{ name, [line: line], args }, typespec_to_ast(result)] }
+    ast_args = lc arg inlist args, do: typespec_to_ast(arg)
+    ast = { :::, [line: line], [{ name, [line: line], ast_args }, typespec_to_ast(result)] }
+
+    vars = args ++ [result]
+      |> Enum.flat_map(&collect_vars/1)
+      |> Enum.uniq
+      |> Enum.map(&{ &1, { :var, [line: line], nil } })
+
+    unless vars == [] do
+      ast = { :when, [line: line], [ast, vars] }
+    end
+
+    ast
   end
 
   def spec_to_ast(name, { :type, line, :fun, [] }) do
@@ -252,15 +263,21 @@ defmodule Kernel.Typespec do
   end
 
   def spec_to_ast(name, { :type, line, :bounded_fun, [{ :type, _, :fun, [{ :type, _, :product, args }, result] }, constraints] }) do
-    [h|t] =
-      lc {:type, line, :constraint, [{:atom, _, :is_subtype}, [var, type]]} inlist constraints do
-        { :is_subtype, [line: line], [typespec_to_ast(var), typespec_to_ast(type)] }
+    guards =
+      lc {:type, _, :constraint, [{:atom, _, :is_subtype}, [{ :var, _, var }, type]]} inlist constraints do
+        { var, typespec_to_ast(type) }
       end
 
-    args = lc arg inlist args, do: typespec_to_ast(arg)
-    guards = Enum.reduce t, h, fn(x, acc) -> { :and, line, [acc, x] } end
+    ast_args = lc arg inlist args, do: typespec_to_ast(arg)
 
-    { :when, [line: line], [{ :::, [line: line], [{ name, [line: line], args }, typespec_to_ast(result)] }, guards] }
+    vars = args ++ [result]
+      |> Enum.flat_map(&collect_vars/1)
+      |> Enum.uniq
+
+    vars = vars -- Keyword.keys(guards)
+      |> Enum.map(&{ &1, { :var, [line: line], nil } })
+
+    { :when, [line: line], [{ :::, [line: line], [{ name, [line: line], ast_args }, typespec_to_ast(result)] }, guards ++ vars] }
   end
 
   @doc """
@@ -418,28 +435,27 @@ defmodule Kernel.Typespec do
   end
 
   @doc false
-  def defspec(type, { :when, _, [{ :::, _, [{ name, meta, args }, return] }, constraints_guard] }, caller) do
+  def defspec(type, { :when, meta2, [{ :::, _, [{ name, meta, args }, return] }, guard] }, caller) do
     if is_atom(args), do: args = []
-    vars = guard_to_vars(constraints_guard)
-    constraints = guard_to_constraints(constraints_guard, vars, caller)
+
+    unless Keyword.keyword?(guard) do
+      guard = Macro.to_string(guard)
+      compile_error caller, "invalid guard in function type specification `#{guard}`"
+    end
+
+    vars = Keyword.keys(guard)
+    constraints = guard_to_constraints(guard, vars, meta2, caller)
+
     spec = { :type, line(meta), :fun, fn_args(meta, args, return, vars, caller) }
     if constraints != [] do
       spec = { :type, line(meta), :bounded_fun, [spec, constraints] }
     end
+
     code = { { name, Kernel.length(args) }, spec }
     Module.compile_typespec(caller.module, type, code)
     code
   end
 
-  def defspec(type, { :when, _, [fun, { :::, _, [guards, return] }] } = spec, caller) do
-    new_spec = { :when, [], [{ :::, [], [fun, return] }, guards] }
-    IO.write "typespec format is deprecated `#{Macro.to_string(spec)}`\n" <>
-      "new format is: `#{Macro.to_string(new_spec)}`\n" <>
-      Exception.format_stacktrace
-
-    defspec(type, new_spec, caller)
-  end
-
   def defspec(type, { :::, _, [{ name, meta, args }, return] }, caller) do
     if is_atom(args), do: args = []
     spec = { :type, line(meta), :fun, fn_args(meta, args, return, [], caller) }
@@ -453,41 +469,49 @@ defmodule Kernel.Typespec do
     compile_error caller, "invalid function type specification `#{spec}`"
   end
 
-  defp guard_to_vars({ :is_subtype, _, [{ name, _, _ }, _] }) do
-    [name]
-  end
-
-  defp guard_to_vars({ :is_var, _, [{ name, _, _ }] }) do
-    [name]
-  end
-
-  defp guard_to_vars({ :and, _, [left, right] }) do
-    guard_to_vars(left) ++ guard_to_vars(right)
-  end
-
-  defp guard_to_constraints({ :is_subtype, meta, [{ name, _, context }, type] }, vars, caller)
-      when is_atom(name) and is_atom(context) do
+  defp guard_to_constraints(guard, vars, meta, caller) do
     line = line(meta)
-    contraints = [{ :atom, line, :is_subtype }, [{:var, line, name}, typespec(type, vars, caller)]]
-    [{ :type, line, :constraint, contraints }]
-  end
 
-  defp guard_to_constraints({ :is_var, _, [{ name, _, context }] }, _, _)
-      when is_atom(name) and is_atom(context) do
-    []
-  end
-
-  defp guard_to_constraints({ :and, _, [left, right] }, vars, caller) do
-    guard_to_constraints(left, vars, caller) ++ guard_to_constraints(right, vars, caller)
-  end
-
-  defp guard_to_constraints(other, _vars, caller) do
-    guard = Macro.to_string(other)
-    compile_error caller, "invalid guard in function type specification `#{guard}`"
+    Enum.reduce(guard, [], fn
+      { _name, { :var, _, context } }, acc when is_atom(context) ->
+        acc
+      { name, type }, acc ->
+        constraint = [{ :atom, line, :is_subtype }, [{:var, line, name}, typespec(type, vars, caller)]]
+        type = { :type, line, :constraint, constraint }
+        [type|acc]
+    end) |> Enum.reverse
   end
 
   ## To AST conversion
 
+  defp collect_vars({ :ann_type, _line, args }) do
+    Enum.flat_map(args, &collect_vars/1)
+  end
+
+  defp collect_vars({ :type, _line, _kind, args }) do
+    Enum.flat_map(args, &collect_vars/1)
+  end
+
+  defp collect_vars({ :remote_type, _line, args }) do
+    Enum.flat_map(args, &collect_vars/1)
+  end
+
+  defp collect_vars({ :typed_record_field, _line, type }) do
+    collect_vars(type)
+  end
+
+  defp collect_vars({:paren_type, _line, [type]}) do
+    collect_vars(type)
+  end
+
+  defp collect_vars({:var, _line, var}) do
+    [erl_to_ex_var(var)]
+  end
+
+  defp collect_vars(_) do
+    []
+  end
+
   defp typespec_to_ast({ :type, line, :tuple, :any }) do
     typespec_to_ast({:type, line, :tuple, []})
   end
@@ -549,14 +573,7 @@ defmodule Kernel.Typespec do
   end
 
   defp typespec_to_ast({ :var, line, var }) do
-    var =
-      case atom_to_binary(var) do
-        <<"_", c :: [binary, size(1)], rest :: binary>> ->
-          binary_to_atom("_#{String.downcase(c)}#{rest}")
-        <<c :: [binary, size(1)], rest :: binary>> ->
-          binary_to_atom("#{String.downcase(c)}#{rest}")
-      end
-    { var, line, nil }
+    { erl_to_ex_var(var), line, nil }
   end
 
   # Special shortcut(s)
@@ -598,6 +615,15 @@ defmodule Kernel.Typespec do
 
   defp typespec_to_ast(other), do: other
 
+  defp erl_to_ex_var(var) do
+    case atom_to_binary(var) do
+      <<"_", c :: [binary, size(1)], rest :: binary>> ->
+        binary_to_atom("_#{String.downcase(c)}#{rest}")
+      <<c :: [binary, size(1)], rest :: binary>> ->
+        binary_to_atom("#{String.downcase(c)}#{rest}")
+    end
+  end
+
   ## From AST conversion
 
   defp line(meta) do

lib/elixir/lib/list.ex

@@ -48,7 +48,7 @@ defmodule List do
       iex> List.duplicate([1, 2], 2)
       [[1,2],[1,2]]
   """
-  @spec duplicate(elem, non_neg_integer) :: [elem] when is_var(elem)
+  @spec duplicate(elem, non_neg_integer) :: [elem] when elem: var
   def duplicate(elem, n) do
     :lists.duplicate(n, elem)
   end
@@ -62,7 +62,7 @@ defmodule List do
       [1,2,3]
 
   """
-  @spec flatten(deep_list) :: list when is_subtype(deep_list, [any | deep_list])
+  @spec flatten(deep_list) :: list when deep_list: [any | deep_list]
   def flatten(list) do
     :lists.flatten(list)
   end
@@ -78,7 +78,7 @@ defmodule List do
       [1,2,3,4,5]
 
   """
-  @spec flatten(deep_list, [elem]) :: [elem] when is_subtype(deep_list, [elem | deep_list]) and is_var(elem)
+  @spec flatten(deep_list, [elem]) :: [elem] when elem: var, deep_list: [elem | deep_list]
   def flatten(list, tail) do
     :lists.flatten(list, tail)
   end
@@ -96,7 +96,7 @@ defmodule List do
       2
 
   """
-  @spec foldl([elem], acc, (elem, acc -> acc)) :: acc when is_var(elem) and is_var(acc)
+  @spec foldl([elem], acc, (elem, acc -> acc)) :: acc when elem: var, acc: var
   def foldl(list, acc, function) when is_list(list) and is_function(function) do
     :lists.foldl(function, acc, list)
   end
@@ -111,7 +111,7 @@ defmodule List do
       -2
 
   """
-  @spec foldr([elem], acc, (elem, acc -> acc)) :: acc when is_var(elem) and is_var(acc)
+  @spec foldr([elem], acc, (elem, acc -> acc)) :: acc when elem: var, acc: var
   def foldr(list, acc, function) when is_list(list) and is_function(function) do
     :lists.foldr(function, acc, list)
   end
@@ -129,7 +129,7 @@ defmodule List do
       3
 
   """
-  @spec last([elem]) :: nil | elem when is_var(elem)
+  @spec last([elem]) :: nil | elem when elem: var
 
   def last([]), do: nil
 
@@ -399,7 +399,7 @@ defmodule List do
       [1, 2, 3]
 
   """
-  @spec update_at([elem], integer, (elem -> any)) :: list when is_var(elem)
+  @spec update_at([elem], integer, (elem -> any)) :: list when elem: var
   def update_at(list, index, fun) do
     if index < 0 do
       do_update_at(list, length(list) + index, fun)

lib/elixir/lib/macro.ex

@@ -49,8 +49,8 @@ defmodule Macro do
   end
 
   @doc """
-  Breaks a pipeline expression into a list. 
-  
+  Breaks a pipeline expression into a list.
+
   Raises if the pipeline is ill-formed.
   """
   @spec unpipe(Macro.t) :: [Macro.t]
@@ -152,8 +152,8 @@ defmodule Macro do
   end
 
   @doc %S"""
-  Unescape the given chars. 
-  
+  Unescape the given chars.
+
   This is the unescaping behavior
   used by default in Elixir single- and double-quoted strings.
   Check `unescape_string/2` for information on how to customize
@@ -298,7 +298,14 @@ defmodule Macro do
 
   # Tuple containers
   def to_string({ :{}, _, args } = ast, fun) do
-    fun.(ast, "{" <> Enum.map_join(args, ", ", &to_string(&1, fun)) <> "}")
+    if match?([_], args) do
+      tuple = "{" <> Enum.map_join(args, ", ", &to_string(&1, fun)) <> "}"
+    else
+      args = args_to_string(args, fun)
+      tuple = "{" <> args <> "}"
+    end
+
+    fun.(ast, tuple)
   end
 
   # Fn keyword
@@ -321,6 +328,17 @@ defmodule Macro do
     fun.(ast, "(" <> arrow_to_string(ast, fun, true) <> ")")
   end
 
+  # left when right
+  def to_string({ :when, _, [left, right] } = ast, fun) do
+    if right != [] and Keyword.keyword?(right) do
+      right = kw_list_to_string(right, fun)
+    else
+      right = fun.(ast, op_to_string(right, fun, :when, :right))
+    end
+
+    fun.(ast, op_to_string(left, fun, :when, :left) <> " when " <> right)
+  end
+
   # Binary ops
   def to_string({ op, _, [left, right] } = ast, fun) when op in @binary_ops do
     fun.(ast, op_to_string(left, fun, op, :left) <> " #{op} " <> op_to_string(right, fun, op, :right))
@@ -362,11 +380,15 @@ defmodule Macro do
 
   # Lists
   def to_string(list, fun) when is_list(list) do
-    if Keyword.keyword?(list) do
-      fun.(list, "[" <> kw_list_to_string(list, fun) <> "]")
-    else
-      fun.(list, "[" <> Enum.map_join(list, ", ", &to_string(&1, fun)) <> "]")
-    end
+    fun.(list, cond do
+      Keyword.keyword?(list) ->
+        "[" <> kw_list_to_string(list, fun) <> "]"
+      not match?([_], list) ->
+        args = args_to_string(list, fun)
+        "[" <> args <> "]"
+      true ->
+        "[" <> Enum.map_join(list, ", ", &to_string(&1, fun)) <> "]"
+    end)
   end
 
   # All other structures
@@ -389,18 +411,20 @@ defmodule Macro do
   defp call_to_string(other, fun),                    do: to_string(other, fun)
 
   defp call_to_string_with_args(target, args, fun) do
-    { list, last } = :elixir_utils.split_last(args)
     target = call_to_string(target, fun)
+    args = args_to_string(args, fun)
+    target <> "(" <> args <> ")"
+  end
 
-    case last != [] and Keyword.keyword?(last) do
-      true  ->
-        args = Enum.map_join(list, ", ", &to_string(&1, fun))
-        if list != [], do: args = args <> ", "
-        args = args <> kw_list_to_string(last, fun)
-        target <> "(" <> args <> ")"
-      false ->
-        args = Enum.map_join(args, ", ", &to_string(&1, fun))
-        target <> "(" <> args <> ")"
+  defp args_to_string(args, fun) do
+    { list, last } = :elixir_utils.split_last(args)
+
+    if last != [] and Keyword.keyword?(last) do
+      args = Enum.map_join(list, ", ", &to_string(&1, fun))
+      if list != [], do: args = args <> ", "
+      args <> kw_list_to_string(last, fun)
+    else
+      Enum.map_join(args, ", ", &to_string(&1, fun))
     end
   end
 
@@ -483,8 +507,8 @@ defmodule Macro do
   end
 
   @doc """
-  Receives an AST node and expands it once. 
-  
+  Receives an AST node and expands it once.
+
   The following contents are expanded:
 
   * Macros (local or remote);
@@ -665,8 +689,8 @@ defmodule Macro do
 
   @doc """
   Receives an AST node and expands it until it no longer represents
-  a macro. 
-  
+  a macro.
+
   Check `expand_once/2` for more information on how
   expansion works.
   """
@@ -715,7 +739,7 @@ defmodule Macro do
   @doc """
   Recursively traverses the quoted expression checking if all sub-terms are
   safe.
- 
+
   Terms are considered safe if they represent data structures and don't actually
   evaluate code. Returns `:ok` unless a given term is unsafe,
   which is returned as `{ :unsafe, term }`.

lib/elixir/src/elixir_parser.yrl

@@ -87,6 +87,28 @@ expr -> matched_expr : '$1'.
 expr -> no_parens_expr : '$1'.
 expr -> unmatched_expr : '$1'.
 
+%% In Elixir we have three main call syntaxes: with parentheses,
+%% without parentheses and with do blocks. They are represented
+%% in the AST as matched, no_parens and unmatched.
+%%
+%% The distinction is required because we can't, for example, have
+%% a function call with a do block as argument inside another do
+%% block call, unless there are parentheses:
+%%
+%%   if if true do true else false end do  #=> invalid
+%%   if(if true do true else false end) do #=> valid
+%%
+%% Similarly, it is not possible to nest calls without parentheses
+%% if their arity is more than 1:
+%%
+%%   foo a, bar b, c  #=> invalid
+%%   foo(a, bar b, c) #=> invalid
+%%   foo a, bar b     #=> valid
+%%   foo a, bar(b, c) #=> valid
+%%
+%% So the different grammar rules need to take into account
+%% if calls without parentheses are do blocks in particular
+%% segments and act accordingly.
 matched_expr -> matched_expr matched_op_expr : build_op(element(1, '$2'), '$1', element(2, '$2')).
 matched_expr -> matched_expr no_parens_op_expr : build_op(element(1, '$2'), '$1', element(2, '$2')).
 matched_expr -> unary_op_eol matched_expr : build_unary_op('$1', '$2').
@@ -140,13 +162,16 @@ no_parens_op_expr -> tail_op_eol no_parens_expr : { '$1', '$2' }.
 no_parens_op_expr -> than_op_eol no_parens_expr : { '$1', '$2' }.
 no_parens_op_expr -> in_op_eol no_parens_expr : { '$1', '$2' }.
 no_parens_op_expr -> inc_op_eol no_parens_expr : { '$1', '$2' }.
-no_parens_op_expr -> when_op_eol no_parens_expr : { '$1', '$2' }.
 no_parens_op_expr -> range_op_eol no_parens_expr : { '$1', '$2' }.
 no_parens_op_expr -> default_op_eol no_parens_expr : { '$1', '$2' }.
 no_parens_op_expr -> type_op_eol no_parens_expr : { '$1', '$2' }.
 no_parens_op_expr -> comp_op_eol no_parens_expr : { '$1', '$2' }.
 no_parens_op_expr -> arrow_op_eol no_parens_expr : { '$1', '$2' }.
 
+%% Allow when (and only when) with keywords
+no_parens_op_expr -> when_op_eol no_parens_expr : { '$1', '$2' }.
+no_parens_op_expr -> when_op_eol call_args_no_parens_kw : { '$1', '$2' }.
+
 matched_op_expr -> match_op_eol matched_expr : { '$1', '$2' }.
 matched_op_expr -> add_op_eol matched_expr : { '$1', '$2' }.
 matched_op_expr -> mult_op_eol matched_expr : { '$1', '$2' }.

lib/elixir/test/elixir/macro_test.exs

@@ -39,7 +39,7 @@ defmodule MacroTest do
   test :escape_works_recursively do
     assert [1,{:{}, [], [:a,:b,:c]}, 3] == Macro.escape([1, { :a, :b, :c }, 3])
   end
-  
+
   test :escape_improper do
     assert [{:|, [], [1,2]}] == Macro.escape([1|2])
     assert [1,{:|, [], [2,3]}] == Macro.escape([1,2|3])
@@ -330,6 +330,8 @@ defmodule MacroTest do
   end
 
   test :containers_to_string do
+    assert Macro.to_string(quote do: {})   == "{}"
+    assert Macro.to_string(quote do: [])   == "[]"
     assert Macro.to_string(quote do: { 1, 2, 3 })   == "{1, 2, 3}"
     assert Macro.to_string(quote do: [ 1, 2, 3 ])   == "[1, 2, 3]"
     assert Macro.to_string(quote do: << 1, 2, 3 >>) == "<<1, 2, 3>>"
@@ -366,10 +368,20 @@ defmodule MacroTest do
   end
 
   test :last_arg_kw_list do
+    assert Macro.to_string(quote do: foo([])) == "foo([])"
     assert Macro.to_string(quote do: foo(x: y)) == "foo(x: y)"
     assert Macro.to_string(quote do: foo(x: 1 + 2)) == "foo(x: 1 + 2)"
     assert Macro.to_string(quote do: foo(x: y, p: q)) == "foo(x: y, p: q)"
     assert Macro.to_string(quote do: foo(a, x: y, p: q)) == "foo(a, x: y, p: q)"
+
+
+    assert Macro.to_string(quote do: { [] }) == "{[]}"
+    assert Macro.to_string(quote do: { [a: b] }) == "{[a: b]}"
+    assert Macro.to_string(quote do: { x, a: b }) == "{x, a: b}"
+    assert Macro.to_string(quote do: []) == "[]"
+    assert Macro.to_string(quote do: [a: b]) == "[a: b]"
+    assert Macro.to_string(quote do: [[a: b]]) == "[[a: b]]"
+    assert Macro.to_string(quote do: [x, a: b]) == "[x, a: b]"
   end
 
   test :to_string_with_fun do

lib/elixir/test/elixir/typespec_test.exs

@@ -322,13 +322,13 @@ defmodule Typespec.TypeTest do
   test "@spec(spec) with guards" do
     { spec1, spec2, spec3 } = test_module do
       def myfun1(x), do: x
-      spec1 = @spec myfun1(x) :: boolean when is_subtype(x, integer)
+      spec1 = @spec myfun1(x) :: boolean when [x: integer]
 
       def myfun2(x), do: x
-      spec2 = @spec myfun2(x) :: x when is_var(x)
+      spec2 = @spec myfun2(x) :: x when [x: var]
 
       def myfun3(_x, y), do: y
-      spec3 = @spec myfun3(x, y) :: y when is_subtype(y, x) and is_var(x)
+      spec3 = @spec myfun3(x, y) :: y when [y: x, x: var]
 
       { spec1, spec2, spec3 }
     end
@@ -455,7 +455,8 @@ defmodule Typespec.TypeTest do
     specs = [
       (quote do: @spec a() :: integer()),
       (quote do: @spec a(atom()) :: integer()),
-      (quote do: @spec a(b) :: integer() when is_subtype(b, integer())),
+      (quote do: @spec a(b) :: integer() when [b: integer()]),
+      (quote do: @spec a(b) :: b when [b: var])
     ]
 
     compiled = test_module do