Warn for unused module attributesjv-module-attributes-warn

Module attributes that are imported from other
contexts are not warned. This keeps the same
semantics regarding variables and function definition
ordering which are not warned when they come from
other contexts.

11 files changed, 108 insertions, 101 deletions

diff --git a/lib/elixir/lib/calendar.ex b/lib/elixir/lib/calendar.ex
index 007461030..7393e8c44 100644
--- a/lib/elixir/lib/calendar.ex
+++ b/lib/elixir/lib/calendar.ex
@@ -801,8 +801,6 @@ defmodule NaiveDateTime do
                             calendar: Calendar.calendar, hour: Calendar.hour, minute: Calendar.minute,
                             second: Calendar.second, microsecond: Calendar.microsecond}
 
-  @unix_epoch :calendar.datetime_to_gregorian_seconds {{1970, 1, 1}, {0, 0, 0}}
-
   @doc """
   Returns the current naive datetime in UTC.
 
diff --git a/lib/elixir/lib/kernel.ex b/lib/elixir/lib/kernel.ex
index 630971842..29934546d 100644
--- a/lib/elixir/lib/kernel.ex
+++ b/lib/elixir/lib/kernel.ex
@@ -2373,63 +2373,65 @@ defmodule Kernel do
   """
   defmacro @(expr)
 
-  defmacro @({name, _, args}) do
-    # Check for Module as it is compiled later than Kernel
-    case bootstrapped?(Module) do
-      false -> nil
-      true  ->
-        assert_module_scope(__CALLER__, :@, 1)
-        function? = __CALLER__.function != nil
+  defmacro @({name, meta, args}) do
+    assert_module_scope(__CALLER__, :@, 1)
+    function? = __CALLER__.function != nil
+
+    cond do
+      # Check for Module as it is compiled later than Kernel
+      not bootstrapped?(Module) ->
+        nil
+
+      not function? and __CALLER__.context == :match ->
+        raise ArgumentError, "invalid write attribute syntax, you probably meant to use: @#{name} expression"
 
-        case not function? and __CALLER__.context == :match do
+      # Typespecs attributes are currently special cased by the compiler
+      macro = is_list(args) and length(args) == 1 and typespec(name) ->
+        case bootstrapped?(Kernel.Typespec) do
           false -> nil
-          true  ->
-            raise ArgumentError, "invalid write attribute syntax, you probably meant to use: @#{name} expression"
+          true  -> quote do: Kernel.Typespec.unquote(macro)(unquote(hd(args)))
         end
 
-        # Typespecs attributes are special cased by the compiler so far
-        case is_list(args) and length(args) == 1 and typespec(name) do
-          false ->
-            do_at(args, name, function?, __CALLER__)
-          macro ->
-            case bootstrapped?(Kernel.Typespec) do
-              false -> nil
-              true  -> quote do: Kernel.Typespec.unquote(macro)(unquote(hd(args)))
-            end
-        end
+      true ->
+        do_at(args, meta, name, function?, __CALLER__)
     end
   end
 
   # @attribute(value)
-  defp do_at([arg], name, function?, env) do
-    case function? do
-      true ->
+  defp do_at([arg], meta, name, function?, env) do
+    read? = :lists.keymember(:context, 1, meta)
+
+    cond do
+      function? ->
         raise ArgumentError, "cannot set attribute @#{name} inside function/macro"
-      false ->
-        cond do
-          name == :behavior ->
-            :elixir_errors.warn env.line, env.file,
-                                "@behavior attribute is not supported, please use @behaviour instead"
-          :lists.member(name, [:moduledoc, :typedoc, :doc]) ->
-            {stack, _} = :elixir_quote.escape(env_stacktrace(env), false)
-            arg = {env.line, arg}
-            quote do: Module.put_attribute(__MODULE__, unquote(name), unquote(arg), unquote(stack))
-          true ->
-            quote do: Module.put_attribute(__MODULE__, unquote(name), unquote(arg))
-        end
+
+      name == :behavior ->
+        :elixir_errors.warn env.line, env.file,
+                            "@behavior attribute is not supported, please use @behaviour instead"
+
+      :lists.member(name, [:moduledoc, :typedoc, :doc]) ->
+        {stack, _} = :elixir_quote.escape(env_stacktrace(env), false)
+        arg = {env.line, arg}
+        quote do: Module.put_attribute(__MODULE__, unquote(name), unquote(arg),
+                                       unquote(stack), unquote(read?))
+
+      true ->
+        quote do: Module.put_attribute(__MODULE__, unquote(name), unquote(arg),
+                                       nil, unquote(read?))
     end
   end
 
   # @attribute or @attribute()
-  defp do_at(args, name, function?, env) when is_atom(args) or args == [] do
+  defp do_at(args, _meta, name, function?, env) when is_atom(args) or args == [] do
     stack = env_stacktrace(env)
-
     doc_attr? = :lists.member(name, [:moduledoc, :typedoc, :doc])
+
     case function? do
       true ->
         value =
-          with {_, doc} when doc_attr? <- Module.get_attribute(env.module, name, stack),
-            do: doc
+          with {_, doc} when doc_attr? <-
+                 Module.get_attribute(env.module, name, stack),
+               do: doc
         try do
           :elixir_quote.escape(value, false)
         rescue
@@ -2438,17 +2440,19 @@ defmodule Kernel do
         else
           {val, _} -> val
         end
+
       false ->
         {escaped, _} = :elixir_quote.escape(stack, false)
         quote do
-          with {_, doc} when unquote(doc_attr?) <- Module.get_attribute(__MODULE__, unquote(name), unquote(escaped)),
-            do: doc
+          with {_, doc} when unquote(doc_attr?) <-
+                 Module.get_attribute(__MODULE__, unquote(name), unquote(escaped)),
+               do: doc
         end
     end
   end
 
   # All other cases
-  defp do_at(args, name, _function?, _env) do
+  defp do_at(args, _meta, name, _function?, _env) do
     raise ArgumentError, "expected 0 or 1 argument for @#{name}, got: #{length(args)}"
   end
 
@@ -3571,6 +3575,7 @@ defmodule Kernel do
           end
         false ->
           quote do
+            _ = @enforce_keys
             def __struct__(kv) do
               :lists.foldl(fn {key, val}, acc ->
                 :maps.update(key, val, acc)
diff --git a/lib/elixir/lib/module.ex b/lib/elixir/lib/module.ex
index 1ad374cdf..67380b4b4 100644
--- a/lib/elixir/lib/module.ex
+++ b/lib/elixir/lib/module.ex
@@ -855,7 +855,7 @@ defmodule Module do
   """
   @spec put_attribute(module, key :: atom, value :: term) :: term
   def put_attribute(module, key, value) do
-    put_attribute(module, key, value, nil)
+    put_attribute(module, key, value, nil, false)
   end
 
   @doc """
@@ -910,7 +910,7 @@ defmodule Module do
     assert_not_compiled!(:delete_attribute, module)
     table = data_table_for(module)
     case :ets.take(table, key) do
-      [{_, value, true, _}] ->
+      [{_, value, _accumulated? = true, _}] ->
         :ets.insert(table, {key, [], true, true})
         value
       [{_, value, _, _}] ->
@@ -961,7 +961,7 @@ defmodule Module do
     end
 
     if Keyword.get(opts, :accumulate) do
-      :ets.insert_new(table, {new, [], true, true}) ||
+      :ets.insert_new(table, {new, [], _accumulated? = true, _read? = true}) ||
         :ets.update_element(table, new, {3, true})
     end
 
@@ -1059,22 +1059,22 @@ defmodule Module do
   @doc false
   # Used internally by Kernel's @.
   # This function is private and must be used only internally.
-  def put_attribute(module, key, value, stack) when is_atom(key) do
+  def put_attribute(module, key, value, stack, read?) when is_atom(key) do
     assert_not_compiled!(:put_attribute, module)
     table = data_table_for(module)
     value = preprocess_attribute(key, value)
 
     case :ets.lookup(table, key) do
-      [{^key, {line, <<_::binary>>}, acc, _read?}]
+      [{^key, {line, <<_::binary>>}, accumulated?, _read?}]
           when key in [:doc, :typedoc, :moduledoc] and is_list(stack) ->
         IO.warn "redefining @#{key} attribute previously set at line #{line}", stack
-        :ets.insert(table, {key, value, acc, false})
+        :ets.insert(table, {key, value, accumulated?, read?})
 
-      [{^key, current, true, _read?}] ->
-        :ets.insert(table, {key, [value | current], true, false})
+      [{^key, current, _accumulated? = true, _read?}] ->
+        :ets.insert(table, {key, [value | current], true, read?})
 
       _ ->
-        :ets.insert(table, {key, value, false, false})
+        :ets.insert(table, {key, value, false, read?})
     end
 
     value
diff --git a/lib/elixir/src/elixir_module.erl b/lib/elixir/src/elixir_module.erl
index 47e7a1a52..302743a2b 100644
--- a/lib/elixir/src/elixir_module.erl
+++ b/lib/elixir/src/elixir_module.erl
@@ -70,6 +70,7 @@ do_compile(Line, Module, Block, Vars, E) ->
     put_compiler_modules([Module|CompilerModules]),
     {Result, NE} = eval_form(Line, Module, Data, Block, Vars, E),
 
+    PersistedAttrs = ets:lookup_element(Data, ?persisted_attr, 2),
     CompileOpts = ets:lookup_element(Data, compile, 2),
     OnLoad = ets:lookup_element(Data, 'on_load', 2),
     [elixir_locals:record_local(Tuple, Module) || Tuple <- OnLoad],
@@ -80,16 +81,11 @@ do_compile(Line, Module, Block, Vars, E) ->
     {All, Forms0} = functions_form(Line, File, Module, Def, Defp,
                                    Defmacro, Defmacrop, Exports, Functions),
     Forms1 = specs_form(Data, Defmacro, Defmacrop, Unreachable, Forms0),
-    Forms2 = types_form(Line, File, Data, Forms1),
-    Forms3 = attributes_form(Line, File, Data, Forms2),
+    Forms2 = types_form(Data, Forms1),
+    Forms3 = attributes_form(Line, File, Data, PersistedAttrs, Forms2),
 
     elixir_locals:ensure_no_import_conflict(Line, File, Module, All),
-
-    %% TODO: Remove this at it will apply for all attributes.
-    case Docs of
-      true  -> warn_unused_docs(Line, File, Data, doc);
-      false -> false
-    end,
+    warn_unused_attributes(Line, File, Data, PersistedAttrs),
 
     Location = {elixir_utils:characters_to_list(elixir_utils:relative_to_cwd(File)), Line},
 
@@ -200,9 +196,8 @@ eval_form(Line, Module, Data, Block, Vars, E) ->
   {Value, EC}.
 
 eval_callbacks(Line, Data, Name, Args, E) ->
-  Callbacks = lists:reverse(ets:lookup_element(Data, Name, 2)),
-
-  lists:foldl(fun({M, F}, Acc) ->
+  Callbacks = ets:lookup_element(Data, Name, 2),
+  lists:foldr(fun({M, F}, Acc) ->
     expand_callback(Line, M, F, Args, Acc#{vars := [], export_vars := nil},
                     fun(AM, AF, AA) -> apply(AM, AF, AA) end)
   end, E, Callbacks).
@@ -219,8 +214,7 @@ functions_form(Line, File, Module, Def, Defp, Defmacro, Defmacrop, Exports, Body
 
 %% Add attributes handling to the form
 
-attributes_form(Line, File, Data, Current) ->
-  PersistedAttrs = ets:lookup_element(Data, ?persisted_attr, 2),
+attributes_form(Line, File, Data, PersistedAttrs, Current) ->
   Transform = fun(Key, Acc) when is_atom(Key) ->
     case ets:lookup(Data, Key) of
       [{Key, Values, true, _}] ->
@@ -247,21 +241,18 @@ process_external_resource(Line, File, Value) ->
 
 %% Types
 
-types_form(Line, File, Data, Forms0) ->
+types_form(Data, Forms0) ->
   case elixir_compiler:get_opt(internal) of
     false ->
       Types0 = take_type_spec(Data, type) ++
                take_type_spec(Data, typep) ++
                take_type_spec(Data, opaque),
-
       Types1 = ['Elixir.Kernel.Typespec':translate_type(Kind, Expr, Caller) ||
                 {Kind, Expr, Caller} <- Types0],
 
-      warn_unused_docs(Line, File, Data, typedoc),
       Forms1 = types_attributes(Types1, Forms0),
       Forms2 = export_types_attributes(Types1, Forms1),
       Forms2;
-
     true ->
       Forms0
   end.
@@ -332,8 +323,8 @@ translate_macro_spec({{callback, NameArity, Spec}, Line}, _Defmacro, _Defmacrop)
 spec_for_macro({type, Line, 'fun', [{type, _, product, Args} | T]}) ->
   NewArgs = [{type, Line, term, []} | Args],
   {type, Line, 'fun', [{type, Line, product, NewArgs} | T]};
-
-spec_for_macro(Else) -> Else.
+spec_for_macro(Else) ->
+  Else.
 
 optional_callbacks_attributes(OptionalCallbacksTypespec, Specs) ->
   % We only take the specs of the callbacks (which are both callbacks and
@@ -434,13 +425,11 @@ check_module_availability(Line, File, Module) ->
       ok
   end.
 
-warn_unused_docs(Line, File, Data, Attribute) ->
-  case ets:member(Data, Attribute) of
-    true ->
-      elixir_errors:form_warn([{line, Line}], File, ?MODULE, {unused_doc, Attribute});
-    _ ->
-      ok
-  end.
+warn_unused_attributes(Line, File, Data, PersistedAttrs) ->
+  ReservedAttrs = [after_compile, before_compile, moduledoc, on_definition | PersistedAttrs],
+  Keys = ets:select(Data, [{{'$1', '_', '_', false}, [{is_atom, '$1'}], ['$1']}]),
+  [elixir_errors:form_warn([{line, Line}], File, ?MODULE, {unused_attribute, Key}) ||
+   Key <- Keys, not lists:member(Key, ReservedAttrs)].
 
 % __INFO__
 
@@ -554,10 +543,12 @@ prune_stacktrace(Info, []) ->
 format_error({invalid_external_resource, Value}) ->
   io_lib:format("expected a string value for @external_resource, got: ~p",
     ['Elixir.Kernel':inspect(Value)]);
-format_error({unused_doc, typedoc}) ->
-  "@typedoc provided but no type follows it";
-format_error({unused_doc, doc}) ->
-  "@doc provided but no definition follows it";
+format_error({unused_attribute, typedoc}) ->
+  "module attribute @typedoc was set but no type follows it";
+format_error({unused_attribute, doc}) ->
+  "module attribute @doc was set but no definition follows it";
+format_error({unused_attribute, Attr}) ->
+  io_lib:format("module attribute @~ts was set but never used", [Attr]);
 format_error({internal_function_overridden, {Name, Arity}}) ->
   io_lib:format("function ~ts/~B is internal and should not be overridden", [Name, Arity]);
 format_error({invalid_module, Module}) ->
diff --git a/lib/elixir/src/elixir_quote.erl b/lib/elixir/src/elixir_quote.erl
index 950d9afef..1ac251de3 100644
--- a/lib/elixir/src/elixir_quote.erl
+++ b/lib/elixir/src/elixir_quote.erl
@@ -3,7 +3,7 @@
 -export([dot/5, tail_list/3, list/2]). %% Quote callbacks
 
 -include("elixir.hrl").
--define(defs(Kind), Kind == def; Kind == defp; Kind == defmacro; Kind == defmacrop).
+-define(defs(Kind), Kind == def; Kind == defp; Kind == defmacro; Kind == defmacrop; Kind == '@').
 -define(lexical(Kind), Kind == import; Kind == alias; Kind == require).
 -compile({inline, [keyfind/2, keystore/3, keydelete/2, keyreplace/3, keynew/3]}).
 
diff --git a/lib/elixir/test/elixir/gen_event_test.exs b/lib/elixir/test/elixir/gen_event_test.exs
index 6b9442d56..603bd5c67 100644
--- a/lib/elixir/test/elixir/gen_event_test.exs
+++ b/lib/elixir/test/elixir/gen_event_test.exs
@@ -112,8 +112,6 @@ defmodule GenEventTest do
     end
   end
 
-  @receive_timeout 1000
-
   test "start/1" do
     assert {:ok, pid} = GenEvent.start()
     assert GenEvent.which_handlers(pid) == []
diff --git a/lib/elixir/test/elixir/kernel/warning_test.exs b/lib/elixir/test/elixir/kernel/warning_test.exs
index 3a2aa16dc..9a4895e82 100644
--- a/lib/elixir/test/elixir/kernel/warning_test.exs
+++ b/lib/elixir/test/elixir/kernel/warning_test.exs
@@ -680,6 +680,21 @@ defmodule Kernel.WarningTest do
     purge Sample
   end
 
+  test "attribute with no use" do
+    content = capture_err(fn ->
+      Code.eval_string """
+      defmodule Sample do
+        @at "Something"
+        Module.put_attribute(__MODULE__, :put_attribute, "Something")
+      end
+      """
+    end)
+    assert content =~ "module attribute @at was set but never used"
+    assert content =~ "module attribute @put_attribute was set but never used"
+  after
+    purge Sample
+  end
+
   test "typedoc with no type" do
     assert capture_err(fn ->
       Code.eval_string """
@@ -687,7 +702,7 @@ defmodule Kernel.WarningTest do
         @typedoc "Something"
       end
       """
-    end) =~ "@typedoc provided but no type follows it"
+    end) =~ "module attribute @typedoc was set but no type follows it"
   after
     purge Sample
   end
@@ -699,7 +714,7 @@ defmodule Kernel.WarningTest do
         @doc "Something"
       end
       """
-    end) =~ "@doc provided but no definition follows it"
+    end) =~ "module attribute @doc was set but no definition follows it"
   after
     purge Sample
   end
diff --git a/lib/elixir/test/elixir/macro_test.exs b/lib/elixir/test/elixir/macro_test.exs
index 70b79a2ce..39442cc56 100644
--- a/lib/elixir/test/elixir/macro_test.exs
+++ b/lib/elixir/test/elixir/macro_test.exs
@@ -212,11 +212,9 @@ defmodule MacroTest do
     assert Macro.expand_once(expr, __ENV__) == expr
   end
 
-  @foo 1
-
   test "expand once does not expand module attributes" do
     message = "could not call get_attribute on module #{inspect(__MODULE__)} " <>
-      "because it was already compiled"
+              "because it was already compiled"
     assert_raise ArgumentError, message, fn ->
       Macro.expand_once(quote(do: @foo), __ENV__)
     end
diff --git a/lib/elixir/test/elixir/module_test.exs b/lib/elixir/test/elixir/module_test.exs
index 0ba10a797..34a1bbf5d 100644
--- a/lib/elixir/test/elixir/module_test.exs
+++ b/lib/elixir/test/elixir/module_test.exs
@@ -58,34 +58,35 @@ defmodule ModuleTest do
     end
   end
 
-  test "return value" do
+  test "module attributes returns value" do
     in_module do
       assert (@return [:foo, :bar]) == [:foo, :bar]
+      _ = @return
     end
   end
 
-  test "in memory" do
+  test "in memory modules are tagged as so" do
     assert :code.which(__MODULE__) == :in_memory
   end
 
   ## Eval
 
-  test "eval quoted" do
+  test "executes eval_quoted definitions" do
     assert eval_quoted_info() == {ModuleTest, "sample.ex", 13}
   end
 
-  test "line from macro" do
+  test "retrieves line from macros" do
     assert ModuleTest.ToUse.line == 36
   end
 
   ## Callbacks
 
-  test "compile callback hook" do
+  test "executes custom before_compile callback" do
     assert ModuleTest.ToUse.callback_value(true) == true
     assert ModuleTest.ToUse.callback_value(false) == false
   end
 
-  test "before compile callback hook" do
+  test "executes default before_compile callback" do
     assert ModuleTest.ToUse.before_compile == []
   end
 
@@ -99,7 +100,7 @@ defmodule ModuleTest do
     assert {:+, _, [{:foo, _, nil}, {:bar, _, nil}]} = expr
   end
 
-  test "on definition" do
+  test "executes on definition callback" do
     defmodule OnDefinition do
       @on_definition ModuleTest
 
@@ -118,7 +119,7 @@ defmodule ModuleTest do
     end
   end
 
-  test "overridable inside before compile" do
+  test "may set overridable inside before_compile callback" do
     defmodule OverridableWithBeforeCompile do
       @before_compile ModuleTest
     end
diff --git a/lib/iex/lib/iex/config.ex b/lib/iex/lib/iex/config.ex
index c6cdf69e0..fe5a49475 100644
--- a/lib/iex/lib/iex/config.ex
+++ b/lib/iex/lib/iex/config.ex
@@ -4,8 +4,6 @@ defmodule IEx.Config do
   @table __MODULE__
   @agent __MODULE__
   @keys [:colors, :inspect, :history_size, :default_prompt, :alive_prompt, :width]
-  @colors [:eval_interrupt, :eval_result, :eval_error, :eval_info,
-           :stack_app, :stack_info, :ls_directory, :ls_device]
 
   # Read API
 
diff --git a/lib/mix/test/mix/local_test.exs b/lib/mix/test/mix/local_test.exs
index 692878c7d..1418a85f4 100644
--- a/lib/mix/test/mix/local_test.exs
+++ b/lib/mix/test/mix/local_test.exs
@@ -66,6 +66,9 @@ defmodule Mix.LocalTest do
   llLXgJJE2tGpDhEXBA3idg==
   """
 
+  # We don't actually use it but it exists for documentation purposes.
+  _ = @private_key
+
   setup_all do
     File.mkdir_p!(Mix.PublicKey.public_keys_path)