Merge pull request #2350 from ericmj/keyword-error

Raise if giving non-keywords to Keyword

Author: José Valim

lib/elixir/lib/keyword.ex

@@ -24,6 +24,7 @@ defmodule Keyword do
   in `Enum` and `List` can also be applied.
   """
 
+  @compile :inline_list_funcs
   @behaviour Dict
 
   @type key :: atom
@@ -174,7 +175,12 @@ defmodule Keyword do
   """
   @spec get_values(t, key) :: [value]
   def get_values(keywords, key) when is_list(keywords) and is_atom(key) do
-    for {k, v} <- keywords, key == k, do: v
+    fun = fn
+      {k, v} when k === key -> {true, v}
+      {_, _} -> false
+    end
+
+    :lists.filtermap(fun, keywords)
   end
 
   @doc """
@@ -192,7 +198,7 @@ defmodule Keyword do
   """
   @spec keys(t) :: [key]
   def keys(keywords) when is_list(keywords) do
-    for {key, _} <- keywords, do: key
+    :lists.map(fn {k, _} -> k end, keywords)
   end
 
   @doc """
@@ -206,11 +212,11 @@ defmodule Keyword do
   """
   @spec values(t) :: [value]
   def values(keywords) when is_list(keywords) do
-    for {_, value} <- keywords, do: value
+    :lists.map(fn {_, v} -> v end, keywords)
   end
 
   @doc """
-  Deletes the entry in the keyword list for a `key` with `value`.
+  Deletes the entries in the keyword list for a `key` with `value`.
   If no `key` with `value` exists, returns the keyword list unchanged.
 
   ## Examples
@@ -227,11 +233,11 @@ defmodule Keyword do
   """
   @spec delete(t, key, value) :: t
   def delete(keywords, key, value) when is_list(keywords) and is_atom(key) do
-    for {k, v} = tuple <- keywords, key != k or value != v, do: tuple
+    :lists.filter(fn {k, v} -> k != key or v != value end, keywords)
   end
 
   @doc """
-  Deletes all entries in the keyword list for a specific `key`.
+  Deletes the entries in the keyword list for a specific `key`.
   If the `key` does not exist, returns the keyword list unchanged.
   Use `delete_first` to delete just the first entry in case of
   duplicated keys.
@@ -250,7 +256,7 @@ defmodule Keyword do
   """
   @spec delete(t, key) :: t
   def delete(keywords, key) when is_list(keywords) and is_atom(key) do
-    for {k, _} = tuple <- keywords, key != k, do: tuple
+    :lists.filter(fn {k, _} -> k != key end, keywords)
   end
 
   @doc """
@@ -339,7 +345,8 @@ defmodule Keyword do
   """
   @spec merge(t, t) :: t
   def merge(d1, d2) when is_list(d1) and is_list(d2) do
-    d2 ++ for({k, _} = tuple <- d1, not has_key?(d2, k), do: tuple)
+    fun = fn {k, _v} -> not has_key?(d2, k) end
+    d2 ++ :lists.filter(fun, d1)
   end
 
   @doc """
@@ -467,16 +474,16 @@ defmodule Keyword do
 
   """
   def split(keywords, keys) when is_list(keywords) do
-    acc = {[], []}
-
-    {take, drop} = Enum.reduce keywords, acc, fn({k, v}, {take, drop}) ->
+    fun = fn {k, v}, {take, drop} ->
       case k in keys do
         true  -> {[{k, v}|take], drop}
         false -> {take, [{k, v}|drop]}
       end
     end
 
-    {Enum.reverse(take), Enum.reverse(drop)}
+    acc = {[], []}
+    {take, drop} = :lists.foldl(fun, acc, keywords)
+    {:lists.reverse(take), :lists.reverse(drop)}
   end
 
   @doc """
@@ -497,7 +504,7 @@ defmodule Keyword do
 
   """
   def take(keywords, keys) when is_list(keywords) do
-    for {k, _} = tuple <- keywords, k in keys, do: tuple
+    :lists.filter(fn {k, _} -> k in keys end, keywords)
   end
 
   @doc """
@@ -517,7 +524,7 @@ defmodule Keyword do
 
   """
   def drop(keywords, keys) when is_list(keywords) do
-    for {k, _} = tuple <- keywords, not k in keys, do: tuple
+    :lists.filter(fn {k, _} -> not k in keys end, keywords)
   end
 
   @doc """

lib/elixir/test/elixir/keyword_test.exs

@@ -62,17 +62,39 @@ defmodule KeywordTest do
   test "keys/1" do
     assert Keyword.keys(create_keywords) == [:first_key, :second_key]
     assert Keyword.keys(create_empty_keywords) == []
+
+    assert_raise FunctionClauseError, fn ->
+      Keyword.keys([:foo])
+    end
   end
 
   test "values/1" do
     assert Keyword.values(create_keywords) == [1, 2]
     assert Keyword.values(create_empty_keywords) == []
+
+    assert_raise FunctionClauseError, fn ->
+      Keyword.values([:foo])
+    end
   end
 
   test "delete/2" do
     assert Keyword.delete(create_keywords, :second_key) == [first_key: 1]
     assert Keyword.delete(create_keywords, :other_key) == [first_key: 1, second_key: 2]
     assert Keyword.delete(create_empty_keywords, :other_key) == []
+
+    assert_raise FunctionClauseError, fn ->
+      Keyword.delete([:foo], :foo)
+    end
+  end
+
+  test "delete/3" do
+    keywords = [a: 1, b: 2, c: 3, a: 2]
+    assert Keyword.delete(keywords, :a, 2) == [a: 1, b: 2, c: 3]
+    assert Keyword.delete(keywords, :a, 1) == [b: 2, c: 3, a: 2]
+
+    assert_raise FunctionClauseError, fn ->
+      Keyword.delete([:foo], :foo, 0)
+    end
   end
 
   test "put/3" do
@@ -90,6 +112,10 @@ defmodule KeywordTest do
     assert Keyword.merge(create_keywords, create_empty_keywords) == [first_key: 1, second_key: 2]
     assert Keyword.merge(create_keywords, create_keywords) == [first_key: 1, second_key: 2]
     assert Keyword.merge(create_empty_keywords, create_empty_keywords) == []
+
+    assert_raise FunctionClauseError, fn ->
+      Keyword.delete([:foo], [:foo])
+    end
   end
 
   test "merge/3" do
@@ -136,6 +162,10 @@ defmodule Keyword.DuplicatedTest do
     assert Keyword.get_values(create_keywords, :first_key) == [1, 2]
     assert Keyword.get_values(create_keywords, :second_key) == [2]
     assert Keyword.get_values(create_keywords, :other_key) == []
+
+    assert_raise FunctionClauseError, fn ->
+      Keyword.get_values([:foo], :foo)
+    end
   end
 
   test "keys/1" do
@@ -191,6 +221,28 @@ defmodule Keyword.DuplicatedTest do
     assert Keyword.has_key?([a: 1], :b) == false
   end
 
+  test "take/2" do
+    assert Keyword.take([], []) == []
+    assert Keyword.take([a: 0, b: 1, a: 2], []) == []
+    assert Keyword.take([a: 0, b: 1, a: 2], [:a]) == [a: 0, a: 2]
+    assert Keyword.take([a: 0, b: 1, a: 2], [:b]) == [b: 1]
+
+    assert_raise FunctionClauseError, fn ->
+      Keyword.take([:foo], [:foo])
+    end
+  end
+
+  test "drop/2" do
+    assert Keyword.drop([], []) == []
+    assert Keyword.drop([a: 0, b: 1, a: 2], []) == [a: 0, b: 1, a: 2]
+    assert Keyword.drop([a: 0, b: 1, a: 2], [:a]) == [b: 1]
+    assert Keyword.drop([a: 0, b: 1, a: 2], [:b]) == [a: 0, a: 2]
+
+    assert_raise FunctionClauseError, fn ->
+      Keyword.drop([:foo], [:foo])
+    end
+  end
+
   defp create_empty_keywords, do: []
   defp create_keywords, do: [first_key: 1, first_key: 2, second_key: 2]
 end