From 7e9006988d3d6a0782d658d5b30bba085a8c91dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vin=C3=ADcius=20M=C3=BCller?= Date: Sun, 3 Sep 2023 19:14:51 +0000 Subject: [PATCH 01/12] Add span field to diagnostics (#12843) --- lib/elixir/lib/code.ex | 1 + lib/elixir/lib/exception.ex | 11 ++- lib/elixir/lib/io.ex | 9 +- lib/elixir/lib/kernel/parallel_compiler.ex | 13 ++- lib/elixir/lib/module/parallel_checker.ex | 3 +- lib/elixir/src/elixir_errors.erl | 71 ++++++++------- .../test/elixir/kernel/diagnostics_test.exs | 86 +++++++++++++++++++ lib/mix/lib/mix/compilers/elixir.ex | 17 ++-- lib/mix/lib/mix/task.compiler.ex | 4 +- 9 files changed, 167 insertions(+), 48 deletions(-) diff --git a/lib/elixir/lib/code.ex b/lib/elixir/lib/code.ex index 8a123c58fb7..163de0b296e 100644 --- a/lib/elixir/lib/code.ex +++ b/lib/elixir/lib/code.ex @@ -203,6 +203,7 @@ defmodule Code do required(:message) => String.t(), required(:position) => position, required(:stacktrace) => Exception.stacktrace(), + required(:span) => {non_neg_integer, non_neg_integer} | nil, optional(any()) => any() } diff --git a/lib/elixir/lib/exception.ex b/lib/elixir/lib/exception.ex index 3ca79087d21..6f5b7d39a95 100644 --- a/lib/elixir/lib/exception.ex +++ b/lib/elixir/lib/exception.ex @@ -953,7 +953,7 @@ defmodule SyntaxError do }) when not is_nil(snippet) and not is_nil(column) do snippet = - :elixir_errors.format_snippet({line, column}, file, description, snippet, :error, []) + :elixir_errors.format_snippet({line, column}, file, description, snippet, :error, [], nil) format_message(file, line, column, snippet) end @@ -965,7 +965,8 @@ defmodule SyntaxError do column: column, description: description }) do - snippet = :elixir_errors.format_snippet({line, column}, file, description, nil, :error, []) + snippet = + :elixir_errors.format_snippet({line, column}, file, description, nil, :error, [], nil) padded = " " <> String.replace(snippet, "\n", "\n ") format_message(file, line, column, padded) @@ -1002,7 +1003,7 @@ defmodule TokenMissingError do }) when not is_nil(snippet) and not is_nil(column) do snippet = - :elixir_errors.format_snippet({line, column}, file, description, snippet, :error, []) + :elixir_errors.format_snippet({line, column}, file, description, snippet, :error, [], nil) format_message(file, line, column, snippet) end @@ -1014,7 +1015,9 @@ defmodule TokenMissingError do column: column, description: description }) do - snippet = :elixir_errors.format_snippet({line, column}, file, description, nil, :error, []) + snippet = + :elixir_errors.format_snippet({line, column}, file, description, nil, :error, [], nil) + padded = " " <> String.replace(snippet, "\n", "\n ") format_message(file, line, column, padded) end diff --git a/lib/elixir/lib/io.ex b/lib/elixir/lib/io.ex index 55cc98c9ce4..ed6745cabcf 100644 --- a/lib/elixir/lib/io.ex +++ b/lib/elixir/lib/io.ex @@ -329,7 +329,10 @@ defmodule IO do def warn(message, %Macro.Env{line: line, file: file} = env) do message = to_chardata(message) - :elixir_errors.emit_diagnostic(:warning, line, file, message, Macro.Env.stacktrace(env), true) + + :elixir_errors.emit_diagnostic(:warning, line, file, message, Macro.Env.stacktrace(env), + read_snippet: true + ) end def warn(message, [{_, _} | _] = keyword) do @@ -351,12 +354,12 @@ defmodule IO do def warn(message, []) do message = to_chardata(message) - :elixir_errors.emit_diagnostic(:warning, 0, nil, message, [], false) + :elixir_errors.emit_diagnostic(:warning, 0, nil, message, [], read_snippet: false) end def warn(message, [{_, _, _, _} | _] = stacktrace) do message = to_chardata(message) - :elixir_errors.emit_diagnostic(:warning, 0, nil, message, stacktrace, false) + :elixir_errors.emit_diagnostic(:warning, 0, nil, message, stacktrace, read_snippet: false) end @doc false diff --git a/lib/elixir/lib/kernel/parallel_compiler.ex b/lib/elixir/lib/kernel/parallel_compiler.ex index a936d31b9b1..b86d8dc38fe 100644 --- a/lib/elixir/lib/kernel/parallel_compiler.ex +++ b/lib/elixir/lib/kernel/parallel_compiler.ex @@ -864,7 +864,8 @@ defmodule Kernel.ParallelCompiler do file: Path.absname(file), position: nil, message: description, - stacktrace: stacktrace + stacktrace: stacktrace, + span: nil } end end @@ -892,7 +893,15 @@ defmodule Kernel.ParallelCompiler do line = get_line(file, reason, stack) file = Path.absname(file) message = :unicode.characters_to_binary(Kernel.CLI.format_error(kind, reason, stack)) - %{file: file, position: line || 0, message: message, severity: :error, stacktrace: stack} + + %{ + file: file, + position: line || 0, + message: message, + severity: :error, + stacktrace: stack, + span: nil + } end defp get_line(_file, %{line: line, column: column}, _stack) diff --git a/lib/elixir/lib/module/parallel_checker.ex b/lib/elixir/lib/module/parallel_checker.ex index 2b55d3e22c3..e6fd474075e 100644 --- a/lib/elixir/lib/module/parallel_checker.ex +++ b/lib/elixir/lib/module/parallel_checker.ex @@ -328,7 +328,8 @@ defmodule Module.ParallelChecker do file: file, position: position_to_tuple(position), message: IO.iodata_to_binary(message), - stacktrace: [to_stacktrace(file, position, mfa)] + stacktrace: [to_stacktrace(file, position, mfa)], + span: nil } end diff --git a/lib/elixir/src/elixir_errors.erl b/lib/elixir/src/elixir_errors.erl index cfa8dfcab79..7efee514b2f 100644 --- a/lib/elixir/src/elixir_errors.erl +++ b/lib/elixir/src/elixir_errors.erl @@ -6,7 +6,7 @@ -module(elixir_errors). -export([compile_error/1, compile_error/3, parse_error/5]). -export([function_error/4, module_error/4, file_error/4]). --export([format_snippet/6]). +-export([format_snippet/7]). -export([erl_warn/3, file_warn/4]). -export([print_diagnostic/2, emit_diagnostic/6]). -export([print_warning/2, print_warning/3]). @@ -19,21 +19,23 @@ %% TODO: Remove me on Elixir v2.0. %% Called by deprecated Kernel.ParallelCompiler.print_warning. print_warning(Position, File, Message) -> - Output = format_snippet(Position, File, Message, nil, warning, []), + Output = format_snippet(Position, File, Message, nil, warning, [], nil), io:put_chars(standard_error, [Output, $\n, $\n]). %% Called by Module.ParallelChecker. print_warning(Message, Diagnostic) -> #{file := File, position := Position, stacktrace := S} = Diagnostic, Snippet = get_snippet(File, Position), - Output = format_snippet(Position, File, Message, Snippet, warning, S), + Span = get_span(Diagnostic), + Output = format_snippet(Position, File, Message, Snippet, warning, S, Span), io:put_chars(standard_error, [Output, $\n, $\n]). %% Called by Module.ParallelChecker. print_warning_group(Message, [Diagnostic | Others]) -> #{file := File, position := Position, stacktrace := S} = Diagnostic, Snippet = get_snippet(File, Position), - Formatted = format_snippet(Position, File, Message, Snippet, warning, S), + Span = get_span(Diagnostic), + Formatted = format_snippet(Position, File, Message, Snippet, warning, S, Span), LineNumber = extract_line(Position), LineDigits = get_line_number_digits(LineNumber, 1), Padding = case Snippet of @@ -43,6 +45,9 @@ print_warning_group(Message, [Diagnostic | Others]) -> Locations = [["\n", n_spaces(Padding), "└─ ", 'Elixir.Exception':format_stacktrace_entry(ES)] || #{stacktrace := [ES]} <- Others], io:put_chars(standard_error, [Formatted, Locations, $\n, $\n]). +get_span(#{span := nil}) -> nil; +get_span(#{span := Span}) -> Span. + get_snippet(nil, _Position) -> nil; get_snippet(File, Position) -> @@ -59,7 +64,8 @@ print_diagnostic(#{severity := Severity, message := M, stacktrace := Stacktrace, false -> nil end, - Output = format_snippet(P, F, M, Snippet, Severity, Stacktrace), + Span = get_span(Diagnostic), + Output = format_snippet(P, F, M, Snippet, Severity, Stacktrace, Span), MaybeStack = case (F /= nil) orelse elixir_config:is_bootstrap() of @@ -70,13 +76,21 @@ print_diagnostic(#{severity := Severity, message := M, stacktrace := Stacktrace, io:put_chars(standard_error, [Output, MaybeStack, $\n, $\n]), Diagnostic. -emit_diagnostic(Severity, Position, File, Message, Stacktrace, ReadSnippet) -> +emit_diagnostic(Severity, Position, File, Message, Stacktrace, Options) -> + ReadSnippet = proplists:get_value(read_snippet, Options, false), + + Span = case lists:keyfind(span, 1, Options) of + {span, {EndLine, EndCol}} -> {EndLine, EndCol}; + _ -> nil + end, + Diagnostic = #{ severity => Severity, file => File, position => Position, message => unicode:characters_to_binary(Message), - stacktrace => Stacktrace + stacktrace => Stacktrace, + span => Span }, case get(elixir_code_diagnostics) of @@ -116,11 +130,11 @@ do_get_file_line(IoDevice, N) -> %% Format snippets %% "Snippet" here refers to the source code line where the diagnostic/error occured -format_snippet(_Position, nil, Message, nil, Severity, _Stacktrace) -> +format_snippet(_Position, nil, Message, nil, Severity, _Stacktrace, _Span) -> Formatted = [prefix(Severity), Message], unicode:characters_to_binary(Formatted); -format_snippet(Position, File, Message, nil, Severity, Stacktrace) -> +format_snippet(Position, File, Message, nil, Severity, Stacktrace, _Span) -> Location = location_format(Position, File, Stacktrace), Formatted = io_lib:format( @@ -131,28 +145,21 @@ format_snippet(Position, File, Message, nil, Severity, Stacktrace) -> unicode:characters_to_binary(Formatted); -format_snippet(Position, File, Message, Snippet, Severity, Stacktrace) -> - {Content, Column} = - case Snippet of - S when is_map(Snippet) -> - #{content := C, offset := O} = S, - {C, O}; - - _ when is_binary(Snippet) -> - {Snippet, extract_column(Position)} - end, - +format_snippet(Position, File, Message, Snippet, Severity, Stacktrace, Span) -> + Column = extract_column(Position), LineNumber = extract_line(Position), LineDigits = get_line_number_digits(LineNumber, 1), Spacing = n_spaces(max(2, LineDigits) + 1), LineNumberSpacing = if LineDigits =:= 1 -> 1; true -> 0 end, - {FormattedLine, ColumnsTrimmed} = format_line(Content), + {FormattedLine, ColumnsTrimmed} = format_line(Snippet), Location = location_format(Position, File, Stacktrace), Highlight = case Column of nil -> highlight_below_line(FormattedLine, Severity); - _ -> highlight_at_position(Column - ColumnsTrimmed, Severity) + _ -> + Length = calculate_span_length({LineNumber, Column}, Span), + highlight_at_position(Column - ColumnsTrimmed, Severity, Length) end, Formatted = io_lib:format( @@ -173,6 +180,10 @@ format_snippet(Position, File, Message, Snippet, Severity, Stacktrace) -> unicode:characters_to_binary(Formatted). +calculate_span_length({StartLine, StartCol}, {StartLine, EndCol}) -> EndCol - StartCol; +calculate_span_length({StartLine, _}, {EndLine, _}) when EndLine > StartLine -> 1; +calculate_span_length({_, _}, nil) -> 1. + format_line(Line) -> case trim_line(Line, 0) of {Trimmed, SpacesMatched} when SpacesMatched >= 27 -> @@ -196,11 +207,11 @@ pad_line([Last], _Padding) -> [Last]; pad_line([First, <<"">> | Rest], Padding) -> [First, "\n" | pad_line([<<"">> | Rest], Padding)]; pad_line([First | Rest], Padding) -> [First, Padding | pad_line(Rest, Padding)]. -highlight_at_position(Column, Severity) -> +highlight_at_position(Column, Severity, Length) -> Spacing = n_spaces(max(Column - 1, 0)), case Severity of - warning -> highlight([Spacing, $~], warning); - error -> highlight([Spacing, $^], error) + warning -> highlight([Spacing, lists:duplicate(Length, $~)], warning); + error -> highlight([Spacing, lists:duplicate(Length, $^)], error) end. highlight_below_line(Line, Severity) -> @@ -228,7 +239,7 @@ n_spaces(N) -> lists:duplicate(N, " "). erl_warn(none, File, Warning) -> erl_warn(0, File, Warning); erl_warn(Location, File, Warning) when is_binary(File) -> - emit_diagnostic(warning, Location, File, Warning, [], true). + emit_diagnostic(warning, Location, File, Warning, [], [{read_snippet, true}]). -spec file_warn(list(), binary() | #{file := binary(), _ => _}, module(), any()) -> ok. file_warn(Meta, File, Module, Desc) when is_list(Meta), is_binary(File) -> @@ -240,7 +251,7 @@ file_warn(Meta, E, Module, Desc) when is_list(Meta) -> false -> {EnvPosition, EnvFile, EnvStacktrace} = env_format(Meta, E), Message = Module:format_error(Desc), - emit_diagnostic(warning, EnvPosition, EnvFile, Message, EnvStacktrace, true) + emit_diagnostic(warning, EnvPosition, EnvFile, Message, EnvStacktrace, [{read_snippet, true}]) end. -spec file_error(list(), binary() | #{file := binary(), _ => _}, module(), any()) -> no_return(). @@ -273,7 +284,7 @@ function_error(Meta, Env, Module, Desc) -> print_error(Meta, Env, Module, Desc) -> {EnvPosition, EnvFile, EnvStacktrace} = env_format(Meta, Env), Message = Module:format_error(Desc), - emit_diagnostic(error, EnvPosition, EnvFile, Message, EnvStacktrace, true), + emit_diagnostic(error, EnvPosition, EnvFile, Message, EnvStacktrace, [{read_snippet, true}]), ok. %% Compilation error. @@ -398,12 +409,12 @@ raise_snippet(Location, File, Input, Kind, Message) when is_binary(File) -> snippet_line(InputString, Location, StartLine) -> {line, Line} = lists:keyfind(line, 1, Location), case lists:keyfind(column, 1, Location) of - {column, Column} -> + {column, _} -> Lines = string:split(InputString, "\n", all), Snippet = (lists:nth(Line - StartLine + 1, Lines)), case string:trim(Snippet, leading) of [] -> nil; - _ -> #{content => elixir_utils:characters_to_binary(Snippet), offset => Column} + _ -> elixir_utils:characters_to_binary(Snippet) end; false -> diff --git a/lib/elixir/test/elixir/kernel/diagnostics_test.exs b/lib/elixir/test/elixir/kernel/diagnostics_test.exs index ab22899dc05..9eb2f769c54 100644 --- a/lib/elixir/test/elixir/kernel/diagnostics_test.exs +++ b/lib/elixir/test/elixir/kernel/diagnostics_test.exs @@ -684,6 +684,92 @@ defmodule Kernel.DiagnosticsTest do end end + describe "Code.print_diagnostic" do + @tag :tmp_dir + test "handles diagnostic with span", %{tmp_dir: tmp_dir} do + path = make_relative_tmp(tmp_dir, "diagnostic_length.ex") + + diagnostic = %{ + file: path, + severity: :error, + message: "Diagnostic span test", + stacktrace: [], + position: {4, 7}, + span: {4, 10}, + compiler_name: "Elixir" + } + + source = """ + defmodule Sample do + @file "#{path}" + + def bar do + nil + end + end + """ + + File.write!(path, source) + + result = + ExUnit.CaptureIO.capture_io(:stderr, fn -> + Code.print_diagnostic(diagnostic) + end) + + assert result == """ + error: Diagnostic span test + │ + 4 │ def bar do + │ ^^^ + │ + └─ #{path}:4:7 + + """ + end + + @tag :tmp_dir + test "prints single marker for multiline diagnostic", %{tmp_dir: tmp_dir} do + path = make_relative_tmp(tmp_dir, "diagnostic_length.ex") + + diagnostic = %{ + file: path, + severity: :error, + message: "Diagnostic span test", + stacktrace: [], + position: {4, 7}, + span: {5, 2}, + compiler_name: "Elixir" + } + + source = """ + defmodule Sample do + @file "#{path}" + + def bar do + nil + end + end + """ + + File.write!(path, source) + + result = + ExUnit.CaptureIO.capture_io(:stderr, fn -> + Code.print_diagnostic(diagnostic) + end) + + assert result == """ + error: Diagnostic span test + │ + 4 │ def bar do + │ ^ + │ + └─ #{path}:4:7 + + """ + end + end + defp make_relative_tmp(tmp_dir, filename) do # Compiler outputs relative, so we just grab the tmp dir tmp_dir diff --git a/lib/mix/lib/mix/compilers/elixir.ex b/lib/mix/lib/mix/compilers/elixir.ex index 5383fa62701..4668ef57e41 100644 --- a/lib/mix/lib/mix/compilers/elixir.ex +++ b/lib/mix/lib/mix/compilers/elixir.ex @@ -1,7 +1,7 @@ defmodule Mix.Compilers.Elixir do @moduledoc false - @manifest_vsn 20 + @manifest_vsn 21 @checkpoint_vsn 2 import Record @@ -720,14 +720,15 @@ defmodule Mix.Compilers.Elixir do runtime_warnings: runtime_warnings ) <- sources, file = Path.absname(source), - {position, message} <- compile_warnings ++ runtime_warnings do + {position, message, span} <- compile_warnings ++ runtime_warnings do diagnostic = %Mix.Task.Compiler.Diagnostic{ severity: :warning, file: file, position: position, message: message, compiler_name: "Elixir", - stacktrace: [] + stacktrace: [], + span: span } if print? do @@ -740,8 +741,8 @@ defmodule Mix.Compilers.Elixir do end defp apply_warnings(sources, %{runtime_warnings: r_warnings, compile_warnings: c_warnings}) do - runtime_group = Enum.group_by(r_warnings, & &1.file, &{&1.position, &1.message}) - compile_group = Enum.group_by(c_warnings, & &1.file, &{&1.position, &1.message}) + runtime_group = Enum.group_by(r_warnings, & &1.file, &{&1.position, &1.message, &1.span}) + compile_group = Enum.group_by(c_warnings, & &1.file, &{&1.position, &1.message, &1.span}) for source( source: source_path, @@ -762,7 +763,8 @@ defmodule Mix.Compilers.Elixir do position: position, message: message, severity: severity, - stacktrace: stacktrace + stacktrace: stacktrace, + span: span }) do %Mix.Task.Compiler.Diagnostic{ file: file, @@ -770,7 +772,8 @@ defmodule Mix.Compilers.Elixir do message: message, severity: severity, compiler_name: "Elixir", - stacktrace: stacktrace + stacktrace: stacktrace, + span: span } end diff --git a/lib/mix/lib/mix/task.compiler.ex b/lib/mix/lib/mix/task.compiler.ex index dbaaeb0b70e..be968b2ee25 100644 --- a/lib/mix/lib/mix/task.compiler.ex +++ b/lib/mix/lib/mix/task.compiler.ex @@ -40,7 +40,8 @@ defmodule Mix.Task.Compiler do position: position, compiler_name: String.t(), details: any, - stacktrace: Exception.stacktrace() + stacktrace: Exception.stacktrace(), + span: {non_neg_integer, non_neg_integer} | nil } @typedoc """ @@ -82,6 +83,7 @@ defmodule Mix.Task.Compiler do :message, :position, :compiler_name, + span: nil, details: nil, stacktrace: [] ] From ce79c2c83d6cb84be4b4efad9df3baf5b8396df5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vin=C3=ADcius=20M=C3=BCller?= Date: Sun, 3 Sep 2023 19:33:02 +0000 Subject: [PATCH 02/12] Add Access.at/2 (#12906) --- lib/elixir/lib/access.ex | 18 ++++++++++-------- lib/elixir/test/elixir/access_test.exs | 17 +++++++++++++++++ 2 files changed, 27 insertions(+), 8 deletions(-) diff --git a/lib/elixir/lib/access.ex b/lib/elixir/lib/access.ex index 4c893b93286..9c13cc56481 100644 --- a/lib/elixir/lib/access.ex +++ b/lib/elixir/lib/access.ex @@ -650,6 +650,8 @@ defmodule Access do the longer it will take to access its index. Therefore index-based operations are generally avoided in favor of other functions in the `Enum` module. + A `default` value can be given since Elixir v1.16. + The returned function is typically passed as an accessor to `Kernel.get_in/2`, `Kernel.get_and_update_in/3`, and friends. @@ -694,20 +696,20 @@ defmodule Access do ** (RuntimeError) Access.at/1 expected a list, got: %{} """ - @spec at(integer) :: access_fun(data :: list, current_value :: term) - def at(index) when is_integer(index) do - fn op, data, next -> at(op, data, index, next) end + @spec at(integer, term) :: access_fun(data :: list, current_value :: term) + def at(index, default \\ nil) when is_integer(index) do + fn op, data, next -> at(op, data, index, next, default) end end - defp at(:get, data, index, next) when is_list(data) do - data |> Enum.at(index) |> next.() + defp at(:get, data, index, next, default) when is_list(data) do + data |> Enum.at(index, default) |> next.() end - defp at(:get_and_update, data, index, next) when is_list(data) do - get_and_update_at(data, index, next, [], fn -> nil end) + defp at(:get_and_update, data, index, next, default) when is_list(data) do + get_and_update_at(data, index, next, [], fn -> default end) end - defp at(_op, data, _index, _next) do + defp at(_op, data, _index, _next, _default) do raise "Access.at/1 expected a list, got: #{inspect(data)}" end diff --git a/lib/elixir/test/elixir/access_test.exs b/lib/elixir/test/elixir/access_test.exs index b1ae1fb8848..7a376be8fab 100644 --- a/lib/elixir/test/elixir/access_test.exs +++ b/lib/elixir/test/elixir/access_test.exs @@ -239,6 +239,23 @@ defmodule AccessTest do end end + describe "at/2" do + @test_list [1, 2, 3, 4, 5, 6] + + test "accepts default value (get_in)" do + assert :default = get_in(@test_list, [Access.at(10, :default)]) + end + + test "accepts default value (get_and_update_in)" do + list = @test_list + + assert {:default, ^list} = + get_and_update_in(list, [Access.at(10, :default)], fn :default -> + {nil, nil} + end) + end + end + describe "at!/1" do @test_list [1, 2, 3, 4, 5, 6] From bdf66e7d6e6558fc821571c4c23e7fb81c24ab09 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Mon, 4 Sep 2023 10:50:23 +0200 Subject: [PATCH 03/12] Revert "Add Access.at/2 (#12906)" This reverts commit ce79c2c83d6cb84be4b4efad9df3baf5b8396df5. See #10158. --- lib/elixir/lib/access.ex | 18 ++++++++---------- lib/elixir/test/elixir/access_test.exs | 17 ----------------- 2 files changed, 8 insertions(+), 27 deletions(-) diff --git a/lib/elixir/lib/access.ex b/lib/elixir/lib/access.ex index 9c13cc56481..4c893b93286 100644 --- a/lib/elixir/lib/access.ex +++ b/lib/elixir/lib/access.ex @@ -650,8 +650,6 @@ defmodule Access do the longer it will take to access its index. Therefore index-based operations are generally avoided in favor of other functions in the `Enum` module. - A `default` value can be given since Elixir v1.16. - The returned function is typically passed as an accessor to `Kernel.get_in/2`, `Kernel.get_and_update_in/3`, and friends. @@ -696,20 +694,20 @@ defmodule Access do ** (RuntimeError) Access.at/1 expected a list, got: %{} """ - @spec at(integer, term) :: access_fun(data :: list, current_value :: term) - def at(index, default \\ nil) when is_integer(index) do - fn op, data, next -> at(op, data, index, next, default) end + @spec at(integer) :: access_fun(data :: list, current_value :: term) + def at(index) when is_integer(index) do + fn op, data, next -> at(op, data, index, next) end end - defp at(:get, data, index, next, default) when is_list(data) do - data |> Enum.at(index, default) |> next.() + defp at(:get, data, index, next) when is_list(data) do + data |> Enum.at(index) |> next.() end - defp at(:get_and_update, data, index, next, default) when is_list(data) do - get_and_update_at(data, index, next, [], fn -> default end) + defp at(:get_and_update, data, index, next) when is_list(data) do + get_and_update_at(data, index, next, [], fn -> nil end) end - defp at(_op, data, _index, _next, _default) do + defp at(_op, data, _index, _next) do raise "Access.at/1 expected a list, got: #{inspect(data)}" end diff --git a/lib/elixir/test/elixir/access_test.exs b/lib/elixir/test/elixir/access_test.exs index 7a376be8fab..b1ae1fb8848 100644 --- a/lib/elixir/test/elixir/access_test.exs +++ b/lib/elixir/test/elixir/access_test.exs @@ -239,23 +239,6 @@ defmodule AccessTest do end end - describe "at/2" do - @test_list [1, 2, 3, 4, 5, 6] - - test "accepts default value (get_in)" do - assert :default = get_in(@test_list, [Access.at(10, :default)]) - end - - test "accepts default value (get_and_update_in)" do - list = @test_list - - assert {:default, ^list} = - get_and_update_in(list, [Access.at(10, :default)], fn :default -> - {nil, nil} - end) - end - end - describe "at!/1" do @test_list [1, 2, 3, 4, 5, 6] From 6d31f098cca7dff7e4d346b9c2086492bb115138 Mon Sep 17 00:00:00 2001 From: felipe stival <14948182+v0idpwn@users.noreply.github.com> Date: Mon, 4 Sep 2023 13:53:58 +0300 Subject: [PATCH 04/12] Fix remark in mix format docs (#12908) The text was true at some point, but now the code is always compiled before running the formatter if formatter.exs includes plugins, so it doesn't apply anymore. --- lib/mix/lib/mix/tasks/format.ex | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/mix/lib/mix/tasks/format.ex b/lib/mix/lib/mix/tasks/format.ex index c81aeb2c8b7..7603d572a48 100644 --- a/lib/mix/lib/mix/tasks/format.ex +++ b/lib/mix/lib/mix/tasks/format.ex @@ -129,9 +129,8 @@ defmodule Mix.Tasks.Format do inputs: ["{mix,.formatter}.exs", "{config,lib,test}/**/*.{ex,exs}", "posts/*.{md,markdown}"] ] - Remember that, when running the formatter with plugins, you must make - sure that your dependencies and your application have been compiled, - so the relevant plugin code can be loaded. Otherwise a warning is logged. + Notice that, when running the formatter with plugins, your code will be + compiled first. In addition, the order by which you input your plugins is the format order. So, in the above `.formatter.exs`, the `MixMarkdownFormatter` will format From 30c2eae6aee84c2088c4eab0368a95a55bcd1dea Mon Sep 17 00:00:00 2001 From: felipe stival <14948182+v0idpwn@users.noreply.github.com> Date: Mon, 4 Sep 2023 14:47:17 +0300 Subject: [PATCH 05/12] Fix error message for invalid list of plugins (#12909) --- lib/mix/lib/mix/tasks/format.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/mix/lib/mix/tasks/format.ex b/lib/mix/lib/mix/tasks/format.ex index 7603d572a48..8869941d8b1 100644 --- a/lib/mix/lib/mix/tasks/format.ex +++ b/lib/mix/lib/mix/tasks/format.ex @@ -245,7 +245,7 @@ defmodule Mix.Tasks.Format do plugins = Keyword.get(formatter_opts, :plugins, []) if not is_list(plugins) do - Mix.raise("Expected :plugins to return a list of directories, got: #{inspect(plugins)}") + Mix.raise("Expected :plugins to return a list of modules, got: #{inspect(plugins)}") end if plugins != [] do From e9d548b1220ebc1246fce7142ee7e6da8218c09c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Mon, 4 Sep 2023 14:00:49 +0200 Subject: [PATCH 06/12] Improve error message on Access module --- lib/elixir/lib/access.ex | 61 ++++++++++++++++++++++++++++++++++++---- 1 file changed, 55 insertions(+), 6 deletions(-) diff --git a/lib/elixir/lib/access.ex b/lib/elixir/lib/access.ex index 4c893b93286..a6e4569f095 100644 --- a/lib/elixir/lib/access.ex +++ b/lib/elixir/lib/access.ex @@ -191,9 +191,13 @@ defmodule Access do case __STACKTRACE__ do [unquote(top) | _] -> reason = - "#{inspect(unquote(module))} does not implement the Access behaviour. " <> - "If you are using get_in/put_in/update_in, you can specify the field " <> - "to be accessed using Access.key!/1" + """ + #{inspect(unquote(module))} does not implement the Access behaviour + + You can use the "struct.field" syntax to access struct fields. \ + You can also use Access.key!/1 to access struct fields dynamically \ + inside get_in/put_in/update_in\ + """ %{unquote(exception) | reason: reason} @@ -326,9 +330,23 @@ defmodule Access do end end + def get(list, key, _default) when is_list(list) and is_integer(key) do + raise ArgumentError, """ + the Access module does not support accessing lists by index, got: #{inspect(key)} + + Accessing a list by index is typically discouraged in Elixir, \ + instead we prefer to use the Enum module to manipulate lists \ + as a whole. If you really must access a list element by index, \ + you can Enum.at/1 or the functions in the List module\ + """ + end + def get(list, key, _default) when is_list(list) do - raise ArgumentError, - "the Access calls for keywords expect the key to be an atom, got: " <> inspect(key) + raise ArgumentError, """ + the Access module supports only keyword lists (with atom keys), got: #{inspect(key)} + + If you want to search lists of tuples, use List.keyfind/3\ + """ end def get(nil, _key, default) do @@ -377,10 +395,26 @@ defmodule Access do Map.get_and_update(map, key, fun) end - def get_and_update(list, key, fun) when is_list(list) do + def get_and_update(list, key, fun) when is_list(list) and is_atom(key) do Keyword.get_and_update(list, key, fun) end + def get_and_update(list, key, _fun) when is_list(list) and is_integer(key) do + raise ArgumentError, """ + the Access module does not support accessing lists by index, got: #{inspect(key)} + + Accessing a list by index is typically discouraged in Elixir, \ + instead we prefer to use the Enum module to manipulate lists \ + as a whole. If you really must mostify a list element by index, \ + you can Access.at/1 or the functions in the List module\ + """ + end + + def get_and_update(list, key, _fun) when is_list(list) do + raise ArgumentError, + "the Access module supports only keyword lists (with atom keys), got: " <> inspect(key) + end + def get_and_update(nil, key, _fun) do raise ArgumentError, "could not put/update key #{inspect(key)} on a nil value" end @@ -507,6 +541,21 @@ defmodule Access do iex> get_in(map, [Access.key!(:user), Access.key!(:unknown)]) ** (KeyError) key :unknown not found in: %{name: \"john\"} + The examples above could be partially written as: + + iex> map = %{user: %{name: "john"}} + iex> map.user.name + "john" + iex> get_and_update_in(map.user.name, fn prev -> + ...> {prev, String.upcase(prev)} + ...> end) + {"john", %{user: %{name: "JOHN"}}} + + However, it is not possible to remove fields using the dot notation, + as it is implified those fields must also be present. In any case, + `Access.key!/1` is useful when the key is not known in advance + and must be accessed dynamically. + An error is raised if the accessed structure is not a map/struct: iex> get_in([], [Access.key!(:foo)]) From 91d0b9f65cb6086a2d80d8f6f511176e0c9222e2 Mon Sep 17 00:00:00 2001 From: Artem Solomatin Date: Tue, 5 Sep 2023 09:15:12 +0300 Subject: [PATCH 07/12] Fix supervisor docs grammar (#12912) Co-authored-by: Andrea Leopardi Co-authored-by: Jean Klingler --- lib/elixir/lib/supervisor.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/elixir/lib/supervisor.ex b/lib/elixir/lib/supervisor.ex index be16ae5bb56..e56527fa994 100644 --- a/lib/elixir/lib/supervisor.ex +++ b/lib/elixir/lib/supervisor.ex @@ -875,7 +875,7 @@ defmodule Supervisor do `module.child_spec([])`. After the child specification is retrieved, the fields on `overrides` - are directly applied on the child spec. If `overrides` has keys that + are directly applied to the child spec. If `overrides` has keys that do not map to any child specification field, an error is raised. See the "Child specification" section in the module documentation From 1bc8bc5e76e20c6b0180e024e9d280ab5fc0af94 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 5 Sep 2023 08:29:39 +0200 Subject: [PATCH 08/12] Bump actions/checkout from 3 to 4 (#12911) Bumps [actions/checkout](https://github.com/actions/checkout) from 3 to 4. - [Release notes](https://github.com/actions/checkout/releases) - [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md) - [Commits](https://github.com/actions/checkout/compare/v3...v4) --- updated-dependencies: - dependency-name: actions/checkout dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- .github/workflows/builds.hex.pm.yml | 2 +- .github/workflows/ci-markdown.yml | 2 +- .github/workflows/ci.yml | 6 +++--- .github/workflows/notify.yml | 2 +- .github/workflows/release.yml | 2 +- 5 files changed, 7 insertions(+), 7 deletions(-) diff --git a/.github/workflows/builds.hex.pm.yml b/.github/workflows/builds.hex.pm.yml index 5fc9c7dedaf..7cfb1d790d9 100644 --- a/.github/workflows/builds.hex.pm.yml +++ b/.github/workflows/builds.hex.pm.yml @@ -32,7 +32,7 @@ jobs: build_docs: build_docs runs-on: ubuntu-22.04 steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 with: fetch-depth: 50 - name: Get tags diff --git a/.github/workflows/ci-markdown.yml b/.github/workflows/ci-markdown.yml index 0a356cf7eff..0fb12d49b1e 100644 --- a/.github/workflows/ci-markdown.yml +++ b/.github/workflows/ci-markdown.yml @@ -22,7 +22,7 @@ jobs: steps: - name: Check out the repository - uses: actions/checkout@v3 + uses: actions/checkout@v4 with: fetch-depth: 10 diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8873bf1fce5..afe1f6b4b55 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -36,7 +36,7 @@ jobs: development: true runs-on: ubuntu-20.04 steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 with: fetch-depth: 50 - uses: erlef/setup-beam@v1 @@ -82,7 +82,7 @@ jobs: steps: - name: Configure Git run: git config --global core.autocrlf input - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 with: fetch-depth: 50 - uses: erlef/setup-beam@v1 @@ -107,7 +107,7 @@ jobs: name: Check POSIX-compliant runs-on: ubuntu-20.04 steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 with: fetch-depth: 50 - name: Install Shellcheck diff --git a/.github/workflows/notify.yml b/.github/workflows/notify.yml index cdc48380c8c..482d5ad241c 100644 --- a/.github/workflows/notify.yml +++ b/.github/workflows/notify.yml @@ -13,7 +13,7 @@ jobs: runs-on: ubuntu-20.04 name: Notify steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 with: fetch-depth: 50 - uses: erlef/setup-beam@v1 diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index d862ea1104b..8723e4752bb 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -42,7 +42,7 @@ jobs: build_docs: build_docs runs-on: ubuntu-22.04 steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 with: fetch-depth: 50 - uses: ./.github/workflows/release_pre_built From 29b29c5fbfa4453df2c4afe1ded88d5fc02c6313 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Thu, 7 Sep 2023 05:46:52 -0400 Subject: [PATCH 09/12] Add an introduction to module names --- .../pages/getting-started/erlang-libraries.md | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/lib/elixir/pages/getting-started/erlang-libraries.md b/lib/elixir/pages/getting-started/erlang-libraries.md index f9ae1112c9e..153aa59168f 100644 --- a/lib/elixir/pages/getting-started/erlang-libraries.md +++ b/lib/elixir/pages/getting-started/erlang-libraries.md @@ -2,6 +2,19 @@ Elixir provides excellent interoperability with Erlang libraries. In fact, Elixir discourages simply wrapping Erlang libraries in favor of directly interfacing with Erlang code. In this section, we will present some of the most common and useful Erlang functionality that is not found in Elixir. +Erlang modules have a different naming convention than in Elixir and start in lowercase. In both cases, module names are atoms and we invoke functions by dispatching to the module name: + +```elixir +iex> is_atom(String) +true +iex> String.first("hello") +"h" +iex> is_atom(:binary) +true +iex> :binary.first("hello") +104 +``` + As you grow more proficient in Elixir, you may want to explore the Erlang [STDLIB Reference Manual](http://www.erlang.org/doc/apps/stdlib/index.html) in more detail. ## The binary module @@ -9,9 +22,9 @@ As you grow more proficient in Elixir, you may want to explore the Erlang [STDLI The built-in Elixir String module handles binaries that are UTF-8 encoded. [The `:binary` module](`:binary`) is useful when you are dealing with binary data that is not necessarily UTF-8 encoded. ```elixir -iex> String.to_charlist "Ø" +iex> String.to_charlist("Ø") [216] -iex> :binary.bin_to_list "Ø" +iex> :binary.bin_to_list("Ø") [195, 152] ``` @@ -25,7 +38,7 @@ Elixir does not contain a function similar to `printf` found in C and other lang iex> :io.format("Pi is approximately given by:~10.3f~n", [:math.pi]) Pi is approximately given by: 3.142 :ok -iex> to_string :io_lib.format("Pi is approximately given by:~10.3f~n", [:math.pi]) +iex> to_string(:io_lib.format("Pi is approximately given by:~10.3f~n", [:math.pi])) "Pi is approximately given by: 3.142\n" ``` From f5a9a65c4246e88598871fa7d9c82b73698a6f9c Mon Sep 17 00:00:00 2001 From: chengshq Date: Sat, 9 Sep 2023 01:44:55 +0800 Subject: [PATCH 10/12] Fix index on split (#12915) --- lib/elixir/pages/getting-started/lists-and-tuples.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/elixir/pages/getting-started/lists-and-tuples.md b/lib/elixir/pages/getting-started/lists-and-tuples.md index 01f06ed09b3..ef4ea6b4b1c 100644 --- a/lib/elixir/pages/getting-started/lists-and-tuples.md +++ b/lib/elixir/pages/getting-started/lists-and-tuples.md @@ -157,7 +157,7 @@ On the other hand, `String.split_at/2` splits a string in two parts at a given p ```elixir iex> String.split_at("hello world", 3) {"hel", "lo world"} -iex> String.split_at("hello world", 3) +iex> String.split_at("hello world", -4) {"hello w", "orld"} ``` From 0d139c15df7f72ac2180fa9646f12b149b244786 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vin=C3=ADcius=20M=C3=BCller?= Date: Sun, 10 Sep 2023 11:09:56 +0000 Subject: [PATCH 11/12] Save column information on start delimiter tokens (#12916) --- lib/elixir/src/elixir_tokenizer.erl | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/elixir/src/elixir_tokenizer.erl b/lib/elixir/src/elixir_tokenizer.erl index c9f9b9bf8fd..34fcb6b7c1d 100644 --- a/lib/elixir/src/elixir_tokenizer.erl +++ b/lib/elixir/src/elixir_tokenizer.erl @@ -144,7 +144,7 @@ tokenize([], Line, Column, #elixir_tokenizer{cursor_completion=Cursor} = Scope, AccTokens = cursor_complete(Line, CursorColumn, CursorTerminators, CursorTokens), {ok, Line, Column, AllWarnings, AccTokens}; -tokenize([], EndLine, Column, #elixir_tokenizer{terminators=[{Start, StartLine, _} | _]} = Scope, Tokens) -> +tokenize([], EndLine, Column, #elixir_tokenizer{terminators=[{Start, {StartLine, _, _}, _} | _]} = Scope, Tokens) -> End = terminator(Start), Hint = missing_terminator_hint(Start, End, Scope), Message = "missing terminator: ~ts (for \"~ts\" starting at line ~B)", @@ -1376,12 +1376,12 @@ handle_terminator(Rest, Line, Column, Scope, Token, Tokens) -> tokenize(Rest, Line, Column, New, [Token | Tokens]) end. -check_terminator({Start, {Line, _, _}}, Terminators, Scope) +check_terminator({Start, Meta}, Terminators, Scope) when Start == '('; Start == '['; Start == '{'; Start == '<<' -> Indentation = Scope#elixir_tokenizer.indentation, - {ok, Scope#elixir_tokenizer{terminators=[{Start, Line, Indentation} | Terminators]}}; + {ok, Scope#elixir_tokenizer{terminators=[{Start, Meta, Indentation} | Terminators]}}; -check_terminator({Start, {Line, _, _}}, Terminators, Scope) when Start == 'fn'; Start == 'do' -> +check_terminator({Start, Meta}, Terminators, Scope) when Start == 'fn'; Start == 'do' -> Indentation = Scope#elixir_tokenizer.indentation, NewScope = @@ -1394,7 +1394,7 @@ check_terminator({Start, {Line, _, _}}, Terminators, Scope) when Start == 'fn'; Scope end, - {ok, NewScope#elixir_tokenizer{terminators=[{Start, Line, Indentation} | Terminators]}}; + {ok, NewScope#elixir_tokenizer{terminators=[{Start, Meta, Indentation} | Terminators]}}; check_terminator({'end', {EndLine, _, _}}, [{'do', _, Indentation} | Terminators], Scope) -> NewScope = @@ -1410,7 +1410,7 @@ check_terminator({'end', {EndLine, _, _}}, [{'do', _, Indentation} | Terminators {ok, NewScope#elixir_tokenizer{terminators=Terminators}}; -check_terminator({End, {EndLine, EndColumn, _}}, [{Start, StartLine, _} | Terminators], Scope) +check_terminator({End, {EndLine, EndColumn, _}}, [{Start, {StartLine, _, _}, _} | Terminators], Scope) when End == 'end'; End == ')'; End == ']'; End == '}'; End == '>>' -> case terminator(Start) of End -> @@ -1449,7 +1449,7 @@ unexpected_token_or_reserved(_) -> "unexpected token: ". missing_terminator_hint(Start, End, #elixir_tokenizer{mismatch_hints=Hints}) -> case lists:keyfind(Start, 1, Hints) of - {Start, HintLine, _} -> + {Start, {HintLine, _, _}, _} -> io_lib:format("\n\n HINT: it looks like the \"~ts\" on line ~B does not have a matching \"~ts\"", [Start, HintLine, End]); false -> From 4774c2759af01b9dce3e3796b31c5e0f43c3c6ae Mon Sep 17 00:00:00 2001 From: Lucas Francisco da Matta Vegi Date: Mon, 11 Sep 2023 05:31:06 -0300 Subject: [PATCH 12/12] "Alternative return types" added (Anti-patterns documentation) (#12905) --- .../anti-patterns/design-anti-patterns.md | 61 ++++++++++++++++++- 1 file changed, 60 insertions(+), 1 deletion(-) diff --git a/lib/elixir/pages/anti-patterns/design-anti-patterns.md b/lib/elixir/pages/anti-patterns/design-anti-patterns.md index 937d153c2a6..3dea9a9e54c 100644 --- a/lib/elixir/pages/anti-patterns/design-anti-patterns.md +++ b/lib/elixir/pages/anti-patterns/design-anti-patterns.md @@ -69,7 +69,66 @@ The following arguments were given to MyLibrary.foo/1: ## Alternative return types -TODO +#### Problem + +This anti-pattern refers to functions that receive options (for example, *keyword list*) parameters that drastically change their return type. Because options are optional and sometimes set dynamically, if they change the return type it may be hard to understand what the function actually returns. + +#### Example + +An example of this anti-pattern, as shown below, is when a function has many alternative return types, depending on the options received as a parameter. + +```elixir +defmodule AlternativeInteger do + @spec parse(String.t(), keyword()) :: integer() | {integer(), String.t()} | :error + def parse(string, options \\ []) when is_list(options) do + if Keyword.get(options, :discard_rest, false) do + Integer.parse(string) + else + case Integer.parse(string) do + {int, _rest} -> int + :error -> :error + end + end + end +end +``` + +```elixir +iex> AlternativeInteger.parse("13") +{13, ""} +iex> AlternativeInteger.parse("13", discard_rest: true) +13 +iex> AlternativeInteger.parse("13", discard_rest: false) +{13, ""} +``` + +#### Refactoring + +To refactor this anti-pattern, as shown next, add a specific function for each return type (for example, `parse_discard_rest/1`), no longer delegating this to options passed as arguments. + +```elixir +defmodule AlternativeInteger do + @spec parse(String.t()) :: {integer(), String.t()} | :error + def parse(string) do + Integer.parse(string) + end + + @spec parse_discard_rest(String.t()) :: integer() | :error + def parse_discard_rest(string) do + case Integer.parse(string) do + {int, _rest} -> int + :error -> :error + end + end +end +``` + +```elixir +iex> AlternativeInteger.parse("13") +{13, ""} +iex> AlternativeInteger.parse_discard_rest("13") +13 +``` ## Unrelated multi-clause function