Merge branch 'anders/diameter/dict_error_identification/OTP-9639'

* anders/diameter/dict_error_identification/OTP-9639: (27 commits)
  Update documentation
  Improve base_rfc3588.dia formatting
  Make typo fix backwards compatible
  Fix base_rfc3588.dia typo
  Check compiler dependencies in app suite
  Move type definitions into diameter.erl
  Fix interpretation of vendor id in @grouped
  Add range checks on dictionary integers
  Don't explicitly load inherited modules
  Tweak diameter_make interface
  Add format testcase to compiler suite
  Add diameter_dict_util:format/1 for reconstructing a dictionary file
  Make diameter_types usable with @codecs
  Minor codegen tweaks
  Remove unnecessary includes
  Add compiler suite
  Update app suite
  Update codec suite
  Vendor id fixes
  No longer inherit common dictionary in relay dictionary
  ...

9 files changed, 342 insertions, 41 deletions

diff --git a/lib/diameter/test/diameter_app_SUITE.erl b/lib/diameter/test/diameter_app_SUITE.erl
index 7f53a4ddd4..7da145fb3b 100644
--- a/lib/diameter/test/diameter_app_SUITE.erl
+++ b/lib/diameter/test/diameter_app_SUITE.erl
@@ -41,6 +41,19 @@
 
 -define(A, list_to_atom).
 
+%% Modules not in the app and that should not have dependencies on it
+%% for build reasons.
+-define(COMPILER_MODULES, [diameter_codegen,
+                           diameter_dict_scanner,
+                           diameter_dict_parser,
+                           diameter_dict_util,
+                           diameter_exprecs,
+                           diameter_make]).
+
+-define(HELP_MODULES, [diameter_callback,
+                       diameter_dbg,
+                       diameter_info]).
+
 %% ===========================================================================
 
 suite() ->
@@ -93,14 +106,8 @@ vsn(Config) ->
 modules(Config) ->
     Mods = fetch(modules, fetch(app, Config)),
     Installed = code_mods(),
-    Help = [diameter_callback,
-            diameter_codegen,
-            diameter_dbg,
-            diameter_exprecs,
-            diameter_info,
-            diameter_make,
-            diameter_spec_scan,
-            diameter_spec_util],
+    Help = lists:sort(?HELP_MODULES ++ ?COMPILER_MODULES),
+
     {[], Help} = {Mods -- Installed, lists:sort(Installed -- Mods)}.
 
 code_mods() ->
@@ -167,14 +174,12 @@ xref(Config) ->
     %% stop xref from complaining about calls to module erlang, which
     %% was previously in kernel. Erts isn't an application however, in
     %% the sense that there's no .app file, and isn't listed in
-    %% applications. Seems less than ideal. Also, diameter_tcp does
-    %% call ssl despite ssl not being listed as a dependency in the
-    %% app file since ssl is only required for TLS security: it's up
-    %% to a client who wants TLS it to start ssl.
+    %% applications.
     ok = lists:foreach(fun(A) -> add_application(XRef, A) end,
                        [?APP, erts | fetch(applications, App)]),
 
     {ok, Undefs} = xref:analyze(XRef, undefined_function_calls),
+    {ok, Called} = xref:analyze(XRef, {module_call, ?COMPILER_MODULES}),
 
     xref:stop(XRef),
 
@@ -183,7 +188,21 @@ xref(Config) ->
                               lists:member(F, Mods)
                                   andalso {F,T} /= {diameter_tcp, ssl}
                       end,
-                      Undefs).
+                      Undefs),
+    %% diameter_tcp does call ssl despite the latter not being listed
+    %% as a dependency in the app file since ssl is only required for
+    %% TLS security: it's up to a client who wants TLS it to start
+    %% ssl.
+
+    [] = lists:filter(fun is_bad_dependency/1, Called).
+
+%% It's not strictly necessary that diameter compiler modules not
+%% depend on other diameter modules but it's a simple source of build
+%% errors if not encoded in the makefile (hence the test) so guard
+%% against it.
+is_bad_dependency(Mod) ->
+    lists:prefix("diameter", atom_to_list(Mod))
+        andalso not lists:member(Mod, ?COMPILER_MODULES).
 
 add_application(XRef, App) ->
     add_application(XRef, App, code:lib_dir(App)).
diff --git a/lib/diameter/test/diameter_codec_test.erl b/lib/diameter/test/diameter_codec_test.erl
index 8046ca4c04..fbd38067a8 100644
--- a/lib/diameter/test/diameter_codec_test.erl
+++ b/lib/diameter/test/diameter_codec_test.erl
@@ -30,6 +30,9 @@
 -define(BASE, diameter_gen_base_rfc3588).
 -define(BOOL, [true, false]).
 
+-define(A, list_to_atom).
+-define(S, atom_to_list).
+
 %% ===========================================================================
 %% Interface.
 
@@ -42,7 +45,7 @@ gen(Mod) ->
                                                       command_codes,
                                                       avp_types,
                                                       grouped,
-                                                      enums,
+                                                      enum,
                                                       import_avps,
                                                       import_groups,
                                                       import_enums]]).
@@ -133,7 +136,7 @@ types() ->
 
 gen(M, T) ->
     [] = run(lists:map(fun(X) -> {?MODULE, [gen, M, T, X]} end,
-                       fetch(T, M:dict()))).
+                       fetch(T, dict(M)))).
 
 fetch(T, Spec) ->
     case orddict:find(T, Spec) of
@@ -143,6 +146,10 @@ fetch(T, Spec) ->
             []
     end.
 
+gen(M, messages = T, {Name, Code, Flags, ApplId, Avps})
+  when is_list(Name) ->
+    gen(M, T, {?A(Name), Code, Flags, ApplId, Avps});
+
 gen(M, messages, {Name, Code, Flags, _, _}) ->
     Rname = M:msg2rec(Name),
     Name = M:rec2msg(Rname),
@@ -156,22 +163,16 @@ gen(M, messages, {Name, Code, Flags, _, _}) ->
            end,
     [] = arity(M, Name, Rname);
 
-gen(M, command_codes = T, {Code, {Req, Abbr}, Ans}) ->
-    Rname = M:msg2rec(Req),
-    Rname = M:msg2rec(Abbr),
-    gen(M, T, {Code, Req, Ans});
-
-gen(M, command_codes = T, {Code, Req, {Ans, Abbr}}) ->
-    Rname = M:msg2rec(Ans),
-    Rname = M:msg2rec(Abbr),
-    gen(M, T, {Code, Req, Ans});
-
 gen(M, command_codes, {Code, Req, Ans}) ->
-    Msgs = orddict:fetch(messages, M:dict()),
+    Msgs = orddict:fetch(messages, dict(M)),
     {_, Code, _, _, _} = lists:keyfind(Req, 1, Msgs),
     {_, Code, _, _, _} = lists:keyfind(Ans, 1, Msgs);
 
-gen(M, avp_types, {Name, Code, Type, _Flags, _Encr}) ->
+gen(M, avp_types = T, {Name, Code, Type, Flags})
+  when is_list(Name) ->
+    gen(M, T, {?A(Name), Code, ?A(Type), Flags});
+
+gen(M, avp_types, {Name, Code, Type, _Flags}) ->
     {Code, Flags, VendorId} = M:avp_header(Name),
     0 = Flags band 2#00011111,
     V = undefined /= VendorId,
@@ -181,11 +182,19 @@ gen(M, avp_types, {Name, Code, Type, _Flags, _Encr}) ->
     B = z(B),
     [] = avp_decode(M, Type, Name);
 
+gen(M, grouped = T, {Name, Code, Vid, Avps})
+  when is_list(Name) ->
+    gen(M, T, {?A(Name), Code, Vid, Avps});
+
 gen(M, grouped, {Name, _, _, _}) ->
     Rname = M:name2rec(Name),
     [] = arity(M, Name, Rname);
 
-gen(M, enums, {Name, ED}) ->
+gen(M, enum = T, {Name, ED})
+  when is_list(Name) ->
+    gen(M, T, {?A(Name), lists:map(fun({E,D}) -> {?A(E), D} end, ED)});
+
+gen(M, enum, {Name, ED}) ->
     [] = run([{?MODULE, [enum, M, Name, T]} || T <- ED]);
 
 gen(M, Tag, {_Mod, L}) ->
@@ -253,17 +262,17 @@ arity(M, Name, AvpName, Rec) ->
 
 %% enum/3
 
-enum(M, Name, {E,_}) ->
+enum(M, Name, {_,E}) ->
     B = <<E:32/integer>>,
     B = M:avp(encode, E, Name),
     E = M:avp(decode, B, Name).
 
 retag(import_avps)   -> avp_types;
 retag(import_groups) -> grouped;
-retag(import_enums)  -> enums;
+retag(import_enums)  -> enum;
 
 retag(avp_types) -> import_avps;
-retag(enums)     -> import_enums.
+retag(enum)     -> import_enums.
 
 %% ===========================================================================
 
@@ -370,8 +379,8 @@ values('Time') ->
 %% wrapped as for values/1.
 
 values('Enumerated', Name, Mod) ->
-    {_Name, Vals} = lists:keyfind(Name, 1, types(enums, Mod)),
-    lists:map(fun({N,_}) -> N end, Vals);
+    {_Name, Vals} = lists:keyfind(?S(Name), 1, types(enum, Mod)),
+    lists:map(fun({_,N}) -> N end, Vals);
 
 values('Grouped', Name, Mod) ->
     Rname = Mod:name2rec(Name),
@@ -400,8 +409,8 @@ values('AVP', _) ->
 
 values(Name, Mod) ->
     Avps = types(avp_types, Mod),
-    {Name, _Code, Type, _Flags, _Encr} = lists:keyfind(Name, 1, Avps),
-    b(values(Type, Name, Mod)).
+    {_Name, _Code, Type, _Flags} = lists:keyfind(?S(Name), 1, Avps),
+    b(values(?A(Type), Name, Mod)).
 
 %% group/5
 %%
@@ -467,7 +476,7 @@ types(T, Mod) ->
     types(T, retag(T), Mod).
 
 types(T, IT, Mod) ->
-    Dict = Mod:dict(),
+    Dict = dict(Mod),
     fetch(T, Dict) ++ lists:flatmap(fun({_,As}) -> As end, fetch(IT, Dict)).
 
 %% random/[12]
@@ -498,3 +507,8 @@ flatten({_, {{badmatch, [{_, {{badmatch, _}, _}} | _] = L}, _}}) ->
     L;
 flatten(T) ->
     [T].
+
+%% dict/1
+
+dict(Mod) ->
+    tl(Mod:dict()).
diff --git a/lib/diameter/test/diameter_compiler_SUITE.erl b/lib/diameter/test/diameter_compiler_SUITE.erl
new file mode 100644
index 0000000000..cc4b1ddac5
--- /dev/null
+++ b/lib/diameter/test/diameter_compiler_SUITE.erl
@@ -0,0 +1,272 @@
+%%
+%% %CopyrightBegin%
+%%
+%% Copyright Ericsson AB 2010-2011. 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
+%% compliance with the License. You should have received a copy of the
+%% Erlang Public License along with this software. If not, it can be
+%% retrieved online at http://www.erlang.org/.
+%%
+%% Software distributed under the License is distributed on an "AS IS"
+%% basis, WITHOUT WARRANTY OF ANY KIND, either express or implied. See
+%% the License for the specific language governing rights and limitations
+%% under the License.
+%%
+%% %CopyrightEnd%
+%%
+
+%%
+%% Tests of the dictionary file compiler.
+%%
+
+-module(diameter_compiler_SUITE).
+-compile({no_auto_import, [error/2]}).
+
+-export([suite/0,
+         all/0,
+         init_per_suite/1,
+         end_per_suite/1]).
+
+%% testcases
+-export([format/1,
+         replace/1, replace/2]).
+
+-export([dict/0]).  %% fake dictionary module
+
+-define(base, "base_rfc3588.dia").
+-define(util, diameter_util).
+-define(S, atom_to_list).
+
+%% ===========================================================================
+
+%% RE/Replacement (in the sense of re:replace/4) pairs for morphing
+%% base_rfc3588.dia. The key is 'ok' or the the expected error as
+%% returned in the first element of the error tuple returned by
+%% diameter_dict_util:parse/2.
+-define(REPLACE,
+        [{scan,
+          "@id 0",
+          "@id \\&"},
+         {scan,
+          "@name ",
+          "&'"},
+         {parse,
+          "@id 0",
+          "@id @id"},
+         {avp_code_already_defined,
+          "480",
+          "485"},
+         {uint32_out_of_range,
+          "@id 0",
+          "@id 4294967296"},
+         {uint32_out_of_range,
+          "@vendor 0",
+          "@vendor 4294967296"},
+         {uint32_out_of_range,
+          [{"^ *Failed-AVP .*$", "&V"},
+           {"@avp_types", "@avp_vendor_id 4294967296 Failed-AVP\n&"}]},
+         {imported_avp_already_defined,
+          "@avp_types",
+          "@inherits diameter_gen_base_rfc3588 &"},
+         {duplicate_import,
+          [{"@avp_types", "@inherits diameter_gen_base_rfc3588 Class\n&"},
+           {"@avp_types", "@inherits diameter_gen_base_rfc3588\n&"},
+           {"^@avp_types[^@]*", ""},
+           {"^@enum[^&]*", ""}]},
+         {duplicate_section,
+          "@prefix",
+          "@name"},
+         {already_declared,
+          "@enum Termination-Cause",
+          "& XXX 0\n &"},
+         {already_declared,
+          "@define Result-Code",
+          "& XXX 1000 &"},
+         {inherited_avp_already_defined,
+          "@id",
+          "@inherits nomod Origin-Host &"},
+         {avp_already_defined,
+          "@avp_types",
+          "@inherits m XXX\nXXX\n&"},
+         {avp_already_defined,
+          "@avp_types",
+          "@inherits mod1 XXX\n@inherits mod2 XXX\n&"},
+         {key_already_defined,
+          "DIAMETER_SUCCESS",
+          "& 2001\n&"},
+         {messages_without_id,
+          "@id 0",
+          ""},
+         {avp_name_already_defined,
+          "Class",
+          "& 666 Time M\n&"},
+         {avp_has_unknown_type,
+          "Enumerated",
+          "Enum"},
+         {avp_has_invalid_flag,
+          " -",
+          " X"},
+         {avp_has_duplicate_flag,
+          " -",
+          " MM"},
+         {avp_has_vendor_id,
+          "@avp_types",
+          "@avp_vendor_id 667 Class\n&"},
+         {avp_has_no_vendor,
+          [{"^ *Class .*$", "&V"},
+           {"@vendor .*", ""}]},
+         {group_already_defined,
+          "@grouped",
+          "& Failed-AVP ::= < AVP Header: 279 > " "{AVP}\n&"},
+         {grouped_avp_code_mismatch,
+          "(Failed-AVP ::= [^0-9]*27)9",
+          "&8"},
+         {grouped_avp_has_wrong_type,
+          "(Failed-AVP *279 *)Grouped",
+          "\\1Time"},
+         {grouped_avp_not_defined,
+          "Failed-AVP *.*",
+          ""},
+         {grouped_vendor_id_without_flag,
+          "(Failed-AVP .*)>",
+          "\\1 668>"},
+         {grouped_vendor_id_mismatch,
+          [{"(Failed-AVP .*)>", "\\1 17>"},
+           {"^ *Failed-AVP .*$", "&V"},
+           {"@avp_types", "@avp_vendor_id 18 Failed-AVP\n&"}]},
+         {ok,
+          [{"(Failed-AVP .*)>", "\\1 17>"},
+           {"^ *Failed-AVP .*$", "&V"}]},
+         {message_name_already_defined,
+          "CEA ::= .*:",
+          "& 257 > {Result-Code}\n&"},
+         {message_code_already_defined,
+          "CEA( ::= .*)",
+          "XXX\\1 {Result-Code}\n&"},
+         {message_has_duplicate_flag,
+          "(CER ::=.*)>",
+          "\\1, REQ>"},
+         {message_application_id_mismatch,
+         "(CER ::=.*)>",
+          "\\1 1>"},
+         {invalid_avp_order,
+          "CEA ::=",
+          "{Result-Code} &"},
+         {invalid_qualifier,
+          "CEA ::=.*",
+          "& 3*2"},
+         {avp_already_referenced,
+          "CER ::=.*",
+          "& {Origin-Host}"},
+         {message_missing,
+          "CER ::=",
+          "XXR ::= < Diameter-Header: 666, REQ > {Origin-Host} &"},
+         {requested_avp_not_found,
+          [{"@id", "@inherits diameter_gen_base_rfc3588 XXX &"},
+           {"CEA ::=", "<XXX> &"}]},
+         {requested_avp_not_found,
+          [{"@id", "@inherits diameter_gen_base_rfc3588 'X X X' &"},
+           {"CEA ::=", "<'X X X'> &"}]},
+         {enumerated_avp_has_wrong_local_type,
+          "Enumerated",
+          "Time"},
+         {enumerated_avp_not_defined,
+         [{"{ Disconnect-Cause }", ""},
+          {"^ *Disconnect-Cause .*", ""}]},
+         {avp_not_defined,
+          "CEA ::=",
+          "<XXX> &"},
+         {not_loaded,
+          [{"@avp_types", "@inherits nomod XXX &"},
+           {"CEA ::=", "<XXX> &"}]},
+         {recompile,
+          [{"@avp_types", "@inherits " ++ ?S(?MODULE) ++ " XXX &"},
+           {"CEA ::=", "<XXX> &"}]},
+         {no_dict,
+          [{"@avp_types", "@inherits diameter XXX &"},
+           {"CEA ::=", "<XXX> &"}]},
+         {ok,
+          "@avp_types",
+          "@end & bad syntax"},
+         {parse,
+          "@avp_types",
+          "& bad syntax"},
+         {ok,
+          [{"@avp_types", "& 3XXX 666 Time M 'X X X' 667 Time -"},
+           {"^ *Class .*", "@avp_types"},
+           {"^ *Failed-AVP ", "@avp_types &"},
+           {"@grouped", "&&"},
+           {"^ *Failed-AVP ::=", "@grouped &"},
+           {"CEA ::=", "<'Class'> &"},
+           {"@avp_types", "@inherits diameter_gen_base_rfc3588 Class\n&"},
+           {"@avp_types", "@custom_types mymod "
+                              "Product-Name Firmware-Revision\n"
+                          "@codecs mymod "
+                              "Origin-Host Origin-Realm\n&"}]}]).
+
+%% ===========================================================================
+
+suite() ->
+    [{timetrap, {seconds, 5}}].
+
+all() ->
+    [format,
+     replace].
+
+%% Error handling testcases will make an erroneous dictionary out of
+%% the base dictionary and check that the expected error results.
+%% ?REPLACE encodes the modifications and expected error.
+init_per_suite(Config) ->
+    Path = filename:join([code:lib_dir(diameter, src), "dict", ?base]),
+    {ok, Bin} = file:read_file(Path),
+    [{base, Bin} | Config].
+
+end_per_suite(_Config) ->
+    ok.
+
+%% ===========================================================================
+%% testcases
+
+%% Ensure that parse o format is the identity map.
+format(Config) ->
+    Bin = proplists:get_value(base, Config),
+    {ok, Dict} = diameter_dict_util:parse(Bin, []),
+    {ok, D} = diameter_dict_util:parse(diameter_dict_util:format(Dict), []),
+    {Dict, Dict} = {Dict, D}.
+
+%% replace/1
+
+replace(Config) ->
+    Bin = proplists:get_value(base, Config),
+    [] = ?util:run([{?MODULE, [replace, E, Bin]} || E <- ?REPLACE]).
+
+replace({E, RE, Repl}, Bin) ->
+    replace({E, [{RE, Repl}]}, Bin);
+
+replace({E, Mods}, Bin) ->
+    B = iolist_to_binary(lists:foldl(fun re/2, Bin, Mods)),
+    case {E, diameter_dict_util:parse(B, [{include, here()}]), Mods} of
+        {ok, {ok, Dict}, _} ->
+            Dict;
+        {_, {error, {E,_} = T}, _} ->
+            S = diameter_dict_util:format_error(T),
+            true = nochar($", S, E),
+            true = nochar($', S, E),
+            S
+    end.
+
+re({RE, Repl}, Bin) ->
+    re:replace(Bin, RE, Repl, [multiline]).
+
+%% ===========================================================================
+
+nochar(Char, Str, Err) ->
+    Err == parse orelse not lists:member(Char, Str) orelse Str.
+
+here() ->
+    filename:dirname(code:which(?MODULE)).
+
+dict() ->
+    [0 | orddict:new()].
diff --git a/lib/diameter/test/diameter_failover_SUITE.erl b/lib/diameter/test/diameter_failover_SUITE.erl
index f4d62f94c6..aef2447587 100644
--- a/lib/diameter/test/diameter_failover_SUITE.erl
+++ b/lib/diameter/test/diameter_failover_SUITE.erl
@@ -59,7 +59,6 @@
 
 -include("diameter.hrl").
 -include("diameter_gen_base_rfc3588.hrl").
--include("diameter_ct.hrl").
 
 %% ===========================================================================
 
diff --git a/lib/diameter/test/diameter_relay_SUITE.erl b/lib/diameter/test/diameter_relay_SUITE.erl
index 40cbdf805a..b8e194ba9e 100644
--- a/lib/diameter/test/diameter_relay_SUITE.erl
+++ b/lib/diameter/test/diameter_relay_SUITE.erl
@@ -66,7 +66,6 @@
 
 -include("diameter.hrl").
 -include("diameter_gen_base_rfc3588.hrl").
--include("diameter_ct.hrl").
 
 %% ===========================================================================
 
diff --git a/lib/diameter/test/diameter_tls_SUITE.erl b/lib/diameter/test/diameter_tls_SUITE.erl
index 127e3435dc..4abc7bb23f 100644
--- a/lib/diameter/test/diameter_tls_SUITE.erl
+++ b/lib/diameter/test/diameter_tls_SUITE.erl
@@ -69,7 +69,6 @@
 
 -include("diameter.hrl").
 -include("diameter_gen_base_rfc3588.hrl").
--include("diameter_ct.hrl").
 
 %% ===========================================================================
 
diff --git a/lib/diameter/test/diameter_traffic_SUITE.erl b/lib/diameter/test/diameter_traffic_SUITE.erl
index 55c5fc7c54..78131b4ec4 100644
--- a/lib/diameter/test/diameter_traffic_SUITE.erl
+++ b/lib/diameter/test/diameter_traffic_SUITE.erl
@@ -89,7 +89,6 @@
 
 -include("diameter.hrl").
 -include("diameter_gen_base_rfc3588.hrl").
--include("diameter_ct.hrl").
 
 %% ===========================================================================
 
diff --git a/lib/diameter/test/diameter_transport_SUITE.erl b/lib/diameter/test/diameter_transport_SUITE.erl
index c22adc3334..3bd96ee422 100644
--- a/lib/diameter/test/diameter_transport_SUITE.erl
+++ b/lib/diameter/test/diameter_transport_SUITE.erl
@@ -46,7 +46,6 @@
 
 -include_lib("kernel/include/inet_sctp.hrl").
 -include("diameter.hrl").
--include("diameter_ct.hrl").
 
 -define(util, diameter_util).
 
diff --git a/lib/diameter/test/modules.mk b/lib/diameter/test/modules.mk
index f88258c232..54978d820c 100644
--- a/lib/diameter/test/modules.mk
+++ b/lib/diameter/test/modules.mk
@@ -24,6 +24,7 @@ MODULES = \
 	diameter_ct \
 	diameter_util \
 	diameter_enum \
+	diameter_compiler_SUITE \
 	diameter_codec_SUITE \
 	diameter_codec_test \
 	diameter_app_SUITE \