Improve error message on do-block in module attr, closes #10544

Author: josevalim

lib/elixir/lib/kernel.ex

@@ -3123,7 +3123,15 @@ defmodule Kernel do
 
       not function? and __CALLER__.context == :match ->
         raise ArgumentError,
-              "invalid write attribute syntax, you probably meant to use: @#{name} expression"
+              """
+              invalid write attribute syntax. If you want to define an attribute, don't do this:
+
+                  @foo = :value
+
+              Instead, do this:
+
+                  @foo :value
+              """
 
       # Typespecs attributes are currently special cased by the compiler
       is_list(args) and typespec?(name) ->
@@ -3219,7 +3227,21 @@ defmodule Kernel do
     end
   end
 
-  # All other cases
+  # Error cases
+  defp do_at([{call, meta, ctx_or_args}, [{:do, _} | _] = kw], _meta, name, _function?, _env) do
+    args = if is_atom(ctx_or_args), do: [], else: ctx_or_args
+    code = "\n@#{name} (#{Macro.to_string({call, meta, args ++ [kw]})})"
+
+    raise ArgumentError, """
+    expected 0 or 1 argument for @#{name}, got 2.
+
+    It seems you are trying to use the do-syntax with @module attributes \
+    but the do-block is binding to the attribute name. You probably want \
+    to wrap the argument value in parentheses, like this:
+    #{String.replace(code, "\n", "\n    ")}
+    """
+  end
+
   defp do_at(args, _meta, name, _function?, _env) do
     raise ArgumentError, "expected 0 or 1 argument for @#{name}, got: #{length(args)}"
   end

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

@@ -692,16 +692,6 @@ defmodule Kernel.ErrorsTest do
     end
   end
 
-  test "match attribute in module" do
-    msg = "invalid write attribute syntax, you probably meant to use: @foo expression"
-
-    assert_raise ArgumentError, msg, fn ->
-      defmodule MatchAttributeInModule do
-        @foo = 42
-      end
-    end
-  end
-
   test "invalid case clauses" do
     assert_eval_raise CompileError,
                       "nofile:1: expected one argument for :do clauses (->) in \"case\"",

lib/elixir/test/elixir/kernel_test.exs

@@ -592,16 +592,6 @@ defmodule KernelTest do
       _ = a
     end
 
-    test "setting attribute with uppercase" do
-      message = "module attributes set via @ cannot start with an uppercase letter"
-
-      assert_raise ArgumentError, message, fn ->
-        defmodule UpcaseAttrSample do
-          @Upper
-        end
-      end
-    end
-
     test "in module body" do
       defmodule InSample do
         @foo [:a, :b]
@@ -782,6 +772,40 @@ defmodule KernelTest do
     end
   end
 
+  describe "@" do
+    test "setting attribute with do-block" do
+      exception =
+        catch_error(
+          defmodule UpcaseAttrSample do
+            @foo quote do
+              :ok
+            end
+          end
+        )
+
+      assert exception.message =~ "expected 0 or 1 argument for @foo, got 2"
+      assert exception.message =~ "You probably want to wrap the argument value in parentheses"
+    end
+
+    test "setting attribute with uppercase" do
+      message = "module attributes set via @ cannot start with an uppercase letter"
+
+      assert_raise ArgumentError, message, fn ->
+        defmodule UpcaseAttrSample do
+          @Upper
+        end
+      end
+    end
+
+    test "matching attribute" do
+      assert_raise ArgumentError, ~r"invalid write attribute syntax", fn ->
+        defmodule MatchAttributeInModule do
+          @foo = 42
+        end
+      end
+    end
+  end
+
   describe "defdelegate" do
     defdelegate my_flatten(list), to: List, as: :flatten