Introduce a new mechanism for structured error handling

5 files changed, 126 insertions, 59 deletions

diff --git a/lib/asn1/src/asn1ct.erl b/lib/asn1/src/asn1ct.erl
index b41af6ab1b..15aa4efe7d 100644
--- a/lib/asn1/src/asn1ct.erl
+++ b/lib/asn1/src/asn1ct.erl
@@ -301,8 +301,10 @@ run_passes_1([{pass,Name,Pass}|Passes], #st{run=Run}=St0)
     try Run(Name, Pass, St0) of
 	{ok,St} ->
 	    run_passes_1(Passes, St);
-	{error,#st{error=Error}} ->
-	    {error,Error};
+	{error,#st{error=Errors}} ->
+	    {Structured,AllErrors} = clean_errors(Errors),
+	    print_structured_errors(Structured),
+	    {error,AllErrors};
 	done ->
 	    ok
     catch
@@ -315,6 +317,21 @@ run_passes_1([{pass,Name,Pass}|Passes], #st{run=Run}=St0)
 run_passes_1([], _St) ->
     ok.
 
+clean_errors(Errors) when is_list(Errors) ->
+    F = fun({structured_error,_,_,_}) -> true;
+	   (_) -> false
+	end,
+    {Structured0,AdHoc} = lists:partition(F, Errors),
+    Structured = lists:sort(Structured0),
+    {Structured,Structured ++ AdHoc};
+clean_errors(AdHoc) -> {[],AdHoc}.
+
+print_structured_errors([_|_]=Errors) ->
+    _ = [io:format("~ts:~w: ~ts\n", [F,L,M:format_error(E)]) ||
+	    {structured_error,{F,L},M,E} <- Errors],
+    ok;
+print_structured_errors(_) -> ok.
+
 compile1(File, #st{opts=Opts}=St0) ->
     verbose("Erlang ASN.1 version ~p, compiling ~p~n", [?vsn,File], Opts),
     verbose("Compiler Options: ~p~n", [Opts], Opts),
diff --git a/lib/asn1/src/asn1ct_check.erl b/lib/asn1/src/asn1ct_check.erl
index bb0469d7a5..3c9e0dd8d2 100644
--- a/lib/asn1/src/asn1ct_check.erl
+++ b/lib/asn1/src/asn1ct_check.erl
@@ -25,7 +25,7 @@
 %-compile(export_all).
 %% Avoid warning for local function error/1 clashing with autoimported BIF.
 -compile({no_auto_import,[error/1]}).
--export([check/2,storeindb/2]).
+-export([check/2,storeindb/2,format_error/1]).
 %-define(debug,1).
 -include("asn1_records.hrl").
 %%% The tag-number for universal types
@@ -175,8 +175,9 @@ check(S,{Types,Values,ParameterizedTypes,Classes,Objects,ObjectSets}) ->
 	     {NewTypes,NewValues,ParameterizedTypes,NewClasses,
 	      lists:subtract(NewObjects,ExclO)++InlinedObjects,
 	      lists:subtract(NewObjectSets,ExclOS)++ParObjectSetNames}};
-	_ ->{error,{asn1,lists:flatten([Terror3,Verror5,Cerror,
-					Oerror,Exporterror,ImportError])}}
+	_ ->
+	    {error,lists:flatten([Terror3,Verror5,Cerror,
+				  Oerror,Exporterror,ImportError])}
     end.
 
 context_switch_in_spec() ->
@@ -6946,60 +6947,27 @@ storeindb(S,M) when is_record(M,module) ->
     NewM = M#module{typeorval=findtypes_and_values(TVlist)},
     asn1_db:dbnew(NewM#module.name),
     asn1_db:dbput(NewM#module.name,'MODULE',  NewM),
-    Res = storeindb(NewM#module.name,TVlist,[]),
+    Res = storeindb(#state{mname=NewM#module.name}, TVlist, []),
     include_default_class(S,NewM#module.name),
     include_default_type(NewM#module.name),
     Res.
 
-storeindb(Module,[H|T],ErrAcc) when is_record(H,typedef) ->
-    storeindb(Module,H#typedef.name,H,T,ErrAcc);
-storeindb(Module,[H|T],ErrAcc) when is_record(H,valuedef) ->
-    storeindb(Module,H#valuedef.name,H,T,ErrAcc);
-storeindb(Module,[H|T],ErrAcc) when is_record(H,ptypedef) ->
-    storeindb(Module,H#ptypedef.name,H,T,ErrAcc);
-storeindb(Module,[H|T],ErrAcc) when is_record(H,classdef) ->
-    storeindb(Module,H#classdef.name,H,T,ErrAcc);
-storeindb(Module,[H|T],ErrAcc) when is_record(H,pvaluesetdef) ->
-    storeindb(Module,H#pvaluesetdef.name,H,T,ErrAcc);
-storeindb(Module,[H|T],ErrAcc) when is_record(H,pobjectdef) ->
-    storeindb(Module,H#pobjectdef.name,H,T,ErrAcc);
-storeindb(Module,[H|T],ErrAcc) when is_record(H,pvaluedef) ->
-    storeindb(Module,H#pvaluedef.name,H,T,ErrAcc);
-storeindb(_,[],[]) -> ok;
-storeindb(_,[],ErrAcc) -> 
-    {error,ErrAcc}.
-
-storeindb(Module,Name,H,T,ErrAcc) ->
-    case asn1_db:dbget(Module,Name) of
+storeindb(#state{mname=Module}=S, [H|T], Errors) ->
+    Name = asn1ct:get_name_of_def(H),
+    case asn1_db:dbget(Module, Name) of
 	undefined ->
-	    asn1_db:dbput(Module,Name,H),
-	    storeindb(Module,T,ErrAcc);
-	_ -> 
-	    case H of 
-		_Type when is_record(H,typedef) ->
-		    error({type,"already defined",
-			   #state{mname=Module,type=H,tname=Name}});
-		_Type when is_record(H,valuedef) ->
-		    error({value,"already defined",
-			   #state{mname=Module,value=H,vname=Name}});
-		_Type when is_record(H,ptypedef) ->
-		    error({ptype,"already defined",
-			   #state{mname=Module,type=H,tname=Name}});
-		_Type when is_record(H,pobjectdef) ->
-		    error({ptype,"already defined",
-			   #state{mname=Module,type=H,tname=Name}});
-		_Type when is_record(H,pvaluesetdef) ->
-		    error({ptype,"already defined",
-			   #state{mname=Module,type=H,tname=Name}});
-		_Type when is_record(H,pvaluedef) ->
-		    error({ptype,"already defined",
-			   #state{mname=Module,type=H,tname=Name}});
-		_Type when is_record(H,classdef) ->
-		    error({class,"already defined",
-			   #state{mname=Module,value=H,vname=Name}})
-	    end,
-	    storeindb(Module,T,[H|ErrAcc])
-    end.
+	    asn1_db:dbput(Module, Name, H),
+	    storeindb(S, T, Errors);
+	Prev ->
+	    PrevLine = asn1ct:get_pos_of_def(Prev),
+	    {error,Error} = asn1_error(S, H, {already_defined,Name,PrevLine}),
+	    storeindb(S, T, [Error|Errors])
+    end;
+storeindb(_, [], []) ->
+    ok;
+storeindb(_, [], [_|_]=Errors) ->
+    {error,Errors}.
+
 
 findtypes_and_values(TVList) ->
     findtypes_and_values(TVList,[],[],[],[],[],[]).%% Types,Values,
@@ -7039,7 +7007,15 @@ findtypes_and_values([],Tacc,Vacc,Pacc,Cacc,Oacc,OSacc) ->
     {lists:reverse(Tacc),lists:reverse(Vacc),lists:reverse(Pacc),
      lists:reverse(Cacc),lists:reverse(Oacc),lists:reverse(OSacc)}.
     
-
+asn1_error(#state{mname=Where}, Item, Error) ->
+    Pos = asn1ct:get_pos_of_def(Item),
+    {error,{structured_error,{Where,Pos},?MODULE,Error}}.
+
+format_error({already_defined,Name,PrevLine}) ->
+    io_lib:format("the name ~p has already been defined at line ~p",
+		  [Name,PrevLine]);
+format_error(Other) ->
+    io_lib:format("~p", [Other]).
 
 error({export,Msg,#state{mname=Mname,type=Ref,tname=Typename}}) ->
     Pos = Ref#'Externaltypereference'.pos,
diff --git a/lib/asn1/test/Makefile b/lib/asn1/test/Makefile
index 6e6ab4639c..15b97df972 100644
--- a/lib/asn1/test/Makefile
+++ b/lib/asn1/test/Makefile
@@ -114,7 +114,8 @@ MODULES= \
 	asn1_app_test \
 	asn1_appup_test \
 	asn1_wrapper \
-	asn1_SUITE
+	asn1_SUITE \
+	error_SUITE
 
 SUITE= asn1_SUITE.erl
 
diff --git a/lib/asn1/test/asn1_SUITE.erl b/lib/asn1/test/asn1_SUITE.erl
index 5ec201fd78..2b3ff07980 100644
--- a/lib/asn1/test/asn1_SUITE.erl
+++ b/lib/asn1/test/asn1_SUITE.erl
@@ -860,7 +860,7 @@ testInvokeMod(Config, Rule, Opts) ->
     {ok, _Result2} = 'PrimStrings':encode('Bs1', [1, 0, 1, 0]).
 
 testExport(Config) ->
-    {error, {asn1, _Reason}} =
+    {error, _} =
 	asn1ct:compile(filename:join(?config(data_dir, Config),
 				     "IllegalExport"),
 		       [{outdir, ?config(case_dir, Config)}]).
@@ -906,8 +906,8 @@ testOpenTypeImplicitTag(Config, Rule, Opts) ->
 duplicate_tags(Config) ->
     DataDir = ?config(data_dir, Config),
     CaseDir = ?config(case_dir, Config),
-    {error, {asn1, [{error, {type, _, _, 'SeqOpt1Imp',
-			     {asn1, {duplicates_of_the_tags, _}}}}]}} =
+    {error, [{error, {type, _, _, 'SeqOpt1Imp',
+			     {asn1, {duplicates_of_the_tags, _}}}}]} =
 	asn1ct:compile(filename:join(DataDir, "SeqOptional2"),
 		       [abs, {outdir, CaseDir}]).
 
diff --git a/lib/asn1/test/error_SUITE.erl b/lib/asn1/test/error_SUITE.erl
new file mode 100644
index 0000000000..4dd4d58aad
--- /dev/null
+++ b/lib/asn1/test/error_SUITE.erl
@@ -0,0 +1,73 @@
+%%
+%% %CopyrightBegin%
+%%
+%% Copyright Ericsson AB 2013. 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%
+%%
+
+-module(error_SUITE).
+-export([suite/0,all/0,groups/0,
+	 already_defined/1]).
+
+-include_lib("test_server/include/test_server.hrl").
+
+suite() -> [{ct_hooks, [ts_install_cth]}].
+
+all() ->
+    [{group,p}].
+
+groups() ->
+    [{p,parallel(),[already_defined]}].
+
+parallel() ->
+    case erlang:system_info(schedulers) > 1 of
+        true  -> [parallel];
+        false -> []
+    end.
+
+already_defined(Config) ->
+    M = 'Already',
+    P = {M,
+	 <<"Already DEFINITIONS ::= BEGIN\n"
+	   "  I ::= INTEGER\n"
+	   "  i I ::= 42\n"
+	   "  I ::= OCTET STRING\n"
+	   "  I ::= CLASS { &Type }\n"
+	   "  MYCLASS ::= CLASS { &Type }\n"
+	   "  i MYCLASS ::= { &Type INTEGER }\n"
+	   "  o MYCLASS ::= { &Type INTEGER }\n"
+	   "  I MYCLASS ::= { o }\n"
+	   "  I{T} ::= SEQUENCE OF T\n"
+	   "  I{INTEGER:x} INTEGER ::= { 1 | 2 | x }\n"
+	   "  i{T} MYCLASS ::= { &Type T }\n"
+	   "END\n">>},
+    {error,
+     [
+      {structured_error,{M,4},asn1ct_check,{already_defined,'I',2}},
+      {structured_error,{M,5},asn1ct_check,{already_defined,'I',2}},
+      {structured_error,{M,7},asn1ct_check,{already_defined,'i',3}},
+      {structured_error,{M,9},asn1ct_check,{already_defined,'I',2}},
+      {structured_error,{M,10},asn1ct_check,{already_defined,'I',2}},
+      {structured_error,{M,11},asn1ct_check,{already_defined,'I',2}},
+      {structured_error,{M,12},asn1ct_check,{already_defined,'i',3}}
+     ]
+    } = run(P, Config),
+    ok.
+
+run({Mod,Spec}, Config) ->
+    Base = atom_to_list(Mod) ++ ".asn1",
+    File = filename:join(?config(priv_dir, Config), Base),
+    ok = file:write_file(File, Spec),
+    asn1ct:compile(File).