Merge pull request #517 from tsloughter/dict_parse

catch error if config can't be parsed into dict to print error message

6 files changed, 53 insertions, 18 deletions

diff --git a/src/rebar3.erl b/src/rebar3.erl
index 4e90d82..e324687 100644
--- a/src/rebar3.erl
+++ b/src/rebar3.erl
@@ -120,8 +120,7 @@ init_config() ->
                  ConfigFile ->
                      rebar_config:consult_file(ConfigFile)
              end,
-
-    Config1 = rebar_config:merge_locks(Config, rebar_config:consult_file(?LOCK_FILE)),
+    Config1 = rebar_config:merge_locks(Config, rebar_config:consult_lock_file(?LOCK_FILE)),
 
     %% If $HOME/.config/rebar3/config exists load and use as global config
     GlobalConfigFile = rebar_dir:global_config(),
@@ -129,7 +128,8 @@ init_config() ->
                 true ->
                     ?DEBUG("Load global config file ~p",
                            [GlobalConfigFile]),
-                    GlobalConfig = rebar_state:new(rebar_config:consult_file(GlobalConfigFile)),
+                    GlobalConfigTerms = rebar_config:consult_file(GlobalConfigFile),
+                    GlobalConfig = rebar_state:new(GlobalConfigTerms),
 
                     %% We don't want to worry about global plugin install state effecting later
                     %% usage. So we throw away the global profile state used for plugin install.
diff --git a/src/rebar_config.erl b/src/rebar_config.erl
index b8222bc..3e06c38 100644
--- a/src/rebar_config.erl
+++ b/src/rebar_config.erl
@@ -27,7 +27,10 @@
 -module(rebar_config).
 
 -export([consult/1
+        ,consult_app_file/1
         ,consult_file/1
+        ,consult_lock_file/1
+        ,verify_config_format/1
         ,format_error/1
 
         ,merge_locks/2]).
@@ -43,10 +46,21 @@
 consult(Dir) ->
     consult_file(filename:join(Dir, ?DEFAULT_CONFIG_FILE)).
 
--spec consult_file(file:name()) -> [any()].
-consult_file(File) when is_binary(File) ->
-    consult_file(binary_to_list(File));
+consult_app_file(File) ->
+    consult_file_(File).
+
+consult_lock_file(File) ->
+    consult_file_(File).
+
 consult_file(File) ->
+    Terms = consult_file_(File),
+    true = verify_config_format(Terms),
+    Terms.
+
+-spec consult_file_(file:name()) -> [any()].
+consult_file_(File) when is_binary(File) ->
+    consult_file_(binary_to_list(File));
+consult_file_(File) ->
     case filename:extension(File) of
         ".script" ->
             consult_and_eval(remove_script_ext(File), File);
@@ -61,6 +75,13 @@ consult_file(File) ->
             end
     end.
 
+verify_config_format([]) ->
+    true;
+verify_config_format([{_Key, _Value} | T]) ->
+    verify_config_format(T);
+verify_config_format([Term | _]) ->
+    throw(?PRV_ERROR({bad_config_format, Term})).
+
 %% no lockfile
 merge_locks(Config, []) ->
     Config;
@@ -78,6 +99,8 @@ merge_locks(Config, [Locks]) ->
     NewDeps = find_newly_added(ConfigDeps, Locks),
     [{{locks, default}, Locks}, {{deps, default}, NewDeps++Deps} | Config].
 
+format_error({bad_config_format, Term}) ->
+    io_lib:format("Unable to parse config. Term is not in {Key, Value} format:~n~p", [Term]);
 format_error({bad_dep_name, Dep}) ->
     io_lib:format("Dependency name must be an atom, instead found: ~p", [Dep]).
 
diff --git a/src/rebar_otp_app.erl b/src/rebar_otp_app.erl
index a8c2d62..9f61e71 100644
--- a/src/rebar_otp_app.erl
+++ b/src/rebar_otp_app.erl
@@ -203,7 +203,7 @@ consult_app_file(Filename) ->
                 false ->
                     file:consult(Filename);
                 true ->
-                    {ok, rebar_config:consult_file(Filename)}
+                    {ok, rebar_config:consult_app_file(Filename)}
             end
     end.
 
diff --git a/src/rebar_state.erl b/src/rebar_state.erl
index 5b59eeb..7c61db2 100644
--- a/src/rebar_state.erl
+++ b/src/rebar_state.erl
@@ -1,6 +1,7 @@
 -module(rebar_state).
 
 -export([new/0, new/1, new/2, new/3,
+
          get/2, get/3, set/3,
 
          has_all_artifacts/1,
@@ -42,6 +43,7 @@
          providers/1, providers/2, add_provider/2]).
 
 -include("rebar.hrl").
+-include_lib("providers/include/providers.hrl").
 
 -record(state_t, {dir                               :: file:name(),
                   opts                = dict:new()  :: rebar_dict(),
@@ -80,7 +82,9 @@ new() ->
 new(Config) when is_list(Config) ->
     BaseState = base_state(),
     Deps = proplists:get_value(deps, Config, []),
-    Opts = dict:from_list([{{deps, default}, Deps} | Config]),
+    Terms = [{{deps, default}, Deps} | Config],
+    true = rebar_config:verify_config_format(Terms),
+    Opts = dict:from_list(Terms),
     BaseState#state_t { dir = rebar_dir:get_cwd(),
                         default = Opts,
                         opts = Opts }.
@@ -90,7 +94,10 @@ new(Profile, Config) when is_atom(Profile)
                         , is_list(Config) ->
     BaseState = base_state(),
     Deps = proplists:get_value(deps, Config, []),
-    Opts = dict:from_list([{{deps, default}, Deps} | Config]),
+
+    Terms = [{{deps, default}, Deps} | Config],
+    true = rebar_config:verify_config_format(Terms),
+    Opts = dict:from_list(Terms),
     BaseState#state_t { dir = rebar_dir:get_cwd(),
                         current_profiles = [Profile],
                         default = Opts,
@@ -103,15 +110,20 @@ new(ParentState=#state_t{}, Config) ->
 -spec new(t(), list(), file:name()) -> t().
 new(ParentState, Config, Dir) ->
     Opts = ParentState#state_t.opts,
-    LocalOpts = case rebar_config:consult_file(filename:join(Dir, ?LOCK_FILE)) of
+    LocalOpts = case rebar_config:consult_lock_file(filename:join(Dir, ?LOCK_FILE)) of
                     [D] ->
-                        %% We want the top level deps only from the lock file.
-                        %% This ensures deterministic overrides for configs.
-                        Deps = [X || X <- D, element(3, X) =:= 0],
-                        dict:from_list([{{locks, default}, D}, {{deps, default}, Deps} | Config]);
+                    %% We want the top level deps only from the lock file.
+                    %% This ensures deterministic overrides for configs.
+                    Deps = [X || X <- D, element(3, X) =:= 0],
+
+                    Terms = [{{locks, default}, D}, {{deps, default}, Deps} | Config],
+                    true = rebar_config:verify_config_format(Terms),
+                    dict:from_list(Terms);
                     _ ->
-                        D = proplists:get_value(deps, Config, []),
-                        dict:from_list([{{deps, default}, D} | Config])
+                    D = proplists:get_value(deps, Config, []),
+                    Terms = [{{deps, default}, D} | Config],
+                    true = rebar_config:verify_config_format(Terms),
+                    dict:from_list(Terms)
                 end,
 
     NewOpts = merge_opts(LocalOpts, Opts),
diff --git a/test/rebar_deps_SUITE.erl b/test/rebar_deps_SUITE.erl
index 6b902a5..afd487e 100644
--- a/test/rebar_deps_SUITE.erl
+++ b/test/rebar_deps_SUITE.erl
@@ -215,7 +215,7 @@ newly_added_dep(Config) ->
     {ok, RebarConfig2} = file:consult(rebar_test_utils:create_config(AppDir, [{deps, TopDeps2}])),
     LockFile = filename:join(AppDir, "rebar.lock"),
     RebarConfig3 = rebar_config:merge_locks(RebarConfig2,
-                                           rebar_config:consult_file(LockFile)),
+                                           rebar_config:consult_lock_file(LockFile)),
 
     %% a should now be installed and c should not change
     rebar_test_utils:run_and_check(
diff --git a/test/rebar_test_utils.erl b/test/rebar_test_utils.erl
index f53ba20..9c181d8 100644
--- a/test/rebar_test_utils.erl
+++ b/test/rebar_test_utils.erl
@@ -164,7 +164,7 @@ check_results(AppDir, Expected) ->
     GlobalPluginDirs = filelib:wildcard(filename:join([AppDir, "global", "plugins"])),
     CheckoutsDir = filename:join([AppDir, "_checkouts"]),
     LockFile = filename:join([AppDir, "rebar.lock"]),
-    Locks = lists:flatten(rebar_config:consult_file(LockFile)),
+    Locks = lists:flatten(rebar_config:consult_lock_file(LockFile)),
 
     InvalidApps = rebar_app_discover:find_apps(BuildDirs, invalid),
     ValidApps = rebar_app_discover:find_apps(BuildDirs, valid),