From 4136f7a307dcf4c232a049ca00e9db725ab643be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Thu, 20 Dec 2018 23:16:26 +0100 Subject: [PATCH] Fix cond scope and improve coverage, closes #8542 --- lib/elixir/src/elixir_erl_pass.erl | 18 +++---- .../test/elixir/fixtures/dialyzer/cond.ex | 27 ++++++++++ .../test/elixir/kernel/dialyzer_test.exs | 5 ++ .../test/elixir/kernel/special_forms_test.exs | 49 +++++++++++++++++++ 4 files changed, 90 insertions(+), 9 deletions(-) create mode 100644 lib/elixir/test/elixir/fixtures/dialyzer/cond.ex diff --git a/lib/elixir/src/elixir_erl_pass.erl b/lib/elixir/src/elixir_erl_pass.erl index ba21718975f..6436d6d6f97 100644 --- a/lib/elixir/src/elixir_erl_pass.erl +++ b/lib/elixir/src/elixir_erl_pass.erl @@ -109,16 +109,16 @@ translate({fn, Meta, Clauses}, S) -> %% Cond translate({'cond', CondMeta, [[{do, Clauses}]]}, S) -> - [{'->', Meta, [[Condition], Body]} = H | T] = lists:reverse(Clauses), - - Case = - case Condition of - X when is_atom(X) and (X /= false) and (X /= nil) -> - build_cond_clauses(T, Body, Meta); - _ -> - Error = {{'.', Meta, [erlang, error]}, [], [cond_clause]}, - build_cond_clauses([H | T], Error, Meta) + [{'->', Meta, [[Condition], _]} = H | T] = lists:reverse(Clauses), + + FirstMeta = + if + is_atom(Condition), Condition /= false, Condition /= nil -> ?generated(Meta); + true -> Meta end, + + Error = {{'.', Meta, [erlang, error]}, [], [cond_clause]}, + Case = build_cond_clauses([H | T], Error, FirstMeta), translate(replace_case_meta(CondMeta, Case), S); %% Case diff --git a/lib/elixir/test/elixir/fixtures/dialyzer/cond.ex b/lib/elixir/test/elixir/fixtures/dialyzer/cond.ex new file mode 100644 index 00000000000..9419b9b8e36 --- /dev/null +++ b/lib/elixir/test/elixir/fixtures/dialyzer/cond.ex @@ -0,0 +1,27 @@ +defmodule Dialyzer.Cond do + def one_boolean do + cond do + true -> :ok + end + end + + def two_boolean do + cond do + List.flatten([]) == [] -> :ok + true -> :ok + end + end + + def one_otherwise do + cond do + :otherwise -> :ok + end + end + + def two_otherwise do + cond do + List.flatten([]) == [] -> :ok + :otherwise -> :ok + end + end +end diff --git a/lib/elixir/test/elixir/kernel/dialyzer_test.exs b/lib/elixir/test/elixir/kernel/dialyzer_test.exs index de6176b7100..2ea2816e0ae 100644 --- a/lib/elixir/test/elixir/kernel/dialyzer_test.exs +++ b/lib/elixir/test/elixir/kernel/dialyzer_test.exs @@ -121,6 +121,11 @@ defmodule Kernel.DialyzerTest do assert_dialyze_no_warnings!(context) end + test "no warnings on cond", context do + copy_beam!(context, Dialyzer.Cond) + assert_dialyze_no_warnings!(context) + end + test "no warnings on for comprehensions with bitstrings", context do copy_beam!(context, Dialyzer.ForBitstring) assert_dialyze_no_warnings!(context) diff --git a/lib/elixir/test/elixir/kernel/special_forms_test.exs b/lib/elixir/test/elixir/kernel/special_forms_test.exs index 22f85186e87..a07b00821c8 100644 --- a/lib/elixir/test/elixir/kernel/special_forms_test.exs +++ b/lib/elixir/test/elixir/kernel/special_forms_test.exs @@ -4,4 +4,53 @@ defmodule Kernel.SpecialFormsTest do use ExUnit.Case, async: true doctest Kernel.SpecialForms + + describe "cond" do + test "does not leak variables for one clause" do + x = 0 + + cond do + true -> + x = 1 + x + end + + assert x == 0 + end + + test "does not leak variables for one clause with non-boolean as catch-all" do + x = 0 + + cond do + :otherwise -> + x = 1 + x + end + + assert x == 0 + end + + test "does not leak variables for multiple clauses" do + x = 0 + + cond do + List.flatten([]) == [] -> + x = 1 + x + + true -> + x = 1 + x + end + + assert x == 0 + end + + test "does not warn on non-boolean as catch-all" do + cond do + List.flatten([]) == [] -> :good + :otherwise -> :also_good + end + end + end end