Merge pull request #118 from inaka/jfacorro.107.improve.error.handlin…

…g.and.reporting

[Closes #107] Improve error handling and reporting.

Makefile

@@ -22,7 +22,7 @@ ERLC_OPTS += +warn_export_vars +warn_exported_vars +warn_missing_spec +warn_unty
 # Commont Test Config
 
 TEST_ERLC_OPTS += +'{parse_transform, lager_transform}'
-CT_SUITES = project elvis style git
+CT_SUITES = elvis style project git
 CT_OPTS = -cover test/elvis.coverspec  -erl_args -config config/test.config
 
 # Builds the elvis escript.

src/elvis.erl

@@ -36,7 +36,7 @@ main(Args) ->
         {ok, {Options, Commands}} ->
             process_options(Options, Commands);
         {error, {Reason, Data}} ->
-            io:format("Error: ~s ~p~n~n", [Reason, Data]),
+            elvis_utils:error_prn("~s ~p~n~n", [Reason, Data]),
             help()
     end.
 
@@ -48,36 +48,42 @@ rock() ->
     rock(Config).
 
 -spec rock(elvis_config:config()) -> ok | {fail, [elvis_result:file()]}.
-rock(Config) when is_list(Config) ->
-    Results = lists:map(fun rock/1, Config),
-    lists:foldl(fun combine_results/2, ok, Results);
-rock(Config = #{files := Files, rules := _Rules}) ->
-    elvis_utils:info("Loading files...~n"),
-    Fun = fun (File) ->
-                  Path = elvis_file:path(File),
-                  elvis_utils:info("Loading ~s~n", [Path]),
-                  elvis_file:load_file_data(Config, File)
-          end,
+rock(Config) ->
+    elvis_config:validate(Config),
+    NewConfig = elvis_config:normalize(Config),
+    Results = lists:map(fun do_rock/1, NewConfig),
+    lists:foldl(fun combine_results/2, ok, Results).
+
+%% @private
+-spec do_rock(elvis_config:config()) -> ok | {fail, [elvis_result:file()]}.
+do_rock(Config0) ->
+    elvis_utils:info("Loading files..."),
+    Config = elvis_config:resolve_files(Config0),
+    Files = elvis_config:files(Config),
+    Fun = fun (File) -> load_file_data(Config, File) end,
     LoadedFiles = lists:map(Fun, Files),
-
-    elvis_utils:info("Applying rules...~n"),
+    elvis_utils:info("Applying rules..."),
     Results = [apply_rules(Config, File) || File <- LoadedFiles],
 
     case elvis_result:status(Results) of
         fail -> {fail, Results};
         ok -> ok
-    end;
-rock(Config = #{dirs := _Dirs, rules := _Rules}) ->
-    NewConfig = elvis_config:resolve_files(Config),
-    rock(NewConfig);
-rock(Config = #{src_dirs := Dirs}) ->
-    %% NOTE: Provided for backwards compatibility.
-    %% Rename 'src_dirs' key to 'dirs'.
-    Config1 = maps:remove(src_dirs, Config),
-    Config2 = Config1#{dirs => Dirs},
-    rock(Config2);
-rock(Config) ->
-    throw({invalid_config, Config}).
+    end.
+
+%% @private
+-spec load_file_data(elvis_config:config(), elvis_file:file()) ->
+    elvis_file:file().
+load_file_data(Config, File) ->
+    Path = elvis_file:path(File),
+    elvis_utils:info("Loading ~s", [Path]),
+    try
+        elvis_file:load_file_data(Config, File)
+    catch
+        _:Reason ->
+            Msg = "~p when loading file ~p.",
+            elvis_utils:error_prn(Msg, [Reason, Path]),
+            File
+    end.
 
 %%% Git-Hook Command
 
@@ -120,7 +126,8 @@ combine_results({fail, ItemResults}, {fail, AccResults}) ->
 
 -spec apply_rules(elvis_config:config(), elvis_file:file()) ->
     elvis_result:file().
-apply_rules(Config = #{rules := Rules}, File) ->
+apply_rules(Config, File) ->
+    Rules = elvis_config:rules(Config),
     Acc = {[], Config, File},
     {RulesResults, _, _} = lists:foldl(fun apply_rule/2, Acc, Rules),
 
@@ -129,9 +136,14 @@ apply_rules(Config = #{rules := Rules}, File) ->
     Results.
 
 apply_rule({Module, Function, Args}, {Result, Config, File}) ->
-    Results = Module:Function(Config, File, Args),
-    RuleResult = elvis_result:new(rule, Function, Results),
-
+    RuleResult = try
+                     Results = Module:Function(Config, File, Args),
+                     elvis_result:new(rule, Function, Results)
+                 catch
+                     _:Reason ->
+                         Msg = "'~p' while applying rule '~p'.",
+                         elvis_result:new(error, Msg, [Reason, Function])
+                 end,
     {[RuleResult | Result], Config, File}.
 
 %%% Command Line Interface
@@ -155,7 +167,7 @@ process_options(Options, Commands) ->
         process_options(Options, AtomCommands, Config)
     catch
         throw:Exception ->
-            io:format("Error: ~p.~n", [Exception])
+            elvis_utils:error_prn("~p.", [Exception])
     end.
 
 -spec process_options([atom()], [string()], elvis_config:config()) -> ok.

src/elvis_code.erl

@@ -734,7 +734,6 @@ to_map({type, Attrs, constraint, [Sub, SubType]}) ->
                  subtype => Sub},
       content => to_map(SubType)};
 to_map({type, Attrs, bounded_fun, [FunType, Defs]}) ->
-    %% io:format("~p~n", [Defs]),
     #{type => type,
       attrs => #{location => get_location(Attrs),
                  text => get_text(Attrs),

src/elvis_config.erl

@@ -4,7 +4,13 @@
          default/0,
          load_file/1,
          load/1,
+         validate/1,
+         normalize/1,
+         %% Geters
          dirs/1,
+         files/1,
+         rules/1,
+         %% Files
          resolve_files/1,
          resolve_files/2,
          apply_to_files/2
@@ -55,6 +61,40 @@ ensure_config_list(Config) when is_map(Config) ->
 ensure_config_list(Config) ->
     Config.
 
+-spec validate(config()) -> ok | {error, term()}.
+validate([]) ->
+    throw({invalid_config, empty_config});
+validate(Config) when is_list(Config) ->
+    lists:foreach(fun validate/1, Config);
+validate(RuleGroup) ->
+    case maps:is_key(src_dirs, RuleGroup) or maps:is_key(dirs, RuleGroup) of
+        false -> throw({invalid_config, {missing_dirs, RuleGroup}});
+        true -> ok
+    end,
+    case maps:is_key(dirs, RuleGroup) of
+        true ->
+            case maps:is_key(filter, RuleGroup) of
+                false -> throw({invalid_config, {missing_filter, RuleGroup}});
+                true -> ok
+            end;
+        false -> ok
+    end,
+    case maps:is_key(rules, RuleGroup) of
+        false -> throw({invalid_config, {missing_rules, RuleGroup}});
+        true -> ok
+    end.
+
+-spec normalize(config()) -> config().
+normalize(Config) when is_list(Config) ->
+    lists:map(fun normalize/1, Config);
+normalize(Config = #{src_dirs := Dirs}) ->
+    %% NOTE: Provided for backwards compatibility.
+    %% Rename 'src_dirs' key to 'dirs'.
+    Config1 = maps:remove(src_dirs, Config),
+    Config1#{dirs => Dirs};
+normalize(Config) ->
+    Config.
+
 -spec dirs(config()) -> [string()].
 dirs(Config) when is_list(Config) ->
     lists:flatmap(fun dirs/1, Config);
@@ -63,6 +103,22 @@ dirs(_RuleGroup = #{dirs := Dirs}) ->
 dirs(#{}) ->
     [].
 
+-spec files(config()) -> [string()].
+files(_RuleGroup = #{files := Files}) ->
+    Files;
+files(#{}) ->
+    undefined.
+
+-spec rules(config()) -> [string()].
+rules(_RuleGroup = #{rules := Rules}) ->
+    Rules;
+rules(#{}) ->
+    undefined.
+
+%% @doc Takes a configuration and a list of files, filtering some
+%%      of them according to the 'filter' key, or if not specified
+%%      uses '*.erl'.
+%% @end
 -spec resolve_files(config(), [elvis_file:file()]) -> config().
 resolve_files(Config, Files) when is_list(Config) ->
     Fun = fun(RuleGroup) -> resolve_files(RuleGroup, Files) end,
@@ -75,16 +131,24 @@ resolve_files(RuleGroup, Files) ->
     FilteredFiles = elvis_file:filter_files(Files, ?DEFAULT_FILTER),
     RuleGroup#{files => FilteredFiles}.
 
+%% @doc Takes a configuration and finds all files according to its 'dirs'
+%%      end  'filter' key, or if not specified uses '*.erl'.
+%% @end
 -spec resolve_files(config()) -> config().
 resolve_files(Config) when is_list(Config) ->
     lists:map(fun resolve_files/1, Config);
+resolve_files(RuleGroup = #{files := _Files}) ->
+    RuleGroup;
 resolve_files(RuleGroup = #{dirs := Dirs, filter := Filter}) ->
     Files = elvis_file:find_files(Dirs, Filter, local),
     RuleGroup#{files => Files};
 resolve_files(RuleGroup = #{dirs := Dirs}) ->
     Files = elvis_file:find_files(Dirs, ?DEFAULT_FILTER),
     RuleGroup#{files => Files}.
 
+%% @doc Takes a function and configuration and applies the function to all
+%%      file in the configuration.
+%% @end
 -spec apply_to_files(fun(), config()) -> config().
 apply_to_files(Fun, Config) when is_list(Config) ->
     ApplyFun = fun(RuleGroup) -> apply_to_files(Fun, RuleGroup) end,

src/elvis_result.erl

@@ -58,7 +58,9 @@ new(item, Msg, Info) ->
 new(rule, Name, Results) ->
     #{name => Name, items => Results};
 new(file, File, Rules) ->
-    #{file => File, rules => Rules}.
+    #{file => File, rules => Rules};
+new(error, Msg, Info) ->
+    #{error_msg => Msg, info => Info}.
 
 -spec new(item, term(), any(), any()) -> item().
 new(item, Msg, Info, LineNum) ->
@@ -100,21 +102,24 @@ print([Result | Results]) ->
 print(#{file := File, rules := Rules}) ->
     Path = elvis_file:path(File),
     Status = case status(Rules) of
-                 ok -> "OK";
-                 fail -> "FAIL"
+                 ok -> "{{green-bold}}OK";
+                 fail -> "{{red-bold}}FAIL"
              end,
 
-    elvis_utils:info("# ~s [~s]~n", [Path, Status]),
+    elvis_utils:notice("# ~s [~s{{white-bold}}]", [Path, Status]),
     print(Rules);
-
+%% Rule
 print(#{items := []}) ->
     ok;
 print(#{name := Name, items := Items}) ->
-    elvis_utils:info("  - ~s~n", [atom_to_list(Name)]),
+    elvis_utils:notice("  - ~s", [atom_to_list(Name)]),
     print(Items);
-
+%% Item
 print(#{message := Msg, info := Info}) ->
-    elvis_utils:info("    - " ++ Msg ++ "~n", Info).
+    elvis_utils:notice("    - " ++ Msg, Info);
+%% Error
+print(#{error_msg := Msg, info := Info}) ->
+    elvis_utils:error_prn(Msg, Info).
 
 -spec status([rule()]) -> ok | fail.
 status([]) ->

src/elvis_utils.erl

@@ -14,7 +14,12 @@
 
          %% Output
          info/1,
-         info/2
+         info/2,
+         notice/1,
+         notice/2,
+         error_prn/1,
+         error_prn/2,
+         parse_colors/1
         ]).
 
 -export_type([file/0]).
@@ -130,7 +135,50 @@ info(Message) ->
 
 -spec info(string(), [term()]) -> ok.
 info(Message, Args) ->
+    ColoredMessage = Message ++ "{{reset}}~n",
+    print(ColoredMessage, Args).
+
+-spec notice(string()) -> ok.
+notice(Message) ->
+    notice(Message, []).
+
+-spec notice(string(), [term()]) -> ok.
+notice(Message, Args) ->
+    ColoredMessage = "{{white-bold}}" ++ Message ++ "{{reset}}~n",
+    print(ColoredMessage, Args).
+
+
+-spec error_prn(string()) -> ok.
+error_prn(Message) ->
+    error_prn(Message, []).
+
+-spec error_prn(string(), [term()]) -> ok.
+error_prn(Message, Args) ->
+    ColoredMessage = "{{red}}Error: {{reset}}" ++ Message ++ "{{reset}}~n",
+    print(ColoredMessage, Args).
+
+print(Message, Args) ->
     case application:get_env(elvis, no_output) of
         {ok, true} -> ok;
-        _ -> io:format(Message, Args)
+        _ ->
+            Output = io_lib:format(Message, Args),
+            io:format(parse_colors(Output))
     end.
+
+
+-spec parse_colors(string()) -> string().
+parse_colors(Message) ->
+    Colors = #{"red" => "\e[0;31m",
+               "red-bold" => "\e[1;31m",
+               "green" => "\e[0;32m",
+               "green-bold" => "\e[1;32m",
+               "white" => "\e[0;37m",
+               "white-bold" => "\e[1;37m",
+               "reset" => "\e[0m"},
+    Opts = [global, {return, list}],
+    Fun = fun(Key, Acc) ->
+                  Regex = ["{{", Key, "}}"],
+                  Color = maps:get(Key, Colors),
+                  re:replace(Acc, Regex, Color, Opts)
+          end,
+    lists:foldl(Fun, Message, maps:keys(Colors)).