additional checks

boundary · Apr 6, 2012 · 21e9045 · 21e9045
1 parent 6ce1501
commit 21e9045
Show file tree

Hide file tree

Showing 3 changed files with 70 additions and 47 deletions.
diff --git a/README.md b/README.md
@@ -1,6 +1,6 @@
 ### Erlang Childspec Validator
 
-Validate your childspecs before you use them. This will ensure things start properly and increase the probability hot upgrades will be performed correctly.
+Validate your childspecs before you use them. It attempts to find the most common mistakes in childspecs and let you know about them. This will ensure things start properly and increase the probability hot upgrades will be performed correctly.
 
 ### Usage
 

diff --git a/src/childspec_validator.erl b/src/childspec_validator.erl
@@ -3,52 +3,67 @@
 -export([validate/1]).
 
 validate({Id, {M, F, A}, Restart, Shutdown, Type, Modules}) when is_atom(Id) ->
-    MFARes = validate_and_error(check_mfa(M, F, A),
+    case code:ensure_loaded(M) of
-                                "Childspec validation error [~p]:"
+        {module, _} ->
-                                " MFA check failed, the arity or"
+            MFARes = validate_and_error(check_mfa(M, F, A),
-                                " function name is incorrect.~n",
+                                        "Childspec validation error [~p]:"
-                                [Id]),
+                                        " MFA check failed, make sure the"
-
+                                        " arity and function name are correct~n",
-    RestartRes = validate_and_error(check_restart(Restart),
+                                        [Id]),
-                                    "Childspec validation error [~p]:"
+
-                                    " Restart is not permanent, temporary"
+            RestartRes = validate_and_error(check_restart(Restart),
-                                    " or transient.~n",
+                                            "Childspec validation error [~p]:"
-                                    [Id]),
+                                            " Restart is not permanent, temporary"
-
+                                            " or transient.~n",
-    ShutdownRes = validate_and_error(check_shutdown(Shutdown),
+                                            [Id]),
-                                     "Childspec validation error [~p]:"
+
-                                     " Shutdown is not brutal_kill,"
+            ShutdownRes = validate_and_error(check_shutdown(Shutdown),
-                                     " infinity or an integer.~n",
+                                             "Childspec validation error [~p]:"
-                                     [Id]),
+                                             " Shutdown is not brutal_kill,"
-
+                                             " infinity or an integer.~n",
-    TypeRes = validate_and_error(check_type(M, Type),
+                                             [Id]),
-                                 "Childspec validation error [~p]:"
+
-                                 " Module type mismatch, if module is"
+            TypeRes = validate_and_error(check_type(M, Type),
-                                 " a supervisor it should be the atom"
+                                         "Childspec validation error [~p]:"
-                                 " 'supervisor', if not it should be"
+                                         " Module type mismatch, if module is"
-                                 " the atom 'worker'.~n",
+                                         " a supervisor it should be the atom"
-                                 [Id]),
+                                         " 'supervisor', if not it should be"
-
+                                         " the atom 'worker'.~n",
-    ModsRes = validate_and_error(check_modules(M, Modules),
+                                         [Id]),
-                                 "Childspec validation error [~p]:"
+
-                                 " Modules mismatch, if gen_event it"
+            ModsRes = validate_and_error(check_modules(M, Modules),
-                                 " should be the atom 'dynamic', else"
+                                         "Childspec validation error [~p]:"
-                                 " should be a list including the"
+                                         " Modules mismatch, if gen_event it"
-                                 " callback module as its only"
+                                         " should be the atom 'dynamic', else"
-                                 " element.~n",
+                                         " should be a list including the"
-                                 [Id]),
+                                         " callback module as its only"
-
+                                         " element.~n",
-    Results = [MFARes, RestartRes, ShutdownRes, TypeRes, ModsRes],
+                                         [Id]),
-
+
-    case lists:member(false, Results) of
+            Results = [MFARes, RestartRes, ShutdownRes, TypeRes, ModsRes],
-        true ->
+
-            false;
+            case lists:member(false, Results) of
-        _ ->
+                true ->
-            true
+                    false;
+                _ ->
+                    true
+            end;
+        {error, _} ->
+            error_logger:error_msg("Childspec validation error [~p]:"
+                                   " The module specified in childspec"
+                                   " is not on the code path.~n",
+                                   [Id]),
+            false
     end;
-validate(_) ->
+
+validate({Id, {_, _, _}, _, _, _, _}) when is_atom(Id) == false ->
     error_logger:error_msg("Childspec validation error:"
                            " Childspec ID is not an atom.~n", []),
+    false;
+validate(_) ->
+    error_logger:error_msg("Childspec validation error:"
+                           " Childspec does not match correct tuple"
+                           " pattern.~n", []),
     false.
 
 %%
@@ -76,7 +91,7 @@ check_mfa(M, F, A) ->
             case proplists:get_value(F, Exports) of
                 Arity ->
                     ok;
-                _ ->
+                        _ ->
                     error
             end;
         _ ->

diff --git a/test/childspec_validator_tests.erl b/test/childspec_validator_tests.erl
@@ -56,6 +56,14 @@ negative() ->
 
     false = childspec_validator:validate(Web),
 
-    NotAtomID = {123, {m, f, []}, blah, werker, []},
+    NotAtomID = {123, {m, f, []}, blah, 123, werker, []},
 
-    false = childspec_validator:validate(NotAtomID).
+    false = childspec_validator:validate(NotAtomID),
+
+    NotOnPath = {asdf, {blahblahblah123, f, []}, blah, 123, blah, []},
+
+    false = childspec_validator:validate(NotOnPath),
+
+    BadPattern = {zxc, asdf, asdf, 123, asdf},
+
+    false = childspec_validator:validate(BadPattern).