Repository: keathley/norm (elixir)

Issue:
Proposal to make parameter names in a @contract optional
One currently has to name each parameter in a contract, but the name doesn't have to match those used in the function.

For example:

```elixir
defmodule Test do
  use Norm
  
  def age(), do spec(is_integer())
  def name(), do: spec(is_binary())
  def greeting(), do: spec(is_binary())
	
  @contract hello(name :: name(), age :: age()) :: greeting()
  def hello(name, age), do: "Hello #{name}. You are #{age}!"
end
```

(Excuse the "looseness" of the `spec` definitions above; in use, I'd be more explicit with the use of regular expressions.)

In the example above, we have the contract:
```elixir
@contract hello(name :: name(), age :: age()) :: greeting()
```

The repetition, if one is naming specs semantically, can make it a little less readable and a little more error-prone.

We can currently write a slightly cleaner:
```elixir
@contract hello(n :: name(), a :: age()) :: greeting()
```

But we can't do the following, which in retrospect might confuse the intent since we're not ignoring the spec, only the parameter name:
```elixir
@contract hello(_ :: name(), _ :: age()) :: greeting()
```

It gives the following error:
```elixir
** (CompileError) invalid use of _. "_" represents a value to be ignored in a pattern and cannot be used in expressions
```

---
What I'd prefer is an option to write:
```elixir
@contract hello(name(), age()) :: greeting()
```

For me, this more concise notation encourages semantically richer naming of specs that communicate their intent better. Of course, there will be those who abuse the shorthand, but they're the same coders who will have lazy parameter names.

This is perhaps a little more controversial, but since `@contract` must be specified immediately above the function(s) it defines, one could conceivably drop the function name:

```elixir
@contract (name(), age()) :: greeting()
```

Or, if it does look a bit like function currying, which Elixir doesn't support without third-party libraries, the even cleaner:

```elixir
@contract name() :: age() :: greeting()
```

I do copy and paste contracts between functions, and sometimes forget to rename them.

---
Note that I'd still like the option to have parameter names in my contracts. I sometimes like to arrange my contract parameters vertically over several lines. And I often use semi-generic naming.

For example (with the necessary module, spec and schema definitions), this is valid code:

```elixir
@contract complex_image_processing_function(
  poster :: Image.s(),
  stills :: coll_of(Image.s()),
  perceptual_hashes :: coll_of(hash()),
  md5s :: coll_of(hash())
) :: coll_of(Image.s())
def complex_image_processing_function(poster, stills, perceptual_hashes, md5s), do: ...  
```

Pull request description:
Contract improvements
Based on #29. Closes #35.

First of all, we improve contract internals by leveraging
defoverridable/1.

Secondly, we add contract reflection: for each defined contract there
would be a `__contract__({name, arity})` clause defined on the module.
For now, reflection is considered private API and can change anytime,
but it might become public API eventually.

Finally, we make contract argument names optional, this is now valid:

    @contract rgb_to_hex(rgb(), rgb(), rgb()) :: hex()

Under the hood, the arguments are given names like `arg1`, `arg2` etc
but that's an implementation detail of reflection API that is again
private for now.

Patch:
diff --git a/lib/norm/contract.ex b/lib/norm/contract.ex
index cf5f759..661e11d 100644
--- a/lib/norm/contract.ex
+++ b/lib/norm/contract.ex
@@ -27,46 +27,35 @@ defmodule Norm.Contract do
 
   """
 
-  @doc false
-  def __before_compile__(env) do
-    contracts = Module.get_attribute(env.module, :norm_contracts)
-    definitions = Module.definitions_in(env.module)
-
-    for {fun, arity} <- contracts do
-      unless {:"__#{fun}_without_contract__", arity} in definitions do
-        raise ArgumentError, "contract for undefined function #{fun}/#{arity}"
-      end
-    end
-  end
+  defstruct [:args, :result]
 
   @doc false
   defmacro __using__(_) do
     quote do
-      import Kernel, except: [@: 1, def: 2]
-      import Norm.Contract
+      import Kernel, except: [@: 1]
       Module.register_attribute(__MODULE__, :norm_contracts, accumulate: true)
       @before_compile Norm.Contract
+      import Norm.Contract
     end
   end
 
   @doc false
-  defmacro def(call, expr) do
-    quote do
-      if unquote(fa(call)) in @norm_contracts do
-        unless Module.defines?(__MODULE__, unquote(fa(call))) do
-          Kernel.def(unquote(wrapper_call(call)), do: unquote(wrapper_body(call)))
-        end
+  defmacro __before_compile__(env) do
+    definitions = Module.definitions_in(env.module)
+    contracts = Module.get_attribute(env.module, :norm_contracts)
 
-        Kernel.def(unquote(call_without_contract(call)), unquote(expr))
-      else
-        Kernel.def(unquote(call), unquote(expr))
+    for {name, arity, line} <- contracts do
+      unless {name, arity} in definitions do
+        raise ArgumentError, "contract for undefined function #{name}/#{arity}"
       end
+
+      defconformer(name, arity, line)
     end
   end
 
   @doc false
   defmacro @{:contract, _, expr} do
-    defcontract(expr)
+    defcontract(expr, __CALLER__.line)
   end
 
   defmacro @other do
@@ -75,97 +64,59 @@ defmodule Norm.Contract do
     end
   end
 
-  ## Internals
+  defp defconformer(name, arity, line) do
+    args = Macro.generate_arguments(arity, nil)
 
-  defp defcontract(expr) do
-    if Application.get_env(:norm, :enable_contracts, true) do
-      do_defcontract(expr)
-    end
-  end
+    quote line: line do
+      defoverridable [{unquote(name), unquote(arity)}]
 
-  defp do_defcontract(expr) do
-    {call, result_spec} =
-      case expr do
-        [{:"::", _, [call, result_spec]}] ->
-          {call, result_spec}
+      def unquote(name)(unquote_splicing(args)) do
+        contract = __MODULE__.__contract__({unquote(name), unquote(arity)})
 
-        _ ->
-          actual = Macro.to_string({:@, [], [{:contract, [], expr}]})
-
-          raise ArgumentError,
-                "contract must be in the form " <>
-                  "`@contract function(arg :: spec) :: result_spec`, got: `#{actual}`"
-      end
-
-    {name, call_meta, arg_specs} = call
-
-    arg_vars =
-      for arg_spec <- arg_specs do
-        case arg_spec do
-          {:"::", _, [{arg_name, _, _}, _spec]} ->
-            Macro.var(arg_name, nil)
-
-          _ ->
-            raise ArgumentError,
-                  "argument spec must be in the form `arg :: spec`, " <>
-                    "got: `#{Macro.to_string(arg_spec)}`"
+        for {value, {_name, spec}} <- Enum.zip(unquote(args), contract.args) do
+          Norm.conform!(value, spec)
         end
-      end
 
-    conform_args =
-      for {:"::", _, [{arg_name, _, _}, spec]} <- arg_specs do
-        arg = Macro.var(arg_name, nil)
-
-        quote do
-          Norm.conform!(unquote(arg), unquote(spec))
-        end
+        result = super(unquote_splicing(args))
+        Norm.conform!(result, contract.result)
       end
+    end
+  end
 
-    conform_args = {:__block__, [], conform_args}
-    result = Macro.var(:result, nil)
-    call = {name, call_meta, arg_vars}
+  defp defcontract(expr, line) do
+    if Application.get_env(:norm, :enable_contracts, true) do
+      {name, args, result} = parse_contract_expr(expr)
+      arity = length(args)
 
-    quote do
-      @norm_contracts unquote(fa(call))
+      quote do
+        @doc false
+        def __contract__({unquote(name), unquote(arity)}) do
+          %Norm.Contract{args: unquote(args), result: unquote(result)}
+        end
 
-      def unquote(call_with_contract(call)) do
-        unquote(conform_args)
-        unquote(result) = unquote(call_without_contract(call))
-        Norm.conform!(unquote(result), unquote(result_spec))
-        unquote(result)
+        @norm_contracts {unquote(name), unquote(arity), unquote(line)}
       end
     end
   end
 
-  ## Utilities
-
-  defp fix_call_parens({name, meta, nil}), do: {name, meta, []}
-  defp fix_call_parens({name, meta, args}), do: {name, meta, args}
-
-  defp wrapper_call(call) do
-    {name, meta, args} = fix_call_parens(call)
-    args = for {_, index} <- Enum.with_index(args), do: Macro.var(:"arg#{index}", nil)
-    {name, meta, args}
+  defp parse_contract_expr([{:"::", _, [{name, _, args}, result]}]) do
+    args = args |> Enum.with_index(1) |> Enum.map(&parse_arg/1)
+    {name, args, result}
   end
 
-  defp wrapper_body(call) do
-    {name, meta, args} = fix_call_parens(call)
-    args = for {_, index} <- Enum.with_index(args), do: Macro.var(:"arg#{index}", nil)
-    {:"__#{name}_with_contract__", meta, args}
-  end
+  defp parse_contract_expr(expr) do
+    actual = Macro.to_string({:@, [], [{:contract, [], expr}]})
 
-  defp call_with_contract(call) do
-    {name, meta, args} = call
-    {:"__#{name}_with_contract__", meta, args}
+    raise ArgumentError,
+          "contract must be in the form " <>
+            "`@contract function(arg1, arg2) :: spec`, got: `#{actual}`"
   end
 
-  defp call_without_contract(call) do
-    {name, meta, args} = call
-    {:"__#{name}_without_contract__", meta, args}
+  defp parse_arg({{:"::", _, [{name, _, _}, spec]}, _index}) do
+    {name, spec}
   end
 
-  defp fa(call) do
-    {name, _meta, args} = fix_call_parens(call)
-    {name, length(args)}
+  defp parse_arg({spec, index}) do
+    {:"arg#{index}", spec}
   end
 end


Test patch:
diff --git a/test/norm/contract_test.exs b/test/norm/contract_test.exs
index 2489c3c..4004a07 100644
--- a/test/norm/contract_test.exs
+++ b/test/norm/contract_test.exs
@@ -49,17 +49,6 @@ defmodule Norm.ContractTest do
     end
   end
 
-  test "bad arg" do
-    assert_raise ArgumentError, ~r/`arg :: spec`, got: `spec\(is_integer\(\)\)`/, fn ->
-      defmodule BadArg do
-        use Norm
-
-        @contract foo(spec(is_integer())) :: spec(is_integer())
-        def foo(n), do: n
-      end
-    end
-  end
-
   test "no function" do
     assert_raise ArgumentError, "contract for undefined function foo/0", fn ->
       defmodule NoFunction do
@@ -100,4 +89,20 @@ defmodule Norm.ContractTest do
     assert WithoutParentheses2.fun(50) == 100
     assert WithoutParentheses2.other() == "Hello, world!"
   end
+
+  test "reflection" do
+    defmodule Reflection do
+      use Norm
+
+      def int(), do: spec(is_integer())
+
+      @contract foo(a :: int(), int()) :: int()
+      def foo(a, b), do: a + b
+    end
+
+    contract = Reflection.__contract__({:foo, 2})
+
+    assert inspect(contract) ==
+             "%Norm.Contract{args: [a: #Norm.Spec<is_integer()>, arg2: #Norm.Spec<is_integer()>], result: #Norm.Spec<is_integer()>}"
+  end
 end