Warn on underived derives, closes #11799

Author: josevalim

lib/elixir/lib/kernel.ex

@@ -5178,9 +5178,7 @@ defmodule Kernel do
   """
   defmacro defstruct(fields) do
     quote bind_quoted: [fields: fields, bootstrapped?: bootstrapped?(Enum)] do
-      {struct, keys, derive, body} = Kernel.Utils.defstruct(__MODULE__, fields, bootstrapped?)
-      @__struct__ struct
-      @enforce_keys keys
+      {struct, derive, body} = Kernel.Utils.defstruct(__MODULE__, fields, bootstrapped?)
 
       case derive do
         [] -> :ok

lib/elixir/lib/kernel/utils.ex

@@ -164,9 +164,9 @@ defmodule Kernel.Utils do
             _ ->
               quote do
                 {map, keys} =
-                  Enum.reduce(var!(kv), {@__struct__, @enforce_keys}, fn {key, val},
-                                                                         {map, keys} ->
-                    {Map.replace!(map, key, val), List.delete(keys, key)}
+                  Enum.reduce(var!(kv), {@__struct__, unquote(enforce_keys)}, fn
+                    {key, val}, {map, keys} ->
+                      {Map.replace!(map, key, val), List.delete(keys, key)}
                   end)
 
                 case keys do
@@ -183,8 +183,6 @@ defmodule Kernel.Utils do
 
         false ->
           quote do
-            _ = @enforce_keys
-
             :lists.foldl(
               fn {key, val}, acc -> Map.replace!(acc, key, val) end,
               @__struct__,
@@ -195,7 +193,10 @@ defmodule Kernel.Utils do
 
     case enforce_keys -- :maps.keys(struct) do
       [] ->
-        {struct, enforce_keys, Module.get_attribute(module, :derive), body}
+        derive = Module.get_attribute(module, :derive)
+        Module.put_attribute(module, :__struct__, struct)
+        Module.put_attribute(module, :__derived__, derive)
+        {struct, Module.get_attribute(module, :derive), body}
 
       error_keys ->
         raise ArgumentError,

lib/elixir/lib/module.ex

@@ -1787,7 +1787,8 @@ defmodule Module do
   defp args_count([], total, defaults), do: {total, defaults}
 
   @doc false
-  def check_behaviours_and_impls(env, _set, bag, all_definitions) do
+  def check_derive_behaviours_and_impls(env, set, bag, all_definitions) do
+    check_derive(env, set, bag)
     behaviours = bag_lookup_element(bag, {:accumulate, :behaviour}, 2)
     impls = bag_lookup_element(bag, :impls, 2)
     callbacks = check_behaviours(env, behaviours)
@@ -1805,6 +1806,30 @@ defmodule Module do
     :ok
   end
 
+  defp check_derive(env, set, bag) do
+    case bag_lookup_element(bag, {:accumulate, :derive}, 2) do
+      [] ->
+        :ok
+
+      to_derive ->
+        case :ets.lookup(set, :__derived__) do
+          [{_, derived, _}] ->
+            if to_derive != :lists.reverse(derived) do
+              message =
+                "warning: module attribute @derive was set after defstruct, all @derive calls must come before defstruct"
+
+              IO.warn(message, Macro.Env.stacktrace(env))
+            end
+
+          [] ->
+            message =
+              "warning: module attribute @derive was set but never used (it must come before defstruct)"
+
+            IO.warn(message, Macro.Env.stacktrace(env))
+        end
+    end
+  end
+
   defp check_behaviours(env, behaviours) do
     Enum.reduce(behaviours, %{}, fn behaviour, acc ->
       cond do

lib/elixir/src/elixir_module.erl

@@ -127,7 +127,7 @@ compile(Line, Module, Block, Vars, E) ->
         make_readonly(Module),
 
         (not elixir_config:is_bootstrap()) andalso
-         'Elixir.Module':check_behaviours_and_impls(E, DataSet, DataBag, AllDefinitions),
+         'Elixir.Module':check_derive_behaviours_and_impls(E, DataSet, DataBag, AllDefinitions),
 
         RawCompileOpts = bag_lookup_element(DataBag, {accumulate, compile}, 2),
         CompileOpts = validate_compile_opts(RawCompileOpts, AllDefinitions, Unreachable, File, Line),

lib/elixir/test/elixir/protocol_test.exs

@@ -332,6 +332,25 @@ defmodule ProtocolTest do
       assert message =~
                "cannot define @optional_callbacks inside protocol, all of the protocol definitions are required"
     end
+
+    test "when deriving after struct" do
+      assert capture_io(:stderr, fn ->
+               defmodule DeriveTooLate do
+                 defstruct []
+                 @derive [{Derivable, :ok}]
+               end
+             end) =~
+               "module attribute @derive was set after defstruct, all @derive calls must come before defstruct"
+    end
+
+    test "when deriving with no struct" do
+      assert capture_io(:stderr, fn ->
+               defmodule DeriveNeverUsed do
+                 @derive [{Derivable, :ok}]
+               end
+             end) =~
+               "module attribute @derive was set but never used (it must come before defstruct)"
+    end
   end
 
   describe "errors" do