Include proper line numbers on unused module attribute warning, closes #5490

Signed-off-by: José Valim <jose.valim@plataformatec.com.br>

Author: José Valim

lib/elixir/lib/kernel.ex

@@ -2399,7 +2399,11 @@ defmodule Kernel do
 
   # @attribute(value)
   defp do_at([arg], meta, name, function?, env) do
-    read? = :lists.keymember(:context, 1, meta)
+    line =
+      case :lists.keymember(:context, 1, meta) do
+        true -> nil
+        false -> env.line
+      end
 
     cond do
       function? ->
@@ -2413,11 +2417,11 @@ defmodule Kernel do
         {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?))
+                                       unquote(stack), unquote(line))
 
       true ->
         quote do: Module.put_attribute(__MODULE__, unquote(name), unquote(arg),
-                                       nil, unquote(read?))
+                                       nil, unquote(line))
     end
   end
 

lib/elixir/lib/module.ex

@@ -854,7 +854,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, false)
+    put_attribute(module, key, value, nil, nil)
   end
 
   @doc """
@@ -910,7 +910,7 @@ defmodule Module do
     table = data_table_for(module)
     case :ets.take(table, key) do
       [{_, value, _accumulated? = true, _}] ->
-        :ets.insert(table, {key, [], true, true})
+        :ets.insert(table, {key, [], true, nil})
         value
       [{_, value, _, _}] ->
         value
@@ -960,7 +960,7 @@ defmodule Module do
     end
 
     if Keyword.get(opts, :accumulate) do
-      :ets.insert_new(table, {new, [], _accumulated? = true, _read? = true}) ||
+      :ets.insert_new(table, {new, [], _accumulated? = true, _unread_line = nil}) ||
         :ets.update_element(table, new, {3, true})
     end
 
@@ -1031,7 +1031,7 @@ defmodule Module do
         [] -> [value]
       end
 
-    :ets.insert(table, {key, new, true, true})
+    :ets.insert(table, {key, new, true, nil})
   end
 
   @doc false
@@ -1043,7 +1043,7 @@ defmodule Module do
 
     case :ets.lookup(table, key) do
       [{^key, val, _, _}] ->
-        :ets.update_element(table, key, {4, true})
+        :ets.update_element(table, key, {4, nil})
         val
       [] when is_list(stack) ->
         # TODO: Consider raising instead of warning on v2.0 as it usually cascades
@@ -1058,22 +1058,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, read?) when is_atom(key) do
+  def put_attribute(module, key, value, stack, unread_line) 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>>}, accumulated?, _read?}]
+      [{^key, {line, <<_::binary>>}, accumulated?, _unread_line}]
           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, accumulated?, read?})
+        :ets.insert(table, {key, value, accumulated?, unread_line})
 
       [{^key, current, _accumulated? = true, _read?}] ->
-        :ets.insert(table, {key, [value | current], true, read?})
+        :ets.insert(table, {key, [value | current], true, unread_line})
 
       _ ->
-        :ets.insert(table, {key, value, false, read?})
+        :ets.insert(table, {key, value, false, unread_line})
     end
 
     value

lib/elixir/src/elixir_module.erl

@@ -85,7 +85,7 @@ do_compile(Line, Module, Block, Vars, E) ->
     Forms3 = attributes_form(Line, File, Data, PersistedAttrs, Forms2),
 
     elixir_locals:ensure_no_import_conflict(Line, File, Module, All),
-    warn_unused_attributes(Line, File, Data, PersistedAttrs),
+    warn_unused_attributes(File, Data, PersistedAttrs),
 
     Location = {elixir_utils:characters_to_list(elixir_utils:relative_to_cwd(File)), Line},
 
@@ -152,26 +152,26 @@ build(Line, File, Module, Docs, Lexical) ->
     end,
 
   ets:insert(Data, [
-    % {Key, Value, Accumulate?, Read?}
-    {after_compile, [], true, true},
-    {before_compile, [], true, true},
-    {behaviour, [], true, true},
-    {compile, [], true, true},
-    {derive, [], true, true},
-    {dialyzer, [], true, true},
-    {external_resource, [], true, true},
-    {moduledoc, nil, false, true},
-    {on_definition, OnDefinition, true, true},
-    {on_load, [], true, true},
+    % {Key, Value, Accumulate?, UnreadLine}
+    {after_compile, [], true, nil},
+    {before_compile, [], true, nil},
+    {behaviour, [], true, nil},
+    {compile, [], true, nil},
+    {derive, [], true, nil},
+    {dialyzer, [], true, nil},
+    {external_resource, [], true, nil},
+    {moduledoc, nil, false, nil},
+    {on_definition, OnDefinition, true, nil},
+    {on_load, [], true, nil},
 
     % Types
-    {callback, [], true, true},
-    {opaque, [], true, true},
-    {optional_callbacks, [], true, true},
-    {macrocallback, [], true, true},
-    {spec, [], true, true},
-    {type, [], true, true},
-    {typep, [], true, true}
+    {callback, [], true, nil},
+    {opaque, [], true, nil},
+    {optional_callbacks, [], true, nil},
+    {macrocallback, [], true, nil},
+    {spec, [], true, nil},
+    {type, [], true, nil},
+    {typep, [], true, nil}
   ]),
 
   Persisted = [behaviour, on_load, compile, external_resource, dialyzer, vsn],
@@ -425,11 +425,11 @@ check_module_availability(Line, File, Module) ->
       ok
   end.
 
-warn_unused_attributes(Line, File, Data, PersistedAttrs) ->
+warn_unused_attributes(File, Data, PersistedAttrs) ->
   ReservedAttrs = [after_compile, before_compile, moduledoc, on_definition | PersistedAttrs],
-  Keys = ets:select(Data, [{{'$1', '_', '_', false}, [{is_atom, '$1'}], ['$1']}]),
+  Keys = ets:select(Data, [{{'$1', '_', '_', '$2'}, [{is_atom, '$1'}, {is_integer, '$2'}], [['$1', '$2']]}]),
   [elixir_errors:form_warn([{line, Line}], File, ?MODULE, {unused_attribute, Key}) ||
-   Key <- Keys, not lists:member(Key, ReservedAttrs)].
+   [Key, Line] <- Keys, not lists:member(Key, ReservedAttrs)].
 
 % __INFO__
 

lib/elixir/test/elixir/kernel/warning_test.exs

@@ -685,12 +685,11 @@ defmodule Kernel.WarningTest do
       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"
+    assert content =~ "nofile:2"
   after
     purge Sample
   end