Merge branch 'anders/diameter/undefined_group/OTP-11561'

* anders/diameter/undefined_group/OTP-11561:
  Ensure that Grouped AVP's are fully defined in dictionaries
  Don't format diameter_make:codec/2 errors
  Compiler suite fix

6 files changed, 61 insertions, 24 deletions

diff --git a/lib/diameter/bin/diameterc b/lib/diameter/bin/diameterc
index d31f341c36..2c9a8f555c 100755
--- a/lib/diameter/bin/diameterc
+++ b/lib/diameter/bin/diameterc
@@ -4,7 +4,7 @@
 %%
 %% %CopyrightBegin%
 %%
-%% Copyright Ericsson AB 2010-2013. All Rights Reserved.
+%% Copyright Ericsson AB 2010-2014. All Rights Reserved.
 %%
 %% The contents of this file are subject to the Erlang Public License,
 %% Version 1.1, (the "License"); you may not use this file except in
@@ -74,7 +74,7 @@ compile(#argv{file = File, options = Opts, output = Out}) ->
         ok ->
             0;
         {error, Reason} ->
-            error_msg(Reason, []),
+            error_msg(diameter_make:format_error(Reason), []),
             1
     catch
         error: Reason ->
diff --git a/lib/diameter/doc/src/diameter_make.xml b/lib/diameter/doc/src/diameter_make.xml
index e1673378df..13ec5bbfc1 100644
--- a/lib/diameter/doc/src/diameter_make.xml
+++ b/lib/diameter/doc/src/diameter_make.xml
@@ -16,7 +16,7 @@
 <header>
 <copyright>
 <year>2012</year>
-<year>2013</year>
+<year>2014</year>
 <holder>Ericsson AB. All Rights Reserved.</holder>
 </copyright>
 <legalnotice>
@@ -175,6 +175,10 @@ Note that a dictionary's <c>&dict_name;</c>, together with the
 The <c>&dict_name;</c> of a literal input dictionary defaults to
 <c>dictionary</c>.</p>
 
+<p>
+A returned error reason can be converted into a readable string using
+&format_error;.</p>
+
 </desc>
 </func>
 
@@ -206,6 +210,18 @@ The return value is also a parsed dictionary.</p>
 </desc>
 </func>
 
+<!-- ===================================================================== -->
+
+<func>
+<name>format_error(Reason) -> string()</name>
+<fsummary>Turn an error reason into a readable string.</fsummary>
+<desc>
+
+<p>
+Turn an error reason returned by &codec; into a readable string.</p>
+</desc>
+</func>
+
 </funcs>
 
 <!-- ===================================================================== -->
diff --git a/lib/diameter/doc/src/seealso.ent b/lib/diameter/doc/src/seealso.ent
index 7bf7460351..44541afb9b 100644
--- a/lib/diameter/doc/src/seealso.ent
+++ b/lib/diameter/doc/src/seealso.ent
@@ -4,7 +4,7 @@
 
 %CopyrightBegin%
 
-Copyright Ericsson AB 2012-2013. All Rights Reserved.
+Copyright Ericsson AB 2012-2014. All Rights Reserved.
 
 The contents of this file are subject to the Erlang Public License,
 Version 1.1, (the "License"); you may not use this file except in
@@ -117,6 +117,7 @@ significant.
 <!ENTITY make_codec '<seealso marker="diameter_make#codec-2">diameter_make:codec/2</seealso>'>
 <!ENTITY make_format '<seealso marker="diameter_make#format-1">diameter_make:format/1</seealso>'>
 <!ENTITY make_flatten '<seealso marker="diameter_make#flatten-1">diameter_make:flatten/1</seealso>'>
+<!ENTITY make_format_error '<seealso marker="diameter_make#format_error-1">diameter_make:format_error/1</seealso>'>
 
 <!-- diameter_transport -->
 
diff --git a/lib/diameter/src/compiler/diameter_dict_util.erl b/lib/diameter/src/compiler/diameter_dict_util.erl
index 3941f30e03..136bba16cb 100644
--- a/lib/diameter/src/compiler/diameter_dict_util.erl
+++ b/lib/diameter/src/compiler/diameter_dict_util.erl
@@ -1,7 +1,7 @@
 %%
 %% %CopyrightBegin%
 %%
-%% Copyright Ericsson AB 2010-2013. All Rights Reserved.
+%% Copyright Ericsson AB 2010-2014. All Rights Reserved.
 %%
 %% The contents of this file are subject to the Erlang Public License,
 %% Version 1.1, (the "License"); you may not use this file except in
@@ -155,6 +155,8 @@ fmt(grouped_avp_has_wrong_type) ->
     "Grouped AVP ~s at line ~p defined with type ~s at line ~p";
 fmt(grouped_avp_not_defined) ->
     "Grouped AVP ~s on line ~p not defined in @avp_types";
+fmt(grouped_avp_not_grouped) ->
+    "Grouped AVP ~s on line ~p not defined in @grouped";
 fmt(grouped_vendor_id_without_flag) ->
     "Grouped AVP ~s at line ~p has vendor id "
     "but definition at line ~p does not specify V flag";
@@ -401,9 +403,9 @@ read(File) ->
     {ok, iolist_to_binary([File])}.
 
 make_dict(Parse, Opts) ->
-    make_orddict(pass4(pass3(pass2(pass1(reset(make_dict(Parse),
-                                               Opts))),
-                             Opts))).
+    Dict = pass3(pass2(pass1(reset(make_dict(Parse), Opts))), Opts),
+    ok = examine(Dict),
+    make_orddict(Dict).
 
 %% make_orddict/1
 
@@ -1168,7 +1170,7 @@ import_avps(Dict, Opts) ->
     Import = inherit(Dict, Opts),
     report(imported, Import),
 
-    %% pass4/1 tests that all referenced AVP's are either defined
+    %% examine/1 tests that all referenced AVP's are either defined
     %% or imported.
 
     dict:store(import_avps,
@@ -1276,21 +1278,21 @@ dict(Mod) ->
     end.
 
 %% ===========================================================================
-%% pass4/1
+%% examine/1
 %%
 %% Sanity checks.
 
-pass4(Dict) ->
-    dict:fold(fun(K, V, _) -> p4(K, V, Dict) end, ok, Dict),
-    Dict.
+examine(Dict) ->
+    dict:fold(fun(K, V, _) -> x(K, V, Dict) end, ok, Dict),
+    ok.
 
 %% Ensure enum AVP's have type Enumerated.
-p4({enum, Name}, [Line | _], Dict)
+x({enum, Name}, [Line | _], Dict)
   when is_list(Name) ->
     true = is_enumerated_avp(Name, Dict, Line);
 
 %% Ensure all referenced AVP's are either defined locally or imported.
-p4({K, {Name, AvpName}}, [Line | _], Dict)
+x({K, {Name, AvpName}}, [Line | _], Dict)
   when (K == grouped orelse K == messages),
        is_list(Name),
        is_list(AvpName),
@@ -1298,13 +1300,22 @@ p4({K, {Name, AvpName}}, [Line | _], Dict)
     true = avp_is_defined(AvpName, Dict, Line);
 
 %% Ditto.
-p4({K, AvpName}, [Line | _], Dict)
+x({K, AvpName}, [Line | _], Dict)
   when K == avp_vendor_id;
        K == custom_types;
        K == codecs ->
     true = avp_is_defined(AvpName, Dict, Line);
 
-p4(_, _, _) ->
+%% Ensure that all local AVP's of type Grouped are also present in @grouped.
+x({avp_types, Name}, [Line | Toks], Dict)
+  when 0 < Line, is_list(Name) ->
+    [{number, _, _Code}, {word, _, Type}, {word, _, _Flags}] = Toks,
+    "Grouped" == Type
+        andalso error == dict:find({grouped, Name}, Dict)
+        andalso ?RETURN(grouped_avp_not_grouped, [Name, Line]),
+    ok;
+
+x(_, _, _) ->
     ok.
 
 %% has_enumerated_type/3
diff --git a/lib/diameter/src/compiler/diameter_make.erl b/lib/diameter/src/compiler/diameter_make.erl
index 2f314b7e57..adc7808e49 100644
--- a/lib/diameter/src/compiler/diameter_make.erl
+++ b/lib/diameter/src/compiler/diameter_make.erl
@@ -1,7 +1,7 @@
 %%
 %% %CopyrightBegin%
 %%
-%% Copyright Ericsson AB 2010-2013. All Rights Reserved.
+%% Copyright Ericsson AB 2010-2014. All Rights Reserved.
 %%
 %% The contents of this file are subject to the Erlang Public License,
 %% Version 1.1, (the "License"); you may not use this file except in
@@ -33,7 +33,8 @@
 -export([codec/2,
          codec/1,
          format/1,
-         flatten/1]).
+         flatten/1,
+         format_error/1]).
 
 -export_type([opt/0]).
 
@@ -81,8 +82,8 @@ codec(File, Opts) ->
     case parse(Dict, Opts) of
         {ok, ParseD} ->
             make(Path, default(Opts), ParseD);
-        {error = E, Reason} ->
-            {E, diameter_dict_util:format_error(Reason)}
+        {error, _} = E ->
+            E
     end.
 
 codec(File) ->
@@ -115,6 +116,11 @@ flatten([?VERSION = V | Dict]) ->
                       [grouped, import_groups],
                       [enum, import_enums]])].
 
+%% format_error/1
+
+format_error(T) ->
+    diameter_dict_util:format_error(T).
+
 %% ===========================================================================
 
 %% flatten/2
diff --git a/lib/diameter/test/diameter_compiler_SUITE.erl b/lib/diameter/test/diameter_compiler_SUITE.erl
index ed369e8af3..df4dde6240 100644
--- a/lib/diameter/test/diameter_compiler_SUITE.erl
+++ b/lib/diameter/test/diameter_compiler_SUITE.erl
@@ -1,7 +1,7 @@
 %%
 %% %CopyrightBegin%
 %%
-%% Copyright Ericsson AB 2010-2013. All Rights Reserved.
+%% Copyright Ericsson AB 2010-2014. All Rights Reserved.
 %%
 %% The contents of this file are subject to the Erlang Public License,
 %% Version 1.1, (the "License"); you may not use this file except in
@@ -138,6 +138,9 @@
          {grouped_avp_not_defined,
           "Failed-AVP *.*",
           ""},
+         {grouped_avp_not_grouped,
+          "Failed-AVP ::=.*\n.*}",
+          ""},
          {grouped_vendor_id_without_flag,
           "(Failed-AVP .*)>",
           "\\1 668>"},
@@ -397,8 +400,8 @@ replace({E, Mods}, Bin) ->
     case {E, parse(B, [{include, here()}]), Mods} of
         {ok, {ok, Dict}, _} ->
             Dict;
-        {_, {error, S}, _} ->
-            S
+        {_, {error, {E,_} = T}, _} when E /= ok ->
+            diameter_make:format_error(T)
     end.
 
 re({RE, Repl}, Bin) ->