Improve coverage for lexical scope on try and case

Author: José Valim

lib/elixir/src/elixir_macros.erl

@@ -3,7 +3,7 @@
 -module(elixir_macros).
 -export([translate/2]).
 -import(elixir_translator, [translate_each/2, translate_args/2, translate_apply/7]).
--import(elixir_scope, [umergec/2]).
+-import(elixir_scope, [umergec/2, umergea/2]).
 -import(elixir_errors, [syntax_error/3, syntax_error/4,
   assert_no_function_scope/3, assert_module_scope/3, assert_no_match_or_guard_scope/3]).
 
@@ -154,13 +154,13 @@ translate({'try', Meta, [Clauses]}, S) ->
   { TDo, SB } = elixir_translator:translate_each(Do, S#elixir_scope{noname=true}),
 
   Catch = [Tuple || { X, _ } = Tuple <- Clauses, X == 'rescue' orelse X == 'catch'],
-  { TCatch, SC } = elixir_try:clauses(Meta, Catch, umergec(S, SB)),
+  { TCatch, SC } = elixir_try:clauses(Meta, Catch, umergea(S, SB)),
 
   After = proplists:get_value('after', Clauses, nil),
-  { TAfter, SA } = elixir_translator:translate_each(After, umergec(S, SC)),
+  { TAfter, SA } = elixir_translator:translate_each(After, umergea(S, SC)),
 
   Else = elixir_clauses:get_pairs(Meta, else, Clauses, S),
-  { TElse, SE } = elixir_clauses:match(Meta, Else, umergec(S, SA)),
+  { TElse, SE } = elixir_clauses:match(Meta, Else, umergea(S, SA)),
 
   SF = (umergec(S, SE))#elixir_scope{noname=S#elixir_scope.noname},
   { { 'try', ?line(Meta), pack(TDo), TElse, TCatch, pack(TAfter) }, SF };

lib/elixir/src/elixir_scope.erl

@@ -5,7 +5,7 @@
   serialize/1, deserialize/1,
   serialize_with_vars/2, deserialize_with_vars/2,
   to_erl_env/1, to_ex_env/1,
-  umergev/2, umergec/2, merge_clause_vars/2
+  umergev/2, umergec/2, umergea/2, merge_clause_vars/2
 ]).
 -include("elixir.hrl").
 
@@ -140,8 +140,8 @@ deserialize_with_vars({ File, Functions, Requires, Macros,
     counter=[{'',length(Vars)}]
   }.
 
-% Receives two scopes and return a new scope based on the second
-% with their variables merged.
+%% Receives two scopes and return a new scope based on the second
+%% with their variables merged.
 
 umergev(S1, S2) ->
   V1 = S1#elixir_scope.vars,
@@ -153,11 +153,22 @@ umergev(S1, S2) ->
     clause_vars=merge_clause_vars(C1, C2)
   }.
 
-% Receives two scopes and return the later scope
-% keeping the variables from the first (counters,
-% imports and everything else are passed forward).
+%% Receives two scopes and return the first scope with
+%% counters and flags from the later.
 
 umergec(S1, S2) ->
+  S1#elixir_scope{
+    counter=S2#elixir_scope.counter,
+    extra_guards=S2#elixir_scope.extra_guards,
+    super=S2#elixir_scope.super,
+    caller=S2#elixir_scope.caller
+  }.
+
+%% Receives two scopes and return the later scope
+%% keeping the variables from the first (counters,
+%% imports and everything else are passed forward).
+
+umergea(S1, S2) ->
   S2#elixir_scope{
     vars=S1#elixir_scope.vars,
     temp_vars=S1#elixir_scope.temp_vars,

lib/elixir/src/elixir_tracker.erl

@@ -122,7 +122,7 @@ format_error({import_conflict,{Receivers, Name, Arity}}) ->
   io_lib:format("imported ~ts.~ts/~B conflicts with local function",
     [elixir_errors:inspect(hd(Receivers)), Name, Arity]);
 
-format_error({ unused_import, Module }) ->
+format_error({unused_import, Module}) ->
   io_lib:format("unused import ~ts", [elixir_errors:inspect(Module)]);
 
 format_error({unused_args,{Name, Arity}}) ->

lib/elixir/src/elixir_translator.erl

@@ -4,7 +4,7 @@
 -export([forms/4, 'forms!'/4]).
 -export([translate/2, translate_each/2, translate_arg/2,
          translate_args/2, translate_apply/7, translate_fn/3]).
--import(elixir_scope, [umergev/2, umergec/2]).
+-import(elixir_scope, [umergev/2, umergec/2, umergea/2]).
 -import(elixir_errors, [syntax_error/3, syntax_error/4, parse_error/4,
   assert_function_scope/3, assert_module_scope/3, assert_no_guard_scope/3,
   assert_no_match_or_guard_scope/3]).
@@ -607,14 +607,14 @@ translate_local(Meta, Name, Args, S) ->
 
 translate_arg(Arg, { Acc, S }) ->
   { TArg, TAcc } = translate_each(Arg, Acc),
-  { TArg, { umergec(Acc, TAcc), umergev(S, TAcc) } }.
+  { TArg, { umergea(Acc, TAcc), umergev(S, TAcc) } }.
 
 translate_args(Args, #elixir_scope{context=match} = S) ->
   translate(Args, S);
 
 translate_args(Args, S) ->
   { TArgs, { SC, SV } } = lists:mapfoldl(fun translate_arg/2, {S, S}, Args),
-  { TArgs, umergec(SV, SC) }.
+  { TArgs, umergea(SV, SC) }.
 
 %% Translate apply
 %% Used by both apply and external function invocation macros.

lib/elixir/test/elixir/kernel/import_test.exs

@@ -1,134 +1,149 @@
 Code.require_file "../test_helper.exs", __DIR__
 
-defmodule Kernel.ImportAvailable do
-  defmacro flatten do
-    [flatten: 1]
-  end
-end
-
-defmodule Kernel.ImportOnlyTest do
+defmodule Kernel.ImportTest do
   use ExUnit.Case, async: true
 
-  test :import_with_only do
-    require Kernel.ImportAvailable
-    import :lists, only: Kernel.ImportAvailable.flatten
-    assert flatten([1, [2], 3]) == [1, 2, 3]
+  defmodule ImportAvailable do
+    defmacro flatten do
+      [flatten: 1]
+    end
   end
 
-  test :import_all do
+
+  test "import all" do
     import :lists
     assert flatten([1, [2], 3]) == [1, 2, 3]
   end
 
-  test :import_except_none do
+  test "import except none" do
     import :lists, except: []
     assert flatten([1, [2], 3]) == [1, 2, 3]
   end
 
-  test :import_with_except_erlang do
+  test "import except one" do
     import :lists, except: [each: 2]
     assert flatten([1, [2], 3]) == [1, 2, 3]
   end
 
+  test "import only via macro" do
+    require ImportAvailable
+    import :lists, only: ImportAvailable.flatten
+    assert flatten([1, [2], 3]) == [1, 2, 3]
+  end
+
   defmacrop dynamic_opts do
-    [except: [each: 2]]
+    [only: [flatten: 1]]
   end
 
-  test :import_with_dynamic_opts do
+  test "import with options via macro" do
     import :lists, dynamic_opts
     assert flatten([1, [2], 3]) == [1, 2, 3]
   end
-end
-
-defmodule Kernel.DoubleImportTest do
-  use ExUnit.Case, async: true
 
-  test :import_double_except do
-    import :lists, except: [flatten: 1]
+  test "import with double except" do
+    import :lists, except: [duplicate: 2]
     import :lists, except: [each: 2]
     assert append([1], [2, 3]) == [1, 2, 3]
-    assert flatten([1, [2], 3]) == [1, [2], 3]
+    # Buggy local duplicate is untouched
+    assert duplicate([1], 2) == [1]
   end
 
-  def flatten(list), do: list
-end
-
-defmodule Kernel.MessedBitwise do
-  defmacro bnot(x),   do: x
-  defmacro bor(x, _), do: x
-end
-
-defmodule Kernel.Underscored do
-  def hello(x),          do: x
-  def __underscore__(x), do: x
-end
-
-defmodule Kernel.ExplicitUnderscored do
-  def __underscore__(x), do: x * 2
-end
+  defmodule Underscored do
+    def hello(x),          do: x
+    def __underscore__(x), do: x
+  end
 
-defmodule Kernel.ImportUnderscoreTest do
-  use ExUnit.Case, async: true
+  defmodule ExplicitUnderscored do
+    def __underscore__(x), do: x * 2
+  end
 
-  test :includes_only_underscore do
-    import Kernel.Underscored, only: [__underscore__: 1]
+  test "import only with underscore" do
+    import Underscored, only: [__underscore__: 1]
     assert __underscore__(3) == 3
   end
 
-  import :all, Kernel.ExplicitUnderscored
-
-  test :does_not_include_underscored do
-    import Kernel.Underscored
+  test "import all includes underscores" do
+    import :all, ExplicitUnderscored
+    import Underscored
     assert __underscore__(2) == 4
   end
 
-  test :includes_remaining do
-    import Kernel.Underscored
+  test "import non underscored" do
+    import :all, ExplicitUnderscored
+    import Underscored
     assert hello(2) == 2
   end
-end
 
-defmodule Kernel.ImportMacrosTest do
-  use ExUnit.Case, async: true
+  defmodule MessedBitwise do
+    defmacro bnot(x),   do: x
+    defmacro bor(x, _), do: x
+  end
 
   import :macros, Bitwise
 
-  test :import_true do
-    assert band(1, 1) == 1
-    assert bor(0, 1) == 1
-    assert bnot(0) == -1
-  end
-
-  test :function_import_with_only do
+  test "conflicing imports with only and except" do
     import :macros, Bitwise, except: [bnot: 1]
-    import :macros, Kernel.MessedBitwise, only: [bnot: 1]
+    import :macros, MessedBitwise, only: [bnot: 1]
     assert bnot(0) == 0
     assert bor(0, 1) == 1
   end
 
-  # This test is asserting that the requires done
-  # inside the function do not affect outer ones.
-  test :import_true_not_affected do
+  # This test is asserting that the imports in the
+  # test above do not affect this test.
+  test "imports from other functions do not leak" do
     assert band(1, 1) == 1
     assert bor(0, 1) == 1
     assert bnot(0) == -1
   end
-end
 
-defmodule Kernel.MultipleImportTest do
-  use ExUnit.Case, async: true
-
-  test :import_ambiguous do
+  test "import ambiguous" do
     # Simply make sure that we can indeed import functions with
     # the same name and arity from different modules without the
     # import itself causing any errors.
     import List
     import String
   end
 
-  test :import_many do
+  test "import many" do
     [import(List), import(String)]
     assert capitalize("foo")  == "Foo"
     assert flatten([1, [2], 3]) == [1, 2, 3]
   end
+
+  test "import lexical on if" do
+    if false do
+      import :lists
+      flatten([1, [2], 3])
+      flunk
+    else
+      # Buggy local duplicate is untouched
+      assert duplicate([1], 2) == [1]
+    end
+  end
+
+  test "import lexical on case" do
+    case true do
+      false ->
+        import :lists
+        flatten([1, [2], 3])
+        flunk
+      true ->
+        # Buggy local duplicate is untouched
+        assert duplicate([1], 2) == [1]
+    end
+  end
+
+  test "import lexical on try" do
+    try do
+      import :lists
+      flatten([1, [2], 3])
+    catch
+      _ -> flatten([:a, [:b], :c])
+    end
+
+    # Buggy local duplicate is untouched
+    assert duplicate([1], 2) == [1]
+  end
+
+  defp duplicate(list, _), do: list
 end