Skip to content

Commit

Permalink
Unify caret position in diagnostics
Browse files Browse the repository at this point in the history
Closes #12995.
  • Loading branch information
josevalim committed Oct 7, 2023
1 parent e3d5bb7 commit a4c700b
Show file tree
Hide file tree
Showing 7 changed files with 74 additions and 74 deletions.
6 changes: 3 additions & 3 deletions lib/elixir/lib/module/types/expr.ex
Original file line number Diff line number Diff line change
Expand Up @@ -391,10 +391,10 @@ defmodule Module.Types.Expr do
end

# expr.fun(arg)
def of_expr({{:., meta1, [expr1, fun]}, _meta2, args} = expr2, _expected, stack, context) do
def of_expr({{:., _meta1, [expr1, fun]}, meta2, args} = expr2, _expected, stack, context) do
# TODO: Use expected type to infer intersection return type

context = Of.remote(expr1, fun, length(args), meta1, context)
context = Of.remote(expr1, fun, length(args), meta2, context)
stack = push_expr_stack(expr2, stack)

with {:ok, _expr_type, context} <- of_expr(expr1, :dynamic, stack, context),
Expand All @@ -407,7 +407,7 @@ defmodule Module.Types.Expr do

# &Foo.bar/1
def of_expr(
{:&, meta, [{:/, _, [{{:., _, [module, fun]}, _, []}, arity]}]},
{:&, _, [{:/, _, [{{:., _, [module, fun]}, meta, []}, arity]}]},
_expected,
_stack,
context
Expand Down
22 changes: 11 additions & 11 deletions lib/elixir/src/elixir_expand.erl
Original file line number Diff line number Diff line change
Expand Up @@ -209,12 +209,12 @@ expand({'&', Meta, [{super, SuperMeta, Args} = Expr]}, S, E) when is_list(Args)
expand_fn_capture(Meta, Expr, S, E)
end;

expand({'&', Meta, [{'/', _, [{super, _, Context}, Arity]} = Expr]}, S, E) when is_atom(Context), is_integer(Arity) ->
expand({'&', Meta, [{'/', ArityMeta, [{super, SuperMeta, Context}, Arity]} = Expr]}, S, E) when is_atom(Context), is_integer(Arity) ->
assert_no_match_or_guard_scope(Meta, "&", S, E),

case resolve_super(Meta, Arity, E) of
{Kind, Name, _} when Kind == def; Kind == defp ->
{{'&', Meta, [{'/', [], [{Name, [], Context}, Arity]}]}, S, E};
{{'&', Meta, [{'/', ArityMeta, [{Name, SuperMeta, Context}, Arity]}]}, S, E};
_ ->
expand_fn_capture(Meta, Expr, S, E)
end;
Expand Down Expand Up @@ -510,15 +510,15 @@ resolve_super(Meta, Arity, E) ->

expand_fn_capture(Meta, Arg, S, E) ->
case elixir_fn:capture(Meta, Arg, S, E) of
{{remote, Remote, Fun, Arity}, RemoteMeta, SE, EE} ->
{{remote, Remote, Fun, Arity}, RequireMeta, DotMeta, SE, EE} ->
is_atom(Remote) andalso
elixir_env:trace({remote_function, RemoteMeta, Remote, Fun, Arity}, E),
AttachedMeta = attach_context_module(Remote, Meta, E),
{{'&', AttachedMeta, [{'/', [], [{{'.', [], [Remote, Fun]}, [], []}, Arity]}]}, SE, EE};
{{local, Fun, Arity}, _LocalMeta, _SE, #{function := nil}} ->
elixir_env:trace({remote_function, RequireMeta, Remote, Fun, Arity}, E),
AttachedMeta = attach_context_module(Remote, RequireMeta, E),
{{'&', Meta, [{'/', [], [{{'.', DotMeta, [Remote, Fun]}, AttachedMeta, []}, Arity]}]}, SE, EE};
{{local, Fun, Arity}, _, _, _SE, #{function := nil}} ->
file_error(Meta, E, ?MODULE, {undefined_local_capture, Fun, Arity});
{{local, Fun, Arity}, _LocalMeta, SE, EE} ->
{{'&', Meta, [{'/', [], [{Fun, [], nil}, Arity]}]}, SE, EE};
{{local, Fun, Arity}, LocalMeta, _, SE, EE} ->
{{'&', Meta, [{'/', [], [{Fun, LocalMeta, nil}, Arity]}]}, SE, EE};
{expand, Expr, SE, EE} ->
expand(Expr, SE, EE)
end.
Expand Down Expand Up @@ -864,10 +864,10 @@ expand_remote(Receiver, DotMeta, Right, Meta, NoParens, Args, S, SL, #{context :
{{{'.', DotMeta, [Receiver, Right]}, Meta, []}, SL, E};

true ->
AttachedDotMeta = attach_context_module(Receiver, DotMeta, E),
AttachedMeta = attach_context_module(Receiver, Meta, E),
{EArgs, {SA, _}, EA} = mapfold(fun expand_arg/3, {SL, S}, E, Args),

case rewrite(Context, Receiver, AttachedDotMeta, Right, Meta, EArgs, S) of
case rewrite(Context, Receiver, DotMeta, Right, AttachedMeta, EArgs, S) of
{ok, Rewritten} ->
maybe_warn_comparison(Rewritten, Args, E),
{Rewritten, elixir_env:close_write(SA, S), EA};
Expand Down
28 changes: 14 additions & 14 deletions lib/elixir/src/elixir_fn.erl
Original file line number Diff line number Diff line change
Expand Up @@ -41,14 +41,14 @@ fn_arity(Args) -> length(Args).
capture(Meta, {'/', _, [{{'.', _, [M, F]} = Dot, RequireMeta, []}, A]}, S, E) when is_atom(F), is_integer(A) ->
Args = args_from_arity(Meta, A, E),
handle_capture_possible_warning(Meta, RequireMeta, M, F, A, E),
capture_require(Meta, {Dot, RequireMeta, Args}, S, E, true);
capture_require({Dot, RequireMeta, Args}, S, E, true);

capture(Meta, {'/', _, [{F, ImportMeta, C}, A]}, S, E) when is_atom(F), is_integer(A), is_atom(C) ->
Args = args_from_arity(Meta, A, E),
capture_import(Meta, {F, ImportMeta, Args}, S, E, true);
capture_import({F, ImportMeta, Args}, S, E, true);

capture(Meta, {{'.', _, [_, Fun]}, _, Args} = Expr, S, E) when is_atom(Fun), is_list(Args) ->
capture_require(Meta, Expr, S, E, is_sequential_and_not_empty(Args));
capture(_Meta, {{'.', _, [_, Fun]}, _, Args} = Expr, S, E) when is_atom(Fun), is_list(Args) ->
capture_require(Expr, S, E, is_sequential_and_not_empty(Args));

capture(Meta, {{'.', _, [_]}, _, Args} = Expr, S, E) when is_list(Args) ->
capture_expr(Meta, Expr, S, E, false);
Expand All @@ -59,8 +59,8 @@ capture(Meta, {'__block__', _, [Expr]}, S, E) ->
capture(Meta, {'__block__', _, _} = Expr, _S, E) ->
file_error(Meta, E, ?MODULE, {block_expr_in_capture, Expr});

capture(Meta, {Atom, _, Args} = Expr, S, E) when is_atom(Atom), is_list(Args) ->
capture_import(Meta, Expr, S, E, is_sequential_and_not_empty(Args));
capture(_Meta, {Atom, _, Args} = Expr, S, E) when is_atom(Atom), is_list(Args) ->
capture_import(Expr, S, E, is_sequential_and_not_empty(Args));

capture(Meta, {Left, Right}, S, E) ->
capture(Meta, {'{}', Meta, [Left, Right]}, S, E);
Expand All @@ -74,12 +74,12 @@ capture(Meta, Integer, _S, E) when is_integer(Integer) ->
capture(Meta, Arg, _S, E) ->
invalid_capture(Meta, Arg, E).

capture_import(Meta, {Atom, ImportMeta, Args} = Expr, S, E, Sequential) ->
capture_import({Atom, ImportMeta, Args} = Expr, S, E, Sequential) ->
Res = Sequential andalso
elixir_dispatch:import_function(ImportMeta, Atom, length(Args), E),
handle_capture(Res, Meta, Expr, S, E, Sequential).
handle_capture(Res, ImportMeta, ImportMeta, Expr, S, E, Sequential).

capture_require(Meta, {{'.', DotMeta, [Left, Right]}, RequireMeta, Args}, S, E, Sequential) ->
capture_require({{'.', DotMeta, [Left, Right]}, RequireMeta, Args}, S, E, Sequential) ->
case escape(Left, E, []) of
{EscLeft, []} ->
{ELeft, SE, EE} = elixir_expand:expand(EscLeft, S, E),
Expand All @@ -94,17 +94,17 @@ capture_require(Meta, {{'.', DotMeta, [Left, Right]}, RequireMeta, Args}, S, E,
end,

Dot = {{'.', DotMeta, [ELeft, Right]}, RequireMeta, Args},
handle_capture(Res, RequireMeta, Dot, SE, EE, Sequential);
handle_capture(Res, RequireMeta, DotMeta, Dot, SE, EE, Sequential);

{EscLeft, Escaped} ->
Dot = {{'.', DotMeta, [EscLeft, Right]}, RequireMeta, Args},
capture_expr(Meta, Dot, S, E, Escaped, Sequential)
capture_expr(RequireMeta, Dot, S, E, Escaped, Sequential)
end.

handle_capture(false, Meta, Expr, S, E, Sequential) ->
handle_capture(false, Meta, _DotMeta, Expr, S, E, Sequential) ->
capture_expr(Meta, Expr, S, E, Sequential);
handle_capture(LocalOrRemote, Meta, _Expr, S, E, _Sequential) ->
{LocalOrRemote, Meta, S, E}.
handle_capture(LocalOrRemote, Meta, DotMeta, _Expr, S, E, _Sequential) ->
{LocalOrRemote, Meta, DotMeta, S, E}.

capture_expr(Meta, Expr, S, E, Sequential) ->
capture_expr(Meta, Expr, S, E, [], Sequential).
Expand Down
2 changes: 1 addition & 1 deletion lib/elixir/test/elixir/code_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ defmodule CodeTest do
end
"""

assert {_, [%{position: {3, 17}}]} =
assert {_, [%{position: {3, 18}}]} =
Code.with_diagnostics(fn ->
quoted = Code.string_to_quoted!(sample, columns: true)
Code.eval_quoted(quoted, [])
Expand Down
28 changes: 14 additions & 14 deletions lib/elixir/test/elixir/kernel/diagnostics_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -489,9 +489,9 @@ defmodule Kernel.DiagnosticsTest do
warning: Unknown.b/0 is undefined (module Unknown is not available or is yet to be defined)
3 │ defp a, do: Unknown.b()
│ ~
~
└─ #{path}:3:22: Sample.a/0
└─ #{path}:3:23: Sample.a/0
"""

assert capture_eval(source) =~ expected
Expand Down Expand Up @@ -535,7 +535,7 @@ defmodule Kernel.DiagnosticsTest do

expected = """
warning: Unknown.b/0 is undefined (module Unknown is not available or is yet to be defined)
└─ nofile:2:22: Sample.a/0
└─ nofile:2:23: Sample.a/0
"""

assert capture_eval(source) =~ expected
Expand Down Expand Up @@ -661,9 +661,9 @@ defmodule Kernel.DiagnosticsTest do
warning: Unknown.bar/1 is undefined (module Unknown is not available or is yet to be defined)
5 │ ... Unknown.bar(:test)
│ ~
~
└─ #{path}:5:52: Sample.a/0
└─ #{path}:5:53: Sample.a/0
"""

Expand Down Expand Up @@ -710,10 +710,10 @@ defmodule Kernel.DiagnosticsTest do

expected = """
warning: Unknown.bar/0 is undefined (module Unknown is not available or is yet to be defined)
└─ nofile:3:12: Sample.a/0
└─ nofile:4:12: Sample.a/0
└─ nofile:5:12: Sample.a/0
└─ nofile:6:12: Sample.a/0
└─ nofile:3:13: Sample.a/0
└─ nofile:4:13: Sample.a/0
└─ nofile:5:13: Sample.a/0
└─ nofile:6:13: Sample.a/0
"""

Expand Down Expand Up @@ -745,12 +745,12 @@ defmodule Kernel.DiagnosticsTest do
warning: Unknown.bar/0 is undefined (module Unknown is not available or is yet to be defined)
5 │ Unknown.bar()
│ ~
~
└─ #{path}:5:12: Sample.a/0
└─ #{path}:6:12: Sample.a/0
└─ #{path}:7:12: Sample.a/0
└─ #{path}:8:12: Sample.a/0
└─ #{path}:5:13: Sample.a/0
└─ #{path}:6:13: Sample.a/0
└─ #{path}:7:13: Sample.a/0
└─ #{path}:8:13: Sample.a/0
"""

Expand Down
4 changes: 2 additions & 2 deletions lib/elixir/test/elixir/kernel/expansion_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -1143,10 +1143,10 @@ defmodule Kernel.ExpansionTest do
test "expands remotes" do
assert expand(quote(do: &List.flatten/2)) ==
quote(do: &:"Elixir.List".flatten/2)
|> clean_meta([:imports, :context, :no_parens])
|> clean_meta([:imports, :context])

assert expand(quote(do: &Kernel.is_atom/1)) ==
quote(do: &:erlang.is_atom/1) |> clean_meta([:imports, :context, :no_parens])
quote(do: &:erlang.is_atom/1) |> clean_meta([:imports, :context])
end

test "expands macros" do
Expand Down
Loading

0 comments on commit a4c700b

Please sign in to comment.