From 9c0fa3cfef652e117116662f50f2f9f5346967e0 Mon Sep 17 00:00:00 2001 From: Jean Klingler Date: Sat, 23 Sep 2023 00:27:44 +0900 Subject: [PATCH 01/10] Warn when matching on 0.0 and generate erl AST for +/-0.0 (#12949) --- lib/elixir/src/elixir_erl.erl | 8 +++++++ lib/elixir/src/elixir_expand.erl | 6 +++++ .../test/elixir/kernel/expansion_test.exs | 17 ++++++++++++-- .../test/elixir/module/types/pattern_test.exs | 22 ++++++++++++++++++- lib/elixir/test/erlang/control_test.erl | 6 +++++ 5 files changed, 56 insertions(+), 3 deletions(-) diff --git a/lib/elixir/src/elixir_erl.erl b/lib/elixir/src/elixir_erl.erl index 41a519bf0a4..2a6e04c7213 100644 --- a/lib/elixir/src/elixir_erl.erl +++ b/lib/elixir/src/elixir_erl.erl @@ -74,6 +74,14 @@ elixir_to_erl(Tree, Ann) when is_atom(Tree) -> {atom, Ann, Tree}; elixir_to_erl(Tree, Ann) when is_integer(Tree) -> {integer, Ann, Tree}; +elixir_to_erl(Tree, Ann) when is_float(Tree), Tree == 0.0 -> + % 0.0 needs to be rewritten as the AST for +0.0 in matches + Op = + case <> of + <<1:1,_:63>> -> '-'; + _ -> '+' + end, + {op, Ann, Op, {float, Ann, 0.0}}; elixir_to_erl(Tree, Ann) when is_float(Tree) -> {float, Ann, Tree}; elixir_to_erl(Tree, Ann) when is_binary(Tree) -> diff --git a/lib/elixir/src/elixir_expand.erl b/lib/elixir/src/elixir_expand.erl index 62b1f7c5583..c72d2124362 100644 --- a/lib/elixir/src/elixir_expand.erl +++ b/lib/elixir/src/elixir_expand.erl @@ -456,6 +456,10 @@ expand(Pid, S, E) when is_pid(Pid) -> {Pid, E} end; +expand(Zero, S, #{context := match} = E) when is_float(Zero), Zero == 0.0 -> + elixir_errors:file_warn([], E, ?MODULE, invalid_match_on_zero_float), + {Zero, S, E}; + expand(Other, S, E) when is_number(Other); is_atom(Other); is_binary(Other) -> {Other, S, E}; @@ -1163,6 +1167,8 @@ guard_context(_) -> "guards". format_error({remote_nullary_no_parens, Expr}) -> String = 'Elixir.String':replace_suffix('Elixir.Macro':to_string(Expr), <<"()">>, <<>>), io_lib:format("parentheses are required for function calls with no arguments, got: ~ts", [String]); +format_error(invalid_match_on_zero_float) -> + "pattern matching on 0.0 is equivalent to matching only on +0.0 from Erlang/OTP 27+. Instead you must match on +0.0 or -0.0"; format_error({useless_literal, Term}) -> io_lib:format("code block contains unused literal ~ts " "(remove the literal or assign it to _ to avoid warnings)", diff --git a/lib/elixir/test/elixir/kernel/expansion_test.exs b/lib/elixir/test/elixir/kernel/expansion_test.exs index f7042df3a78..8e16612f2df 100644 --- a/lib/elixir/test/elixir/kernel/expansion_test.exs +++ b/lib/elixir/test/elixir/kernel/expansion_test.exs @@ -468,6 +468,16 @@ defmodule Kernel.ExpansionTest do end end + describe "floats" do + test "cannot be 0.0 inside match" do + assert capture_io(:stderr, fn -> expand(quote(do: 0.0 = 0.0)) end) =~ + "pattern matching on 0.0 is equivalent to matching only on +0.0 from Erlang/OTP 27+" + + assert {:=, [], [+0.0, +0.0]} = expand(quote(do: +0.0 = 0.0)) + assert {:=, [], [-0.0, +0.0]} = expand(quote(do: -0.0 = 0.0)) + end + end + describe "tuples" do test "expanded as arguments" do assert expand(quote(do: {after_expansion = 1, a})) == quote(do: {after_expansion = 1, a()}) @@ -715,8 +725,11 @@ defmodule Kernel.ExpansionTest do expand(quote(do: [1] ++ 2 ++ [3] = [1, 2, 3])) end) - assert {:=, _, [-1, {{:., _, [:erlang, :-]}, _, [1]}]} = expand(quote(do: -1 = -1)) - assert {:=, _, [1, {{:., _, [:erlang, :+]}, _, [1]}]} = expand(quote(do: +1 = +1)) + assert {:=, _, [-1, {{:., _, [:erlang, :-]}, _, [1]}]} = + expand(quote(do: -1 = -1)) + + assert {:=, _, [1, {{:., _, [:erlang, :+]}, _, [1]}]} = + expand(quote(do: +1 = +1)) assert {:=, _, [[{:|, _, [1, [{:|, _, [2, 3]}]]}], [1, 2, 3]]} = expand(quote(do: [1] ++ [2] ++ 3 = [1, 2, 3])) diff --git a/lib/elixir/test/elixir/module/types/pattern_test.exs b/lib/elixir/test/elixir/module/types/pattern_test.exs index d495ce9de8c..70e7ca108d6 100644 --- a/lib/elixir/test/elixir/module/types/pattern_test.exs +++ b/lib/elixir/test/elixir/module/types/pattern_test.exs @@ -15,6 +15,20 @@ defmodule Module.Types.PatternTest do end end + defmacrop quoted_pattern_with_diagnostics(patterns) do + {ast, diagnostics} = Code.with_diagnostics(fn -> expand_head(patterns, true) end) + + quote do + {patterns, true} = unquote(Macro.escape(ast)) + + result = + Pattern.of_pattern(patterns, new_stack(), new_context()) + |> lift_result() + + {result, unquote(Macro.escape(diagnostics))} + end + end + defmacrop quoted_head(patterns, guards \\ []) do quote do {patterns, guards} = unquote(Macro.escape(expand_head(patterns, guards))) @@ -76,8 +90,14 @@ defmodule Module.Types.PatternTest do assert quoted_pattern(false) == {:ok, {:atom, false}} assert quoted_pattern(:foo) == {:ok, {:atom, :foo}} assert quoted_pattern(0) == {:ok, :integer} - assert quoted_pattern(0.0) == {:ok, :float} + assert quoted_pattern(+0.0) == {:ok, :float} + assert quoted_pattern(-0.0) == {:ok, :float} assert quoted_pattern("foo") == {:ok, :binary} + + assert {{:ok, :float}, [diagnostic]} = quoted_pattern_with_diagnostics(0.0) + + assert diagnostic.message =~ + "pattern matching on 0.0 is equivalent to matching only on +0.0" end test "list" do diff --git a/lib/elixir/test/erlang/control_test.erl b/lib/elixir/test/erlang/control_test.erl index fbe853bd2d3..7f3337f7880 100644 --- a/lib/elixir/test/erlang/control_test.erl +++ b/lib/elixir/test/erlang/control_test.erl @@ -12,6 +12,12 @@ cond_line_test() -> {clause, 3, _, _, _}] } = to_erl("cond do\n 1 -> :ok\n 2 -> :ok\nend"). +float_match_test() -> + {'case', _, _, + [{clause, _, [{op, _, '+', {float, _, 0.0}}], [], [{atom, _, pos}]}, + {clause, _, [{op, _, '-', {float, _, 0.0}}], [], [{atom, _, neg}]}] + } = to_erl("case X do\n +0.0 -> :pos\n -0.0 -> :neg\nend"). + % Optimized optimized_if_test() -> From d35d60f4c933792bb1dc5282d6b716d9726614cb Mon Sep 17 00:00:00 2001 From: Artem Solomatin Date: Sat, 23 Sep 2023 11:22:38 +0300 Subject: [PATCH 02/10] Add typespecs and docs to 'Range' module (#12953) --- lib/elixir/lib/range.ex | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/lib/elixir/lib/range.ex b/lib/elixir/lib/range.ex index d9bb5cfc94c..ad86e609df0 100644 --- a/lib/elixir/lib/range.ex +++ b/lib/elixir/lib/range.ex @@ -273,6 +273,7 @@ defmodule Range do """ @doc since: "1.14.0" + @spec shift(t, integer) :: t def shift(first..last//step, steps_to_shift) when is_integer(steps_to_shift) do new(first + steps_to_shift * step, last + steps_to_shift * step, step) @@ -364,6 +365,7 @@ defmodule Range do """ @doc since: "1.15.0" + @spec split(t, integer) :: {t, t} def split(first..last//step = range, split) when is_integer(split) do if split >= 0 do split(first, last, step, split) @@ -392,8 +394,17 @@ defmodule Range do @doc """ Converts a range to a list. + + ## Examples + + iex> Range.to_list(0..5) + [0, 1, 2, 3, 4, 5] + iex> Range.to_list(-3..0) + [-3, -2, -1, 0] + """ @doc since: "1.15.0" + @spec to_list(t) :: list(integer) def to_list(first..last//step) when step > 0 and first <= last when step < 0 and first >= last do From 7de603134cd4d08030fc56cfe5d8205d251e029c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Sat, 23 Sep 2023 10:55:24 +0200 Subject: [PATCH 03/10] Perform single pass in ExUnit.Filters --- lib/ex_unit/lib/ex_unit/filters.ex | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/ex_unit/lib/ex_unit/filters.ex b/lib/ex_unit/lib/ex_unit/filters.ex index 317bc329347..c363a801f0b 100644 --- a/lib/ex_unit/lib/ex_unit/filters.ex +++ b/lib/ex_unit/lib/ex_unit/filters.ex @@ -34,10 +34,10 @@ defmodule ExUnit.Filters do |> Enum.split_while(&match?({_, ""}, Integer.parse(&1))) line_numbers = - reversed_line_numbers - |> Enum.reject(&invalid_line_number?/1) - |> Enum.reverse() - |> Enum.map(&{:line, &1}) + for line_number <- reversed_line_numbers, + valid_line_number?(line_number), + reduce: [], + do: (acc -> [line: line_number] ++ acc) path = reversed_path_parts @@ -48,14 +48,14 @@ defmodule ExUnit.Filters do end end - defp invalid_line_number?(arg) do + defp valid_line_number?(arg) do case Integer.parse(arg) do {num, ""} when num > 0 -> - false + true _ -> IO.warn("invalid line number given as ExUnit filter: #{arg}", []) - true + false end end From 55fbc528f97bf4a7805366a85dacd7e1ecacc99c Mon Sep 17 00:00:00 2001 From: Jean Klingler Date: Sat, 23 Sep 2023 18:08:52 +0900 Subject: [PATCH 04/10] Always rewrite signed numbers during expansion (#12955) --- lib/elixir/src/elixir_expand.erl | 3 +++ lib/elixir/src/elixir_rewrite.erl | 2 -- lib/elixir/test/elixir/kernel/expansion_test.exs | 4 ++-- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/lib/elixir/src/elixir_expand.erl b/lib/elixir/src/elixir_expand.erl index c72d2124362..3b6d67a4918 100644 --- a/lib/elixir/src/elixir_expand.erl +++ b/lib/elixir/src/elixir_expand.erl @@ -894,6 +894,9 @@ attach_context_module(Receiver, Meta, #{context_modules := ContextModules}) -> false -> Meta end. +% Signed numbers can be rewritten no matter the context +rewrite(_, erlang, _, '+', _, [Arg], _S) when is_number(Arg) -> {ok, Arg}; +rewrite(_, erlang, _, '-', _, [Arg], _S) when is_number(Arg) -> {ok, -Arg}; rewrite(match, Receiver, DotMeta, Right, Meta, EArgs, _S) -> elixir_rewrite:match_rewrite(Receiver, DotMeta, Right, Meta, EArgs); rewrite(guard, Receiver, DotMeta, Right, Meta, EArgs, S) -> diff --git a/lib/elixir/src/elixir_rewrite.erl b/lib/elixir/src/elixir_rewrite.erl index 8f67e833ea7..ad2cf5905b3 100644 --- a/lib/elixir/src/elixir_rewrite.erl +++ b/lib/elixir/src/elixir_rewrite.erl @@ -302,8 +302,6 @@ increment(Meta, Other) -> %% The allowed operations are very limited. %% The Kernel operators are already inlined by now, we only need to %% care about Erlang ones. -match_rewrite(erlang, _, '+', _, [Arg]) when is_number(Arg) -> {ok, Arg}; -match_rewrite(erlang, _, '-', _, [Arg]) when is_number(Arg) -> {ok, -Arg}; match_rewrite(erlang, _, '++', Meta, [Left, Right]) -> try {ok, static_append(Left, Right, Meta)} catch impossible -> {error, {invalid_match_append, Left}} diff --git a/lib/elixir/test/elixir/kernel/expansion_test.exs b/lib/elixir/test/elixir/kernel/expansion_test.exs index 8e16612f2df..317b2b7b15e 100644 --- a/lib/elixir/test/elixir/kernel/expansion_test.exs +++ b/lib/elixir/test/elixir/kernel/expansion_test.exs @@ -725,10 +725,10 @@ defmodule Kernel.ExpansionTest do expand(quote(do: [1] ++ 2 ++ [3] = [1, 2, 3])) end) - assert {:=, _, [-1, {{:., _, [:erlang, :-]}, _, [1]}]} = + assert {:=, _, [-1, -1]} = expand(quote(do: -1 = -1)) - assert {:=, _, [1, {{:., _, [:erlang, :+]}, _, [1]}]} = + assert {:=, _, [1, 1]} = expand(quote(do: +1 = +1)) assert {:=, _, [[{:|, _, [1, [{:|, _, [2, 3]}]]}], [1, 2, 3]]} = From 6ee16616c057f36d1ae4c9f28c32900dba2b3664 Mon Sep 17 00:00:00 2001 From: Jean Klingler Date: Sat, 23 Sep 2023 18:15:09 +0900 Subject: [PATCH 05/10] Fix OTP 26.1 warning in tests (#12954) --- lib/elixir/test/erlang/control_test.erl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/elixir/test/erlang/control_test.erl b/lib/elixir/test/erlang/control_test.erl index 7f3337f7880..c317166a042 100644 --- a/lib/elixir/test/erlang/control_test.erl +++ b/lib/elixir/test/erlang/control_test.erl @@ -14,8 +14,8 @@ cond_line_test() -> float_match_test() -> {'case', _, _, - [{clause, _, [{op, _, '+', {float, _, 0.0}}], [], [{atom, _, pos}]}, - {clause, _, [{op, _, '-', {float, _, 0.0}}], [], [{atom, _, neg}]}] + [{clause, _, [{op, _, '+', {float, _, +0.0}}], [], [{atom, _, pos}]}, + {clause, _, [{op, _, '-', {float, _, +0.0}}], [], [{atom, _, neg}]}] } = to_erl("case X do\n +0.0 -> :pos\n -0.0 -> :neg\nend"). % Optimized From 3f6c748b1fac0c1aa43d39182ed3778ba1782afa Mon Sep 17 00:00:00 2001 From: Lucas Francisco da Matta Vegi Date: Sat, 23 Sep 2023 09:15:31 -0300 Subject: [PATCH 06/10] "Feature envy" added (Anti-pattern documentation) (#12952) --- .../anti-patterns/design-anti-patterns.md | 50 ++++++++++++++++++- 1 file changed, 49 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 c866062ac3e..376b5182ca6 100644 --- a/lib/elixir/pages/anti-patterns/design-anti-patterns.md +++ b/lib/elixir/pages/anti-patterns/design-anti-patterns.md @@ -207,7 +207,55 @@ end ## Feature envy -TODO +#### Problem + +This anti-pattern occurs when a function accesses more data or calls more functions from another module than from its own. The presence of this anti-pattern can make a module less cohesive and increase code coupling. + +#### Example + +In the following code, all the data used in the `calculate_total_item/1` function of the module `Order` comes from the `OrderItem` module. This increases coupling and decreases code cohesion unnecessarily. + +```elixir +defmodule Order do + # Some functions... + + def calculate_total_item(id) do + item = OrderItem.find_item(id) + total = (item.price + item.taxes) * item.amount + + if discount = OrderItem.find_discount(item) do + total - total * discount + else + total + end + end +end +``` + +#### Refactoring + +To remove this anti-pattern we can move `calculate_total_item/1` to `OrderItem`, decreasing coupling: + +```elixir +defmodule OrderItem do + def find_item(id) + def find_discount(item) + + def calculate_total_item(id) do # <= function moved from Order! + item = find_item(id) + total = (item.price + item.taxes) * item.amount + discount = find_discount(item) + + unless is_nil(discount) do + total - total * discount + else + total + end + end +end +``` + +This refactoring is only possible when you own both modules. If the module you are invoking belongs to another application, then it is not possible to add new functions to it, and your only option is to define an additional module that augments the third-party module. ## Excessive side-effects From af915cfc208c8f9a4e4b061b5bc20e2f6f8c36b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vin=C3=ADcius=20M=C3=BCller?= Date: Sat, 23 Sep 2023 15:24:36 +0000 Subject: [PATCH 07/10] Uncomment assertions (#12960) --- lib/elixir/test/elixir/kernel/parser_test.exs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/elixir/test/elixir/kernel/parser_test.exs b/lib/elixir/test/elixir/kernel/parser_test.exs index bc9091fbeed..6949abe74e9 100644 --- a/lib/elixir/test/elixir/kernel/parser_test.exs +++ b/lib/elixir/test/elixir/kernel/parser_test.exs @@ -876,9 +876,9 @@ defmodule Kernel.ParserTest do msg = &["nofile:1:9:", "syntax error before: sigil ~s starting with content '#{&1}'"] assert_syntax_error(msg.("bar baz"), ~c"~s(foo) ~s(bar baz)") - # assert_syntax_error(msg.(""), ~c"~s(foo) ~s()") - # assert_syntax_error(msg.("bar "), ~c"~s(foo) ~s(bar \#{:baz})") - # assert_syntax_error(msg.(""), ~c"~s(foo) ~s(\#{:bar} baz)") + assert_syntax_error(msg.(""), ~c"~s(foo) ~s()") + assert_syntax_error(msg.("bar "), ~c"~s(foo) ~s(bar \#{:baz})") + assert_syntax_error(msg.(""), ~c"~s(foo) ~s(\#{:bar} baz)") end test "invalid do" do From 9a74a73a00dabc87db75a40f0d4abade6de1df38 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vin=C3=ADcius=20M=C3=BCller?= Date: Sat, 23 Sep 2023 16:59:08 +0000 Subject: [PATCH 08/10] Normalize hint labels (#12958) --- lib/elixir/lib/module/types.ex | 8 +++++--- lib/elixir/src/elixir_errors.erl | 18 +++++++++++------- lib/elixir/src/elixir_tokenizer.erl | 8 ++++---- lib/elixir/test/elixir/kernel/parser_test.exs | 11 +++++++---- .../test/elixir/module/types/types_test.exs | 14 ++++++++------ lib/iex/test/iex/interaction_test.exs | 2 +- 6 files changed, 36 insertions(+), 25 deletions(-) diff --git a/lib/elixir/lib/module/types.ex b/lib/elixir/lib/module/types.ex index 4600d82b048..c1c7efdd5e6 100644 --- a/lib/elixir/lib/module/types.ex +++ b/lib/elixir/lib/module/types.ex @@ -413,14 +413,14 @@ defmodule Module.Types do defp format_message_hint(:inferred_dot) do """ - HINT: "var.field" (without parentheses) implies "var" is a map() while \ + #{hint()} "var.field" (without parentheses) implies "var" is a map() while \ "var.fun()" (with parentheses) implies "var" is an atom() """ end defp format_message_hint(:inferred_bitstring_spec) do """ - HINT: all expressions given to binaries are assumed to be of type \ + #{hint()} all expressions given to binaries are assumed to be of type \ integer() unless said otherwise. For example, <> assumes "expr" \ is an integer. Pass a modifier, such as <> or <>, \ to change the default behaviour. @@ -429,11 +429,13 @@ defmodule Module.Types do defp format_message_hint({:sized_and_unsize_tuples, {size, var}}) do """ - HINT: use pattern matching or "is_tuple(#{Macro.to_string(var)}) and \ + #{hint()} use pattern matching or "is_tuple(#{Macro.to_string(var)}) and \ tuple_size(#{Macro.to_string(var)}) == #{size}" to guard a sized tuple. """ end + defp hint, do: :elixir_errors.prefix(:hint) + defp format_type_hint(type, types, expr, hints) do case format_type_hint(type, types, expr) do {message, hint} -> {message, [hint | hints]} diff --git a/lib/elixir/src/elixir_errors.erl b/lib/elixir/src/elixir_errors.erl index 2bc706df1c3..a7969afb1df 100644 --- a/lib/elixir/src/elixir_errors.erl +++ b/lib/elixir/src/elixir_errors.erl @@ -8,6 +8,7 @@ -export([function_error/4, module_error/4, file_error/4]). -export([format_snippet/7]). -export([erl_warn/3, file_warn/4]). +-export([prefix/1]). -export([print_diagnostic/2, emit_diagnostic/6]). -export([print_warning/2, print_warning/3]). -export([print_warning_group/2]). @@ -135,14 +136,14 @@ extract_column(_) -> nil. %% "Snippet" here refers to the source code line where the diagnostic/error occured format_snippet(_Position, nil, Message, nil, Severity, _Stacktrace, _Span) -> - Formatted = [prefix(Severity), Message], + Formatted = [prefix(Severity), " ", Message], unicode:characters_to_binary(Formatted); format_snippet(Position, File, Message, nil, Severity, Stacktrace, _Span) -> Location = location_format(Position, File, Stacktrace), Formatted = io_lib:format( - "~ts~ts\n" + "~ts ~ts\n" "└─ ~ts", [prefix(Severity), Message, Location] ), @@ -167,7 +168,7 @@ format_snippet(Position, File, Message, Snippet, Severity, Stacktrace, Span) -> end, Formatted = io_lib:format( - " ~ts~ts~ts\n" + " ~ts~ts ~ts\n" " ~ts│\n" " ~ts~p │ ~ts\n" " ~ts│ ~ts\n" @@ -435,18 +436,21 @@ snippet_line(InputString, Location, StartLine) -> %% Helpers -prefix(warning) -> highlight(<<"warning: ">>, warning); -prefix(error) -> highlight(<<"error: ">>, error). +prefix(warning) -> highlight(<<"warning:">>, warning); +prefix(error) -> highlight(<<"error:">>, error); +prefix(hint) -> highlight(<<"hint:">>, hint). highlight(Message, Severity) -> case {Severity, application:get_env(elixir, ansi_enabled, false)} of {warning, true} -> yellow(Message); {error, true} -> red(Message); + {hint, true} -> blue(Message); _ -> Message end. -yellow(Msg) -> io_lib:format("\e[33m~ts\e[0m", [Msg]). -red(Msg) -> io_lib:format("\e[31m~ts\e[0m", [Msg]). +yellow(Msg) -> ["\e[33m", Msg, "\e[0m"]. +blue(Msg) -> ["\e[34m", Msg, "\e[0m"]. +red(Msg) -> ["\e[31m", Msg, "\e[0m"]. env_format(Meta, #{file := EnvFile} = E) -> {File, Position} = meta_location(Meta, EnvFile), diff --git a/lib/elixir/src/elixir_tokenizer.erl b/lib/elixir/src/elixir_tokenizer.erl index 85070770123..6db3de8c6ec 100644 --- a/lib/elixir/src/elixir_tokenizer.erl +++ b/lib/elixir/src/elixir_tokenizer.erl @@ -1434,8 +1434,8 @@ check_terminator({'end', {Line, Column, _}}, [], #elixir_tokenizer{mismatch_hint Suffix = case lists:keyfind('end', 1, Hints) of {'end', HintLine, _Identation} -> - io_lib:format("\n\n HINT: the \"end\" on line ~B may not have a matching \"do\" " - "defined before it (based on indentation)", [HintLine]); + io_lib:format("\n~ts the \"end\" on line ~B may not have a matching \"do\" " + "defined before it (based on indentation)", [elixir_errors:prefix(hint), HintLine]); false -> "" end, @@ -1455,8 +1455,8 @@ 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, _, _}, _} -> - io_lib:format("\n\n HINT: it looks like the \"~ts\" on line ~B does not have a matching \"~ts\"", - [Start, HintLine, End]); + io_lib:format("\n~ts it looks like the \"~ts\" on line ~B does not have a matching \"~ts\"", + [elixir_errors:prefix(hint), Start, HintLine, End]); false -> "" end. diff --git a/lib/elixir/test/elixir/kernel/parser_test.exs b/lib/elixir/test/elixir/kernel/parser_test.exs index 6949abe74e9..cc9e0531574 100644 --- a/lib/elixir/test/elixir/kernel/parser_test.exs +++ b/lib/elixir/test/elixir/kernel/parser_test.exs @@ -554,7 +554,7 @@ defmodule Kernel.ParserTest do ) assert_token_missing( - [~s/HINT: it looks like the "do" on line 2 does not have a matching "end"/], + ["hint:", ~s/it looks like the "do" on line 2 does not have a matching "end"/], ~c""" defmodule MyApp do def one do @@ -819,7 +819,8 @@ defmodule Kernel.ParserTest do assert_syntax_error( [ - "HINT: the \"end\" on line 2 may not have a matching \"do\" defined before it (based on indentation)" + "hint:", + "the \"end\" on line 2 may not have a matching \"do\" defined before it (based on indentation)" ], ~c""" defmodule MyApp do @@ -831,7 +832,8 @@ defmodule Kernel.ParserTest do assert_syntax_error( [ - "HINT: the \"end\" on line 3 may not have a matching \"do\" defined before it (based on indentation)" + "hint:", + "the \"end\" on line 3 may not have a matching \"do\" defined before it (based on indentation)" ], ~c""" defmodule MyApp do @@ -846,7 +848,8 @@ defmodule Kernel.ParserTest do assert_syntax_error( [ - "HINT: the \"end\" on line 6 may not have a matching \"do\" defined before it (based on indentation)" + "hint:", + "the \"end\" on line 6 may not have a matching \"do\" defined before it (based on indentation)" ], ~c""" defmodule MyApp do diff --git a/lib/elixir/test/elixir/module/types/types_test.exs b/lib/elixir/test/elixir/module/types/types_test.exs index d03ee0e28a0..f25f067dab1 100644 --- a/lib/elixir/test/elixir/module/types/types_test.exs +++ b/lib/elixir/test/elixir/module/types/types_test.exs @@ -5,6 +5,8 @@ defmodule Module.Types.TypesTest do alias Module.Types alias Module.Types.{Pattern, Expr} + @hint :elixir_errors.prefix(:hint) + defmacro warning(patterns \\ [], guards \\ [], body) do min_line = min_line(patterns ++ guards ++ [body]) patterns = reset_line(patterns, min_line) @@ -261,7 +263,7 @@ defmodule Module.Types.TypesTest do # types_test.ex:1 <> - HINT: all expressions given to binaries are assumed to be of type \ + #{@hint} all expressions given to binaries are assumed to be of type \ integer() unless said otherwise. For example, <> assumes "expr" \ is an integer. Pass a modifier, such as <> or <>, \ to change the default behaviour. @@ -314,7 +316,7 @@ defmodule Module.Types.TypesTest do # types_test.ex:1 {_} = foo - HINT: use pattern matching or "is_tuple(foo) and tuple_size(foo) == 1" to guard a sized tuple. + #{@hint} use pattern matching or "is_tuple(foo) and tuple_size(foo) == 1" to guard a sized tuple. """ end @@ -454,7 +456,7 @@ defmodule Module.Types.TypesTest do # types_test.ex:4 :atom = foo - HINT: "var.field" (without parentheses) implies "var" is a map() while \ + #{@hint} "var.field" (without parentheses) implies "var" is a map() while \ "var.fun()" (with parentheses) implies "var" is an atom() """ end @@ -489,7 +491,7 @@ defmodule Module.Types.TypesTest do # types_test.ex:4 module.__struct__ - HINT: "var.field" (without parentheses) implies "var" is a map() while \ + #{@hint} "var.field" (without parentheses) implies "var" is a map() while \ "var.fun()" (with parentheses) implies "var" is an atom() """ end @@ -517,7 +519,7 @@ defmodule Module.Types.TypesTest do # types_test.ex:1 foo.__struct__() - HINT: "var.field" (without parentheses) implies "var" is a map() while \ + #{@hint} "var.field" (without parentheses) implies "var" is a map() while \ "var.fun()" (with parentheses) implies "var" is an atom() """ end @@ -632,7 +634,7 @@ defmodule Module.Types.TypesTest do # types_test.ex:5 map2.subkey - HINT: "var.field" (without parentheses) implies "var" is a map() while "var.fun()" (with parentheses) implies "var" is an atom() + #{@hint} "var.field" (without parentheses) implies "var" is a map() while "var.fun()" (with parentheses) implies "var" is an atom() """ end end diff --git a/lib/iex/test/iex/interaction_test.exs b/lib/iex/test/iex/interaction_test.exs index 3f4de7ae5e3..2bf92ae9b00 100644 --- a/lib/iex/test/iex/interaction_test.exs +++ b/lib/iex/test/iex/interaction_test.exs @@ -25,7 +25,7 @@ defmodule IEx.InteractionTest do expected = """ ** (SyntaxError) invalid syntax found on iex:1:4: - \e[31merror: \e[0msyntax error before: '=' + \e[31merror:\e[0m syntax error before: '=' │ 1 │ a += 2 │ \e[31m ^\e[0m From 6935f747e3dd1e05b168d26791507385c3986ed1 Mon Sep 17 00:00:00 2001 From: Andrea Leopardi Date: Mon, 25 Sep 2023 08:39:12 +0200 Subject: [PATCH 09/10] Fix Markdown in docs for the Config module --- lib/elixir/lib/config.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/elixir/lib/config.ex b/lib/elixir/lib/config.ex index 9b885c6ad9d..cc39e43baa5 100644 --- a/lib/elixir/lib/config.ex +++ b/lib/elixir/lib/config.ex @@ -86,7 +86,7 @@ defmodule Config do the `mix.exs` file and inside custom Mix tasks, which always within the `Mix.Tasks` namespace. - ## config/runtime.exs + ## `config/runtime.exs` For runtime configuration, you can use the `config/runtime.exs` file. It is executed right before applications start in both Mix and releases From c7bd0fe7e77eeae31a07eef0ff9bdd31a3259dfe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Mon, 25 Sep 2023 12:55:17 +0200 Subject: [PATCH 10/10] Do not emit duplicate warnings from tokenizer, closes #12961 --- lib/eex/lib/eex/compiler.ex | 8 ++-- lib/eex/test/eex/tokenizer_test.exs | 6 --- lib/eex/test/eex_test.exs | 72 ++++++++++++++++------------- 3 files changed, 44 insertions(+), 42 deletions(-) diff --git a/lib/eex/lib/eex/compiler.ex b/lib/eex/lib/eex/compiler.ex index 292ddf86633..4ace24fde19 100644 --- a/lib/eex/lib/eex/compiler.ex +++ b/lib/eex/lib/eex/compiler.ex @@ -71,11 +71,9 @@ defmodule EEx.Compiler do {:ok, expr, new_line, new_column, rest} -> {key, expr} = case :elixir_tokenizer.tokenize(expr, 1, file: "eex", check_terminators: false) do - {:ok, _line, _column, warnings, tokens} -> - Enum.each(Enum.reverse(warnings), fn {location, msg} -> - :elixir_errors.erl_warn(location, state.file, msg) - end) - + {:ok, _line, _column, _warnings, tokens} -> + # We ignore warnings because the code will be tokenized + # again later with the right line+column info token_key(tokens, expr) {:error, _, _, _, _} -> diff --git a/lib/eex/test/eex/tokenizer_test.exs b/lib/eex/test/eex/tokenizer_test.exs index 328bf6dc3b3..f452bfbcf2e 100644 --- a/lib/eex/test/eex/tokenizer_test.exs +++ b/lib/eex/test/eex/tokenizer_test.exs @@ -5,12 +5,6 @@ defmodule EEx.TokenizerTest do @opts [indentation: 0, trim: false] - test "tokenizer warning" do - assert ExUnit.CaptureIO.capture_io(:stderr, fn -> - EEx.tokenize(~c"foo <% :'bar' %>", @opts) - end) =~ "found quoted atom \"bar\" but the quotes are not required" - end - test "simple charlists" do assert EEx.tokenize(~c"foo", @opts) == {:ok, [{:text, ~c"foo", %{column: 1, line: 1}}, {:eof, %{column: 4, line: 1}}]} diff --git a/lib/eex/test/eex_test.exs b/lib/eex/test/eex_test.exs index 739e572e569..2e60d90354b 100644 --- a/lib/eex/test/eex_test.exs +++ b/lib/eex/test/eex_test.exs @@ -286,6 +286,19 @@ defmodule EExTest do end end + test "when <%!-- is not closed" do + message = """ + my_file.eex:1:5: expected closing '--%>' for EEx expression + | + 1 | foo <%!-- bar + | ^\ + """ + + assert_raise EEx.SyntaxError, message, fn -> + EEx.compile_string("foo <%!-- bar", file: "my_file.eex") + end + end + test "when the token is invalid" do message = """ nofile:1:5: expected closing '%>' for EEx expression @@ -476,39 +489,24 @@ defmodule EExTest do end end - test "when middle expression has a modifier" do - assert ExUnit.CaptureIO.capture_io(:stderr, fn -> - EEx.compile_string("foo <%= if true do %>true<%= else %>false<% end %>") - end) =~ ~s[unexpected beginning of EEx tag \"<%=\" on \"<%= else %>\"] - end - - test "when end expression has a modifier" do - assert ExUnit.CaptureIO.capture_io(:stderr, fn -> - EEx.compile_string("foo <%= if true do %>true<% else %>false<%= end %>") - end) =~ - ~s[unexpected beginning of EEx tag \"<%=\" on \"<%= end %>\"] - end - - test "when trying to use marker '/' without implementation" do + test "when trying to use marker '|' without implementation" do msg = - ~r/unsupported EEx syntax <%\/ %> \(the syntax is valid but not supported by the current EEx engine\)/ + ~r/unsupported EEx syntax <%| %> \(the syntax is valid but not supported by the current EEx engine\)/ assert_raise EEx.SyntaxError, msg, fn -> - EEx.compile_string("<%/ true %>") + EEx.compile_string("<%| true %>") end end - test "when trying to use marker '|' without implementation" do + test "when trying to use marker '/' without implementation" do msg = - ~r/unsupported EEx syntax <%| %> \(the syntax is valid but not supported by the current EEx engine\)/ + ~r/unsupported EEx syntax <%\/ %> \(the syntax is valid but not supported by the current EEx engine\)/ assert_raise EEx.SyntaxError, msg, fn -> - EEx.compile_string("<%| true %>") + EEx.compile_string("<%/ true %>") end end - end - describe "error messages" do test "honor line numbers" do assert_raise EEx.SyntaxError, "nofile:100:6: expected closing '%>' for EEx expression", @@ -529,18 +527,30 @@ defmodule EExTest do EEx.compile_string("foo <%= bar", file: "my_file.eex") end end + end - test "when <%!-- is not closed" do - message = """ - my_file.eex:1:5: expected closing '--%>' for EEx expression - | - 1 | foo <%!-- bar - | ^\ - """ + describe "warnings" do + test "when middle expression has a modifier" do + assert ExUnit.CaptureIO.capture_io(:stderr, fn -> + EEx.compile_string("foo <%= if true do %>true<%= else %>false<% end %>") + end) =~ ~s[unexpected beginning of EEx tag \"<%=\" on \"<%= else %>\"] + end - assert_raise EEx.SyntaxError, message, fn -> - EEx.compile_string("foo <%!-- bar", file: "my_file.eex") - end + test "when end expression has a modifier" do + assert ExUnit.CaptureIO.capture_io(:stderr, fn -> + EEx.compile_string("foo <%= if true do %>true<% else %>false<%= end %>") + end) =~ + ~s[unexpected beginning of EEx tag \"<%=\" on \"<%= end %>\"] + end + + test "from tokenizer" do + warning = + ExUnit.CaptureIO.capture_io(:stderr, fn -> + EEx.compile_string(~s'<%= :"foo" %>', file: "tokenizer.ex") + end) + + assert warning =~ "found quoted atom \"foo\" but the quotes are not required" + assert warning =~ "tokenizer.ex:1:5" end end