From c46c1c3f3f8e92a85d3282c55b0e271da9fa5df4 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 c3f93b9a..7a278ae5 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 8e73bb83..1cbfe382 100644
--- a/test/test_core.jl
+++ b/test/test_core.jl
@@ -48,6 +48,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
@@ -145,14 +147,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