From 67034cb8b42a570e2a33aa88e6d5daf9cd309c11 Mon Sep 17 00:00:00 2001
From: Casey Rodarmor <casey@rodarmor.com>
Date: Sat, 2 Nov 2024 11:11:24 -0700
Subject: [PATCH] Don't export constants (#2449)

---
 src/binding.rs     |  2 ++
 src/command_ext.rs |  2 +-
 src/evaluator.rs   | 37 +++++++++++++++++++++++--------------
 src/parser.rs      |  5 +++--
 src/scope.rs       | 24 ++++++++++--------------
 tests/constants.rs | 16 ++++++++++++++++
 6 files changed, 55 insertions(+), 31 deletions(-)

diff --git a/src/binding.rs b/src/binding.rs
index 7e7890c7..69dcc3b0 100644
--- a/src/binding.rs
+++ b/src/binding.rs
@@ -3,6 +3,8 @@ use super::*;
 /// A binding of `name` to `value`
 #[derive(Debug, Clone, PartialEq, Serialize)]
 pub(crate) struct Binding<'src, V = String> {
+  #[serde(skip)]
+  pub(crate) constant: bool,
   pub(crate) export: bool,
   #[serde(skip)]
   pub(crate) file_depth: u32,
diff --git a/src/command_ext.rs b/src/command_ext.rs
index 6bd7208d..40d9da16 100644
--- a/src/command_ext.rs
+++ b/src/command_ext.rs
@@ -39,7 +39,7 @@ impl CommandExt for Command {
     }
 
     for binding in scope.bindings() {
-      if settings.export || binding.export {
+      if binding.export || (settings.export && !binding.constant) {
         self.env(binding.name.lexeme(), &binding.value);
       }
     }
diff --git a/src/evaluator.rs b/src/evaluator.rs
index d48cece8..b83c8cf7 100644
--- a/src/evaluator.rs
+++ b/src/evaluator.rs
@@ -32,12 +32,14 @@ impl<'src, 'run> Evaluator<'src, 'run> {
 
     for (name, value) in overrides {
       if let Some(assignment) = module.assignments.get(name) {
-        scope.bind(
-          assignment.export,
-          assignment.name,
-          assignment.private,
-          value.clone(),
-        );
+        scope.bind(Binding {
+          constant: false,
+          export: assignment.export,
+          file_depth: 0,
+          name: assignment.name,
+          private: assignment.private,
+          value: value.clone(),
+        });
       } else {
         unknown_overrides.push(name.clone());
       }
@@ -68,12 +70,14 @@ impl<'src, 'run> Evaluator<'src, 'run> {
 
     if !self.scope.bound(name) {
       let value = self.evaluate_expression(&assignment.value)?;
-      self.scope.bind(
-        assignment.export,
-        assignment.name,
-        assignment.private,
+      self.scope.bind(Binding {
+        constant: false,
+        export: assignment.export,
+        file_depth: 0,
+        name: assignment.name,
+        private: assignment.private,
         value,
-      );
+      });
     }
 
     Ok(self.scope.value(name).unwrap())
@@ -340,9 +344,14 @@ impl<'src, 'run> Evaluator<'src, 'run> {
         rest = &rest[1..];
         value
       };
-      evaluator
-        .scope
-        .bind(parameter.export, parameter.name, false, value);
+      evaluator.scope.bind(Binding {
+        constant: false,
+        export: parameter.export,
+        file_depth: 0,
+        name: parameter.name,
+        private: false,
+        value,
+      });
     }
 
     Ok((evaluator.scope, positional))
diff --git a/src/parser.rs b/src/parser.rs
index e03ab783..00246bf3 100644
--- a/src/parser.rs
+++ b/src/parser.rs
@@ -500,11 +500,12 @@ impl<'run, 'src> Parser<'run, 'src> {
     }
 
     Ok(Assignment {
-      file_depth: self.file_depth,
+      constant: false,
       export,
+      file_depth: self.file_depth,
       name,
-      value,
       private: private || name.lexeme().starts_with('_'),
+      value,
     })
   }
 
diff --git a/src/scope.rs b/src/scope.rs
index 36bbedc8..78d12ca0 100644
--- a/src/scope.rs
+++ b/src/scope.rs
@@ -21,9 +21,11 @@ impl<'src, 'run> Scope<'src, 'run> {
     };
 
     for (key, value) in constants() {
-      root.bind(
-        false,
-        Name {
+      root.bind(Binding {
+        constant: true,
+        export: false,
+        file_depth: 0,
+        name: Name {
           token: Token {
             column: 0,
             kind: TokenKind::Identifier,
@@ -34,22 +36,16 @@ impl<'src, 'run> Scope<'src, 'run> {
             src: key,
           },
         },
-        false,
-        (*value).into(),
-      );
+        private: false,
+        value: (*value).into(),
+      });
     }
 
     root
   }
 
-  pub(crate) fn bind(&mut self, export: bool, name: Name<'src>, private: bool, value: String) {
-    self.bindings.insert(Binding {
-      export,
-      file_depth: 0,
-      name,
-      private,
-      value,
-    });
+  pub(crate) fn bind(&mut self, binding: Binding<'src>) {
+    self.bindings.insert(binding);
   }
 
   pub(crate) fn bound(&self, name: &str) -> bool {
diff --git a/tests/constants.rs b/tests/constants.rs
index 59f8b9bd..c6a3a853 100644
--- a/tests/constants.rs
+++ b/tests/constants.rs
@@ -43,3 +43,19 @@ fn constants_can_be_redefined() {
     .stdout("foo")
     .run();
 }
+
+#[test]
+fn constants_are_not_exported() {
+  Test::new()
+    .justfile(
+      "
+        set export
+
+        foo:
+          echo $HEXUPPER
+      ",
+    )
+    .stderr_regex(".*HEXUPPER: unbound variable.*")
+    .status(127)
+    .run();
+}