From 25613cfaf3a20f737ac347f7d7144a5dcaa709a5 Mon Sep 17 00:00:00 2001
From: Laurenz <laurmaedje@gmail.com>
Date: Sat, 16 Sep 2023 20:38:27 +0200
Subject: [PATCH] Fix missing capturing of assignments

The previous commit was a bit overambitious. The left-hand side of assignments should actually be fully captured: Argument lists in `at` calls can contain captured variables. And if the assigned variable itself is captured, then the function is faulty anyway. (And we ensure the correct error message by capturing it.)

Fixes #2169
---
 crates/typst-syntax/src/ast.rs | 12 ------------
 crates/typst/src/eval/func.rs  | 11 +----------
 tests/typ/compiler/closure.typ | 18 ++++++++++++++++++
 3 files changed, 19 insertions(+), 22 deletions(-)

diff --git a/crates/typst-syntax/src/ast.rs b/crates/typst-syntax/src/ast.rs
index 0df3d6d37..cb1d05b20 100644
--- a/crates/typst-syntax/src/ast.rs
+++ b/crates/typst-syntax/src/ast.rs
@@ -1440,18 +1440,6 @@ impl BinOp {
         })
     }
 
-    /// Whether this is an assignment operator.
-    pub fn is_assignment(self) -> bool {
-        matches!(
-            self,
-            Self::Assign
-                | Self::AddAssign
-                | Self::SubAssign
-                | Self::MulAssign
-                | Self::DivAssign
-        )
-    }
-
     /// The precedence of this operator.
     pub fn precedence(self) -> usize {
         match self {
diff --git a/crates/typst/src/eval/func.rs b/crates/typst/src/eval/func.rs
index b7951cefa..3e4ea3704 100644
--- a/crates/typst/src/eval/func.rs
+++ b/crates/typst/src/eval/func.rs
@@ -688,16 +688,6 @@ impl<'a> CapturesVisitor<'a> {
                 }
             }
 
-            // Don't capture left-hand side of an assignment.
-            Some(ast::Expr::Binary(binary)) if binary.op().is_assignment() => {
-                self.visit(binary.rhs().to_untyped());
-            }
-
-            // Don't capture the left-hand side of a destructuring assignment.
-            Some(ast::Expr::DestructAssign(assignment)) => {
-                self.visit(assignment.value().to_untyped());
-            }
-
             // A for loop contains one or two bindings in its pattern. These are
             // active after the iterable is evaluated but before the body is
             // evaluated.
@@ -828,5 +818,6 @@ mod tests {
         test("#(body = 1)", &[]);
         test("#(body += y)", &["y"]);
         test("#{ (body, a) = (y, 1) }", &["y"]);
+        test("#(x.at(y) = 5)", &["x", "y"])
     }
 }
diff --git a/tests/typ/compiler/closure.typ b/tests/typ/compiler/closure.typ
index 92d01446a..85e9dbe25 100644
--- a/tests/typ/compiler/closure.typ
+++ b/tests/typ/compiler/closure.typ
@@ -131,6 +131,24 @@
   f(1, "two", () => x)
 }
 
+---
+// Mutable method with capture in argument.
+#let x = "b"
+#let f() = {
+  let a = (b: 5)
+  a.at(x) = 10
+  a
+}
+#f()
+
+---
+#let x = ()
+#let f() = {
+  // Error: 3-4 variables from outside the function are read-only and cannot be modified
+  x.at(1) = 2
+}
+#f()
+
 ---
 // Named arguments.
 #{