Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

convert erl_anno:anno() to integer() at transform #4

Merged
merged 2 commits into from
Sep 20, 2023
Merged

convert erl_anno:anno() to integer() at transform #4

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9d8df66
Revert "followed the abstract format change (line -> anno)"
dw-kihara Sep 14, 2023
21ddee7
convert erl_anno:anno() to integer at transform
dw-kihara Sep 14, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 27 additions & 35 deletions src/logi_location.erl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@

-export_type([location/0]).
-export_type([map_form/0]).
-export_type([application/0, line/0, line_or_anno/0]).
-export_type([application/0, line/0]).

%%----------------------------------------------------------------------------------------------------------------------
%% Macros & Records & Types
Expand All @@ -56,11 +56,11 @@

-record(?LOCATION,
{
process :: pid(),
application :: application(),
module :: module(),
function :: atom(),
line_or_anno :: line_or_anno()
process :: pid(),
application :: application(),
module :: module(),
function :: atom(),
line :: line()
}).

-opaque location() :: #?LOCATION{}.
Expand All @@ -84,41 +84,34 @@
%%
%% `0' means "Unknown Line"

-type line_or_anno() :: line() | erl_anno:anno().
%% A line number or an erl_anno:anno()
%%
%% Starting from OTP 23, the abstract format uses annos instead of line numbers.
%% This type absorbs the change.
%% See: https://www.erlang.org/docs/23/apps/erts/absform.html

%%----------------------------------------------------------------------------------------------------------------------
%% Exported Functions
%%----------------------------------------------------------------------------------------------------------------------
%% @equiv new(self(), guess_application(Module), Module, Function, Line)
-spec new(module(), atom(), line_or_anno()) -> location().
new(Module, Function, LineOrAnno) ->
new(self(), guess_application(Module), Module, Function, LineOrAnno).
-spec new(module(), atom(), line()) -> location().
new(Module, Function, Line) ->
new(self(), guess_application(Module), Module, Function, Line).

%% @doc Creates a new location object
-spec new(pid(), application(), module(), atom(), line_or_anno()) -> location().
new(Pid, Application, Module, Function, LineOrAnno) ->
Args = [Pid, Application, Module, Function, LineOrAnno],
-spec new(pid(), application(), module(), atom(), line()) -> location().
new(Pid, Application, Module, Function, Line) ->
Args = [Pid, Application, Module, Function, Line],
_ = is_pid(Pid) orelse error(badarg, Args),
_ = is_atom(Application) orelse error(badarg, Args),
_ = is_atom(Module) orelse error(badarg, Args),
_ = is_atom(Function) orelse error(badarg, Args),
_ = (is_integer(LineOrAnno) andalso LineOrAnno >= 0) orelse erl_anno:is_anno(LineOrAnno) orelse error(badarg, Args),
unsafe_new(Pid, Application, Module, Function, LineOrAnno).
_ = (is_integer(Line) andalso Line >= 0) orelse error(badarg, Args),
unsafe_new(Pid, Application, Module, Function, Line).

%% @doc Equivalent to {@link new/5} except omission of the arguments validation
-spec unsafe_new(pid(), application(), module(), atom(), line_or_anno()) -> location().
unsafe_new(Pid, Application, Module, Function, LineOrAnno) ->
-spec unsafe_new(pid(), application(), module(), atom(), line()) -> location().
unsafe_new(Pid, Application, Module, Function, Line) ->
#?LOCATION{
process = Pid,
application = Application,
module = Module,
function = Function,
line_or_anno = LineOrAnno
process = Pid,
application = Application,
module = Module,
function = Function,
line = Line
}.

%% @doc Returns `true' if `X' is a location object, `false' otherwise.
Expand Down Expand Up @@ -152,11 +145,11 @@ from_map(Map) ->
-spec to_map(Location :: location()) -> map_form().
to_map(L) ->
#{
process => get_process(L),
application => get_application(L),
module => get_module(L),
function => get_function(L),
line => get_line(L)
process => L#?LOCATION.process,
application => L#?LOCATION.application,
module => L#?LOCATION.module,
function => L#?LOCATION.function,
line => L#?LOCATION.line
}.

%% @doc Guesses the location where the function is called (parse transformation fallback)
Expand Down Expand Up @@ -225,5 +218,4 @@ get_function(#?LOCATION{function = Function}) -> Function.

%% @doc Gets the line of `Location'
-spec get_line(Location :: location()) -> line().
get_line(#?LOCATION{line_or_anno = LineOrAnno}) when is_integer(LineOrAnno) -> LineOrAnno;
get_line(#?LOCATION{line_or_anno = LineOrAnno}) -> erl_anno:line(LineOrAnno).
get_line(#?LOCATION{line = Line}) -> Line.
4 changes: 2 additions & 2 deletions src/logi_transform.erl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ transform_call(logi, Severity0, {_, _, _, Args} = Call, Loc = #location{line_or_
[Logger, {string, _, _} = Fmt] ->
Opts = {cons, LineOrAnno, {tuple, LineOrAnno, [{atom, LineOrAnno, logger}, Logger]}, {nil, LineOrAnno}},
logi_call_expr(Severity, Fmt, {nil, LineOrAnno}, Opts, Loc);
[Logger, {string, _, _} = Fmt, {nil, LineOrAnno} = Data] ->
[Logger, {string, _, _} = Fmt, {nil, _} = Data] ->
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When anno contains column, LineOrAnno that is bound at #location{line_or_anno = LineOrAnno} will not match here.

Opts = {cons, LineOrAnno, {tuple, LineOrAnno, [{atom, LineOrAnno, logger}, Logger]}, {nil, LineOrAnno}},
logi_call_expr(Severity, Fmt, Data, Opts, Loc);
[Logger, {string, _, _} = Fmt, {cons, _, _, _} = Data] ->
Expand All @@ -136,7 +136,7 @@ logi_location_expr(Loc = #location{line_or_anno = LineOrAnno}) ->
{atom, LineOrAnno, Loc#location.application},
{atom, LineOrAnno, Loc#location.module},
{atom, LineOrAnno, Loc#location.function},
{integer, LineOrAnno, LineOrAnno}
{integer, LineOrAnno, logi_transform_utils:line_or_anno_to_line(LineOrAnno)}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When anno contains column, this made malformed abstract format values like {integer, {10, 12}, {10, 12}} wherer the third element must be an integer.

]).

-spec logi_call_expr(logi:severity(), expr(), expr(), expr(), #location{}) -> expr().
Expand Down
15 changes: 8 additions & 7 deletions src/logi_transform_utils.erl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
-export([get_module/1]).
-export([make_var/2]).
-export([make_call_remote/4]).
-export([line_or_anno_to_line/1]).

%%----------------------------------------------------------------------------------------------------------------------
%% Exported Functions
Expand Down Expand Up @@ -39,14 +40,20 @@ make_var(LineOrAnno, Prefix) ->
Seq0 -> Seq0
end,
_ = put({?MODULE, seq}, Seq + 1),
Name = list_to_atom(Prefix ++ "_line" ++ line_or_anno_to_string(LineOrAnno) ++ "_" ++ integer_to_list(Seq)),
Name = list_to_atom(Prefix ++ "_line" ++ integer_to_list(line_or_anno_to_line(LineOrAnno)) ++ "_" ++ integer_to_list(Seq)),
{var, LineOrAnno, Name}.

%% @doc Makes a abstract term for external function call
-spec make_call_remote(logi_transform:line_or_anno(), module(), atom(), [logi_transform:expr()]) -> logi_transform:expr_call_remote().
make_call_remote(LineOrAnno, Module, Function, ArgsExpr) ->
{call, LineOrAnno, {remote, LineOrAnno, {atom, LineOrAnno, Module}, {atom, LineOrAnno, Function}}, ArgsExpr}.

-spec line_or_anno_to_line(logi_transform:line_or_anno()) -> integer().
line_or_anno_to_line(LineOrAnno) when is_integer(LineOrAnno)
-> LineOrAnno;
line_or_anno_to_line(LineOrAnno)
-> erl_anno:line(LineOrAnno).

%%----------------------------------------------------------------------------------------------------------------------
%% Internal Functions
%%----------------------------------------------------------------------------------------------------------------------
Expand All @@ -61,9 +68,3 @@ find_app_file([Dir | Dirs]) ->
end;
_ -> find_app_file(Dirs)
end.

-spec line_or_anno_to_string(logi_transform:line_or_anno()) -> string().
line_or_anno_to_string(LineOrAnno) when is_integer(LineOrAnno)
-> integer_to_list(LineOrAnno);
line_or_anno_to_string(LineOrAnno)
-> integer_to_list(erl_anno:line(LineOrAnno)).
15 changes: 0 additions & 15 deletions test/logi_location_tests.erl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,18 +59,3 @@ guess_test_() ->
?assertEqual(undefined, logi_location:guess_application('UNDEFINED_MODULE'))
end}
]}.

anno_test_() ->
[
{"Creates a new location object",
fun () ->
L = logi_location:new(lists, map, erl_anno:new({12, 10})),
?assert(logi_location:is_location(L)),
?assertEqual(12, logi_location:get_line(L))
end},
{"Converts to a map without anno",
fun () ->
L = logi_location:new(lists, map, erl_anno:new({12, 10})),
?assertMatch(#{line := 12}, logi_location:to_map(L))
end}
].
15 changes: 15 additions & 0 deletions test/logi_transform_tests.erl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,21 @@ log_test_() ->
end),
?assert(logi:is_logger(Logger))
end},
{"`logi:Severity/Arity` with logger is transformed",
fun () ->
InstallSink(info),

%% [ERROR] function call
?assertError(_, apply(logi, info, [logi:new([]), "hello world", []])),

%% [NO ERROR] transformed call
Logger = logi:info(logi:new([]), "hello world", []),
?assertLog("hello world", [], fun (C) ->
?assertEqual(info, logi_context:get_severity(C)),
?assertEqual(71, logi_location:get_line(logi_context:get_location(C)))
end),
?assert(logi:is_logger(Logger))
end},
{"`Data` arugment will not be evaluated if it is unnecessary",
fun () ->
InstallSink(info),
Expand Down