From 029e46ebbffeb73a9d28fe7575c6d6409bbdadc1 Mon Sep 17 00:00:00 2001
From: Alexander Plavin <alexander@plav.in>
Date: Tue, 4 Apr 2023 20:42:13 +0300
Subject: [PATCH] @reset immediately throws if the target isn't a symbol

---
 src/sugar.jl      | 13 +++++++++++--
 test/test_core.jl | 18 +++++++++++-------
 2 files changed, 22 insertions(+), 9 deletions(-)

diff --git a/src/sugar.jl b/src/sugar.jl
index 30a01269..0f60e906 100644
--- a/src/sugar.jl
+++ b/src/sugar.jl
@@ -303,7 +303,7 @@ function setmacro(optictransform, ex::Expr; overwrite::Bool=false)
         f = :($_UpdateOp($op,$val))
         :($modify($f, $obj, ($optictransform)($optic)))
     end
-    return overwrite ? :($obj = $ret) : ret
+    return _macro_expression_result(obj, ret; overwrite=overwrite)
 end
 
 """
@@ -336,9 +336,18 @@ function insertmacro(optictransform, ex::Expr; overwrite::Bool=false)
     obj, optic = parse_obj_optic(ref)
     val = esc(val)
     ret = :($insert($obj, ($optictransform)($optic), $val))
-    return overwrite ? :($obj = $ret) : ret
+    return _macro_expression_result(obj, ret; overwrite=overwrite)
 end
 
+_macro_expression_result(obj, ret; overwrite) =
+    if overwrite
+        @assert Meta.isexpr(obj, :escape)
+        only(obj.args) isa Symbol || throw(ArgumentError("Rebinding macros can only be used with plain variables as targets. Got expression: $obj"))
+        return :($obj = $ret)
+    else
+        return ret
+    end
+
 """
     @optic
 
diff --git a/test/test_core.jl b/test/test_core.jl
index c7c51592..5341dd4a 100644
--- a/test/test_core.jl
+++ b/test/test_core.jl
@@ -45,6 +45,8 @@ end
     nt = (a=1,)
     @reset nt.a = 5
     @test nt === (a=5,)
+
+    @test_throws Exception eval(:(@reset func(x, y) = 100))
 end
 
 @testset "@set" begin
@@ -142,14 +144,16 @@ end
     x_orig = x
     @test (@set $(x)[2] = 100) == [1, 100, 3]
     @test (@set $(x[2]) = 100) == 100
-    @test (@set $(x)[2] + 2 = 100) == [1, 98, 3]  # impossible without $
-    @test (@set $(x[2]) + 2 = 100) == 98  # impossible without $
-    @test x_orig === x == [1, 2, 3]
+    # these are impossible with @set without $:
+    @test (@set $(x)[2] + 2 = 100) == [1, 98, 3]
+    @test (@set $(x[2]) + 2 = 100) == 98
+    @test (@set first($x, 2) = [10, 20]) == [10, 20, 3]
 
-    @test (@reset $(x[2]) = 100) == 100
-    @test x_orig === x == [1, 100, 3]
-    @test (@reset $(x)[2] = 200) == [1, 200, 3]
-    @test x_orig !== x == [1, 200, 3]
+    @test_throws Exception eval(:(@reset $(x[2]) = 100))
+    @test_throws Exception eval(:(@reset $(x)[2] = 200))
+
+    # ensure the object itself didn't change:
+    @test x_orig === x == [1, 2, 3]
 end