fix(variables): keep dynamic well-known vars live across declare -a / array assign

brush-builtins/src/declare.rs

@@ -281,17 +281,41 @@ impl DeclareCommand {
             EnvironmentLookup::Anywhere
         };
 
+        // If the variable currently holds a dynamic value, resolve its current
+        // reading now, before taking a mutable borrow of the shell below. This
+        // lets a scalar dynamic (e.g. RANDOM) that ends up getting materialized
+        // by a shape conversion freeze at its actual current value instead of
+        // an empty string.
+        let resolved_dynamic_value = context
+            .shell
+            .env()
+            .get_using_policy(name.as_str(), lookup)
+            .and_then(|var| {
+                matches!(var.value(), brush_core::ShellValue::Dynamic { .. })
+                    .then(|| var.resolve_value(context.shell))
+            });
+
         // Look up the variable.
         if let Some(var) = context
             .shell
             .env_mut()
             .get_mut_using_policy(name.as_str(), lookup)
         {
             if self.make_associative_array.is_some() {
-                var.convert_to_associative_array()?;
+                if initial_value.is_some() {
+                    var.convert_to_associative_array_for_reassignment(
+                        resolved_dynamic_value.as_ref(),
+                    )?;
+                } else {
+                    var.convert_to_associative_array(resolved_dynamic_value.as_ref())?;
+                }
             }
             if self.make_indexed_array.is_some() {
-                var.convert_to_indexed_array()?;
+                if initial_value.is_some() {
+                    var.convert_to_indexed_array_for_reassignment(resolved_dynamic_value.as_ref())?;
+                } else {
+                    var.convert_to_indexed_array(resolved_dynamic_value.as_ref())?;
+                }
             }
 
             self.apply_attributes_before_update(var)?;

brush-core/src/expansion.rs

@@ -1819,6 +1819,10 @@ impl<'a, SE: extensions::ShellExtensions> WordExpander<'a, SE> {
                         var.value(),
                         ShellValue::AssociativeArray(_)
                             | ShellValue::Unset(ShellValueUnsetType::AssociativeArray)
+                            | ShellValue::Dynamic {
+                                kind: variables::DynamicValueKind::AssociativeArray,
+                                ..
+                            }
                     )
                 } else {
                     false

brush-core/src/variables.rs

@@ -188,36 +188,123 @@ impl ShellVariable {
     }
 
     /// Converts the variable to an indexed array.
-    pub fn convert_to_indexed_array(&mut self) -> Result<(), error::Error> {
+    ///
+    /// # Arguments
+    ///
+    /// * `resolved_dynamic_value` - If this variable currently holds a dynamic
+    ///   value, the caller may pass in the value it currently resolves to
+    ///   (e.g. via [`Self::resolve_value`]) so that a scalar dynamic (such as
+    ///   `RANDOM`) that ends up getting materialized freezes at its actual
+    ///   current reading rather than an empty string. Pass `None` if no such
+    ///   snapshot is available, or if the variable is known not to be dynamic.
+    pub fn convert_to_indexed_array(
+        &mut self,
+        resolved_dynamic_value: Option<&ShellValue>,
+    ) -> Result<(), error::Error> {
+        self.convert_to_indexed_array_impl(false, resolved_dynamic_value)
+    }
+
+    /// Like [`Self::convert_to_indexed_array`], but for use when a `declare -a
+    /// name=value` is about to immediately overwrite this variable's value.
+    ///
+    /// Bash lets that accompanying assignment fully replace (and permanently
+    /// freeze) a shape-mismatched dynamic special variable such as
+    /// `BASH_ALIASES`, rather than rejecting the conversion the way a bare
+    /// `declare -a name` (with no value) would. A real (non-dynamic) associative
+    /// array is still rejected either way, matching bash.
+    pub fn convert_to_indexed_array_for_reassignment(
+        &mut self,
+        resolved_dynamic_value: Option<&ShellValue>,
+    ) -> Result<(), error::Error> {
+        self.convert_to_indexed_array_impl(true, resolved_dynamic_value)
+    }
+
+    fn convert_to_indexed_array_impl(
+        &mut self,
+        allow_dynamic_mismatch: bool,
+        resolved_dynamic_value: Option<&ShellValue>,
+    ) -> Result<(), error::Error> {
         match self.value() {
             ShellValue::IndexedArray(_) => Ok(()),
             ShellValue::AssociativeArray(_) => {
                 Err(error::ErrorKind::ConvertingAssociativeArrayToIndexedArray.into())
             }
+            // Dynamic variables that are already indexed-array-shaped (e.g. PIPESTATUS)
+            // are backed by getter/setter closures. Bash keeps these live across
+            // `declare -a` rather than freezing a snapshot, so accept the declaration
+            // syntactically but leave the binding untouched.
+            ShellValue::Dynamic {
+                kind: DynamicValueKind::IndexedArray,
+                ..
+            } => Ok(()),
+            ShellValue::Dynamic {
+                kind: DynamicValueKind::AssociativeArray,
+                ..
+            } if !allow_dynamic_mismatch => {
+                Err(error::ErrorKind::ConvertingAssociativeArrayToIndexedArray.into())
+            }
+            // Scalars, including scalar-shaped dynamics like RANDOM/SECONDS, get
+            // materialized into a real indexed array and lose their dynamic binding,
+            // matching bash's `declare -a RANDOM` freezing behavior. Shape-mismatched
+            // associative dynamics fall here too when `allow_dynamic_mismatch` is set.
             _ => {
                 let mut new_values = BTreeMap::new();
-                new_values.insert(
-                    0,
-                    self.value.to_cow_str_without_dynamic_support().to_string(),
-                );
+                let source = resolved_dynamic_value.unwrap_or(&self.value);
+                new_values.insert(0, source.to_cow_str_without_dynamic_support().to_string());
                 self.value = ShellValue::IndexedArray(new_values);
                 Ok(())
             }
         }
     }
 
     /// Converts the variable to an associative array.
-    pub fn convert_to_associative_array(&mut self) -> Result<(), error::Error> {
+    ///
+    /// See [`Self::convert_to_indexed_array`] for the meaning of
+    /// `resolved_dynamic_value`.
+    pub fn convert_to_associative_array(
+        &mut self,
+        resolved_dynamic_value: Option<&ShellValue>,
+    ) -> Result<(), error::Error> {
+        self.convert_to_associative_array_impl(false, resolved_dynamic_value)
+    }
+
+    /// Like [`Self::convert_to_associative_array`], but for use when a `declare
+    /// -A name=value` is about to immediately overwrite this variable's value.
+    /// See [`Self::convert_to_indexed_array_for_reassignment`] for why this
+    /// exists.
+    pub fn convert_to_associative_array_for_reassignment(
+        &mut self,
+        resolved_dynamic_value: Option<&ShellValue>,
+    ) -> Result<(), error::Error> {
+        self.convert_to_associative_array_impl(true, resolved_dynamic_value)
+    }
+
+    fn convert_to_associative_array_impl(
+        &mut self,
+        allow_dynamic_mismatch: bool,
+        resolved_dynamic_value: Option<&ShellValue>,
+    ) -> Result<(), error::Error> {
         match self.value() {
             ShellValue::AssociativeArray(_) => Ok(()),
+            ShellValue::Dynamic {
+                kind: DynamicValueKind::AssociativeArray,
+                ..
+            } => Ok(()),
             ShellValue::IndexedArray(_) => {
                 Err(error::ErrorKind::ConvertingIndexedArrayToAssociativeArray.into())
             }
+            ShellValue::Dynamic {
+                kind: DynamicValueKind::IndexedArray,
+                ..
+            } if !allow_dynamic_mismatch => {
+                Err(error::ErrorKind::ConvertingIndexedArrayToAssociativeArray.into())
+            }
             _ => {
                 let mut new_values: BTreeMap<String, String> = BTreeMap::new();
+                let source = resolved_dynamic_value.unwrap_or(&self.value);
                 new_values.insert(
                     String::from("0"),
-                    self.value.to_cow_str_without_dynamic_support().to_string(),
+                    source.to_cow_str_without_dynamic_support().to_string(),
                 );
                 self.value = ShellValue::AssociativeArray(new_values);
                 Ok(())
@@ -261,7 +348,7 @@ impl ShellVariable {
                 // If we're trying to append an array to a string, we first promote the string to be
                 // an array with the string being present at index 0.
                 (ShellValue::String(_), ShellValueLiteral::Array(_)) => {
-                    self.convert_to_indexed_array()?;
+                    self.convert_to_indexed_array(None)?;
                 }
                 _ => (),
             }
@@ -333,8 +420,7 @@ impl ShellVariable {
                     | ShellValue::Unset(
                         ShellValueUnsetType::IndexedArray | ShellValueUnsetType::Untyped,
                     )
-                    | ShellValue::String(_)
-                    | ShellValue::Dynamic { .. },
+                    | ShellValue::String(_),
                     ShellValueLiteral::Array(literal_values),
                 ) => {
                     self.value = ShellValue::indexed_array_from_literals(literal_values);
@@ -385,7 +471,7 @@ impl ShellVariable {
                 self.assign(ShellValueLiteral::Array(ArrayLiteral(vec![])), false)?;
             }
             ShellValue::String(_) => {
-                self.convert_to_indexed_array()?;
+                self.convert_to_indexed_array(None)?;
             }
             _ => (),
         }
@@ -603,6 +689,12 @@ pub enum ShellValue {
     IndexedArray(BTreeMap<u64, String>),
     /// A value that is dynamically computed.
     Dynamic {
+        /// The shape of value that `getter` produces (scalar, indexed array, or
+        /// associative array). This lets conversion logic (e.g. `declare -a`/`-A`)
+        /// treat a dynamic variable the same way it would treat a materialized
+        /// value of that shape, without having to invoke `getter` (which needs a
+        /// shell reference that isn't always available).
+        kind: DynamicValueKind,
         /// Function that can query the value.
         /// TODO(serde): figure out how to serialize/deserialize dynamic values.
         #[cfg_attr(
@@ -620,6 +712,18 @@ pub enum ShellValue {
     },
 }
 
+/// The shape of value produced by a dynamic variable's getter.
+#[derive(Clone, Copy, Debug, PartialEq, Eq)]
+#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
+pub enum DynamicValueKind {
+    /// The dynamic value resolves to a scalar string (e.g. `RANDOM`, `SECONDS`).
+    Scalar,
+    /// The dynamic value resolves to an indexed array (e.g. `PIPESTATUS`).
+    IndexedArray,
+    /// The dynamic value resolves to an associative array (e.g. `BASH_ALIASES`).
+    AssociativeArray,
+}
+
 #[cfg(feature = "serde")]
 fn default_dynamic_value_getter() -> DynamicValueGetter {
     |_shell: &dyn ShellState| ShellValue::String(String::new())