Merge pull request #4076 from lexmag/access-error

Make Access syntax fail to compile for unsupported values

Author: lexmag

lib/elixir/lib/access.ex

@@ -60,6 +60,11 @@ defmodule Access do
     end
   end
 
+  def fetch(list, key) when is_list(list) do
+    raise ArgumentError,
+      "the Access calls for keywords expect the key to be an atom, got: " <> inspect(key)
+  end
+
   def fetch(nil, _key) do
     :error
   end

lib/elixir/lib/kernel.ex

@@ -1643,7 +1643,7 @@ defmodule Kernel do
   @doc """
   Gets a value from a nested structure.
 
-  Uses the `Access` protocol to traverse the structures
+  Uses the `Access` module to traverse the structures
   according to the given `keys`, unless the `key` is a
   function.
 
@@ -1662,7 +1662,7 @@ defmodule Kernel do
       27
 
   In case any of entries in the middle returns `nil`, `nil` will be returned
-  as per the Access protocol:
+  as per the Access module:
 
       iex> users = %{"john" => %{age: 27}, "meg" => %{age: 23}}
       iex> get_in(users, ["unknown", :age])
@@ -1702,7 +1702,7 @@ defmodule Kernel do
   @doc """
   Puts a value in a nested structure.
 
-  Uses the `Access` protocol to traverse the structures
+  Uses the `Access` module to traverse the structures
   according to the given `keys`, unless the `key` is a
   function. If the key is a function, it will be invoked
   as specified in `get_and_update_in/3`.
@@ -1724,7 +1724,7 @@ defmodule Kernel do
   @doc """
   Updates a key in a nested structure.
 
-  Uses the `Access` protocol to traverse the structures
+  Uses the `Access` module to traverse the structures
   according to the given `keys`, unless the `key` is a
   function. If the key is a function, it will be invoked
   as specified in `get_and_update_in/3`.
@@ -1749,7 +1749,7 @@ defmodule Kernel do
   It expects a tuple to be returned, containing the value
   retrieved and the update one.
 
-  Uses the `Access` protocol to traverse the structures
+  Uses the `Access` module to traverse the structures
   according to the given `keys`, unless the `key` is a
   function.
 

lib/elixir/src/elixir_exp.erl

@@ -516,7 +516,7 @@ expand_remote(Receiver, DotMeta, Right, Meta, Args, E, EL) ->
       ok
   end,
   {EArgs, EA} = expand_args(Args, E),
-  {elixir_rewrite:rewrite(Receiver, DotMeta, Right, Meta, EArgs),
+  {elixir_rewrite:rewrite(Receiver, DotMeta, Right, Meta, EArgs, EA),
    elixir_env:mergev(EL, EA)}.
 
 %% Lexical helpers

lib/elixir/src/elixir_rewrite.erl

@@ -1,10 +1,13 @@
 -module(elixir_rewrite).
--export([rewrite/5, inline/3]).
+-export([rewrite/6, inline/3]).
 -include("elixir.hrl").
 
+-define(is_literal(Arg), (is_binary(Arg) orelse is_number(Arg) orelse is_atom(Arg))).
+
 %% Convenience variables
 
 -define(atom, 'Elixir.Atom').
+-define(access, 'Elixir.Access').
 -define(enum, 'Elixir.Enum').
 -define(float, 'Elixir.Float').
 -define(io, 'Elixir.IO').
@@ -173,9 +176,17 @@ inline(_, _, _) -> false.
 
 %% Complex rewrite rules
 
-rewrite(?string_chars, _DotMeta, 'to_string', _Meta, [String]) when is_binary(String) ->
+rewrite(?access, _DotMeta, 'get', _Meta, [nil, Arg], _Env)
+    when ?is_literal(Arg) orelse (is_atom(element(1, Arg)) andalso element(3, Arg) == nil) ->
+  nil;
+rewrite(?access, _DotMeta, 'get', Meta, [Arg, _], Env)
+    when ?is_literal(Arg) orelse element(1, Arg) == '{}' orelse element(1, Arg) == '<<>>' ->
+  elixir_errors:compile_error(Meta, ?m(Env, file),
+    "the Access syntax and calls to Access.get/2 are not available for the value: ~ts",
+    ['Elixir.Macro':to_string(Arg)]);
+rewrite(?string_chars, _DotMeta, 'to_string', _Meta, [String], _File) when is_binary(String) ->
   String;
-rewrite(?string_chars, DotMeta, 'to_string', Meta, [String]) ->
+rewrite(?string_chars, DotMeta, 'to_string', Meta, [String], _Env) ->
   Var   = {'rewrite', Meta, 'Elixir'},
   Guard = {{'.', ?generated, [erlang, is_binary]}, ?generated, [Var]},
   Slow  = remote(?string_chars, DotMeta, 'to_string', Meta, [Var]),
@@ -186,9 +197,9 @@ rewrite(?string_chars, DotMeta, 'to_string', Meta, [String]) ->
      {'->', ?generated, [[Var], Slow]}]
   }]]};
 
-rewrite(?enum, DotMeta, 'reverse', Meta, [List]) when is_list(List) ->
+rewrite(?enum, DotMeta, 'reverse', Meta, [List], _Env) when is_list(List) ->
   remote(lists, DotMeta, 'reverse', Meta, [List]);
-rewrite(?enum, DotMeta, 'reverse', Meta, [List]) ->
+rewrite(?enum, DotMeta, 'reverse', Meta, [List], _Env) ->
   Var   = {'rewrite', Meta, 'Elixir'},
   Guard = {{'.', ?generated, [erlang, is_list]}, ?generated, [Var]},
   Slow  = remote(?enum, DotMeta, 'reverse', Meta, [Var]),
@@ -199,7 +210,7 @@ rewrite(?enum, DotMeta, 'reverse', Meta, [List]) ->
      {'->', ?generated, [[Var], Slow]}]
   }]]};
 
-rewrite(Receiver, DotMeta, Right, Meta, Args) ->
+rewrite(Receiver, DotMeta, Right, Meta, Args, _Env) ->
   {EReceiver, ERight, EArgs} = rewrite(Receiver, Right, Args),
   remote(EReceiver, DotMeta, ERight, Meta, EArgs).
 

lib/elixir/test/elixir/access_test.exs

@@ -36,8 +36,9 @@ defmodule AccessTest do
     assert Access.fetch([foo: :bar], :foo) == {:ok, :bar}
     assert Access.fetch([foo: :bar], :bar) == :error
 
-    assert_raise FunctionClauseError, fn ->
-      Access.fetch([{"foo", :bar}], "foo")
+    msg = ~r/the Access calls for keywords expect the key to be an atom/
+    assert_raise ArgumentError, msg,  fn ->
+      Access.fetch([], "foo")
     end
 
     assert Access.get([foo: :bar], :foo) == :bar

lib/elixir/test/elixir/kernel/errors_test.exs

@@ -53,6 +53,20 @@ defmodule Kernel.ErrorsTest do
                         'fn 1 end'
   end
 
+  test "invalid Access" do
+    msg = fn(val) ->
+      "nofile:1: the Access syntax and calls to Access.get/2" <>
+      " are not available for the value: " <> val
+    end
+
+    assert_compile_fail CompileError, msg.("1"), "1[:foo]"
+    assert_compile_fail CompileError, msg.("1.1"), "1.1[:foo]"
+    assert_compile_fail CompileError, msg.("{}"), "{}[:foo]"
+    assert_compile_fail CompileError, msg.(":foo"), ":foo[:foo]"
+    assert_compile_fail CompileError, msg.("\"\""), "\"\"[:foo]"
+    assert_compile_fail CompileError, msg.("<<>>"), "<<>>[:foo]"
+  end
+
   test "kw missing space" do
     msg = "nofile:1: keyword argument must be followed by space after: foo:"