Repository: dashbitco/nimble_options (elixir)

Issue:
Passing a "context" for nested options
I have a spec that looks like this:

```elixir
  cluster_options_schema = [
    connections: [type: :pos_integer, default: 2],
    urls: [type: {:custom, __MODULE__, :validate_cluster_urls, []}, required: true]
  ]

  @start_options_schema [
    general_cluster: [type: :keyword_list, keys: cluster_options_schema],
    messages_cluster: [type: :keyword_list, keys: cluster_options_schema],
    ephemeral_cluster: [type: :keyword_list, keys: cluster_options_schema]
  ]
```

Now if I validate this and don't pass the required `:url` option for one of the three clusters, like this:

```elixir
NimbleOptions.validate([general_cluster: []], @start_options_schema)
```

then the returned error doesn't point me to which _cluster_'s options are invalid:

```elixir
{:error, "required option :urls not found, received options: [:channels_per_connection, :connections]"}
```

Should we have a `:context` key that is supported in `:keyword_list`s? Like this:

```elixir
  @start_options_schema [
    general_cluster: [type: :keyword_list, keys: cluster_options_schema, context: :general_cluster],
  ]
```

The error could become:

```elixir
{:error, "required option :urls not found, received options: [:channels_per_connection, :connections] (in :general_cluster)"}
```

Thoughts on how to achieve this, if this belongs in nimble_options, or if there's a better way to do it?

Pull request description:
Support a context when showing error messages
Closes #14.

Implementation is kind of thrown out there, didn't spend too much time making things nice 😄 I'm also very interested in what format you think would work best. Not sure my choice of wording/error message is clear to read. Any ways, this is a first pass to get something working at least, because without this my experience has been that it's really hard to figure out where an error message is coming from. :)

cc @josevalim @msaraiva 

Patch:
diff --git a/lib/nimble_options.ex b/lib/nimble_options.ex
index 8db793d..e2d34f7 100644
--- a/lib/nimble_options.ex
+++ b/lib/nimble_options.ex
@@ -121,7 +121,7 @@ defmodule NimbleOptions do
       ...> ]
       ...>
       ...> NimbleOptions.validate(config, schema)
-      {:error, "required option :module not found, received options: [:concurrency]"}
+      {:error, "(in options [:producer]) required option :module not found, received options: [:concurrency]"}
 
   ## Nested option items
 
@@ -154,7 +154,7 @@ defmodule NimbleOptions do
       ...> ]
       ...>
       ...> NimbleOptions.validate(config, schema)
-      {:error, "expected :interval to be a positive integer, got: :oops!"}
+      {:error, "(in options [:producer, :rate_limiting]) expected :interval to be a positive integer, got: :oops!"}
 
   """
 
@@ -187,13 +187,14 @@ defmodule NimbleOptions do
   @spec validate(keyword(), schema()) ::
           {:ok, validated_options :: keyword()} | {:error, reason :: String.t()}
   def validate(options, schema) do
-    case validate_options_with_schema([root: schema], root: options_schema()) do
-      {:error, message} ->
-        raise ArgumentError,
-              "invalid schema given to NimbleOptions.validate/2. Reason: #{message}"
+    case validate_options_with_schema([root: schema], [root: options_schema()], _path = []) do
+      {:ok, _validated_schema} ->
+        validate_options_with_schema_and_path(options, schema)
 
-      _ ->
-        validate_options_with_schema(options, schema)
+      {:error, message, [:root | path]} ->
+        raise ArgumentError,
+              "invalid schema given to NimbleOptions.validate/2, in options #{inspect(path)}. " <>
+                "Reason: #{message}"
     end
   end
 
@@ -220,10 +221,29 @@ defmodule NimbleOptions do
     @options_schema
   end
 
-  defp validate_options_with_schema(opts, schema) do
-    case validate_unknown_options(opts, schema) do
-      :ok -> validate_options(schema, opts)
-      error -> error
+  defp validate_options_with_schema_and_path(opts, schema) do
+    case validate_options_with_schema(opts, schema, _path = []) do
+      {:ok, options} ->
+        {:ok, options}
+
+      {:error, message, []} ->
+        {:error, message}
+
+      {:error, message, path} ->
+        {:error, "(in options #{inspect(path)}) " <> message}
+    end
+  end
+
+  defp validate_options_with_schema(opts, schema, path) do
+    with :ok <- validate_unknown_options(opts, schema),
+         {:ok, options} <- validate_options(schema, opts) do
+      {:ok, options}
+    else
+      {:error, message} ->
+        {:error, message, path}
+
+      {:error, message, sub_path} ->
+        {:error, message, path ++ sub_path}
     end
   end
 
@@ -241,7 +261,7 @@ defmodule NimbleOptions do
 
   defp validate_options(schema, opts) do
     case Enum.reduce_while(schema, opts, &reduce_options/2) do
-      {:error, _} = result -> result
+      {:error, _, _} = result -> result
       result -> {:ok, result}
     end
   end
@@ -257,7 +277,7 @@ defmodule NimbleOptions do
 
   defp reduce_options({key, schema_opts}, opts) do
     case validate_option(opts, key, schema_opts) do
-      {:error, _} = result ->
+      {:error, _message, _path} = result ->
         {:halt, result}
 
       {:ok, value} ->
@@ -274,14 +294,20 @@ defmodule NimbleOptions do
   end
 
   defp validate_option(opts, key, schema) do
-    with {:ok, value} <- validate_value(opts, key, schema),
-         :ok <- validate_type(schema[:type], key, value) do
-      if schema[:keys] do
-        keys = normalize_keys(schema[:keys], value)
-        validate_options_with_schema(value, keys)
-      else
-        {:ok, value}
+    result =
+      with {:ok, value} <- validate_value(opts, key, schema),
+           :ok <- validate_type(schema[:type], key, value) do
+        if schema[:keys] do
+          keys = normalize_keys(schema[:keys], value)
+          validate_options_with_schema(value, keys, _path = [key])
+        else
+          {:ok, value}
+        end
       end
+
+    case result do
+      {:error, message} -> {:error, message, _path = []}
+      other -> other
     end
   end
 


Test patch:
diff --git a/test/nimble_options_test.exs b/test/nimble_options_test.exs
index 3d10a54..c2d8f3b 100644
--- a/test/nimble_options_test.exs
+++ b/test/nimble_options_test.exs
@@ -25,7 +25,7 @@ defmodule NimbleOptionsTest do
       opts = [stages: 1]
 
       message = """
-      invalid schema given to NimbleOptions.validate/2. \
+      invalid schema given to NimbleOptions.validate/2, in options [:stages]. \
       Reason: invalid option type :foo.
 
       Available types: :any, :keyword_list, :non_empty_keyword_list, :atom, \
@@ -55,7 +55,7 @@ defmodule NimbleOptionsTest do
       ]
 
       message = """
-      invalid schema given to NimbleOptions.validate/2. \
+      invalid schema given to NimbleOptions.validate/2, in options [:producers, :keys, :*, :keys, :module]. \
       Reason: unknown options [:unknown_schema_option], \
       valid options are: [:type, :required, :default, :keys, \
       :deprecated, :rename_to, :doc, :subsection]\
@@ -444,7 +444,7 @@ defmodule NimbleOptionsTest do
 
       assert NimbleOptions.validate(opts, schema) ==
                {:error,
-                "unknown options [:unknown_option1, :unknown_option2], valid options are: [:stages, :min_demand]"}
+                "(in options [:processors]) unknown options [:unknown_option1, :unknown_option2], valid options are: [:stages, :min_demand]"}
     end
 
     test "options with default values" do
@@ -492,7 +492,8 @@ defmodule NimbleOptionsTest do
       opts = [processors: [max_demand: 1]]
 
       assert NimbleOptions.validate(opts, schema) ==
-               {:error, "required option :stages not found, received options: [:max_demand]"}
+               {:error,
+                "(in options [:processors]) required option :stages not found, received options: [:max_demand]"}
     end
 
     test "nested options types" do
@@ -509,7 +510,8 @@ defmodule NimbleOptionsTest do
       opts = [processors: [name: MyModule, stages: :an_atom]]
 
       assert NimbleOptions.validate(opts, schema) ==
-               {:error, "expected :stages to be a positive integer, got: :an_atom"}
+               {:error,
+                "(in options [:processors]) expected :stages to be a positive integer, got: :an_atom"}
     end
   end
 
@@ -554,7 +556,8 @@ defmodule NimbleOptionsTest do
       opts = [producers: [producer1: [module: MyModule, arg: :ok, unknown_option: 1]]]
 
       assert NimbleOptions.validate(opts, schema) ==
-               {:error, "unknown options [:unknown_option], valid options are: [:module, :arg]"}
+               {:error,
+                "(in options [:producers, :producer1]) unknown options [:unknown_option], valid options are: [:module, :arg]"}
     end
 
     test "options with default values" do
@@ -617,7 +620,8 @@ defmodule NimbleOptionsTest do
       opts = [producers: [default: [module: MyModule]]]
 
       assert NimbleOptions.validate(opts, schema) ==
-               {:error, "required option :arg not found, received options: [:module]"}
+               {:error,
+                "(in options [:producers, :default]) required option :arg not found, received options: [:module]"}
     end
 
     test "nested options types" do
@@ -646,7 +650,8 @@ defmodule NimbleOptionsTest do
       ]
 
       assert NimbleOptions.validate(opts, schema) ==
-               {:error, "expected :stages to be a positive integer, got: :an_atom"}
+               {:error,
+                "(in options [:producers, :producer1]) expected :stages to be a positive integer, got: :an_atom"}
     end
 
     test "validate empty keys for :non_empty_keyword_list" do
@@ -719,6 +724,30 @@ defmodule NimbleOptionsTest do
     end
   end
 
+  describe "nested options show up in error messages" do
+    test "for options that we validate" do
+      schema = [
+        socket_options: [
+          type: :keyword_list,
+          keys: [
+            certificates: [
+              type: :keyword_list,
+              keys: [
+                path: [type: :string]
+              ]
+            ]
+          ]
+        ]
+      ]
+
+      opts = [socket_options: [certificates: [path: :not_a_string]]]
+
+      assert NimbleOptions.validate(opts, schema) ==
+               {:error,
+                "(in options [:socket_options, :certificates]) expected :path to be an string, got: :not_a_string"}
+    end
+  end
+
   describe "docs" do
     test "override docs for recursive keys" do
       docs = """