compiler: Fix parse transform column handling

Fix so that parse_transforms are handed the correct AST without
columns if requested by either the user or the parse transform.

Added test to verify that the column stripping works.

5 files changed, 163 insertions, 41 deletions

diff --git a/lib/compiler/doc/src/compile.xml b/lib/compiler/doc/src/compile.xml
index 26555438b8..8906b1c2eb 100644
--- a/lib/compiler/doc/src/compile.xml
+++ b/lib/compiler/doc/src/compile.xml
@@ -363,7 +363,7 @@ module.beam: module.erl \
             since R13B04.</p>
           </item>
 
-	  <tag><c>error_location</c></tag>
+	  <tag><c>{error_location,line | column}</c></tag>
 	  <item>
             <p>If the value of this flag is <c>line</c>, the location
             <seeerl marker="#error_information"><c>ErrorLocation</c></seeerl>
diff --git a/lib/compiler/src/compile.erl b/lib/compiler/src/compile.erl
index c9b902d8ff..515f5facd8 100644
--- a/lib/compiler/src/compile.erl
+++ b/lib/compiler/src/compile.erl
@@ -1049,7 +1049,10 @@ do_parse_module(DefEncoding, #compile{ifile=File,options=Opts,dir=Dir}=St) ->
     end.
 
 with_columns(Opts) ->
-    proplists:get_value(error_location, Opts, column) =:= column.
+    case proplists:get_value(error_location, Opts, column) of
+        column -> true;
+        line -> false
+    end.
 
 consult_abstr(_Code, St) ->
     case file:consult(St#compile.ifile) of
@@ -1138,16 +1141,15 @@ foldl_transform([T|Ts], Code0, St) ->
                           T:parse_transform(Code, S#compile.options)
                   end,
             Run = runner(none, St),
-            try Run({Name, Fun}, Code0, St) of
+            StrippedCode = maybe_strip_columns(Code0, T, St),
+            try Run({Name, Fun}, StrippedCode, St) of
                 {error,Es,Ws} ->
                     {error,St#compile{warnings=St#compile.warnings ++ Ws,
                                       errors=St#compile.errors ++ Es}};
-                {warning, Forms0, Ws} ->
-                    Forms = maybe_strip_columns(Forms0, T),
+                {warning, Forms, Ws} ->
                     foldl_transform(Ts, Forms,
                                     St#compile{warnings=St#compile.warnings ++ Ws});
-                Forms0 ->
-                    Forms = maybe_strip_columns(Forms0, T),
+                Forms ->
                     foldl_transform(Ts, Forms, St)
             catch
                 error:Reason:Stk ->
@@ -1160,28 +1162,43 @@ foldl_transform([T|Ts], Code0, St) ->
                                       {undef_parse_transform,T}}]}],
             {error,St#compile{errors=St#compile.errors ++ Es}}
     end;
-foldl_transform([], Code, St) -> {ok,Code,St}.
-
-%%% It is possible--although unlikely--that parse transforms add
-%%% columns to the abstract code, why this function is called for
-%%% every parse transform.
-maybe_strip_columns(Code, T) ->
-    case erlang:function_exported(T, parse_transform_info, 0) of
-        true ->
-            Info = T:parse_transform_info(),
-            case maps:get(error_location, Info, column) of
-                line ->
-                    strip_columns(Code);
-                _ ->
-                    Code
-            end;
-        false ->
-            Code
+foldl_transform([], Code, St) ->
+    %% We may need to strip columns added by parse transforms before returning
+    %% them back to the compiler. We pass ?MODULE as a bit of a hack to get the
+    %% correct default.
+    {ok, maybe_strip_columns(Code, ?MODULE, St), St}.
+
+%%% If a parse transform does not support column numbers it can say so using
+%%% the parse_transform_info callback. The error_location is taken from both
+%%% compiler options and from the parse transform and if either of them want
+%%% to only use lines, we strip columns.
+maybe_strip_columns(Code, T, St) ->
+    PTErrorLocation =
+        case erlang:function_exported(T, parse_transform_info, 0) of
+            true ->
+                maps:get(error_location, T:parse_transform_info(), column);
+            false ->
+                column
+        end,
+    ConfigErrorLocation = proplists:get_value(error_location, St#compile.options, column),
+    if 
+        PTErrorLocation =:= line; ConfigErrorLocation =:= line ->
+            strip_columns(Code);
+        true -> Code
     end.
 
 strip_columns(Code) ->
     F = fun(A) -> erl_anno:set_location(erl_anno:line(A), A) end,
-    [erl_parse:map_anno(F, Form) || Form <- Code].
+    [case Form of
+         {eof,{Line,_Col}} ->
+             {eof,Line};
+         {ErrorOrWarning,{{Line,_Col},Module,Reason}}
+           when ErrorOrWarning =:= error;
+                ErrorOrWarning =:= warning ->
+             {ErrorOrWarning,{Line,Module,Reason}};
+         Form ->
+             erl_parse:map_anno(F, Form)
+     end || Form <- Code].
 
 get_core_transforms(Opts) -> [M || {core_transform,M} <- Opts].
 
diff --git a/lib/compiler/test/compile_SUITE.erl b/lib/compiler/test/compile_SUITE.erl
index e3f03d4959..7c47d8a289 100644
--- a/lib/compiler/test/compile_SUITE.erl
+++ b/lib/compiler/test/compile_SUITE.erl
@@ -1724,11 +1724,44 @@ do_transforms(Config) ->
     %% Compile a file using line_pt and verify that column numbers
     %% have been stripped.
     Big = filename:join(DataDir, "big"),
-    {[],[_|_]} = compile_partition_warnings(Big, line_pt),
+    {[],[_|_]} = compile_partition_warnings(Big, [{parse_transform,line_pt}]),
 
     %% Compile a file using column_pt and verify that column numbers
     %% have NOT been stripped.
-    {[_|_],[]} = compile_partition_warnings(Big, column_pt),
+    {[_|_],[]} = compile_partition_warnings(Big, [{parse_transform,column_pt}]),
+
+    %% Compile a file using column_pt and error_location=line and verify
+    %% that column numbers have been stripped.
+    {[],[_|_]} = compile_partition_warnings(Big, [{error_location,line},
+                                                  {parse_transform,column_pt}]),
+
+    %% Compile a file using column_pt, line_pt and verify
+    %% that column numbers have been stripped.
+    {[],[_|_]} = compile_partition_warnings(Big, [{parse_transform,column_pt},
+                                                  {parse_transform,line_pt}]),
+
+    %% Compile a file using column_pt that adds columns and error_location=line and
+    %% verify that column numbers have been stripped.
+    {[],[_|_]} = compile_partition_warnings(Big, [{error_location,line},
+                                                  add_columns,
+                                                  {parse_transform,column_pt}]),
+
+    %% Compile a file using column_pt that adds columns and error_location=line and
+    %% then call column_pt again to check that columns are stripped in between calls.
+    %% and then verify that column numbers have been stripped from output.
+    {[],[_|_]} = compile_partition_warnings(Big, [{error_location,line},
+                                                  add_columns,
+                                                  {parse_transform,column_pt},
+                                                  {parse_transform,column_pt}]),
+
+    %% Compile a file using column_pt that adds columns and en error and error_location=line and
+    %% verify that column numbers have been stripped.
+    {error,[{_What,[{Line,_,_}]}],[_|_]} =
+        compile_partition_warnings(Big, [{error_location,line},
+                                         add_columns,
+                                         add_error,
+                                         {parse_transform,column_pt}]),
+    true = is_integer(Line),
 
     %% Cover transform code implementing the `time` option.
     {ok,big,_} = compile:file(Big, [binary, time, report,
@@ -1754,15 +1787,18 @@ do_transforms(Config) ->
 
     ok.
 
-compile_partition_warnings(Source, PT) ->
-    Opts = [binary, return, {core_transform,generic_pt}, {parse_transform,PT}],
-    {ok,big,<<_/binary>>,Ws0} = compile:file(Source, Opts),
-    [{_SourcePath,Ws}] = Ws0,
-
-    %% Return {[ColumnWarning], [LineWarning]}.
-    lists:partition(fun({{L,C},_,_}) when is_integer(L), is_integer(C) -> true;
-                       ({L,_,_}) when is_integer(L) -> false
-                    end, Ws).
+compile_partition_warnings(Source, Opts) ->
+    case compile:file(Source, [binary, return | Opts]) of
+        {ok,big,<<_/binary>>,Ws0} ->
+            [{_SourcePath,Ws}] = Ws0,
+
+            %% Return {[ColumnWarning], [LineWarning]}.
+            lists:partition(fun({{L,C},_,_}) when is_integer(L), is_integer(C) -> true;
+                               ({L,_,_}) when is_integer(L) -> false
+                            end, Ws);
+        Error ->
+            Error
+    end.
 
 %% Cover the erl_compile API used by erlc.
 erl_compile_api(Config) ->
diff --git a/lib/compiler/test/compile_SUITE_data/column_pt.erl b/lib/compiler/test/compile_SUITE_data/column_pt.erl
index 93d56cb5ed..bf599bb38f 100644
--- a/lib/compiler/test/compile_SUITE_data/column_pt.erl
+++ b/lib/compiler/test/compile_SUITE_data/column_pt.erl
@@ -19,11 +19,68 @@
 %%
 
 -module(column_pt).
--export([parse_transform/2, parse_transform_info/0]).
+-export([parse_transform/2, parse_transform_info/0, format_error/1]).
 
 parse_transform_info() ->
     %% Will default to {error_location,column}.
     #{}.
 
-parse_transform(Forms, _Options) ->
-    Forms.
+parse_transform(Forms, Options) ->
+
+    HasColumn =
+        case proplists:get_value(error_location, Options, column) of
+            column ->
+                true;
+            line ->
+                false
+        end,
+
+    AddColumns = proplists:get_value(add_columns, Options, false),
+    AddError = proplists:get_value(add_error, Options, false),
+
+    {eof,LastLocation} = lists:keyfind(eof, 1, Forms),
+    LastLine = erl_anno:line(erl_anno:new(LastLocation)),
+    ExtraFormsAnno =
+        if HasColumn ->
+                erl_anno:new({LastLine,1});
+           not HasColumn ->
+                erl_anno:new(LastLine)
+        end,
+
+    ExtraForms = [{warning,{ExtraFormsAnno,?MODULE,"injected warning"}}] ++
+        [{error,{ExtraFormsAnno,?MODULE,"injected error"}} || AddError],
+
+    lists:map(
+      fun
+          ({eof,Location}) ->
+              Anno = erl_anno:new(Location),
+              HasColumn = erl_anno:column(Anno) =/= undefined,
+              if AddColumns ->
+                      {eof,{erl_anno:line(Anno), 1}};
+                  not AddColumns ->
+                      {eof,Location}
+              end;
+          ({ErrorOrWarning,{Location,Module,Text}})
+            when ErrorOrWarning =:= error;
+                 ErrorOrWarning =:= warning ->
+              Anno = erl_anno:new(Location),
+              HasColumn = erl_anno:column(Anno) =/= undefined,
+              if AddColumns ->
+                      {ErrorOrWarning,{{erl_anno:line(Anno), 1}, Module, Text}};
+                  not AddColumns ->
+                      {ErrorOrWarning, {Location, Module, Text}}
+              end;
+          (Form) ->
+              erl_parse:map_anno(
+                fun(Anno) ->
+                        HasColumn = erl_anno:column(Anno) =/= undefined,
+                        if AddColumns ->
+                                erl_anno:set_location({erl_anno:line(Anno), 1}, Anno);
+                           not AddColumns ->
+                                Anno
+                        end
+                end, Form)
+      end, Forms ++ ExtraForms).
+
+format_error(Error) ->
+    Error.
diff --git a/lib/compiler/test/compile_SUITE_data/line_pt.erl b/lib/compiler/test/compile_SUITE_data/line_pt.erl
index 0e77072d4c..32078081d2 100644
--- a/lib/compiler/test/compile_SUITE_data/line_pt.erl
+++ b/lib/compiler/test/compile_SUITE_data/line_pt.erl
@@ -24,5 +24,17 @@
 parse_transform_info() ->
     #{error_location => line}.
 
-parse_transform(Forms, _Options) ->
-    Forms.
+parse_transform(Forms,_Options) ->
+    lists:map(
+      fun
+          ({eof,Location}) ->
+              Anno = erl_anno:new(Location),
+              false = erl_anno:column(Anno) =/= undefined,
+              {eof,Location};
+          (Form) ->
+              erl_parse:map_anno(
+                fun(Anno) ->
+                        false = erl_anno:column(Anno) =/= undefined,
+                        Anno
+                end, Form)
+      end, Forms).