From e1ded07795631219b7428a0bd3ca2f36286727ad Mon Sep 17 00:00:00 2001
From: WATANABE Yuki <magicant@wonderwand.net>
Date: Sun, 5 Nov 2023 14:09:20 +0900
Subject: [PATCH] typeset: Include read-only variable name in error message

The previous commit modified the error message for assigning to a
read-only variable to include the variable name in the message for the
standard assignment error. This commit does the same for the error
message for the `typeset` builtin.
---
 yash-builtin/src/typeset.rs               | 45 ++++++++++++++++++++---
 yash-builtin/src/typeset/set_variables.rs | 14 ++++---
 2 files changed, 49 insertions(+), 10 deletions(-)

diff --git a/yash-builtin/src/typeset.rs b/yash-builtin/src/typeset.rs
index 717a64f9..fb0934fb 100644
--- a/yash-builtin/src/typeset.rs
+++ b/yash-builtin/src/typeset.rs
@@ -258,7 +258,7 @@ use crate::common::{output, report_error, report_failure};
 use thiserror::Error;
 use yash_env::option::State;
 use yash_env::semantics::Field;
-use yash_env::variable::{AssignError, Variable};
+use yash_env::variable::{Value, Variable};
 use yash_env::Env;
 use yash_syntax::source::pretty::{Annotation, AnnotationType, Message, MessageBase};
 use yash_syntax::source::Location;
@@ -420,6 +420,43 @@ impl Command {
     }
 }
 
+/// Error returned on assigning to a read-only variable
+#[derive(Clone, Debug, Eq, Error, PartialEq)]
+#[error("cannot assign to read-only variable {name:?}")]
+pub struct AssignReadOnlyError {
+    /// Name of the read-only variable
+    pub name: String,
+    /// Value that was being assigned
+    pub new_value: Value,
+    /// Location where the variable was tried to be assigned
+    pub assigned_location: Location,
+    /// Location where the variable was made read-only
+    pub read_only_location: Location,
+}
+
+impl From<AssignReadOnlyError> for yash_env::variable::AssignError {
+    fn from(e: AssignReadOnlyError) -> Self {
+        Self {
+            new_value: e.new_value,
+            assigned_location: Some(e.assigned_location),
+            read_only_location: e.read_only_location,
+        }
+    }
+}
+
+/// This conversion is available only when the optional `yash-semantics`
+/// dependency is enabled.
+#[cfg(feature = "yash-semantics")]
+impl From<AssignReadOnlyError> for yash_semantics::expansion::AssignReadOnlyError {
+    fn from(e: AssignReadOnlyError) -> Self {
+        Self {
+            name: e.name,
+            new_value: e.new_value,
+            read_only_location: e.read_only_location,
+        }
+    }
+}
+
 /// Error that occurs when trying to cancel the read-only attribute of a
 /// variable or function
 #[derive(Clone, Debug, Error, Eq, PartialEq)]
@@ -435,7 +472,7 @@ pub struct UndoReadOnlyError {
 #[derive(Clone, Debug, Error, Eq, PartialEq)]
 pub enum ExecuteError {
     /// Assigning to a read-only variable
-    AssignReadOnlyVariable(#[from] AssignError),
+    AssignReadOnlyVariable(#[from] AssignReadOnlyError),
     /// Cancelling the read-only attribute of a variable
     UndoReadOnlyVariable(UndoReadOnlyError),
     /// Cancelling the read-only attribute of a function
@@ -460,9 +497,7 @@ impl MessageBase for ExecuteError {
 
     fn main_annotation(&self) -> Annotation<'_> {
         let (message, location) = match self {
-            Self::AssignReadOnlyVariable(error) => {
-                (error.to_string(), error.assigned_location.as_ref().unwrap())
-            }
+            Self::AssignReadOnlyVariable(error) => (error.to_string(), &error.assigned_location),
             Self::UndoReadOnlyVariable(error) => (
                 format!("read-only variable `{}`", error.name),
                 &error.name.origin,
diff --git a/yash-builtin/src/typeset/set_variables.rs b/yash-builtin/src/typeset/set_variables.rs
index caa1efc7..6fe5d5b3 100644
--- a/yash-builtin/src/typeset/set_variables.rs
+++ b/yash-builtin/src/typeset/set_variables.rs
@@ -37,17 +37,21 @@ impl SetVariables {
             if let Some((name, value)) = field.value.split_once('=') {
                 value_to_assign = Some(Value::scalar(value));
 
-                // Make the name out of the field value.
+                // Modify the field value so that it contains only the name.
                 field.value.truncate(name.len());
             }
-            let name = field.value.clone();
 
-            let mut variable = env.get_or_create_variable(name, self.scope.into());
+            let mut variable = env.get_or_create_variable(&field.value, self.scope.into());
 
             // Assign the value to the variable.
             if let Some(value) = value_to_assign {
                 if let Err(error) = variable.assign(value, field.origin.clone()) {
-                    errors.push(error.into());
+                    errors.push(ExecuteError::AssignReadOnlyVariable(AssignReadOnlyError {
+                        name: field.value,
+                        new_value: error.new_value,
+                        assigned_location: error.assigned_location.unwrap(),
+                        read_only_location: error.read_only_location,
+                    }));
                     continue;
                 }
             }
@@ -313,7 +317,7 @@ mod tests {
         let errors = sv.execute(&mut env).unwrap_err();
         assert_matches!(&errors[..], [ExecuteError::AssignReadOnlyVariable(error)] => {
             assert_eq!(error.new_value, Value::scalar("foo"));
-            assert_eq!(error.assigned_location.as_ref(), Some(&assigned_location));
+            assert_eq!(error.assigned_location, assigned_location);
             assert_eq!(error.read_only_location, ro_location);
         });
         assert_eq!(env.variables.get("ro"), Some(&ro));