diff --git a/Cargo.lock b/Cargo.lock index e0349c54b..f87ce3c3f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -395,7 +395,6 @@ dependencies = [ "cfg-if", "chrono", "clap", - "fancy-regex", "itertools 0.14.0", "nix 0.31.3", "pretty_assertions", diff --git a/brush-builtins/Cargo.toml b/brush-builtins/Cargo.toml index 4e6c61d4e..ddc2d90a9 100644 --- a/brush-builtins/Cargo.toml +++ b/brush-builtins/Cargo.toml @@ -133,7 +133,6 @@ brush-parser = { version = "^0.4.0", path = "../brush-parser" } cfg-if = "1.0.4" chrono = "0.4.44" clap = { version = "4.6.0", features = ["derive", "wrap_help"] } -fancy-regex = "0.19.0" itertools = "0.14.0" strum = "0.28.0" thiserror = "2.0.18" diff --git a/brush-builtins/src/declare.rs b/brush-builtins/src/declare.rs deleted file mode 100644 index ed47cafd1..000000000 --- a/brush-builtins/src/declare.rs +++ /dev/null @@ -1,722 +0,0 @@ -use clap::Parser; -use itertools::Itertools; -use std::{io::Write, sync::LazyLock}; - -use brush_core::{ - ErrorKind, ExecutionResult, builtins, - env::{self, EnvironmentLookup, EnvironmentScope}, - parser::ast, - variables::{ - self, ArrayLiteral, ShellValue, ShellValueLiteral, ShellValueUnsetType, ShellVariable, - ShellVariableUpdateTransform, - }, -}; - -crate::minus_or_plus_flag_arg!( - MakeIndexedArrayFlag, - 'a', - "Make the variable an indexed array." -); -crate::minus_or_plus_flag_arg!( - MakeAssociativeArrayFlag, - 'A', - "Make the variable an associative array." -); -crate::minus_or_plus_flag_arg!( - CapitalizeValueOnAssignmentFlag, - 'c', - "Enable capitalize-on-assignment for the variable." -); -crate::minus_or_plus_flag_arg!(MakeIntegerFlag, 'i', "Mark the variable as integer-typed"); -crate::minus_or_plus_flag_arg!( - LowercaseValueOnAssignmentFlag, - 'l', - "Enable lowercase-on-assignment for the variable." -); -crate::minus_or_plus_flag_arg!( - MakeNameRefFlag, - 'n', - "Mark the variable as a name reference" -); -crate::minus_or_plus_flag_arg!(MakeReadonlyFlag, 'r', "Mark the variable as read-only."); -crate::minus_or_plus_flag_arg!(MakeTracedFlag, 't', "Enable tracing for the variable."); -crate::minus_or_plus_flag_arg!( - UppercaseValueOnAssignmentFlag, - 'u', - "Enable uppercase-on-assignment for the variable." -); -crate::minus_or_plus_flag_arg!(MakeExportedFlag, 'x', "Mark the variable for export."); - -/// Display or update variables and their attributes. -#[derive(Parser)] -#[clap(override_usage = "declare [OPTIONS] [DECLARATIONS]...")] -pub(crate) struct DeclareCommand { - /// Constrain to function names or definitions. - #[arg(short = 'f')] - function_names_or_defs_only: bool, - - /// Constrain to function names only. - #[arg(short = 'F')] - function_names_only: bool, - - /// Create global variable, if applicable. - #[arg(short = 'g')] - create_global: bool, - - /// When creating a local variable that shadows another variable of the same name, - /// then initialize it with the contents and attributes of the variable being shadowed. - #[arg(short = 'I')] - locals_inherit_from_prev_scope: bool, - - /// Display each item's attributes and values. - #[arg(short = 'p')] - print: bool, - - // - // Attribute options - #[clap(flatten)] // -a - make_indexed_array: MakeIndexedArrayFlag, - #[clap(flatten)] // -A - make_associative_array: MakeAssociativeArrayFlag, - #[clap(flatten)] // -c - capitalize_value_on_assignment: CapitalizeValueOnAssignmentFlag, - #[clap(flatten)] // -i - make_integer: MakeIntegerFlag, - #[clap(flatten)] // -l - lowercase_value_on_assignment: LowercaseValueOnAssignmentFlag, - #[clap(flatten)] // -n - make_nameref: MakeNameRefFlag, - #[clap(flatten)] // -r - make_readonly: MakeReadonlyFlag, - #[clap(flatten)] // -t - make_traced: MakeTracedFlag, - #[clap(flatten)] // -u - uppercase_value_on_assignment: UppercaseValueOnAssignmentFlag, - #[clap(flatten)] // -x - make_exported: MakeExportedFlag, - - // - // Declarations - // - // N.B. These are skipped by clap, but filled in by the BuiltinDeclarationCommand trait. - #[clap(skip)] - declarations: Vec, -} - -#[derive(Clone, Copy)] -enum DeclareVerb { - Declare, - Local, - Readonly, -} - -impl builtins::DeclarationCommand for DeclareCommand { - fn set_declarations(&mut self, declarations: Vec) { - self.declarations = declarations; - } -} - -impl builtins::Command for DeclareCommand { - fn takes_plus_options() -> bool { - true - } - - type Error = brush_core::Error; - - async fn execute( - &self, - mut context: brush_core::ExecutionContext<'_, SE>, - ) -> Result { - let verb = match context.command_name.as_str() { - "local" => DeclareVerb::Local, - "readonly" => DeclareVerb::Readonly, - _ => DeclareVerb::Declare, - }; - - if matches!(verb, DeclareVerb::Local) && !context.shell.in_function() { - writeln!(context.stderr(), "can only be used in a function")?; - return Ok(ExecutionResult::general_error()); - } - - let mut result = ExecutionResult::success(); - if !self.declarations.is_empty() { - for declaration in &self.declarations { - if self.print && !matches!(verb, DeclareVerb::Readonly) { - if !self.try_display_declaration(&context, declaration, verb)? { - result = ExecutionResult::general_error(); - } - } else { - if !self.process_declaration(&mut context, declaration, verb)? { - result = ExecutionResult::general_error(); - } - } - } - } else { - // Display matching declarations from the variable environment. - if !self.function_names_only && !self.function_names_or_defs_only { - self.display_matching_env_declarations(&context, verb)?; - } - - // Do the same for functions. - if !matches!(verb, DeclareVerb::Local | DeclareVerb::Readonly) - && (!self.print || self.function_names_only || self.function_names_or_defs_only) - { - self.display_matching_functions(&context)?; - } - } - - Ok(result) - } -} - -impl DeclareCommand { - fn try_display_declaration( - &self, - context: &brush_core::ExecutionContext<'_, impl brush_core::ShellExtensions>, - declaration: &brush_core::CommandArg, - verb: DeclareVerb, - ) -> Result { - let name = match declaration { - brush_core::CommandArg::String(s) => s, - brush_core::CommandArg::Assignment(_) => { - writeln!(context.stderr(), "declare: {declaration}: not found")?; - return Ok(false); - } - }; - - let lookup = if matches!(verb, DeclareVerb::Local) { - EnvironmentLookup::OnlyInCurrentLocal - } else { - EnvironmentLookup::Anywhere - }; - - if self.function_names_only || self.function_names_or_defs_only { - if let Some(func_registration) = context.shell.funcs().get(name) { - if self.function_names_only { - if self.print { - writeln!(context.stdout(), "declare -f {name}")?; - } else { - writeln!(context.stdout(), "{name}")?; - } - } else { - writeln!(context.stdout(), "{}", func_registration.definition())?; - } - Ok(true) - } else { - // For some reason, bash does not print an error message in this case. - Ok(false) - } - } else if let Some(variable) = context.shell.env().get_using_policy(name, lookup) { - let mut cs = variable.attribute_flags(context.shell); - if cs.is_empty() { - cs.push('-'); - } - - let resolved_value = variable.resolve_value(context.shell); - let separator_str = if matches!(resolved_value, ShellValue::Unset(_)) { - "" - } else { - "=" - }; - - writeln!( - context.stdout(), - "declare -{cs} {name}{separator_str}{}", - resolved_value.format(variables::FormatStyle::DeclarePrint, context.shell)? - )?; - - Ok(true) - } else { - writeln!(context.stderr(), "declare: {name}: not found")?; - Ok(false) - } - } - - /// `declare -f` with attribute flags applies them to the named function rather than - /// displaying it (e.g. `declare -ft name`, `declare -fx name`). - fn apply_function_attributes( - &self, - context: &mut brush_core::ExecutionContext<'_, impl brush_core::ShellExtensions>, - declaration: &brush_core::CommandArg, - ) -> bool { - let func = match declaration { - brush_core::CommandArg::String(name) => context.shell.func_mut(name), - brush_core::CommandArg::Assignment(_) => None, - }; - - // As with display, bash reports failure without printing an error message here. - let Some(func) = func else { - return false; - }; - - match self.make_exported.to_bool() { - Some(true) => func.export(), - Some(false) => func.unexport(), - None => (), - } - - // TODO(declare): function tracing (-t) isn't tracked; it's accepted silently. - true - } - - fn process_declaration( - &self, - context: &mut brush_core::ExecutionContext<'_, impl brush_core::ShellExtensions>, - declaration: &brush_core::CommandArg, - verb: DeclareVerb, - ) -> Result { - let create_var_local = matches!(verb, DeclareVerb::Local) - || (matches!(verb, DeclareVerb::Declare) - && context.shell.in_function() - && !self.create_global); - - if (self.function_names_or_defs_only || self.function_names_only) - && (self.make_traced.to_bool().is_some() || self.make_exported.to_bool().is_some()) - { - return Ok(self.apply_function_attributes(context, declaration)); - } - - if self.function_names_or_defs_only || self.function_names_only { - return self.try_display_declaration(context, declaration, verb); - } - - // Extract the variable name and the initial value being assigned (if any). - let (name, assigned_index, initial_value, name_is_array, append) = - Self::declaration_to_name_and_value(declaration)?; - - // Special-case: `local -` - if name == "-" && matches!(verb, DeclareVerb::Local) { - // TODO(local): `local -` allows shadowing the current `set` options (i.e., $-), with - // subsequent updates getting discarded when the current local scope is popped. - tracing::warn!("not yet implemented: local -"); - return Ok(true); - } - - // Make sure it's a valid name. - if !env::valid_variable_name(name.as_str()) { - writeln!( - context.stderr(), - "{}: {name}: not a valid variable name", - context.command_name - )?; - return Ok(false); - } - - // Figure out where we should look. - let lookup = if create_var_local { - EnvironmentLookup::OnlyInCurrentLocal - } else { - EnvironmentLookup::Anywhere - }; - - // `local -I x[=v]` / `declare -I` (bash 5.0+): the new local inherits - // value and attributes from the nearest same-name variable in an - // enclosing scope instead of starting unset; `+=` appends to the - // inherited value. With no same-name variable anywhere, fall through - // to ordinary creation. - if self.locals_inherit_from_prev_scope && create_var_local { - let inherited = context - .shell - .env() - .get_using_policy(name.as_str(), EnvironmentLookup::Anywhere) - .cloned(); - - if let Some(mut var) = inherited { - self.apply_attributes_before_update(&mut var)?; - - if let Some(initial_value) = initial_value { - var.assign(initial_value, append || assigned_index.is_some())?; - } - - if context.shell.options().export_variables_on_modification - && !var.value().is_array() - { - var.export(); - } - - self.apply_attributes_after_update(&mut var, verb)?; - - context - .shell - .env_mut() - .add(name, var, EnvironmentScope::Local)?; - return Ok(true); - } - } - - // 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 self.make_indexed_array.is_some() { - var.convert_to_indexed_array()?; - } - - self.apply_attributes_before_update(var)?; - - if let Some(initial_value) = initial_value { - // We append for `name+=value`, or if the declaration included - // an explicit index. - var.assign(initial_value, append || assigned_index.is_some())?; - } - - self.apply_attributes_after_update(var, verb)?; - } else { - let unset_type = if self.make_indexed_array.is_some() { - ShellValueUnsetType::IndexedArray - } else if self.make_associative_array.is_some() { - ShellValueUnsetType::AssociativeArray - } else if name_is_array { - ShellValueUnsetType::IndexedArray - } else { - ShellValueUnsetType::Untyped - }; - - let mut var = ShellVariable::new(ShellValue::Unset(unset_type)); - - self.apply_attributes_before_update(&mut var)?; - - if let Some(initial_value) = initial_value { - var.assign(initial_value, append)?; - } - - if context.shell.options().export_variables_on_modification && !var.value().is_array() { - var.export(); - } - - self.apply_attributes_after_update(&mut var, verb)?; - - let scope = if create_var_local { - EnvironmentScope::Local - } else { - EnvironmentScope::Global - }; - - context.shell.env_mut().add(name, var, scope)?; - } - - Ok(true) - } - - #[expect(clippy::type_complexity)] - fn declaration_to_name_and_value( - declaration: &brush_core::CommandArg, - ) -> Result< - ( - String, - Option, - Option, - bool, - bool, - ), - brush_core::Error, - > { - let name; - let assigned_index; - let initial_value; - let name_is_array; - let append; - - match declaration { - brush_core::CommandArg::String(s) => { - // We need to handle the case of someone invoking `declare array[index]`. - // In such case, we ignore the index and treat it as a declaration of - // the array. - #[allow( - clippy::unwrap_in_result, - clippy::unwrap_used, - reason = "regex is valid and should not fail" - )] - static ARRAY_AND_INDEX_RE: LazyLock = - LazyLock::new(|| fancy_regex::Regex::new(r"^(.*?)\[(.*?)\]$").unwrap()); - - if let Some(captures) = ARRAY_AND_INDEX_RE.captures(s)? { - name = captures - .get(1) - .ok_or_else(|| { - brush_core::ErrorKind::InternalError("declaration parse error".into()) - })? - .as_str() - .to_owned(); - - assigned_index = captures.get(2).map(|m| m.as_str().to_owned()); - name_is_array = true; - } else { - name = s.clone(); - assigned_index = None; - name_is_array = false; - } - initial_value = None; - append = false; - } - brush_core::CommandArg::Assignment(assignment) => { - match &assignment.name { - ast::AssignmentName::VariableName(var_name) => { - name = var_name.to_owned(); - assigned_index = None; - } - ast::AssignmentName::ArrayElementName(var_name, index) => { - if matches!(assignment.value, ast::AssignmentValue::Array(_)) { - return Err(ErrorKind::AssigningListToArrayMember.into()); - } - - name = var_name.to_owned(); - assigned_index = Some(index.to_owned()); - } - } - - append = assignment.append; - - match &assignment.value { - ast::AssignmentValue::Scalar(s) => { - if let Some(index) = &assigned_index { - initial_value = Some(ShellValueLiteral::Array(ArrayLiteral(vec![( - Some(index.to_owned()), - s.value.clone(), - )]))); - name_is_array = true; - } else { - initial_value = Some(ShellValueLiteral::Scalar(s.value.clone())); - name_is_array = false; - } - } - ast::AssignmentValue::Array(a) => { - initial_value = Some(ShellValueLiteral::Array(ArrayLiteral( - a.iter() - .map(|(i, v)| { - (i.as_ref().map(|w| w.value.clone()), v.value.clone()) - }) - .collect(), - ))); - name_is_array = true; - } - } - } - } - - Ok((name, assigned_index, initial_value, name_is_array, append)) - } - - fn display_matching_env_declarations( - &self, - context: &brush_core::ExecutionContext<'_, impl brush_core::ShellExtensions>, - verb: DeclareVerb, - ) -> Result<(), brush_core::Error> { - // - // Dump all declarations. Use attribute flags to filter which variables are dumped. - // - - // We start by excluding all variables that are not enumerable. - #[expect(clippy::type_complexity)] - let mut filters: Vec bool>> = - vec![Box::new(|(_, v)| v.is_enumerable())]; - - // Add filters depending on verb. - if matches!(verb, DeclareVerb::Readonly) { - filters.push(Box::new(|(_, v)| v.is_readonly())); - } - - // Add filters depending on attribute flags. - if let Some(value) = self.make_indexed_array.to_bool() { - filters.push(Box::new(move |(_, v)| { - matches!(v.value(), ShellValue::IndexedArray(_)) == value - })); - } - if let Some(value) = self.make_associative_array.to_bool() { - filters.push(Box::new(move |(_, v)| { - matches!(v.value(), ShellValue::AssociativeArray(_)) == value - })); - } - if let Some(value) = self.make_integer.to_bool() { - filters.push(Box::new(move |(_, v)| v.is_treated_as_integer() == value)); - } - if let Some(value) = self.capitalize_value_on_assignment.to_bool() { - filters.push(Box::new(move |(_, v)| { - matches!( - v.get_update_transform(), - ShellVariableUpdateTransform::Capitalize - ) == value - })); - } - if let Some(value) = self.lowercase_value_on_assignment.to_bool() { - filters.push(Box::new(move |(_, v)| { - matches!( - v.get_update_transform(), - ShellVariableUpdateTransform::Lowercase - ) == value - })); - } - if let Some(value) = self.make_nameref.to_bool() { - filters.push(Box::new(move |(_, v)| v.is_treated_as_nameref() == value)); - } - if let Some(value) = self.make_readonly.to_bool() { - filters.push(Box::new(move |(_, v)| v.is_readonly() == value)); - } - if let Some(value) = self.make_readonly.to_bool() { - filters.push(Box::new(move |(_, v)| v.is_trace_enabled() == value)); - } - if let Some(value) = self.uppercase_value_on_assignment.to_bool() { - filters.push(Box::new(move |(_, v)| { - matches!( - v.get_update_transform(), - ShellVariableUpdateTransform::Uppercase - ) == value - })); - } - if let Some(value) = self.make_exported.to_bool() { - filters.push(Box::new(move |(_, v)| v.is_exported() == value)); - } - - let iter_policy = if matches!(verb, DeclareVerb::Local) { - EnvironmentLookup::OnlyInCurrentLocal - } else { - EnvironmentLookup::Anywhere - }; - - // Iterate through an ordered list of all matching declarations tracked in the - // environment. - for (name, variable) in context - .shell - .env() - .iter_using_policy(iter_policy) - .filter(|pair| filters.iter().all(|f| f(*pair))) - .sorted_by_key(|v| v.0) - { - if self.print { - let mut cs = variable.attribute_flags(context.shell); - if cs.is_empty() { - cs.push('-'); - } - - let separator_str = if matches!(variable.value(), ShellValue::Unset(_)) { - "" - } else { - "=" - }; - - writeln!( - context.stdout(), - "declare -{cs} {name}{separator_str}{}", - variable - .value() - .format(variables::FormatStyle::DeclarePrint, context.shell)? - )?; - } else { - writeln!( - context.stdout(), - "{name}={}", - variable - .value() - .format(variables::FormatStyle::Basic, context.shell)? - )?; - } - } - - Ok(()) - } - - fn display_matching_functions( - &self, - context: &brush_core::ExecutionContext<'_, impl brush_core::ShellExtensions>, - ) -> Result<(), brush_core::Error> { - for (name, registration) in context.shell.funcs().iter().sorted_by_key(|v| v.0) { - if self.function_names_only { - writeln!(context.stdout(), "declare -f {name}")?; - } else { - writeln!(context.stdout(), "{}", registration.definition())?; - } - } - - Ok(()) - } - - #[expect(clippy::unnecessary_wraps)] - const fn apply_attributes_before_update( - &self, - var: &mut ShellVariable, - ) -> Result<(), brush_core::Error> { - if let Some(value) = self.make_integer.to_bool() { - if value { - var.treat_as_integer(); - } else { - var.unset_treat_as_integer(); - } - } - if let Some(value) = self.capitalize_value_on_assignment.to_bool() { - if value { - var.set_update_transform(ShellVariableUpdateTransform::Capitalize); - } else if matches!( - var.get_update_transform(), - ShellVariableUpdateTransform::Capitalize - ) { - var.set_update_transform(ShellVariableUpdateTransform::None); - } - } - if let Some(value) = self.lowercase_value_on_assignment.to_bool() { - if value { - var.set_update_transform(ShellVariableUpdateTransform::Lowercase); - } else if matches!( - var.get_update_transform(), - ShellVariableUpdateTransform::Lowercase - ) { - var.set_update_transform(ShellVariableUpdateTransform::None); - } - } - if let Some(value) = self.make_nameref.to_bool() { - if value { - var.treat_as_nameref(); - } else { - var.unset_treat_as_nameref(); - } - } - if let Some(value) = self.make_traced.to_bool() { - if value { - var.enable_trace(); - } else { - var.disable_trace(); - } - } - if let Some(value) = self.uppercase_value_on_assignment.to_bool() { - if value { - var.set_update_transform(ShellVariableUpdateTransform::Uppercase); - } else if matches!( - var.get_update_transform(), - ShellVariableUpdateTransform::Uppercase - ) { - var.set_update_transform(ShellVariableUpdateTransform::None); - } - } - if let Some(value) = self.make_exported.to_bool() { - if value { - var.export(); - } else { - var.unexport(); - } - } - - Ok(()) - } - - fn apply_attributes_after_update( - &self, - var: &mut ShellVariable, - verb: DeclareVerb, - ) -> Result<(), brush_core::Error> { - if matches!(verb, DeclareVerb::Readonly) { - var.set_readonly(); - } else if let Some(value) = self.make_readonly.to_bool() { - if value { - var.set_readonly(); - } else { - var.unset_readonly()?; - } - } - - Ok(()) - } -} diff --git a/brush-builtins/src/declare/display.rs b/brush-builtins/src/declare/display.rs new file mode 100644 index 000000000..8f73f7f9e --- /dev/null +++ b/brush-builtins/src/declare/display.rs @@ -0,0 +1,265 @@ +//! The half of the declaration builtins that reads the environment rather than changing it: +//! listing variables and functions, and the `declare -p` line each is displayed as. + +use itertools::Itertools; +use std::io::Write; + +use brush_core::{ + env::EnvironmentLookup, + variables::{self, ShellValue, ShellVariable, ShellVariableUpdateTransform}, +}; + +use super::{DeclareCommand, DeclareVerb}; + +impl DeclareCommand { + /// Displays the variable or function named by an operand. Returns `true` if it was found. + pub(super) fn try_display_declaration( + &self, + context: &brush_core::ExecutionContext<'_, impl brush_core::ShellExtensions>, + declaration: &brush_core::CommandArg, + verb: DeclareVerb, + ) -> Result { + let name = match declaration { + brush_core::CommandArg::String(s) => s, + brush_core::CommandArg::Assignment(assignment) => { + writeln!( + context.stderr(), + "{}: {assignment}: not found", + context.command_name + )?; + return Ok(false); + } + }; + + let lookup = if matches!(verb, DeclareVerb::Local) { + EnvironmentLookup::OnlyInCurrentLocal + } else { + EnvironmentLookup::Anywhere + }; + + if self.function_names_only || self.function_names_or_defs_only { + if let Some(func_registration) = context.shell.funcs().get(name) { + if self.function_names_only { + if self.print { + writeln!( + context.stdout(), + "declare -{} {name}", + func_registration.attribute_flags() + )?; + } else { + writeln!(context.stdout(), "{name}")?; + } + } else { + writeln!(context.stdout(), "{}", func_registration.definition())?; + } + Ok(true) + } else { + // A shell reports a missing function only through the exit status here. + Ok(false) + } + } else if let Some(variable) = context.shell.env().get_using_policy(name, lookup) { + let resolved_value = variable.resolve_value(context.shell); + write_declare_line(context, name, variable, &resolved_value)?; + Ok(true) + } else { + // Diagnostics name the builtin as invoked (`local`, `typeset`, ...), even though + // displayed declarations always read `declare`. + writeln!( + context.stderr(), + "{}: {name}: not found", + context.command_name + )?; + Ok(false) + } + } + + /// Returns the predicates the attribute options select variables with, one per option given + /// in its `-X` form. They apply as a union: `declare -rt` lists variables that are readonly + /// *or* traced. A plus option (`+x`) selects nothing, as in a shell. + pub(super) fn attribute_selectors(&self) -> Vec { + let mut selectors: Vec = vec![]; + if self.make_indexed_array.to_bool() == Some(true) { + selectors.push(|v| v.value().is_indexed_array()); + } + if self.make_associative_array.to_bool() == Some(true) { + selectors.push(|v| v.value().is_associative_array()); + } + if self.make_integer.to_bool() == Some(true) { + selectors.push(|v| v.is_treated_as_integer()); + } + if self.capitalize_value_on_assignment.to_bool() == Some(true) { + selectors.push(|v| { + matches!( + v.get_update_transform(), + ShellVariableUpdateTransform::Capitalize + ) + }); + } + if self.lowercase_value_on_assignment.to_bool() == Some(true) { + selectors.push(|v| { + matches!( + v.get_update_transform(), + ShellVariableUpdateTransform::Lowercase + ) + }); + } + if self.make_nameref.to_bool() == Some(true) { + selectors.push(|v| v.is_treated_as_nameref()); + } + if self.make_readonly.to_bool() == Some(true) { + selectors.push(|v| v.is_readonly()); + } + if self.make_traced.to_bool() == Some(true) { + selectors.push(|v| v.is_trace_enabled()); + } + if self.uppercase_value_on_assignment.to_bool() == Some(true) { + selectors.push(|v| { + matches!( + v.get_update_transform(), + ShellVariableUpdateTransform::Uppercase + ) + }); + } + if self.make_exported.to_bool() == Some(true) { + selectors.push(|v| v.is_exported()); + } + selectors + } + + /// Displays all variables whose attributes match the requested filters. + pub(super) fn display_matching_env_declarations( + &self, + context: &brush_core::ExecutionContext<'_, impl brush_core::ShellExtensions>, + verb: DeclareVerb, + ) -> Result<(), brush_core::Error> { + // The verb decides which variables are eligible at all: `readonly` and `export` list + // only the variables carrying their attribute. Attribute options then select among them. + let eligible = |v: &ShellVariable| { + v.is_enumerable() + && match verb { + DeclareVerb::Readonly => v.is_readonly(), + DeclareVerb::Export => v.is_exported(), + DeclareVerb::Declare | DeclareVerb::Local => true, + } + }; + let selectors = self.attribute_selectors(); + + // A shell lists in `declare -p` form whenever an attribute option or an + // attribute-implying verb selected the variables, not only under `-p`. + let declare_form = self.print || verb.implies_attribute() || !selectors.is_empty(); + + let iter_policy = if matches!(verb, DeclareVerb::Local) { + EnvironmentLookup::OnlyInCurrentLocal + } else { + EnvironmentLookup::Anywhere + }; + + for (name, variable) in context + .shell + .env() + .iter_using_policy(iter_policy) + .filter(|(_, v)| { + eligible(v) && (selectors.is_empty() || selectors.iter().any(|f| f(v))) + }) + .sorted_by_key(|v| v.0) + { + if declare_form { + write_declare_line(context, name, variable, variable.value())?; + } else { + writeln!( + context.stdout(), + "{name}={}", + variable + .value() + .format(variables::FormatStyle::Basic, context.shell)? + )?; + } + } + + Ok(()) + } + + /// Displays shell functions. An attribute option (`-x`, `-r`, `-t`) or an + /// attribute-implying verb lists only the functions carrying one of those attributes, each + /// definition followed by its attribute line. + pub(super) fn display_matching_functions( + &self, + context: &brush_core::ExecutionContext<'_, impl brush_core::ShellExtensions>, + verb: DeclareVerb, + ) -> Result<(), brush_core::Error> { + use brush_core::functions::Registration; + + let mut selectors: Vec bool> = vec![]; + match verb { + DeclareVerb::Export => selectors.push(Registration::is_exported), + DeclareVerb::Readonly => selectors.push(Registration::is_readonly), + DeclareVerb::Declare | DeclareVerb::Local => (), + } + if self.make_exported.to_bool() == Some(true) { + selectors.push(Registration::is_exported); + } + if self.make_readonly.to_bool() == Some(true) { + selectors.push(Registration::is_readonly); + } + if self.make_traced.to_bool() == Some(true) { + selectors.push(Registration::is_trace_enabled); + } + let filtered = !selectors.is_empty(); + + for (name, registration) in context + .shell + .funcs() + .iter() + .filter(|(_, registration)| !filtered || selectors.iter().any(|s| s(registration))) + .sorted_by_key(|v| v.0) + { + if !self.function_names_only { + writeln!(context.stdout(), "{}", registration.definition())?; + } + if self.function_names_only || filtered { + writeln!( + context.stdout(), + "declare -{} {name}", + registration.attribute_flags() + )?; + } + } + + Ok(()) + } +} + +/// A predicate selecting variables for display. +type VariableSelector = fn(&ShellVariable) -> bool; + +/// Writes the `declare - name=value` line that displays a variable. +/// +/// # Arguments +/// +/// * `value` - The value to display; a dynamic variable's already resolved, if the caller wants +/// it shown. +fn write_declare_line( + context: &brush_core::ExecutionContext<'_, impl brush_core::ShellExtensions>, + name: &str, + variable: &ShellVariable, + value: &ShellValue, +) -> Result<(), brush_core::Error> { + let mut flags = variable.attribute_flags(context.shell); + if flags.is_empty() { + flags.push('-'); + } + + let separator = if matches!(value, ShellValue::Unset(_)) { + "" + } else { + "=" + }; + + writeln!( + context.stdout(), + "declare -{flags} {name}{separator}{}", + value.format(variables::FormatStyle::DeclarePrint, context.shell)? + )?; + + Ok(()) +} diff --git a/brush-builtins/src/declare/mod.rs b/brush-builtins/src/declare/mod.rs new file mode 100644 index 000000000..0f10ae5d8 --- /dev/null +++ b/brush-builtins/src/declare/mod.rs @@ -0,0 +1,1146 @@ +mod display; + +use clap::Parser; +use std::borrow::Cow; +use std::io::Write; + +use brush_core::{ + ErrorKind, ExecutionResult, builtins, + env::{self, EnvironmentLookup, EnvironmentScope}, + expansion::ResolvedAssignment, + parser::ast, + variables::{ + self, ArrayKind, ScalarConversionPolicy, ShellValue, ShellValueLiteral, + ShellValueUnsetType, ShellVariable, ShellVariableUpdateTransform, + }, +}; + +crate::minus_or_plus_flag_arg!( + MakeIndexedArrayFlag, + 'a', + "Make the variable an indexed array." +); +crate::minus_or_plus_flag_arg!( + MakeAssociativeArrayFlag, + 'A', + "Make the variable an associative array." +); +crate::minus_or_plus_flag_arg!( + CapitalizeValueOnAssignmentFlag, + 'c', + "Enable capitalize-on-assignment for the variable." +); +crate::minus_or_plus_flag_arg!(MakeIntegerFlag, 'i', "Mark the variable as integer-typed"); +crate::minus_or_plus_flag_arg!( + InheritLocalsFlag, + 'I', + "Initialize a new local from the contents and attributes of the variable it shadows." +); +crate::minus_or_plus_flag_arg!( + LowercaseValueOnAssignmentFlag, + 'l', + "Enable lowercase-on-assignment for the variable." +); +crate::minus_or_plus_flag_arg!( + MakeNameRefFlag, + 'n', + "Mark the variable as a name reference" +); +crate::minus_or_plus_flag_arg!(MakeReadonlyFlag, 'r', "Mark the variable as read-only."); +crate::minus_or_plus_flag_arg!(MakeTracedFlag, 't', "Enable tracing for the variable."); +crate::minus_or_plus_flag_arg!( + UppercaseValueOnAssignmentFlag, + 'u', + "Enable uppercase-on-assignment for the variable." +); +crate::minus_or_plus_flag_arg!(MakeExportedFlag, 'x', "Mark the variable for export."); + +/// Display or update variables and their attributes. +/// +/// `export` and `readonly` are this command with a verb-implied attribute and a subset of its +/// options; they build one of these from their own command line and run it with +/// [`DeclareCommand::execute_as`]. +#[derive(Parser, Default)] +#[clap(override_usage = "declare [OPTIONS] [DECLARATIONS]...")] +pub(crate) struct DeclareCommand { + /// Constrain to function names or definitions. + #[arg(short = 'f')] + pub(crate) function_names_or_defs_only: bool, + + /// Constrain to function names only. + #[arg(short = 'F')] + pub(crate) function_names_only: bool, + + /// Create global variable, if applicable. + #[arg(short = 'g')] + pub(crate) create_global: bool, + + /// When creating a local variable that shadows another variable of the same name, + /// then initialize it with the contents and attributes of the variable being shadowed. + /// As in a shell, `+I` behaves like `-I`. + #[clap(flatten)] + pub(crate) locals_inherit_from_prev_scope: InheritLocalsFlag, + + /// Display each item's attributes and values. + #[arg(short = 'p')] + pub(crate) print: bool, + + // + // Attribute options + #[clap(flatten)] // -a + pub(crate) make_indexed_array: MakeIndexedArrayFlag, + #[clap(flatten)] // -A + pub(crate) make_associative_array: MakeAssociativeArrayFlag, + #[clap(flatten)] // -c + pub(crate) capitalize_value_on_assignment: CapitalizeValueOnAssignmentFlag, + #[clap(flatten)] // -i + pub(crate) make_integer: MakeIntegerFlag, + #[clap(flatten)] // -l + pub(crate) lowercase_value_on_assignment: LowercaseValueOnAssignmentFlag, + #[clap(flatten)] // -n + pub(crate) make_nameref: MakeNameRefFlag, + #[clap(flatten)] // -r + pub(crate) make_readonly: MakeReadonlyFlag, + #[clap(flatten)] // -t + pub(crate) make_traced: MakeTracedFlag, + #[clap(flatten)] // -u + pub(crate) uppercase_value_on_assignment: UppercaseValueOnAssignmentFlag, + #[clap(flatten)] // -x + pub(crate) make_exported: MakeExportedFlag, + + // + // Declarations + // + // N.B. These are skipped by clap, but filled in by the BuiltinDeclarationCommand trait. + #[clap(skip)] + pub(crate) declarations: Vec, +} + +/// The builtin a declaration was invoked as. All of them share one implementation; the verb +/// selects the scope rules and whether an attribute is implied by the name -- which is where +/// nearly every behavioral difference between them comes from. See +/// [`DeclareVerb::implies_attribute`]. +#[derive(Clone, Copy)] +pub(crate) enum DeclareVerb { + Declare, + // `local` and `readonly` are the only builtins declaring with these verbs, so the variants go + // away with them. + #[cfg_attr( + not(feature = "builtin.declare"), + allow(dead_code, reason = "constructed only by the `local` builtin") + )] + Local, + #[cfg_attr( + not(feature = "builtin.declare"), + allow(dead_code, reason = "constructed only by the `readonly` builtin") + )] + Readonly, + // `export` is the only builtin that declares with this verb, so the variant goes away with it. + #[cfg_attr( + not(feature = "builtin.export"), + allow(dead_code, reason = "constructed only by the `export` builtin") + )] + Export, +} + +impl DeclareVerb { + /// Whether this builtin grants an attribute by virtue of its own name (`export` grants `-x`, + /// `readonly` grants `-r`) rather than taking every attribute from an option, as `declare` + /// and `local` do. + /// + /// Almost everything that separates `export` and `readonly` from `declare` follows from + /// this, so this one predicate stands in for all of it: + /// + /// - the granted attribute is not an option, so `-p` alongside operands has nothing to + /// select and is a no-op, and `-f` applies the attribute instead of displaying; + /// - `-a`/`-A` become modifiers of an assignment rather than the point of the command: they + /// apply only to an operand that assigns a value, may be combined, and `-a` wins; + /// - a subscripted operand is an invalid identifier; + /// - a missing function under `-f` is reported; + /// - each assignment performed is echoed as an extra `set -x` line; + /// - a readonly variable is reported the way a bare assignment reports it, without naming + /// the builtin. + const fn implies_attribute(self) -> bool { + matches!(self, Self::Readonly | Self::Export) + } +} + +#[derive(Clone, Copy)] +struct DeclarationScope { + lookup: EnvironmentLookup, + creation: EnvironmentScope, +} + +/// A declaration whose expansion and structural interpretation are complete. +/// +/// A shell applies what it can before it complains, so preparing an operand can succeed and +/// still have found something wrong: [`Self::stopped_by`] carries that error until the rest of +/// the operand has been applied, in the same two-phase shape as +/// [`ResolvedAssignment::stopped_by`], which is where most of them come from. +struct PreparedDeclaration { + /// The variable being declared. + name: String, + /// The subscript the operand named, if any (present even when nothing is assigned, as in + /// `declare arr[5]`), resolved to the element's final index or key. + subscript: Option, + /// The subscript as the operand wrote it, for diagnostics. Differs from `subscript` only when + /// resolution changed it (`a[i+1]` to `a[2]`). + written_subscript: Option, + /// The value to assign, if any. + initial_value: Option, + /// Whether the operand appended rather than replaced. + append: bool, + /// Whether the operand is unquoted compound syntax (`name=(...)`). A shell's refusal to + /// assign such an operand is an assignment error rather than a builtin failure: nothing is + /// granted and the command list is abandoned. (Only a refusal -- see + /// [`brush_core::ErrorKind::is_assignment_failure`].) + is_compound_syntax: bool, + /// The array kind the target has before this declaration, if it exists and is an array. + current_kind: Option, + /// The array kind the declaration converts its target to, if any. See + /// [`DeclareCommand::conversion_kind`]. + conversion: Option, + /// An error that stopped this operand short while it was prepared -- a bad subscript on + /// its name, or a bad key in its compound value -- raised once what was kept has been + /// applied. + stopped_by: Option, +} + +impl PreparedDeclaration { + /// A declaration that names a variable without assigning to it. Other shapes are built from + /// this one with struct update syntax. + fn bare(name: &str, subscript: Option<&str>) -> Self { + Self { + name: name.to_owned(), + subscript: subscript.map(str::to_owned), + written_subscript: subscript.map(str::to_owned), + initial_value: None, + append: false, + is_compound_syntax: false, + current_kind: None, + conversion: None, + stopped_by: None, + } + } + + /// A declaration that binds its target as an array without assigning to it, which is how a + /// shell binds a target whose subscript turned out to be bad. + /// + /// # Arguments + /// + /// * `name` - The variable being declared. + /// * `current_kind` - The array kind the target already has, if it exists and is an array. + fn bound_as_array(name: &str, current_kind: Option) -> Self { + match current_kind { + // A target that is already an array needs no binding, and must not get one: + // appending to a declared-but-unset array would fill it out and wrongly leave it set. + Some(_) => Self::bare(name, None), + // Appending an empty list is how a shell binds a target as an array without giving + // it a value: a new variable becomes a set, empty array, and a scalar is promoted to + // element 0. + None => Self { + initial_value: Some(ShellValueLiteral::Array(variables::ArrayLiteral(vec![]))), + append: true, + ..Self::bare(name, None) + }, + } + } + + /// Returns the text `readonly` and `export` echo as an extra `set -x` trace line for this + /// declaration: only a scalar assignment to a whole, validly named variable is echoed. + fn render_traced_assignment(&self) -> Option { + let Some(value @ ShellValueLiteral::Scalar(_)) = &self.initial_value else { + return None; + }; + if self.subscript.is_some() || !env::valid_variable_name(self.name.as_str()) { + return None; + } + + let op = if self.append { "+=" } else { "=" }; + Some(std::format!("{}{op}{value}", self.name)) + } +} + +impl builtins::DeclarationCommand for DeclareCommand { + fn set_declarations(&mut self, declarations: Vec) { + self.declarations = declarations; + } +} + +impl builtins::Command for DeclareCommand { + fn takes_plus_options() -> bool { + true + } + + type Error = brush_core::Error; + + async fn execute( + &self, + context: brush_core::ExecutionContext<'_, SE>, + ) -> Result { + self.execute_as(DeclareVerb::Declare, &self.declarations, context) + .await + } +} + +impl DeclareCommand { + /// Executes this command as the given declaration builtin. + /// + /// # Arguments + /// + /// * `verb` - The builtin performing the declaration. + /// * `declarations` - The operands to process. (Taken separately from `self` so a wrapper + /// builtin can lend its own without copying them.) + /// * `context` - The execution context. + pub(crate) async fn execute_as( + &self, + verb: DeclareVerb, + declarations: &[brush_core::CommandArg], + mut context: brush_core::ExecutionContext<'_, SE>, + ) -> Result { + if !verb.implies_attribute() + && self.make_indexed_array.to_bool() == Some(true) + && self.make_associative_array.to_bool() == Some(true) + { + writeln!( + context.stderr(), + "{}: -a: invalid option", + context.command_name + )?; + return Ok(ExecutionResult::new(2)); + } + + if matches!(verb, DeclareVerb::Local) && !context.shell.in_function() { + writeln!( + context.stderr(), + "{}: can only be used in a function", + context.command_name + )?; + return Ok(ExecutionResult::general_error()); + } + + let for_functions = self.function_names_only || self.function_names_or_defs_only; + let mut result = ExecutionResult::success(); + if !declarations.is_empty() { + // Operands are displayed, applied to functions, or applied to variables. `-p` + // selects display and `-f`/`-F` select functions, which are displayed unless an + // attribute is being applied to them. + let display = self.print && !verb.implies_attribute(); + if display || for_functions { + let applies_function_attributes = for_functions + && (verb.implies_attribute() + || self.make_traced.is_some() + || self.make_exported.is_some() + || self.make_readonly.is_some()); + + for declaration in declarations { + // A function cannot be declared by assignment. + if for_functions && matches!(declaration, brush_core::CommandArg::Assignment(_)) + { + writeln!( + context.stderr(), + "{}: cannot use `-f' to make functions", + context.command_name + )?; + result = ExecutionResult::general_error(); + continue; + } + + let succeeded = if applies_function_attributes { + self.apply_function_attributes(&mut context, declaration, verb)? + } else { + self.try_display_declaration(&context, declaration, verb)? + }; + if !succeeded { + result = ExecutionResult::general_error(); + } + } + } else { + let scope = self.declaration_scope(&context, verb); + + // Operands are processed in order, each against the environment its + // predecessors left behind. An assignment error propagates, so the interpreter + // abandons the rest of the command list. + for declaration in declarations { + let prepared = self + .prepare_declaration(&mut context, declaration, verb, scope) + .await?; + + // `export` and `readonly` echo each assignment they perform as a trace + // line of its own, on top of the one the interpreter already wrote for the + // command. + if verb.implies_attribute() + && let Some(line) = prepared.render_traced_assignment() + { + context.trace_extra_line(line).await; + } + + if !self.apply_declaration(&mut context, prepared, verb, scope)? { + result = ExecutionResult::general_error(); + } + } + } + } else { + if !for_functions { + self.display_matching_env_declarations(&context, verb)?; + } + + // Functions are listed under -f/-F, and otherwise only when nothing selected + // variables specifically: `-p`, an attribute option, or a verb that implies one. + if !matches!(verb, DeclareVerb::Local) + && (for_functions + || (!self.print + && !verb.implies_attribute() + && self.attribute_selectors().is_empty())) + { + self.display_matching_functions(&context, verb)?; + } + } + + Ok(result) + } + + /// Resolves the lookup and creation scopes for this invocation. + fn declaration_scope( + &self, + context: &brush_core::ExecutionContext<'_, impl brush_core::ShellExtensions>, + verb: DeclareVerb, + ) -> DeclarationScope { + let create_local = matches!(verb, DeclareVerb::Local) + || (matches!(verb, DeclareVerb::Declare) + && context.shell.in_function() + && !self.create_global); + + let lookup = if create_local { + EnvironmentLookup::OnlyInCurrentLocal + } else if self.create_global { + EnvironmentLookup::OnlyInGlobal + } else { + EnvironmentLookup::Anywhere + }; + + let creation = if create_local { + EnvironmentScope::Local + } else { + EnvironmentScope::Global + }; + + DeclarationScope { lookup, creation } + } + + fn apply_function_attributes( + &self, + context: &mut brush_core::ExecutionContext<'_, impl brush_core::ShellExtensions>, + declaration: &brush_core::CommandArg, + verb: DeclareVerb, + ) -> Result { + let func = match declaration { + brush_core::CommandArg::String(name) => context.shell.func_mut(name), + brush_core::CommandArg::Assignment(_) => None, + }; + + let Some(func) = func else { + if verb.implies_attribute() { + writeln!( + context.stderr(), + "{}: {declaration}: not a function", + context.command_name + )?; + } + return Ok(false); + }; + + if self.make_readonly.to_bool() == Some(false) && func.is_readonly() { + writeln!( + context.stderr(), + "{}: {declaration}: readonly function", + context.command_name + )?; + return Ok(false); + } + + match self.make_exported.to_bool() { + Some(true) => func.export(), + Some(false) => func.unexport(), + None => (), + } + match self.make_traced.to_bool() { + Some(true) => func.enable_trace(), + Some(false) => func.disable_trace(), + None => (), + } + if matches!(verb, DeclareVerb::Readonly) || self.make_readonly.to_bool() == Some(true) { + func.set_readonly(); + } + + Ok(true) + } + + /// Applies one prepared declaration to the variable environment. Returns `true` on success, + /// or `false` for a failure that affects the exit status without stopping the remaining + /// operands. + fn apply_declaration( + &self, + context: &mut brush_core::ExecutionContext<'_, impl brush_core::ShellExtensions>, + declaration: PreparedDeclaration, + verb: DeclareVerb, + scope: DeclarationScope, + ) -> Result { + // `+a`/`+A` cannot remove an array attribute, even from a declared-but-unset array. + let dropping = match declaration.current_kind { + Some(ArrayKind::Indexed) => self.make_indexed_array.to_bool() == Some(false), + Some(ArrayKind::Associative) => self.make_associative_array.to_bool() == Some(false), + None => false, + }; + if dropping { + writeln!( + context.stderr(), + "{}: {}: cannot destroy array variables in this way", + context.command_name, + declaration.name, + )?; + return Ok(false); + } + + // Special-case: `local -` + if declaration.name == "-" && matches!(verb, DeclareVerb::Local) { + // TODO(local): `local -` allows shadowing the current `set` options (i.e., $-), with + // subsequent updates getting discarded when the current local scope is popped. + tracing::warn!("not yet implemented: local -"); + return Ok(true); + } + + if !env::valid_variable_name(declaration.name.as_str()) { + writeln!( + context.stderr(), + "{}: `{}': not a valid identifier", + context.command_name, + declaration.name, + )?; + return Ok(false); + } + + if verb.implies_attribute() + && let Some(subscript) = &declaration.subscript + { + writeln!( + context.stderr(), + "{}: `{}[{subscript}]': not a valid identifier", + context.command_name, + declaration.name, + )?; + return Ok(false); + } + + // A failure is reported against the variable as written. An assignment error + // propagates; any other failure fails just this operand. + let name = declaration.name.clone(); + let subscript = declaration.written_subscript.clone(); + match self.update_declared_variable(context, declaration, verb, scope) { + Ok(()) => Ok(true), + Err(err) => { + let err = err.for_variable(&name, subscript.as_deref()); + if err.is_assignment_error() { + Err(err) + } else { + self.report_recoverable_error(context, err, verb) + } + } + } + } + + /// Reports a recoverable per-operand failure to stderr and returns `Ok(false)`; any other + /// error propagates. + fn report_recoverable_error( + &self, + context: &brush_core::ExecutionContext<'_, impl brush_core::ShellExtensions>, + err: brush_core::Error, + verb: DeclareVerb, + ) -> Result { + match err.kind() { + // Plain `export` and `readonly` report a readonly variable the way a bare assignment + // does, without naming the builtin; a bad subscript is always reported bare. + ErrorKind::ReadonlyVariable + if verb.implies_attribute() && self.requested_array_kind().is_none() => + { + writeln!(context.stderr(), "{err}")?; + } + ErrorKind::BadArraySubscript(_) => writeln!(context.stderr(), "{err}")?, + ErrorKind::ReadonlyVariable + | ErrorKind::ConvertingIndexedArrayToAssociativeArray + | ErrorKind::ConvertingAssociativeArrayToIndexedArray => { + writeln!(context.stderr(), "{}: {err}", context.command_name)?; + } + _ => return Err(err), + } + + Ok(false) + } + + /// Applies one prepared declaration to the environment, updating the variable it names or + /// creating it. + fn update_declared_variable( + &self, + context: &mut brush_core::ExecutionContext<'_, impl brush_core::ShellExtensions>, + declaration: PreparedDeclaration, + verb: DeclareVerb, + scope: DeclarationScope, + ) -> Result<(), brush_core::Error> { + // Read before the env borrow below is taken out. + let auto_export = context.shell.options().export_variables_on_modification; + + // `local -I` / `declare -I` (bash 5.0+) starts the new local from a copy of the nearest + // same-name variable, wherever it lives. The copy is bound first and then updated like + // any existing variable, so a kind conflict still leaves the local in place. A readonly + // inheritee refuses the declaration outright. + let inheritee = self.inheritee(context.shell.env(), &declaration.name, scope); + let inherited = if let Some(inheritee) = inheritee { + if inheritee.is_readonly() { + return Err(ErrorKind::ReadonlyVariable.into()); + } + + let mut var = inheritee.clone(); + // Inherit what a dynamic value (DIRSTACK and friends) resolves to, and inherit a + // nameref's target string as an ordinary scalar. + var.resolve_dynamic(context.shell); + var.unset_treat_as_nameref(); + + context + .shell + .env_mut() + .add(declaration.name.clone(), var, scope.creation)?; + true + } else { + false + }; + + if let Some(var) = context + .shell + .env_mut() + .get_mut_using_policy(declaration.name.as_str(), scope.lookup) + { + // A shell discards a function-local scalar's value on conversion; only a global + // keeps it as element 0. + let policy = if matches!(scope.creation, EnvironmentScope::Local) { + ScalarConversionPolicy::Discard + } else { + ScalarConversionPolicy::PromoteToElementZero + }; + let conversion = declaration.conversion.map(|kind| (kind, policy)); + + // `set -a` exports a new variable outright, but an existing one only when the + // declaration assigns to it. + let export = auto_export && (inherited || declaration.initial_value.is_some()); + self.update_variable(var, declaration, verb, conversion, export)?; + } else { + // `export -n` removes an attribute; unless it also assigns, it has nothing to create. + if matches!(verb, DeclareVerb::Export) + && self.make_exported.to_bool() == Some(false) + && declaration.initial_value.is_none() + { + return Ok(()); + } + + // A local may not shadow a readonly global (a readonly local in an enclosing + // function's scope is fine). + if matches!(scope.creation, EnvironmentScope::Local) + && context + .shell + .env() + .get_using_policy(&declaration.name, EnvironmentLookup::OnlyInGlobal) + .is_some_and(ShellVariable::is_readonly) + { + return Err(ErrorKind::ReadonlyVariable.into()); + } + + let mut var = ShellVariable::new(ShellValue::Unset(ShellValueUnsetType::Untyped)); + // The value is unset, so the scalar policy is moot. + let conversion = declaration + .conversion + .map(|kind| (kind, ScalarConversionPolicy::Discard)); + + // The variable is bound even when assigning to it fails, so a failed element + // assignment still leaves it declared with its kind and no value. + let name = declaration.name.clone(); + let updated = + self.update_variable(&mut var, declaration, verb, conversion, auto_export); + context.shell.env_mut().add(name, var, scope.creation)?; + updated?; + } + + Ok(()) + } + + /// The shared tail of a declaration: converts the variable's kind, assigns any value, and + /// applies attributes, in the order a shell does. + /// + /// The value update runs first (kind conversion, the readonly check, the attributes that + /// shape how a value is stored, then the value). Two separate things can then have gone + /// wrong, and they grant different amounts: + /// + /// | The update | ...and what was recorded in `prepare` | Verb's own attribute | Option attributes, `set -a` | + /// |---|---|---|---| + /// | succeeded | nothing | granted | granted | + /// | succeeded | a bad subscript on the name | granted | granted | + /// | failed | (either way) | granted | not granted | + /// + /// So `declare -rx 'b[*]=1'` still leaves `b` an empty readonly, exported array, while a + /// refused conversion grants only `export`'s `-x` or `readonly`'s `-r`. + /// + /// An unquoted compound operand is the exception to the whole table: any failed outcome is + /// an assignment error, none of these attributes is granted, and the error propagates. The + /// attributes that shape how a value is stored are applied earlier and survive regardless -- + /// see [`Self::apply_pre_assignment_attributes`]. + /// + /// `set -a` never exports an array, outranks an explicit `+x`, but yields to `export -n`. + /// + /// # Arguments + /// + /// * `var` - The variable to update. + /// * `declaration` - The declaration to apply. + /// * `verb` - The builtin performing the declaration. + /// * `conversion` - The array kind to convert the variable to first, if any, with the policy + /// for a set scalar value. + /// * `auto_export` - Whether `set -a` applies to this update. + fn update_variable( + &self, + var: &mut ShellVariable, + mut declaration: PreparedDeclaration, + verb: DeclareVerb, + conversion: Option<(ArrayKind, ScalarConversionPolicy)>, + auto_export: bool, + ) -> Result<(), brush_core::Error> { + let is_compound_syntax = declaration.is_compound_syntax; + let stopped_by = declaration.stopped_by.take(); + let updated = self.update_value(var, declaration, conversion); + let value_assigned = updated.is_ok(); + let outcome = updated.and(stopped_by.map_or(Ok(()), Err)); + // Only a shell's own refusal to assign becomes an assignment error; brush failing to + // carry the assignment out (an unimplemented case, say) fails the operand like any + // other error rather than abandoning the caller's command list. + if let Err(err) = &outcome + && is_compound_syntax + && err.kind().is_assignment_failure() + { + return outcome.map_err(brush_core::Error::into_assignment_error); + } + + let implied_export = matches!(verb, DeclareVerb::Export); + match verb { + DeclareVerb::Export => self.apply_export_flag(var), + DeclareVerb::Readonly => { + var.set_readonly(); + } + DeclareVerb::Declare | DeclareVerb::Local => (), + } + + if value_assigned { + if !implied_export { + self.apply_export_flag(var); + } + self.apply_trace_flag(var); + self.apply_readonly_flag(var)?; + + let auto_export = + auto_export && !(implied_export && self.make_exported.to_bool() == Some(false)); + if auto_export && !var.value().is_array() { + var.export(); + } + } + + outcome + } + + /// Performs the value half of a declaration: the kind conversion, the option attributes that + /// shape how a value is assigned, and the assignment itself. Everything that can refuse the + /// update is checked before the variable is touched, so a refused operand leaves no + /// attribute behind. + fn update_value( + &self, + var: &mut ShellVariable, + declaration: PreparedDeclaration, + conversion: Option<(ArrayKind, ScalarConversionPolicy)>, + ) -> Result<(), brush_core::Error> { + if let Some((kind, policy)) = conversion { + var.convert_to_array_kind(kind, policy)?; + } + + if var.is_readonly() + && (declaration.initial_value.is_some() || self.requests_value_transform()) + { + return Err(ErrorKind::ReadonlyVariable.into()); + } + + self.apply_pre_assignment_attributes(var); + + if let Some(initial_value) = declaration.initial_value { + var.assign_at(declaration.subscript, initial_value, declaration.append)?; + } + + Ok(()) + } + + /// Returns the array kind a declaration converts its target to, if any. An explicit + /// `-a`/`-A` always converts (for `export`/`readonly`, only alongside a value). Otherwise an + /// operand whose shape implies an array -- a subscripted name or a compound value -- makes + /// an indexed array of a target that is not one already. + /// + /// # Arguments + /// + /// * `assigns_value` - Whether the operand assigns a value. + /// * `implies_array` - Whether the operand's shape implies an array target. + /// * `verb` - The builtin performing the declaration. + /// * `current` - The array kind the target currently has, if it exists and is an array. + fn conversion_kind( + &self, + assigns_value: bool, + implies_array: bool, + verb: DeclareVerb, + current: Option, + ) -> Option { + if verb.implies_attribute() && !assigns_value { + return None; + } + + self.requested_array_kind() + .or_else(|| (current.is_none() && implies_array).then_some(ArrayKind::Indexed)) + } + + /// Returns the variable a `-I` declaration would start the new local from: the nearest + /// same-name variable, wherever it lives. `None` when this invocation did not ask to + /// inherit, is not creating a local, or no such variable exists. + fn inheritee<'a>( + &self, + env: &'a env::ShellEnvironment, + name: &str, + scope: DeclarationScope, + ) -> Option<&'a ShellVariable> { + if self.locals_inherit_from_prev_scope.is_some() + && matches!(scope.creation, EnvironmentScope::Local) + { + env.get_using_policy(name, EnvironmentLookup::Anywhere) + } else { + None + } + } + + /// Returns the current array kind of the variable this declaration will update: the one a + /// `-I` declaration inherits, or else one already in the declaration's own scope. `None` + /// when no such variable exists or it is not an array. A dynamic value answers with the + /// kind of what it resolves to. + fn current_target_kind( + &self, + context: &brush_core::ExecutionContext<'_, impl brush_core::ShellExtensions>, + name: &str, + scope: DeclarationScope, + ) -> Option { + let env = context.shell.env(); + self.inheritee(env, name, scope) + .or_else(|| env.get_using_policy(name, scope.lookup)) + .and_then(|var| match var.value() { + ShellValue::Dynamic { .. } => var.resolve_value(context.shell).array_kind(), + value => value.array_kind(), + }) + } + + /// Prepares one operand for application: interprets it, decides the array conversion it + /// calls for, and resolves its subscripts against the resulting kind. + async fn prepare_declaration( + &self, + context: &mut brush_core::ExecutionContext<'_, impl brush_core::ShellExtensions>, + declaration: &brush_core::CommandArg, + verb: DeclareVerb, + scope: DeclarationScope, + ) -> Result { + // Assignment syntax wins over a bare name: it is the only interpretation under which + // the text after `=` is a value, and checking it first keeps a value that merely ends + // in `]` (`x=[a]`) from being mistaken for a subscripted name. Quoting or an expansion + // may have hidden it from the parser; the operand as a whole was already expanded, so + // such a value is taken verbatim. + let assignment = match declaration { + brush_core::CommandArg::Assignment(assignment) => Cow::Borrowed(assignment), + brush_core::CommandArg::String(operand) => { + match brush_parser::word::parse_scalar_assignment( + operand, + &context.shell.parser_options(), + ) { + Ok(assignment) => Cow::Owned(assignment), + Err(_) => return Ok(self.prepare_bare_operand(context, operand, verb, scope)), + } + } + }; + + // Only a parser-recognized compound operand is compound syntax; a quoted one + // (`'a=(1 2)'`) is a scalar until reinterpreted below. + let is_compound_syntax = matches!(declaration, brush_core::CommandArg::Assignment(_)) + && matches!(assignment.value, ast::AssignmentValue::Array(_)); + + // A rejected subscripted operand (see apply_declaration) is reported as written: its + // subscript is never evaluated and its value never assigned. + if verb.implies_attribute() + && let ast::AssignmentName::ArrayElementName(name, subscript) = &assignment.name + { + return Ok(PreparedDeclaration { + is_compound_syntax, + ..PreparedDeclaration::bare(name, Some(subscript)) + }); + } + + // Subscripts resolve against the kind the target will have once this declaration has + // applied: the kind it converts to, else the kind it already has, else indexed. + let name = assignment.name.base_name(); + let current_kind = self.current_target_kind(context, name, scope); + let implies_array = matches!(assignment.name, ast::AssignmentName::ArrayElementName(..)) + || matches!(assignment.value, ast::AssignmentValue::Array(_)); + let conversion = self.conversion_kind(true, implies_array, verb, current_kind); + let target = conversion.or(current_kind).unwrap_or(ArrayKind::Indexed); + + let name = name.to_owned(); + let written_subscript = match &assignment.name { + ast::AssignmentName::VariableName(_) => None, + ast::AssignmentName::ArrayElementName(_, index) => Some(index.clone()), + }; + let resolved = context + .shell + .resolve_assignment_subscripts(&context.params, assignment.into_owned(), target) + .await; + let ResolvedAssignment { + assignment, + stopped_by, + } = match resolved { + Ok(resolved) => resolved, + // A bad subscript on the name: a shell still binds the target as an array (leaving + // one that is already an array exactly as it was) and then fails the operand. + Err(err) if matches!(err.kind(), ErrorKind::BadArraySubscript(_)) => { + return Ok(PreparedDeclaration { + is_compound_syntax, + current_kind, + conversion, + stopped_by: Some(err), + ..PreparedDeclaration::bound_as_array(&name, current_kind) + }); + } + Err(err) => return Err(err), + }; + + // A value that only now looks like compound syntax is reinterpreted. A bad key in it is + // only a warning, as in a shell: the value stops short, but the operand succeeds. + let assignment = match self + .reinterpret_as_compound(context, &assignment, current_kind.is_some(), target) + .await? + { + Some(reinterpreted) => { + if let Some(err) = reinterpreted.stopped_by { + writeln!(context.stderr(), "{err}")?; + } + reinterpreted.assignment + } + None => assignment, + }; + + let (name, subscript) = match assignment.name { + ast::AssignmentName::VariableName(name) => (name, None), + ast::AssignmentName::ArrayElementName(name, index) => (name, Some(index)), + }; + Ok(PreparedDeclaration { + name, + subscript, + written_subscript, + initial_value: Some(assignment.value.into()), + append: assignment.append, + is_compound_syntax, + current_kind, + conversion, + stopped_by, + }) + } + + /// Prepares an operand holding no assignment syntax. `declare array[index]` names an array + /// without assigning to it; the subscript only marks the operand as an array declaration + /// and is never evaluated. An empty subscript, or one followed by more text (`a[1][2]`), is + /// left in the name so that it fails as an invalid identifier. + fn prepare_bare_operand( + &self, + context: &brush_core::ExecutionContext<'_, impl brush_core::ShellExtensions>, + operand: &str, + verb: DeclareVerb, + scope: DeclarationScope, + ) -> PreparedDeclaration { + let (name, subscript) = match operand.strip_suffix(']').and_then(|s| s.split_once('[')) { + Some((name, subscript)) if !subscript.is_empty() && !subscript.contains(']') => { + (name, Some(subscript)) + } + _ => (operand, None), + }; + + let current_kind = self.current_target_kind(context, name, scope); + PreparedDeclaration { + current_kind, + conversion: self.conversion_kind(false, subscript.is_some(), verb, current_kind), + ..PreparedDeclaration::bare(name, subscript) + } + } + + /// Returns the array kind this invocation explicitly requested with `-a` or `-A`, if any. + /// When both are given (which only `export` and `readonly` allow), `-a` wins. + fn requested_array_kind(&self) -> Option { + if self.make_indexed_array.to_bool() == Some(true) { + Some(ArrayKind::Indexed) + } else if self.make_associative_array.to_bool() == Some(true) { + Some(ArrayKind::Associative) + } else { + None + } + } + + /// Reinterprets an expanded assignment's scalar value as a compound array value when the + /// requested attributes or the target's existing type call for it. Returns `None` if the + /// value should stay scalar. + async fn reinterpret_as_compound( + &self, + context: &mut brush_core::ExecutionContext<'_, impl brush_core::ShellExtensions>, + assignment: &ast::Assignment, + target_is_array: bool, + target: ArrayKind, + ) -> Result, brush_core::Error> { + // Without an array attribute or an already-array target, the text stays scalar. A + // subscripted operand is reinterpreted only under an explicit attribute: an existing + // array's element takes the text literally. + let subscripted = matches!(assignment.name, ast::AssignmentName::ArrayElementName(..)); + if self.requested_array_kind().is_none() && (subscripted || !target_is_array) { + return Ok(None); + } + + // Parser-recognized compound assignments already had their elements expanded. + let ast::AssignmentValue::Scalar(value) = &assignment.value else { + return Ok(None); + }; + + let Some(elements) = brush_parser::word::parse_compound_assignment_value( + value.value.as_str(), + &context.shell.parser_options(), + ) else { + return Ok(None); + }; + + // The compound syntax hid the elements from the operand's expansion, so they are + // expanded now, exactly once. A compound value cannot target a single element, so the + // subscript is dropped and the whole array assigned. + let compound = ast::Assignment { + name: ast::AssignmentName::VariableName(assignment.name.base_name().to_owned()), + value: ast::AssignmentValue::Array(elements), + append: assignment.append, + loc: assignment.loc.clone(), + }; + + Ok(Some( + context + .shell + .expand_assignment(&context.params, &compound, target) + .await?, + )) + } + + /// Returns whether an option asks for an attribute that can transform the variable's value: + /// the integer and case transforms (added or removed), or becoming a nameref. A readonly + /// variable refuses these; `+n` and the pure flags (`-x`/`-t`) stay permitted. + fn requests_value_transform(&self) -> bool { + self.make_integer.is_some() + || self.capitalize_value_on_assignment.is_some() + || self.lowercase_value_on_assignment.is_some() + || self.uppercase_value_on_assignment.is_some() + || self.make_nameref.to_bool() == Some(true) + } + + /// Applies the option attributes that have to be on the variable before a value is: the ones + /// that shape how the value is stored (`-i`, `-c`/`-l`/`-u`, `-n`). A shell keeps these even + /// when the assignment then fails, which is what separates them from the flags applied after. + const fn apply_pre_assignment_attributes(&self, var: &mut ShellVariable) { + if let Some(value) = self.make_integer.to_bool() { + if value { + var.treat_as_integer(); + } else { + var.unset_treat_as_integer(); + } + } + if let Some(value) = self.capitalize_value_on_assignment.to_bool() { + if value { + var.set_update_transform(ShellVariableUpdateTransform::Capitalize); + } else if matches!( + var.get_update_transform(), + ShellVariableUpdateTransform::Capitalize + ) { + var.set_update_transform(ShellVariableUpdateTransform::None); + } + } + if let Some(value) = self.lowercase_value_on_assignment.to_bool() { + if value { + var.set_update_transform(ShellVariableUpdateTransform::Lowercase); + } else if matches!( + var.get_update_transform(), + ShellVariableUpdateTransform::Lowercase + ) { + var.set_update_transform(ShellVariableUpdateTransform::None); + } + } + if let Some(value) = self.make_nameref.to_bool() { + if value { + var.treat_as_nameref(); + } else { + var.unset_treat_as_nameref(); + } + } + if let Some(value) = self.uppercase_value_on_assignment.to_bool() { + if value { + var.set_update_transform(ShellVariableUpdateTransform::Uppercase); + } else if matches!( + var.get_update_transform(), + ShellVariableUpdateTransform::Uppercase + ) { + var.set_update_transform(ShellVariableUpdateTransform::None); + } + } + } + + /// Applies the `-x`/`+x` flag, if given. + const fn apply_export_flag(&self, var: &mut ShellVariable) { + match self.make_exported.to_bool() { + Some(true) => { + var.export(); + } + Some(false) => { + var.unexport(); + } + None => (), + } + } + + /// Applies the `-t`/`+t` flag, if given. It shapes nothing about the value, so like `-x` and + /// `-r` it is granted only once the assignment has gone through. + const fn apply_trace_flag(&self, var: &mut ShellVariable) { + match self.make_traced.to_bool() { + Some(true) => { + var.enable_trace(); + } + Some(false) => { + var.disable_trace(); + } + None => (), + } + } + + /// Applies the `-r`/`+r` flag, if given. Errors if readonly status cannot be removed. + fn apply_readonly_flag(&self, var: &mut ShellVariable) -> Result<(), brush_core::Error> { + match self.make_readonly.to_bool() { + Some(true) => { + var.set_readonly(); + } + Some(false) => { + var.unset_readonly()?; + } + None => (), + } + + Ok(()) + } +} diff --git a/brush-builtins/src/export.rs b/brush-builtins/src/export.rs index 5e906bd2b..52ecf07d3 100644 --- a/brush-builtins/src/export.rs +++ b/brush-builtins/src/export.rs @@ -1,12 +1,9 @@ use clap::Parser; -use itertools::Itertools; -use std::io::Write; -use brush_core::{ - ExecutionExitCode, ExecutionResult, builtins, - env::{EnvironmentLookup, EnvironmentScope}, - parser::ast, - variables, +use brush_core::builtins; + +use crate::declare::{ + DeclareCommand, DeclareVerb, MakeAssociativeArrayFlag, MakeExportedFlag, MakeIndexedArrayFlag, }; /// Add or update exported shell variables. @@ -24,6 +21,14 @@ pub(crate) struct ExportCommand { #[arg(short = 'p')] display_exported_names: bool, + /// Make the variable an indexed array when assigning to it. + #[arg(short = 'a')] + make_indexed_array: bool, + + /// Make the variable an associative array when assigning to it. + #[arg(short = 'A')] + make_associative_array: bool, + // // Declarations // @@ -43,132 +48,21 @@ impl builtins::Command for ExportCommand { async fn execute( &self, - mut context: brush_core::ExecutionContext<'_, SE>, + context: brush_core::ExecutionContext<'_, SE>, ) -> Result { - if self.declarations.is_empty() { - display_all_exported_vars(&context)?; - return Ok(ExecutionResult::success()); - } - - let mut result = ExecutionResult::success(); - for decl in &self.declarations { - let current_result = self.process_decl(&mut context, decl)?; - if !current_result.is_success() { - result = current_result; - } - } - - Ok(result) - } -} - -impl ExportCommand { - fn process_decl( - &self, - context: &mut brush_core::ExecutionContext<'_, impl brush_core::ShellExtensions>, - decl: &brush_core::CommandArg, - ) -> Result { - match decl { - brush_core::CommandArg::String(s) => { - // See if this is supposed to be a function name. - if self.names_are_functions { - // Try to find the function already present; if we find it, then mark it - // exported. - if let Some(func) = context.shell.func_mut(s) { - if self.unexport { - func.unexport(); - } else { - func.export(); - } - } else { - writeln!(context.stderr(), "{s}: not a function")?; - return Ok(ExecutionExitCode::InvalidUsage.into()); - } - } - // Try to find the variable already present; if we find it, then mark it - // exported. - else if let Some((_, variable)) = context.shell.env_mut().get_mut(s) { - if self.unexport { - variable.unexport(); - } else { - variable.export(); - } - } - } - brush_core::CommandArg::Assignment(assignment) => { - let name = match &assignment.name { - ast::AssignmentName::VariableName(name) => name, - ast::AssignmentName::ArrayElementName(_, _) => { - writeln!(context.stderr(), "not a valid variable name")?; - return Ok(ExecutionExitCode::InvalidUsage.into()); - } - }; - - let value = match &assignment.value { - ast::AssignmentValue::Scalar(s) => { - variables::ShellValueLiteral::Scalar(s.flatten()) - } - ast::AssignmentValue::Array(a) => { - variables::ShellValueLiteral::Array(variables::ArrayLiteral( - a.iter() - .map(|(k, v)| (k.as_ref().map(|k| k.flatten()), v.flatten())) - .collect(), - )) - } - }; - - // `export name+=value` appends to the existing value, exactly like a - // bare `name+=value`. update_or_add always replaces, so when the - // variable already exists honor the append here. A missing variable - // falls through: appending to nothing is a plain assignment. - if assignment.append - && let Some((_, variable)) = context.shell.env_mut().get_mut(name) - { - variable.assign(value, true)?; - if self.unexport { - variable.unexport(); - } else { - variable.export(); - } - return Ok(ExecutionResult::success()); - } - - // Update the variable with the provided value and then mark it exported. - context.shell.env_mut().update_or_add( - name, - value, - |var| { - if self.unexport { - var.unexport(); - } else { - var.export(); - } - Ok(()) - }, - EnvironmentLookup::Anywhere, - EnvironmentScope::Global, - )?; - } - } - - Ok(ExecutionResult::success()) - } -} - -fn display_all_exported_vars( - context: &brush_core::ExecutionContext<'_, impl brush_core::ShellExtensions>, -) -> Result<(), brush_core::Error> { - // Enumerate variables, sorted by key. - for (name, variable) in context.shell.env().iter().sorted_by_key(|v| v.0) { - if variable.is_exported() { - let value = variable.value().try_get_cow_str(context.shell); - if let Some(value) = value { - writeln!(context.stdout(), "declare -x {name}=\"{value}\"")?; - } else { - writeln!(context.stdout(), "declare -x {name}")?; - } + // `export` is `declare` with `-x` implied (`-n` being `+x`) and only a subset of its + // options accepted; this struct exists so that subset is what the command line takes. + DeclareCommand { + function_names_or_defs_only: self.names_are_functions, + print: self.display_exported_names, + make_indexed_array: MakeIndexedArrayFlag::new(self.make_indexed_array.then_some(true)), + make_associative_array: MakeAssociativeArrayFlag::new( + self.make_associative_array.then_some(true), + ), + make_exported: MakeExportedFlag::new(Some(!self.unexport)), + ..DeclareCommand::default() } + .execute_as(DeclareVerb::Export, &self.declarations, context) + .await } - - Ok(()) } diff --git a/brush-builtins/src/factory.rs b/brush-builtins/src/factory.rs index 99eeb8e44..2e73c9f63 100644 --- a/brush-builtins/src/factory.rs +++ b/brush-builtins/src/factory.rs @@ -84,7 +84,7 @@ pub fn default_builtins( #[cfg(feature = "builtin.declare")] m.insert( "readonly".into(), - decl_builtin::().special(), + decl_builtin::().special(), ); #[cfg(feature = "builtin.times")] m.insert( @@ -119,10 +119,7 @@ pub fn default_builtins( #[cfg(all(feature = "builtin.kill", unix))] m.insert("kill".into(), builtin::()); #[cfg(feature = "builtin.declare")] - m.insert( - "local".into(), - decl_builtin::(), - ); + m.insert("local".into(), decl_builtin::()); #[cfg(feature = "builtin.pwd")] m.insert("pwd".into(), builtin::()); #[cfg(feature = "builtin.read")] diff --git a/brush-builtins/src/lib.rs b/brush-builtins/src/lib.rs index 0dd7fab3e..22fc3d4a0 100644 --- a/brush-builtins/src/lib.rs +++ b/brush-builtins/src/lib.rs @@ -35,7 +35,10 @@ mod command; mod complete; #[cfg(feature = "builtin.continue")] mod continue_; -#[cfg(feature = "builtin.declare")] +// `export` is implemented in terms of `declare` (see `export.rs`), so its code is needed +// whenever either builtin is selected. The `declare`/`typeset`/`local`/`readonly` +// registrations stay gated on `builtin.declare` alone. +#[cfg(any(feature = "builtin.declare", feature = "builtin.export"))] mod declare; #[cfg(feature = "builtin.dirs")] mod dirs; @@ -73,6 +76,8 @@ mod jobs; mod kill; #[cfg(feature = "builtin.let")] mod let_; +#[cfg(feature = "builtin.declare")] +mod local; #[cfg(feature = "builtin.mapfile")] mod mapfile; #[cfg(feature = "builtin.popd")] @@ -85,6 +90,8 @@ mod pushd; mod pwd; #[cfg(feature = "builtin.read")] mod read; +#[cfg(feature = "builtin.declare")] +mod readonly; #[cfg(feature = "builtin.return")] mod return_; #[cfg(feature = "builtin.set")] @@ -150,7 +157,7 @@ fn write_alias_definition( #[macro_export] macro_rules! minus_or_plus_flag_arg { ($struct_name:ident, $flag_char:literal, $desc:literal) => { - #[derive(clap::Parser)] + #[derive(clap::Parser, Default)] pub(crate) struct $struct_name { #[arg(short = $flag_char, name = concat!(stringify!($struct_name), "_enable"), action = clap::ArgAction::SetTrue, help = $desc)] _enable: bool, @@ -165,6 +172,16 @@ macro_rules! minus_or_plus_flag_arg { } impl $struct_name { + /// Builds the flag as if `-X` (`Some(true)`), `+X` (`Some(false)`), or neither + /// (`None`) had been given on the command line. + #[allow(dead_code, reason = "may not be used in all macro instantiations")] + pub const fn new(value: Option) -> Self { + Self { + _enable: matches!(value, Some(true)), + _disable: matches!(value, Some(false)), + } + } + #[allow(dead_code, reason = "may not be used in all macro instantiations")] pub const fn is_some(&self) -> bool { self._enable || self._disable diff --git a/brush-builtins/src/local.rs b/brush-builtins/src/local.rs new file mode 100644 index 000000000..865103598 --- /dev/null +++ b/brush-builtins/src/local.rs @@ -0,0 +1,37 @@ +use clap::Parser; + +use brush_core::builtins; + +use crate::declare::{DeclareCommand, DeclareVerb}; + +/// Create and update local variables inside a shell function. +#[derive(Parser)] +#[clap(override_usage = "local [OPTIONS] [DECLARATIONS]...")] +pub(crate) struct LocalCommand { + /// `local` takes every option `declare` does; only the scope rules differ. + #[clap(flatten)] + declare: DeclareCommand, +} + +impl builtins::DeclarationCommand for LocalCommand { + fn set_declarations(&mut self, declarations: Vec) { + self.declare.set_declarations(declarations); + } +} + +impl builtins::Command for LocalCommand { + fn takes_plus_options() -> bool { + true + } + + type Error = brush_core::Error; + + async fn execute( + &self, + context: brush_core::ExecutionContext<'_, SE>, + ) -> Result { + self.declare + .execute_as(DeclareVerb::Local, &self.declare.declarations, context) + .await + } +} diff --git a/brush-builtins/src/readonly.rs b/brush-builtins/src/readonly.rs new file mode 100644 index 000000000..bc85eedad --- /dev/null +++ b/brush-builtins/src/readonly.rs @@ -0,0 +1,62 @@ +use clap::Parser; + +use brush_core::builtins; + +use crate::declare::{DeclareCommand, DeclareVerb, MakeAssociativeArrayFlag, MakeIndexedArrayFlag}; + +/// Mark shell variables or functions as read-only. +#[derive(Parser)] +pub(crate) struct ReadonlyCommand { + /// Names are treated as function names. + #[arg(short = 'f')] + names_are_functions: bool, + + /// Display all read-only names. + #[arg(short = 'p')] + display_readonly_names: bool, + + /// Make the variable an indexed array when assigning to it. + #[arg(short = 'a')] + make_indexed_array: bool, + + /// Make the variable an associative array when assigning to it. + #[arg(short = 'A')] + make_associative_array: bool, + + // + // Declarations + // + // N.B. These are skipped by clap, but filled in by the BuiltinDeclarationCommand trait. + #[clap(skip)] + declarations: Vec, +} + +impl builtins::DeclarationCommand for ReadonlyCommand { + fn set_declarations(&mut self, declarations: Vec) { + self.declarations = declarations; + } +} + +impl builtins::Command for ReadonlyCommand { + type Error = brush_core::Error; + + async fn execute( + &self, + context: brush_core::ExecutionContext<'_, SE>, + ) -> Result { + // `readonly` is `declare` with the readonly attribute implied by the verb and only a + // subset of its options accepted; this struct exists so that subset is what the command + // line takes. + DeclareCommand { + function_names_or_defs_only: self.names_are_functions, + print: self.display_readonly_names, + make_indexed_array: MakeIndexedArrayFlag::new(self.make_indexed_array.then_some(true)), + make_associative_array: MakeAssociativeArrayFlag::new( + self.make_associative_array.then_some(true), + ), + ..DeclareCommand::default() + } + .execute_as(DeclareVerb::Readonly, &self.declarations, context) + .await + } +} diff --git a/brush-builtins/src/unset.rs b/brush-builtins/src/unset.rs index d07759aa7..309928200 100644 --- a/brush-builtins/src/unset.rs +++ b/brush-builtins/src/unset.rs @@ -1,8 +1,8 @@ -use std::borrow::Cow; +use std::io::Write; use clap::Parser; -use brush_core::{ExecutionResult, Shell, builtins}; +use brush_core::{ExecutionParameters, ExecutionResult, Shell, builtins, variables::ArrayKind}; /// Unset a variable. #[derive(Parser)] @@ -51,6 +51,7 @@ impl builtins::Command for UnsetCommand { } let unspecified = self.name_interpretation.unspecified(); + let mut result = ExecutionResult::success(); #[expect(clippy::needless_continue)] for name in &self.names { @@ -60,61 +61,145 @@ impl builtins::Command for UnsetCommand { if let Ok(parameter) = brush_parser::word::parse_parameter(name, &context.shell.parser_options()) { - let result = match parameter { + // The diagnostic below names the variable, not the operand, so an element's + // base name is kept alongside the outcome. + let (target, removed) = match parameter { brush_parser::word::Parameter::Positional(_) => continue, brush_parser::word::Parameter::Special(_) => continue, brush_parser::word::Parameter::Named(name) => { - context.shell.env_mut().unset(name.as_str())?.is_some() + let removed = context.shell.env_mut().unset(name.as_str()); + (name, removed.map(|prev| prev.is_some())) } brush_parser::word::Parameter::NamedWithIndex { name, index } => { - unset_array_index(context.shell, name.as_str(), index.as_str())? + let removed = unset_array_element( + context.shell, + &context.params, + name.as_str(), + index.as_str(), + ) + .await; + (name, removed) } + // `name[*]` and `name[@]` reach the word parser as their own parameter + // kind, but `unset` reads them as ordinary subscripts. brush_parser::word::Parameter::NamedWithAllIndices { - name: _, - concatenate: _, - } => continue, + name, + concatenate, + } => { + let index = if concatenate { "*" } else { "@" }; + let removed = unset_array_element( + context.shell, + &context.params, + name.as_str(), + index, + ) + .await; + (name, removed) + } }; - if result { - continue; + match removed { + Ok(true) => continue, + Ok(false) => (), + // A readonly variable stays, whether the operand named the whole + // variable or one of its elements; the remaining names are still + // processed. + Err(err) + if matches!(err.kind(), brush_core::ErrorKind::ReadonlyVariable) => + { + writeln!( + context.stderr(), + "{}: {target}: cannot unset: readonly variable", + context.command_name + )?; + result = ExecutionResult::general_error(); + continue; + } + // A subscript on something that is not an array fails this name, but + // the remaining names are still processed. + Err(err) if matches!(err.kind(), brush_core::ErrorKind::NotArray) => { + writeln!( + context.stderr(), + "{}: {target}: not an array variable", + context.command_name + )?; + result = ExecutionResult::general_error(); + continue; + } + Err(err) => return Err(err), } } } - // TODO(unset): Deal with readonly functions if unspecified || self.name_interpretation.shell_functions { - if context.shell.undefine_func(name) { - continue; + match context.shell.undefine_func(name) { + Ok(true) => continue, + Ok(false) => (), + // A readonly function stays; the remaining names are still processed. + Err(err) + if matches!(err.kind(), brush_core::ErrorKind::ReadonlyFunction(_)) => + { + writeln!( + context.stderr(), + "{}: {name}: cannot unset: readonly function", + context.command_name + )?; + result = ExecutionResult::general_error(); + } + Err(err) => return Err(err), } } } - Ok(ExecutionResult::success()) + Ok(result) } } -fn unset_array_index( +/// Unsets the element a `name[subscript]` operand names. Returns whether anything was removed. +/// +/// The subscript is resolved the way every other subscript is -- see +/// [`brush_core::Shell::resolve_array_subscript`] -- with the outcomes a shell reserves for +/// `unset` layered on top: an empty subscript names nothing, `*` and `@` name every element of +/// an indexed array (but are ordinary keys of an associative one), and +/// a variable that is not an array behaves as if it were element 0 of itself. +async fn unset_array_element( shell: &mut Shell, + params: &ExecutionParameters, name: &str, index: &str, ) -> Result { - // First check to see if it's an associative array. - let is_assoc_array = shell - .env() - .get(name) - .is_some_and(|(_, var)| var.value().is_associative_array()); - - // Compute which index we should actually use. For indexed arrays, we need to evaluate - // the index string as an arithmetic expression first. - let index_to_use: Cow<'_, str> = if is_assoc_array { - index.into() - } else { - // First evaluate the index expression. - let index_as_expr = brush_parser::arithmetic::parse(index)?; - let evaluated_index = shell.eval_arithmetic(&index_as_expr)?; - evaluated_index.to_string().into() + let Some((_, var)) = shell.env().get(name) else { + return Ok(false); + }; + + // An empty subscript names no element at all; a shell ignores it silently, and does so + // before it would refuse a readonly variable. + if index.is_empty() { + return Ok(true); + } + if var.is_readonly() { + return Err(brush_core::ErrorKind::ReadonlyVariable.into()); + } + + let kind = var.value().array_kind(); + // `*` and `@` name every element of an indexed array, but an associative array can hold + // either as an ordinary key, so there they name one element like any other key does. + if matches!(index, "*" | "@") && kind != Some(ArrayKind::Associative) { + return shell.env_mut().unset_all_indices(name); + } + + let Some(kind) = kind else { + // A variable that is not an array still answers a subscript that evaluates to 0: it is + // its own element 0, and unsetting that unsets the variable. + let index = shell + .resolve_array_subscript(params, index, ArrayKind::Indexed) + .await?; + if index != "0" { + return Err(brush_core::ErrorKind::NotArray.into()); + } + return Ok(shell.env_mut().unset(name)?.is_some()); }; - // Now we can try to unset, and return the result. - shell.env_mut().unset_index(name, index_to_use.as_ref()) + let index = shell.resolve_array_subscript(params, index, kind).await?; + shell.env_mut().unset_index(name, index.as_str()) } diff --git a/brush-core/src/arithmetic.rs b/brush-core/src/arithmetic.rs index cca76f668..299a474a2 100644 --- a/brush-core/src/arithmetic.rs +++ b/brush-core/src/arithmetic.rs @@ -32,9 +32,10 @@ pub enum EvalError { #[error("failed to access array")] FailedToAccessArray, - /// Failed to update the shell environment in an assignment operator. - #[error("failed to update environment")] - FailedToUpdateEnvironment, + /// Failed to update the shell environment in an assignment operator. Carries the reason, + /// which already names the variable it is about (`a: readonly variable`). + #[error("{0}")] + FailedToUpdateEnvironment(String), /// Failed to parse an arithmetic expression. #[error("failed to parse expression: {0}")] @@ -378,7 +379,7 @@ fn assign( env::EnvironmentLookup::Anywhere, env::EnvironmentScope::Global, ) - .map_err(|_err| EvalError::FailedToUpdateEnvironment)?; + .map_err(|err| EvalError::FailedToUpdateEnvironment(err.to_string()))?; } ast::ArithmeticTarget::ArrayElement(name, index_expr) => { let index_str = eval_expr_impl(index_expr, shell, depth)?.to_string(); @@ -393,7 +394,7 @@ fn assign( env::EnvironmentLookup::Anywhere, env::EnvironmentScope::Global, ) - .map_err(|_err| EvalError::FailedToUpdateEnvironment)?; + .map_err(|err| EvalError::FailedToUpdateEnvironment(err.to_string()))?; } } diff --git a/brush-core/src/builtins.rs b/brush-core/src/builtins.rs index 66692b9c3..e8e743bfa 100644 --- a/brush-core/src/builtins.rs +++ b/brush-core/src/builtins.rs @@ -512,8 +512,14 @@ async fn exec_declaration_builtin_impl< for (i, arg) in args.into_iter().enumerate() { match arg { + // A `+X` word is an option only for a builtin that takes plus options; to any other + // (`export`, `readonly`) it is an operand, which the builtin then rejects as an + // invalid identifier, as a shell does. CommandArg::String(s) - if i == 0 || (s.len() > 1 && (s.starts_with('-') || s.starts_with('+'))) => + if i == 0 + || (s.len() > 1 + && (s.starts_with('-') + || (s.starts_with('+') && T::takes_plus_options()))) => { options.push(s); } diff --git a/brush-core/src/commands.rs b/brush-core/src/commands.rs index 6fc537e99..8ac68d1d8 100644 --- a/brush-core/src/commands.rs +++ b/brush-core/src/commands.rs @@ -72,6 +72,20 @@ impl ExecutionContext<'_, SE> { pub fn iter_fds(&self) -> impl Iterator { self.params.iter_fds(self.shell) } + + /// Writes one extra `set -x` trace line on the builtin's own behalf; the interpreter already + /// traces the command itself. `export` and `readonly` use this to echo each assignment they + /// perform. Self-gating: this is a no-op when command tracing is off, so callers need not + /// check. + /// + /// # Arguments + /// + /// * `line` - The trace line, already quoted for display. + pub async fn trace_extra_line(&mut self, line: String) { + if self.shell.options().print_commands_and_arguments { + self.shell.trace_command(&self.params, line).await; + } + } } /// An argument to a command. @@ -81,6 +95,9 @@ pub enum CommandArg { String(String), /// An assignment/declaration; typically treated as a string, but will /// be specially handled by a limited set of built-in commands. + /// + /// Its words are already expanded. Its subscripts are not yet resolved: only the declaration + /// builtin knows whether the target is an indexed or associative array. Assignment(ast::Assignment), } @@ -106,21 +123,56 @@ impl From<&String> for CommandArg { } impl CommandArg { + /// Renders this argument as `set -x` trace text, quoting the whole argument if a shell would + /// need quoting to reproduce it. pub(crate) fn quote_for_tracing(&self) -> Cow<'_, str> { match self { Self::String(s) => escape::quote_if_needed(s, escape::QuoteMode::SingleQuote), - Self::Assignment(a) => { - let mut s = a.name.to_string(); - let op = if a.append { "+=" } else { "=" }; - s.push_str(op); - s.push_str(&escape::quote_if_needed( - a.value.to_string().as_str(), + // A compound operand was already traced on its own line (see + // `compound_assignment_for_tracing`); the command line carries only its name. + Self::Assignment(ast::Assignment { + name, + value: ast::AssignmentValue::Array(_), + .. + }) => Cow::Owned(name.to_string()), + // Traced as one word: `x=a b` becomes `'x=a b'`, not `x='a b'`. + Self::Assignment(assignment) => Cow::Owned( + escape::quote_if_needed( + assignment.to_string().as_str(), escape::QuoteMode::SingleQuote, - )); - s.into() - } + ) + .into_owned(), + ), } } + + /// Returns the standalone `set -x` line a shell traces for a compound assignment operand + /// (`name=(['k']='v' 'w')`, every key and value single-quoted) ahead of the command that + /// carries it; `None` for any other argument. + pub(crate) fn compound_assignment_for_tracing(&self) -> Option { + let Self::Assignment(ast::Assignment { + name, + value: ast::AssignmentValue::Array(elements), + append, + .. + }) = self + else { + return None; + }; + + let quote = + |word: &ast::Word| escape::force_quote(&word.value, escape::QuoteMode::SingleQuote); + let elements = elements + .iter() + .map(|(key, value)| match key { + Some(key) => std::format!("[{}]={}", quote(key), quote(value)), + None => quote(value), + }) + .join(" "); + let op = if *append { "+=" } else { "=" }; + + Some(std::format!("{name}{op}({elements})")) + } } /// Encapsulates a possibly-owned reference to a `Shell` for command execution. @@ -253,15 +305,17 @@ pub fn compose_std_command, SE: extensions::ShellExtensions>( Ok(cmd) } +/// Runs the pre-execution hooks for a command that is about to be executed. `source_text` is the +/// command's text as it appeared in the source, before any expansion. pub(crate) async fn on_preexecute( cmd: &mut commands::SimpleCommand<'_, impl extensions::ShellExtensions>, + source_text: String, ) -> Result<(), error::Error> { - // Set BASH_COMMAND before invoking the DEBUG trap (and generally before - // executing commands). - let full_cmd = cmd.args.iter().map(|arg| arg.to_string()).join(" "); + // Set BASH_COMMAND before invoking the DEBUG trap (and generally before executing commands). + // It reports the command as written, not as expanded. cmd.shell.env_mut().update_or_add( "BASH_COMMAND", - variables::ShellValueLiteral::Scalar(full_cmd), + variables::ShellValueLiteral::Scalar(source_text), |_| Ok(()), env::EnvironmentLookup::Anywhere, env::EnvironmentScope::Global, @@ -566,15 +620,13 @@ pub(crate) fn execute_external_command( argv0_override: Option<&str>, args: &[CommandArg], ) -> Result { - // Filter out the args; we only want strings. + // An assignment-shaped argument reaching an external command is just text; render it. (This + // is only reachable if a declaration builtin was disabled after its arguments were prepared.) let cmd_args = args .iter() - .filter_map(|e| { - if let CommandArg::String(s) = e { - Some(s) - } else { - None - } + .map(|arg| match arg { + CommandArg::String(value) => Cow::Borrowed(OsStr::new(value)), + CommandArg::Assignment(assignment) => Cow::Owned(assignment.to_string().into()), }) .collect::>(); diff --git a/brush-core/src/env.rs b/brush-core/src/env.rs index dffe15aca..4488adbbf 100644 --- a/brush-core/src/env.rs +++ b/brush-core/src/env.rs @@ -7,7 +7,7 @@ use std::collections::hash_map; use crate::Shell; use crate::error; use crate::extensions; -use crate::variables::{self, ShellValue, ShellValueUnsetType, ShellVariable}; +use crate::variables::{self, ArrayKind, ShellValue, ShellValueUnsetType, ShellVariable}; /// Represents the policy for looking up variables in a shell environment. #[derive(Clone, Copy)] @@ -92,8 +92,6 @@ impl Drop for ScopeGuard<'_, SE> { pub struct ShellEnvironment { /// Stack of scopes, with the top of the stack being the current scope. scopes: Vec<(EnvironmentScope, ShellVariableMap)>, - /// Whether or not to auto-export variables on creation or modification. - export_variables_on_modification: bool, /// Count of total entries (may include duplicates with shadowed variables). entry_count: usize, } @@ -109,7 +107,6 @@ impl ShellEnvironment { pub fn new() -> Self { Self { scopes: vec![(EnvironmentScope::Global, ShellVariableMap::default())], - export_variables_on_modification: false, entry_count: 0, } } @@ -346,6 +343,33 @@ impl ShellEnvironment { } } + /// Tries to unset every element of the named array variable. Returns whether any element was + /// removed; a variable that does not exist removes nothing. + /// + /// # Arguments + /// + /// * `name` - The name of the array variable to clear. + pub fn unset_all_indices(&mut self, name: &str) -> Result { + if let Some((_, var)) = self.get_mut(name) { + var.unset_all_indices() + } else { + Ok(false) + } + } + + /// Returns the array kind a subscript on the named variable resolves against: the kind the + /// variable already has, or [`ArrayKind::Indexed`] when there is no such variable or it is + /// not an array -- either way, a subscripted assignment is about to make it an indexed one. + /// + /// # Arguments + /// + /// * `name` - The name of the variable the subscript names. + pub fn subscript_kind(&self, name: &str) -> ArrayKind { + self.get(name) + .and_then(|(_, var)| var.value().array_kind()) + .unwrap_or(ArrayKind::Indexed) + } + fn try_unset_in_map( map: &mut ShellVariableMap, name: &str, @@ -478,19 +502,12 @@ impl ShellEnvironment { ) -> Result<(), error::Error> { let name = name.into(); - let auto_export = self.export_variables_on_modification; if let Some(var) = self.get_mut_using_policy(&name, lookup_policy) { - var.assign(value, false)?; - if auto_export { - var.export(); - } + var.assign(value, false).map_err(name_it(&name))?; updater(var) } else { let mut var = ShellVariable::new(ShellValue::Unset(ShellValueUnsetType::Untyped)); - var.assign(value, false)?; - if auto_export { - var.export(); - } + var.assign(value, false).map_err(name_it(&name))?; updater(&mut var)?; self.add(name, var, scope_if_creating) @@ -519,7 +536,8 @@ impl ShellEnvironment { let name = name.into(); if let Some(var) = self.get_mut_using_policy(&name, lookup_policy) { - var.assign_at_index(index, value, false)?; + var.assign_at_index(index, value, false) + .map_err(name_it(&name))?; updater(var) } else { let mut var = ShellVariable::new(ShellValue::Unset(ShellValueUnsetType::Untyped)); @@ -529,7 +547,8 @@ impl ShellEnvironment { value, )])), false, - )?; + ) + .map_err(name_it(&name))?; updater(&mut var)?; self.add(name, var, scope_if_creating) @@ -546,13 +565,9 @@ impl ShellEnvironment { pub fn add>( &mut self, name: N, - mut var: ShellVariable, + var: ShellVariable, target_scope: EnvironmentScope, ) -> Result<(), error::Error> { - if self.export_variables_on_modification { - var.export(); - } - for (scope_type, map) in self.scopes.iter_mut().rev() { if *scope_type == target_scope { let prev_var = map.set(name, var); @@ -642,6 +657,14 @@ impl ShellVariableMap { } } +/// Returns a mapper that names `name` as the variable an assignment error is about, so it is +/// reported as `name: message`, the way a shell reports a failed assignment. An error already +/// attributed to a variable is left alone, so a caller with more precise context may attach it +/// first. +fn name_it(name: &str) -> impl Fn(error::Error) -> error::Error + '_ { + move |err| err.for_variable(name, None) +} + /// Checks if the given name is a valid variable name. pub fn valid_variable_name(s: &str) -> bool { let mut cs = s.chars(); diff --git a/brush-core/src/error.rs b/brush-core/src/error.rs index 4c0088a3c..c07c7b903 100644 --- a/brush-core/src/error.rs +++ b/brush-core/src/error.rs @@ -4,18 +4,26 @@ use std::path::PathBuf; use crate::{Shell, ShellFd, extensions, results, sys}; -/// Unified error type for this crate. Contains just a kind for now, -/// but will be extended later with additional context. -#[derive(thiserror::Error, Debug)] -#[error("{kind}")] +/// Unified error type for this crate: a kind plus the context the shell has attached to it. +#[derive(Debug)] pub struct Error { /// The kind of error. - #[source] kind: ErrorKind, + /// The variable this error is about, if it was raised by an assignment to or declaration of + /// one; displayed as a `name: ` prefix, as a shell does. See [`Error::for_variable`]. + /// + /// Boxed because it is `None` on all but the assignment paths, and `Error` is returned by + /// value from nearly every function in this crate. + variable: Option>, + /// Whether or not the error should be considered a "fatal" error that would /// result in abnormal exit of a non-interactive shell. fatal: bool, + + /// Whether or not this error arose from a variable assignment; see + /// [`Error::is_assignment_error`]. + from_assignment: bool, } /// Monolithic error type for the shell @@ -29,12 +37,22 @@ pub enum ErrorKind { #[error("cannot assign list to array member")] AssigningListToArrayMember, + /// An array element was named with a subscript no element can have. Carries the element as + /// written (`name[subscript]`). + #[error("{0}: bad array subscript")] + BadArraySubscript(String), + + /// A compound value keyed an indexed array element with `*` or `@`. Carries the element as + /// written (`[key]=value`). + #[error("{0}: cannot assign to non-numeric index")] + AssigningToNonNumericIndex(String), + /// An attempt was made to convert an associative array to an indexed array. - #[error("cannot convert associative array to indexed array")] + #[error("cannot convert associative to indexed array")] ConvertingAssociativeArrayToIndexedArray, /// An attempt was made to convert an indexed array to an associative array. - #[error("cannot convert indexed array to associative array")] + #[error("cannot convert indexed to associative array")] ConvertingIndexedArrayToAssociativeArray, /// An error occurred while sourcing the indicated script file. @@ -157,9 +175,13 @@ pub enum ErrorKind { Utf8Error(#[from] std::str::Utf8Error), /// An attempt was made to modify a readonly variable. - #[error("cannot mutate readonly variable")] + #[error("readonly variable")] ReadonlyVariable, + /// An attempt was made to redefine or unset a readonly function. + #[error("{0}: readonly function")] + ReadonlyFunction(String), + /// The indicated pattern is invalid. #[error("invalid pattern: '{0}'")] InvalidPattern(String), @@ -320,6 +342,31 @@ pub enum ErrorKind { NoMatch(String), } +impl ErrorKind { + /// Returns whether this is a shell's own refusal to perform an assignment, as opposed to + /// brush failing to carry one out (an unimplemented case, an I/O error). Only the former + /// may be turned into an assignment error, which abandons the rest of the command list; + /// see [`Error::is_assignment_error`]. + pub const fn is_assignment_failure(&self) -> bool { + matches!( + self, + Self::ReadonlyVariable + | Self::AssigningListToArrayMember + | Self::ConvertingIndexedArrayToAssociativeArray + | Self::ConvertingAssociativeArrayToIndexedArray + ) || self.is_bad_element_key() + } + + /// Returns whether this names an array element that no element can be: an empty subscript, + /// or `*`/`@` where only a number will do. + pub const fn is_bad_element_key(&self) -> bool { + matches!( + self, + Self::BadArraySubscript(_) | Self::AssigningToNonNumericIndex(_) + ) + } +} + /// Trait implementable by built-in commands to represent errors. pub trait BuiltinError: std::error::Error + ConvertibleToExitCode + Send + Sync { /// Try to extract a reference to the underlying `std::io::Error`, if any. @@ -329,12 +376,22 @@ pub trait BuiltinError: std::error::Error + ConvertibleToExitCode + Send + Sync fn as_io_error(&self) -> Option<&std::io::Error> { None } + + /// Returns whether this is a variable assignment error; see + /// [`Error::is_assignment_error`]. + fn is_assignment_error(&self) -> bool { + false + } } impl BuiltinError for Error { fn as_io_error(&self) -> Option<&std::io::Error> { self.as_io_error() } + + fn is_assignment_error(&self) -> bool { + self.is_assignment_error() + } } /// Helper trait for converting values to exit codes. @@ -394,8 +451,30 @@ where fn from(convertible_to_kind: T) -> Self { Self { kind: convertible_to_kind.into(), + variable: None, fatal: false, + from_assignment: false, + } + } +} + +impl std::fmt::Display for Error { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + if let Some(name) = &self.variable { + write!(f, "{name}: ")?; } + write!(f, "{}", self.kind) + } +} + +impl std::error::Error for Error { + /// The kind's own source, not the kind itself: this error already displays the kind's + /// message, so returning the kind here would make a consumer that prints the whole chain + /// (`anyhow`, `eyre`, `{:#}`) print it twice. Whatever the kind wraps -- an + /// [`std::io::Error`], say -- stays reachable; the kind itself is reachable through + /// [`Error::kind`]. + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + self.kind.source() } } @@ -412,6 +491,55 @@ impl Error { self.fatal } + /// Names the variable this error is about, so it displays as `name: message`, the way a + /// shell reports a failed assignment or declaration. A list assigned to an element is + /// reported against the element (`name[subscript]`); a kind whose message already names its + /// target (a bad subscript, a readonly function) is left alone, as is an error already + /// attributed to a variable. + /// + /// # Arguments + /// + /// * `name` - The variable's name. + /// * `subscript` - The subscript of the element assigned to, as written, if any. + #[must_use] + pub fn for_variable(mut self, name: &str, subscript: Option<&str>) -> Self { + if self.variable.is_some() { + return self; + } + + self.variable = match (&self.kind, subscript) { + ( + ErrorKind::BadArraySubscript(_) + | ErrorKind::AssigningToNonNumericIndex(_) + | ErrorKind::ReadonlyFunction(_), + _, + ) => None, + (ErrorKind::AssigningListToArrayMember, Some(subscript)) => { + Some(std::format!("{name}[{subscript}]").into_boxed_str()) + } + _ => Some(Box::from(name)), + }; + self + } + + /// Marks this error as a variable assignment error. + #[must_use] + pub const fn into_assignment_error(mut self) -> Self { + self.from_assignment = true; + self + } + + /// Returns whether or not this is a variable assignment error: one that a shell reports and + /// then abandons the rest of the current command list for, instead of letting the command + /// that raised it fail on its own. A failed assignment statement (`x=v`, `a=(...)`) is one, + /// and so is a declaration builtin's failure to assign an unquoted compound operand. The + /// mark is visible through a [`ErrorKind::BuiltinError`] wrapper, so wrapping a builtin's + /// error does not hide it. + pub fn is_assignment_error(&self) -> bool { + self.from_assignment + || matches!(&self.kind, ErrorKind::BuiltinError(inner, _) if inner.is_assignment_error()) + } + /// Returns a reference to the error kind. pub const fn kind(&self) -> &ErrorKind { &self.kind @@ -480,3 +608,78 @@ pub fn unimp(msg: &'static str) -> Result { pub fn unimp_with_issue(msg: &'static str, project_issue_id: u32) -> Result { Err(ErrorKind::UnimplementedAndTracked(msg, project_issue_id).into()) } + +#[cfg(test)] +mod tests { + use std::error::Error as _; + + use super::*; + + #[test] + fn error_displays_its_message_once_and_does_not_repeat_it_as_its_source() { + let io_err = std::io::Error::new(std::io::ErrorKind::NotFound, "no such file"); + let err = Error::from(ErrorKind::IoError(io_err)); + + // The kind's message shows up in this error's own message... + assert_eq!(err.to_string(), "i/o error: no such file"); + // ...so the source has to be what the kind wraps, not the kind itself; otherwise a + // consumer that prints the whole chain prints the same text twice. + let source = err.source().unwrap(); + assert_eq!(source.to_string(), "no such file"); + assert!(source.downcast_ref::().is_some()); + assert!(err.as_io_error().is_some()); + } + + #[test] + fn naming_a_variable_prefixes_the_message() { + let err = Error::from(ErrorKind::ReadonlyVariable).for_variable("v", None); + assert_eq!(err.to_string(), "v: readonly variable"); + + // A list assigned to an element is reported against the element. + let err = Error::from(ErrorKind::AssigningListToArrayMember).for_variable("a", Some("i+1")); + assert_eq!( + err.to_string(), + "a[i+1]: cannot assign list to array member" + ); + + // A kind whose message already names its target is left alone. + let err = + Error::from(ErrorKind::BadArraySubscript(String::from("a[]"))).for_variable("a", None); + assert_eq!(err.to_string(), "a[]: bad array subscript"); + } + + #[test] + fn only_a_shells_own_refusal_counts_as_an_assignment_failure() { + for kind in [ + ErrorKind::ReadonlyVariable, + ErrorKind::AssigningListToArrayMember, + ErrorKind::ConvertingIndexedArrayToAssociativeArray, + ErrorKind::ConvertingAssociativeArrayToIndexedArray, + ErrorKind::BadArraySubscript(String::from("a[]")), + ErrorKind::AssigningToNonNumericIndex(String::from("[*]=1")), + ] { + assert!(kind.is_assignment_failure(), "expected refusal: {kind}"); + } + + // brush failing to carry an assignment out is not the shell refusing to perform it, and + // must not be turned into an assignment error: that would abandon the caller's whole + // command list over an unimplemented case or a broken pipe. + for kind in [ + ErrorKind::Unimplemented("misaligned keys/values"), + ErrorKind::IoError(std::io::Error::other("broken pipe")), + ErrorKind::NotArray, + ] { + assert!( + !kind.is_assignment_failure(), + "expected non-refusal: {kind}" + ); + } + } + + #[test] + fn only_an_unusable_element_key_stops_a_compound_value_short() { + assert!(ErrorKind::BadArraySubscript(String::new()).is_bad_element_key()); + assert!(ErrorKind::AssigningToNonNumericIndex(String::new()).is_bad_element_key()); + assert!(!ErrorKind::ReadonlyVariable.is_bad_element_key()); + } +} diff --git a/brush-core/src/expansion.rs b/brush-core/src/expansion.rs index df57104a6..4f0381704 100644 --- a/brush-core/src/expansion.rs +++ b/brush-core/src/expansion.rs @@ -4,7 +4,10 @@ use std::borrow::Cow; use std::cmp::min; use std::io::Write as _; -use brush_parser::word::{ParameterTransformOp, SubstringMatchKind}; +use brush_parser::{ + ast, + word::{ParameterTransformOp, SubstringMatchKind}, +}; use itertools::Itertools; use crate::ExecutionParameters; @@ -21,9 +24,7 @@ use crate::prompt; use crate::shell::Shell; use crate::sys; use crate::trace_categories; -use crate::variables::ShellValueUnsetType; -use crate::variables::ShellVariable; -use crate::variables::{self, ShellValue}; +use crate::variables::{self, ArrayKind, ShellValue, ShellVariable}; /// Controls how the expander handles a backslash-escape sequence (`\X`) /// when it appears outside any explicit quoting (single, double, ANSI-C). @@ -576,7 +577,8 @@ pub(crate) async fn full_expand_and_split_word_with_options( expander.full_expand_with_splitting(word_str.as_ref()).await } -/// Expands a word in assignment context (enables tilde-after-colon expansion). +/// Expands a word in assignment context and returns the resulting text. Assignment context enables +/// tilde expansion after colons and does not perform field splitting. /// /// # Arguments /// @@ -593,6 +595,247 @@ pub(crate) async fn basic_expand_assignment_word( expander.basic_expand_to_str(word_str.as_ref()).await } +/// An assignment whose value is expanded and whose subscripts are resolved against its target's +/// array kind. +pub struct ResolvedAssignment { + /// The expanded assignment. + pub assignment: ast::Assignment, + /// The error of a bad compound key (empty, or `*`/`@` on an indexed array), if one stopped + /// the value short. The elements before it were kept, as in a shell; the caller assigns them + /// and then raises this. + pub stopped_by: Option, +} + +/// Fully expands a raw parsed assignment whose target kind is already known: an ordinary +/// `name=value` statement, or compound elements a declaration builtin has just recognized inside +/// an operand. (A declaration builtin's operands themselves go through +/// [`expand_assignment_words`] instead.) +/// +/// # Arguments +/// +/// * `shell` - The shell environment in which expansions run. +/// * `params` - The execution parameters used by expansions and command substitutions. +/// * `assignment` - A raw parsed assignment whose words have not yet been expanded. +/// * `target` - The array kind that determines how subscripts and compound keys are resolved. +pub(crate) async fn expand_assignment( + shell: &mut Shell, + params: &ExecutionParameters, + assignment: &ast::Assignment, + target: ArrayKind, +) -> Result { + // The value is expanded before any subscript is resolved, so arithmetic side effects in a + // subscript are not visible to the value. + let value = expand_assignment_value(shell, params, &assignment.value).await?; + + let expanded = ast::Assignment { + name: assignment.name.clone(), + value, + append: assignment.append, + loc: assignment.loc.clone(), + }; + resolve_assignment_subscripts(shell, params, expanded, target).await +} + +/// Expands the words of an assignment that appeared as a command argument, without resolving its +/// subscripts: the value is fully expanded, while the subscript and compound keys get ordinary +/// word expansion only. +/// +/// This is the first of the two stages a declaration builtin's operand goes through. The operand +/// is one command word, so all of it is word-expanded before the command runs. Whether a +/// subscript is then an arithmetic index or a literal key depends on the target's array kind, +/// which only the builtin knows once it has read its options; it finishes with +/// [`resolve_assignment_subscripts`]. The subscript therefore ends up expanded twice, as in a +/// shell: with `k='$x'`, `declare name[$k]=v` assigns to `name[$x]`'s value. +/// +/// # Arguments +/// +/// * `shell` - The shell environment in which expansions run. +/// * `params` - The execution parameters used by expansions and command substitutions. +/// * `assignment` - A raw parsed assignment whose words have not yet been expanded. +pub(crate) async fn expand_assignment_words( + shell: &mut Shell, + params: &ExecutionParameters, + assignment: &ast::Assignment, +) -> Result { + let value = expand_assignment_value(shell, params, &assignment.value).await?; + + // The grammar guarantees a literal identifier for the base name; only a subscript expands. + let name = match &assignment.name { + ast::AssignmentName::VariableName(name) => ast::AssignmentName::VariableName(name.clone()), + ast::AssignmentName::ArrayElementName(name, index) => { + ast::AssignmentName::ArrayElementName( + name.clone(), + basic_expand_word(shell, params, index).await?, + ) + } + }; + + Ok(ast::Assignment { + name, + value, + append: assignment.append, + loc: assignment.loc.clone(), + }) +} + +/// Expands an assignment's value. A scalar value and a keyed compound element are expanded as +/// assignment words (no field splitting); an unkeyed compound element is an ordinary word that +/// is field-split into as many elements as it produces. Compound keys get word expansion only; +/// see [`resolve_assignment_subscripts`] for the rest. +async fn expand_assignment_value( + shell: &mut Shell, + params: &ExecutionParameters, + value: &ast::AssignmentValue, +) -> Result { + match value { + ast::AssignmentValue::Scalar(value) => Ok(ast::AssignmentValue::Scalar( + basic_expand_assignment_word(shell, params, value) + .await? + .into(), + )), + ast::AssignmentValue::Array(elements) => { + let mut expanded = vec![]; + for (key, value) in elements { + if let Some(key) = key { + let expanded_key = basic_expand_word(shell, params, key.as_ref()).await?; + let expanded_value = basic_expand_assignment_word(shell, params, value).await?; + expanded.push((Some(expanded_key.into()), expanded_value.into())); + } else { + let values = full_expand_and_split_word(shell, params, value).await?; + expanded.extend(values.into_iter().map(|value| (None, value.into()))); + } + } + + Ok(ast::AssignmentValue::Array(expanded)) + } + } +} + +/// Resolves one array subscript against the kind of the array it names, and returns the index or +/// key it selects, using a fresh expander. +/// +/// Callers layer their own validation on top -- an assignment rejects the subscripts no element +/// can have (see [`expand_assignment_subscript`]), while `unset` quietly ignores them -- but none +/// of them re-decide how a subscript is read; that rule lives only in +/// [`WordExpander::expand_array_index`], which this defers to. +/// +/// # Arguments +/// +/// * `shell` - The shell environment in which expansion and evaluation run. +/// * `params` - The execution parameters used by expansion. +/// * `index` - The subscript, as written after the operand's own word expansion. +/// * `kind` - The array kind that selects arithmetic or literal semantics. +pub(crate) async fn resolve_array_subscript( + shell: &mut Shell, + params: &ExecutionParameters, + index: &str, + kind: ArrayKind, +) -> Result { + WordExpander::new(shell, params) + .expand_array_index(index, kind) + .await +} + +/// Expands one `name[subscript]=` subscript against a known target kind and returns its final +/// index or key. An empty subscript, or `*`/`@` on an indexed array, is a bad array subscript, +/// reported against the element as written. +async fn expand_assignment_subscript( + shell: &mut Shell, + params: &ExecutionParameters, + name: &str, + index: &str, + target: ArrayKind, +) -> Result { + let bad = || error::ErrorKind::BadArraySubscript(std::format!("{name}[{index}]")); + + // A shell checks an indexed subscript's text before evaluating it (`a[$empty]` is element 0, + // `a[]` is an error) and an associative key's text after expanding it. + if matches!(target, ArrayKind::Indexed) && matches!(index, "" | "*" | "@") { + return Err(bad().into()); + } + + let resolved = resolve_array_subscript(shell, params, index, target).await?; + if matches!(target, ArrayKind::Associative) && resolved.is_empty() { + return Err(bad().into()); + } + + Ok(resolved) +} + +/// Resolves one already-word-expanded compound element key: arithmetically for an indexed +/// array, literally for an associative one. A key no element can have is reported against the +/// element as written. +async fn resolve_compound_key( + shell: &mut Shell, + params: &ExecutionParameters, + key: &str, + value: &str, + target: ArrayKind, +) -> Result { + let element = || std::format!("[{key}]={value}"); + match (target, key) { + (_, "") => Err(error::ErrorKind::BadArraySubscript(element()).into()), + (ArrayKind::Indexed, "*" | "@") => { + Err(error::ErrorKind::AssigningToNonNumericIndex(element()).into()) + } + (ArrayKind::Indexed, _) => Ok(arithmetic::expand_and_eval(shell, params, key, false) + .await? + .to_string()), + (ArrayKind::Associative, _) => Ok(key.to_owned()), + } +} + +/// Resolves the subscripts of an assignment whose words were already expanded by +/// [`expand_assignment_words`]; values are left untouched so they are never expanded twice. +/// +/// A bad `name[subscript]=` subscript fails the whole assignment. A bad compound key stops the +/// value at that element, as in a shell: the elements before it are kept, the rest dropped, and +/// the key's error is returned in [`ResolvedAssignment::stopped_by`]. +/// +/// # Arguments +/// +/// * `shell` - The shell environment in which subscript expansion and evaluation run. +/// * `params` - The execution parameters used by subscript expansion. +/// * `assignment` - The already-word-expanded assignment whose subscripts should be resolved. +/// * `target` - The array kind that selects arithmetic or literal subscript semantics. +pub(crate) async fn resolve_assignment_subscripts( + shell: &mut Shell, + params: &ExecutionParameters, + mut assignment: ast::Assignment, + target: ArrayKind, +) -> Result { + if let ast::AssignmentName::ArrayElementName(name, index) = &mut assignment.name { + *index = expand_assignment_subscript(shell, params, name, index, target).await?; + } + + let mut stopped_by = None; + if let ast::AssignmentValue::Array(elements) = &mut assignment.value { + for (i, (key, value)) in elements.iter_mut().enumerate() { + let Some(key) = key else { + continue; + }; + match resolve_compound_key(shell, params, &key.value, &value.value, target).await { + Ok(resolved) => key.value = resolved, + // A key no element can have stops the value here; anything else (an arithmetic + // error, say) fails the whole expansion. + Err(err) if err.kind().is_bad_element_key() => { + stopped_by = Some((i, err)); + break; + } + Err(err) => return Err(err), + } + } + if let Some((i, _)) = &stopped_by { + elements.truncate(*i); + } + } + + Ok(ResolvedAssignment { + assignment, + stopped_by: stopped_by.map(|(_, err)| err), + }) +} + /// Assigns a value to a named parameter. /// /// # Arguments @@ -1751,19 +1994,8 @@ impl<'a, SE: extensions::ShellExtensions> WordExpander<'a, SE> { let (variable_name, index) = match parameter { brush_parser::word::Parameter::Named(name) => (name, None), brush_parser::word::Parameter::NamedWithIndex { name, index } => { - let is_set_assoc_array = if let Some((_, var)) = self.shell.env().get(name) { - matches!( - var.value(), - ShellValue::AssociativeArray(_) - | ShellValue::Unset(ShellValueUnsetType::AssociativeArray) - ) - } else { - false - }; - - let index_to_use = self - .expand_array_index(index.as_str(), is_set_assoc_array) - .await?; + let kind = self.shell.env().subscript_kind(name); + let index_to_use = self.expand_array_index(index.as_str(), kind).await?; (name, Some(index_to_use)) } brush_parser::word::Parameter::Positional(_) @@ -1778,7 +2010,7 @@ impl<'a, SE: extensions::ShellExtensions> WordExpander<'a, SE> { let value = value.into(); - if let Some(index) = index { + let result = if let Some(index) = index { self.shell.env_mut().update_or_add_array_element( variable_name, index, @@ -1795,7 +2027,9 @@ impl<'a, SE: extensions::ShellExtensions> WordExpander<'a, SE> { env::EnvironmentLookup::Anywhere, env::EnvironmentScope::Global, ) - } + }; + + result.map_err(|err| err.for_variable(variable_name, None)) } async fn try_resolve_parameter_to_variable( @@ -1929,21 +2163,9 @@ impl<'a, SE: extensions::ShellExtensions> WordExpander<'a, SE> { } } brush_parser::word::Parameter::NamedWithIndex { name, index } => { - // First check to see if it's an associative array. - let is_set_assoc_array = if let Some((_, var)) = self.shell.env().get(name) { - matches!( - var.value(), - ShellValue::AssociativeArray(_) - | ShellValue::Unset(ShellValueUnsetType::AssociativeArray) - ) - } else { - false - }; - - // Figure out which index to use. - let index_to_use = self - .expand_array_index(index.as_str(), is_set_assoc_array) - .await?; + // The array kind of the target governs how the index is expanded. + let kind = self.shell.env().subscript_kind(name); + let index_to_use = self.expand_array_index(index.as_str(), kind).await?; // Index into the array. if let Some((_, var)) = self.shell.env().get(name) @@ -2002,20 +2224,34 @@ impl<'a, SE: extensions::ShellExtensions> WordExpander<'a, SE> { } } + /// Resolves one array subscript against the kind of the array it names, and returns the + /// index or key it selects. An indexed array's subscript is an arithmetic expression + /// (expanded, then evaluated); an associative array's is a literal key (expanded only). + /// + /// This is the one place that rule lives. A subscript reached from outside an expansion -- + /// an assignment's, or `unset`'s -- comes here through [`resolve_array_subscript`]; one + /// reached from inside a parameter reference comes here directly, so that the literal key's + /// expansion inherits the surrounding expander's settings. + /// + /// # Arguments + /// + /// * `index` - The subscript, as written after the operand's own word expansion. + /// * `kind` - The array kind that selects arithmetic or literal semantics. async fn expand_array_index( &mut self, index: &str, - for_set_associative_array: bool, + kind: ArrayKind, ) -> Result { - let index_to_use = if for_set_associative_array { - self.basic_expand_to_str(index).await? - } else { - arithmetic::expand_and_eval(self.shell, self.params, index, false) - .await? - .to_string() - }; - - Ok(index_to_use) + match kind { + ArrayKind::Associative => self.basic_expand_to_str(index).await, + ArrayKind::Indexed => { + Ok( + arithmetic::expand_and_eval(self.shell, self.params, index, false) + .await? + .to_string(), + ) + } + } } fn expand_special_parameter( diff --git a/brush-core/src/functions.rs b/brush-core/src/functions.rs index 3815cc1a3..d8095fcc3 100644 --- a/brush-core/src/functions.rs +++ b/brush-core/src/functions.rs @@ -69,6 +69,13 @@ pub struct Registration { source_info: crate::SourceInfo, /// Whether or not this function definition should be exported to children. exported: bool, + /// Whether or not this function may be redefined or unset. + #[cfg_attr(feature = "serde", serde(default))] + readonly: bool, + /// Whether or not this function inherits the DEBUG and RETURN traps. The attribute is + /// recorded and displayed (`declare -ft`) but not yet honored by trap handling. + #[cfg_attr(feature = "serde", serde(default))] + traced: bool, } impl From for Registration { @@ -77,6 +84,8 @@ impl From for Registration { definition: Arc::new(definition), source_info: crate::SourceInfo::default(), exported: false, + readonly: false, + traced: false, } } } @@ -96,6 +105,8 @@ impl Registration { definition: Arc::new(definition), source_info: source_info.clone(), exported: false, + readonly: false, + traced: false, } } @@ -123,4 +134,45 @@ impl Registration { pub const fn is_exported(&self) -> bool { self.exported } + + /// Marks the function readonly: it can no longer be redefined or unset. + pub const fn set_readonly(&mut self) { + self.readonly = true; + } + + /// Returns whether this function is readonly. + pub const fn is_readonly(&self) -> bool { + self.readonly + } + + /// Enables tracing for the function. + pub const fn enable_trace(&mut self) { + self.traced = true; + } + + /// Disables tracing for the function. + pub const fn disable_trace(&mut self) { + self.traced = false; + } + + /// Returns whether tracing is enabled for this function. + pub const fn is_trace_enabled(&self) -> bool { + self.traced + } + + /// Returns the canonical attribute flag string for this function, as displayed by + /// `declare -F`: `f` followed by the attributes the function carries. + pub fn attribute_flags(&self) -> String { + let mut flags = String::from("f"); + if self.readonly { + flags.push('r'); + } + if self.traced { + flags.push('t'); + } + if self.exported { + flags.push('x'); + } + flags + } } diff --git a/brush-core/src/interp.rs b/brush-core/src/interp.rs index b2d342ddc..9a61d947a 100644 --- a/brush-core/src/interp.rs +++ b/brush-core/src/interp.rs @@ -5,7 +5,7 @@ use std::collections::VecDeque; use std::io::Write; use std::path::{Path, PathBuf}; -use crate::arithmetic::{self, ExpandAndEvaluate}; +use crate::arithmetic::ExpandAndEvaluate; use crate::commands::{self, CommandArg}; use crate::env::{EnvironmentLookup, EnvironmentScope, valid_variable_name}; use crate::openfiles::{OpenFile, OpenFiles}; @@ -13,9 +13,7 @@ use crate::results::{ ExecutionExitCode, ExecutionResult, ExecutionSpawnResult, ExecutionWaitResult, }; use crate::shell::Shell; -use crate::variables::{ - ArrayLiteral, ShellValue, ShellValueLiteral, ShellValueUnsetType, ShellVariable, -}; +use crate::variables::{ArrayLiteral, ShellValue, ShellValueLiteral, ShellVariable}; use crate::{ ShellFd, error, expansion, extendedtests, extensions, ioutils, jobs, openfiles, sys, timing, }; @@ -1100,7 +1098,7 @@ impl Execute for ast::FunctionDefinition { async fn execute( &self, shell: &mut Shell, - _params: &ExecutionParameters, + params: &ExecutionParameters, ) -> Result { let func_name = self.fname.value.clone(); @@ -1127,9 +1125,17 @@ impl Execute for ast::FunctionDefinition { .map_or_else(crate::SourceInfo::default, |frame| { frame.adjusted_source_info() }); - shell.define_func(func_name, self.clone(), &source_info); - - let result = ExecutionResult::success(); + // A readonly function refuses redefinition. That is reported here rather than + // propagated, so the rest of the command list still runs, as in a shell. It is written + // as the bare message, like the diagnostics the declaration builtins emit. + let result = match shell.define_func(func_name, self.clone(), &source_info) { + Ok(()) => ExecutionResult::success(), + Err(err) if matches!(err.kind(), error::ErrorKind::ReadonlyFunction(_)) => { + writeln!(params.stderr(shell), "{err}")?; + ExecutionResult::general_error() + } + Err(err) => return Err(err), + }; shell.set_last_exit_status(result.exit_code.into()); Ok(result) @@ -1193,10 +1199,28 @@ impl ExecuteInPipeline for ast::SimpleComma if command_takes_assignments { // This looks like an assignment, and the command being invoked is a // well-known builtin that takes arguments that need to function like - // assignments (but which are processed by the builtin). - let expanded = - expand_assignment(&mut context.shell, ¶ms, assignment).await?; - args.push(CommandArg::Assignment(expanded)); + // assignments (but which are processed by the builtin). Expand its + // words now, exactly as a shell does before the command runs; the + // builtin resolves the subscripts later, once it knows the target + // array type. + let expanded = CommandArg::Assignment( + expansion::expand_assignment_words( + &mut context.shell, + ¶ms, + assignment, + ) + .await?, + ); + + // A compound operand is traced on its own line as it is expanded, + // ahead of the command's own trace line. + if context.shell.options().print_commands_and_arguments + && let Some(line) = expanded.compound_assignment_for_tracing() + { + context.shell.trace_command(¶ms, line).await; + } + + args.push(expanded); } else { // This *looks* like an assignment, but it's really a string we should // fully treat as a regular looking @@ -1287,8 +1311,20 @@ impl ExecuteInPipeline for ast::SimpleComma process_group_id: context.process_group_id, }; - match execute_command(context, params, cmd_name, &assignments, &args).await { + match execute_command( + context, + params, + cmd_name, + self.to_string(), + &assignments, + &args, + ) + .await + { Ok(result) => Ok(result), + // A variable assignment error raised by a declaration builtin abandons the rest + // of the command list, exactly as one raised by an assignment statement does. + Err(err) if err.is_assignment_error() => Err(err), Err(err) => { let _ = parent_shell.display_error(&mut stderr, &err); @@ -1305,9 +1341,7 @@ impl ExecuteInPipeline for ast::SimpleComma assignment, &mut context.shell, ¶ms, - false, - None, - EnvironmentScope::Global, + AssignmentTarget::Shell, ) .await?; } @@ -1337,6 +1371,7 @@ async fn execute_command>( mut context: PipelineExecutionContext<'_, impl extensions::ShellExtensions>, params: ExecutionParameters, cmd_name: T, + source_text: String, assignments: &[&ast::Assignment], args: &[CommandArg], ) -> Result { @@ -1351,9 +1386,7 @@ async fn execute_command>( assignment, guard.shell(), ¶ms, - true, - Some(EnvironmentScope::Command), - EnvironmentScope::Command, + AssignmentTarget::CommandEnvironment, ) .await?; } @@ -1380,7 +1413,7 @@ async fn execute_command>( cmd.post_execute = Some(|shell| shell.env_mut().pop_scope(EnvironmentScope::Command)); // Run through any pre-execution hooks as best effort. - let _ = commands::on_preexecute(&mut cmd).await; + let _ = commands::on_preexecute(&mut cmd, source_text).await; // Execute // TODO(jobs): do we need to move self back to foreground on error here? @@ -1426,202 +1459,128 @@ async fn expand_words( Ok(fields) } -async fn expand_assignment( - shell: &mut Shell, - params: &ExecutionParameters, - assignment: &ast::Assignment, -) -> Result { - let value = expand_assignment_value(shell, params, &assignment.value).await?; - Ok(ast::Assignment { - name: basic_expand_assignment_name(shell, params, &assignment.name).await?, - value, - append: assignment.append, - loc: assignment.loc.clone(), - }) +/// Where an assignment binds, and how the variable it binds is marked. The interpreter performs +/// exactly these two kinds of assignment. +#[derive(Clone, Copy, Eq, PartialEq)] +enum AssignmentTarget { + /// An assignment statement (`x=1`): it updates the variable wherever it is in scope, creates + /// a global if there is none, and exports only when `set -a` says to. + Shell, + /// A `name=value` prefix on a command: it lives in the command's own scope, shadowing + /// anything of the same name, and is exported to the command for its duration. + CommandEnvironment, } -async fn basic_expand_assignment_name( - shell: &mut Shell, - params: &ExecutionParameters, - name: &ast::AssignmentName, -) -> Result { - match name { - ast::AssignmentName::VariableName(name) => { - let expanded = expansion::basic_expand_word(shell, params, name).await?; - Ok(ast::AssignmentName::VariableName(expanded)) - } - ast::AssignmentName::ArrayElementName(name, index) => { - let expanded_name = expansion::basic_expand_word(shell, params, name).await?; - let expanded_index = expansion::basic_expand_word(shell, params, index).await?; - Ok(ast::AssignmentName::ArrayElementName( - expanded_name, - expanded_index, - )) +impl AssignmentTarget { + /// The scope an existing variable must live in for this assignment to update it rather than + /// create a new one; `None` updates whichever variable is in scope. + const fn required_scope(self) -> Option { + match self { + Self::Shell => None, + Self::CommandEnvironment => Some(EnvironmentScope::Command), } } -} -async fn expand_assignment_value( - shell: &mut Shell, - params: &ExecutionParameters, - value: &ast::AssignmentValue, -) -> Result { - let expanded = match value { - ast::AssignmentValue::Scalar(s) => { - let expanded_word = expansion::basic_expand_assignment_word(shell, params, s).await?; - ast::AssignmentValue::Scalar(ast::Word::from(expanded_word)) - } - ast::AssignmentValue::Array(arr) => { - let mut expanded_values = vec![]; - for (key, value) in arr { - if let Some(k) = key { - let expanded_key = expansion::basic_expand_assignment_word(shell, params, k) - .await? - .into(); - let expanded_value = - expansion::basic_expand_assignment_word(shell, params, value) - .await? - .into(); - expanded_values.push((Some(expanded_key), expanded_value)); - } else { - // Array elements are treated as regular words, not assignments - let split_expanded_value = - expansion::full_expand_and_split_word(shell, params, value).await?; - for expanded_value in split_expanded_value { - expanded_values.push((None, expanded_value.into())); - } - } - } - - ast::AssignmentValue::Array(expanded_values) + /// The scope a newly created variable is added to. + const fn creation_scope(self) -> EnvironmentScope { + match self { + Self::Shell => EnvironmentScope::Global, + Self::CommandEnvironment => EnvironmentScope::Command, } - }; - - Ok(expanded) + } } -#[expect(clippy::too_many_lines)] async fn apply_assignment( assignment: &ast::Assignment, shell: &mut Shell, params: &ExecutionParameters, - mut export: bool, - required_scope: Option, - creation_scope: EnvironmentScope, + target_scope: AssignmentTarget, ) -> Result<(), error::Error> { - // Figure out if we are trying to assign to a variable or assign to an element of an existing - // array. - let mut array_index; - let variable_name = match &assignment.name { - ast::AssignmentName::VariableName(name) => { - array_index = None; - name - } - ast::AssignmentName::ArrayElementName(name, index) => { - let expanded = expansion::basic_expand_word(shell, params, index).await?; - array_index = Some(expanded); - name - } - }; - - // Expand the values. - let new_value = match &assignment.value { - ast::AssignmentValue::Scalar(unexpanded_value) => { - let value = - expansion::basic_expand_assignment_word(shell, params, unexpanded_value).await?; - ShellValueLiteral::Scalar(value) - } - ast::AssignmentValue::Array(unexpanded_values) => { - let mut elements = vec![]; - for (unexpanded_key, unexpanded_value) in unexpanded_values { - let key = match unexpanded_key { - Some(unexpanded_key) => Some( - expansion::basic_expand_assignment_word(shell, params, unexpanded_key) - .await?, - ), - None => None, - }; - - if key.is_some() { - let value = - expansion::basic_expand_assignment_word(shell, params, unexpanded_value) - .await?; - elements.push((key, value)); - } else { - // Array elements are treated as regular words, not assignments - let values = - expansion::full_expand_and_split_word(shell, params, unexpanded_value) - .await?; - for value in values { - elements.push((None, value)); - } - } - } - ShellValueLiteral::Array(ArrayLiteral(elements)) - } + // Base names are never expanded, so this stays valid for the expanded assignment below. + let variable_name = assignment.name.base_name(); + let target = shell.env().subscript_kind(variable_name); + let expansion::ResolvedAssignment { + assignment: expanded, + stopped_by, + } = shell.expand_assignment(params, assignment, target).await?; + let ast::Assignment { + name: expanded_name, + value: expanded_value, + append, + .. + } = expanded; + + let array_index = match expanded_name { + ast::AssignmentName::VariableName(_) => None, + ast::AssignmentName::ArrayElementName(_, index) => Some(index), }; + let new_value = ShellValueLiteral::from(expanded_value); if shell.options().print_commands_and_arguments { - let op = if assignment.append { "+=" } else { "=" }; - shell - .trace_command(params, std::format!("{}{op}{new_value}", assignment.name)) - .await; + // A shell traces a scalar assignment with its expanded value under the name as written + // (subscript included), and a compound assignment entirely as written. + let line = if matches!(assignment.value, ast::AssignmentValue::Array(_)) { + assignment.to_string() + } else { + let op = if append { "+=" } else { "=" }; + std::format!("{}{op}{new_value}", assignment.name) + }; + shell.trace_command(params, line).await; } - // See if we need to eval an array index. - if let Some(idx) = &array_index { - // An array subscript is arithmetically evaluated unless the target is an - // associative array (in which case the subscript is used as a literal key). - // A scalar or unset/untyped variable becomes an indexed array, so its - // subscript still needs to be evaluated. - let will_be_indexed_array = - if let Some((_, existing_value)) = shell.env().get(variable_name) { - !matches!( - existing_value.value(), - ShellValue::AssociativeArray(_) - | ShellValue::Unset(ShellValueUnsetType::AssociativeArray) - ) - } else { - true - }; + // A failure here is an assignment error, reported against the variable as written. + let written_subscript = match &assignment.name { + ast::AssignmentName::VariableName(_) => None, + ast::AssignmentName::ArrayElementName(_, index) => Some(index.as_str()), + }; + let as_assignment_error = |err: error::Error| { + err.for_variable(variable_name, written_subscript) + .into_assignment_error() + }; - if will_be_indexed_array { - array_index = Some( - arithmetic::expand_and_eval(shell, params, idx.as_str(), false) - .await? - .to_string(), - ); - } - } + bind_assignment( + shell, + variable_name, + array_index, + new_value, + append, + target_scope, + ) + .map_err(as_assignment_error)?; + // The elements before a bad compound key are now assigned; raise the key's error. + stopped_by.map_or(Ok(()), |err| Err(as_assignment_error(err))) +} + +/// Binds an already-expanded value to a variable, creating the variable if necessary. Kept +/// separate from [`apply_assignment`] because binding an existing variable has to return while +/// the environment is still mutably borrowed. +fn bind_assignment( + shell: &mut Shell, + variable_name: &str, + array_index: Option, + new_value: ShellValueLiteral, + append: bool, + target_scope: AssignmentTarget, +) -> Result<(), error::Error> { // Read option before taking mutable borrow on env. let export_variables_on_modification = shell.options().export_variables_on_modification; + let mut export = target_scope == AssignmentTarget::CommandEnvironment; + let required_scope = target_scope.required_scope(); // See if we can find an existing value associated with the variable. - if let Some((existing_value_scope, existing_value)) = - shell.env_mut().get_mut(variable_name.as_str()) - { + if let Some((existing_value_scope, existing_value)) = shell.env_mut().get_mut(variable_name) { if required_scope.is_none() || Some(existing_value_scope) == required_scope { - if let Some(array_index) = array_index { - match new_value { - ShellValueLiteral::Scalar(s) => { - existing_value.assign_at_index(array_index, s, assignment.append)?; - } - ShellValueLiteral::Array(_) => { - return error::unimp("replacing an array item with an array"); - } - } - } else { - if !export - && export_variables_on_modification - && !matches!(new_value, ShellValueLiteral::Array(_)) - { - export = true; - } - - existing_value.assign(new_value, assignment.append)?; + if array_index.is_none() + && !export + && export_variables_on_modification + && !matches!(new_value, ShellValueLiteral::Array(_)) + { + export = true; } + existing_value.assign_at(array_index, new_value, append)?; + if export { existing_value.export(); } @@ -1638,13 +1597,13 @@ async fn apply_assignment( ShellValue::indexed_array_from_literals(ArrayLiteral(vec![(Some(array_index), s)])) } ShellValueLiteral::Array(_) => { - return error::unimp("cannot assign list to array member"); + return Err(error::ErrorKind::AssigningListToArrayMember.into()); } } } else { match new_value { ShellValueLiteral::Scalar(s) => { - export = export || shell.options().export_variables_on_modification; + export = export || export_variables_on_modification; ShellValue::String(s) } ShellValueLiteral::Array(values) => ShellValue::indexed_array_from_literals(values), @@ -1657,7 +1616,9 @@ async fn apply_assignment( new_var.export(); } - shell.env_mut().add(variable_name, new_var, creation_scope) + shell + .env_mut() + .add(variable_name, new_var, target_scope.creation_scope()) } #[expect(clippy::too_many_lines)] diff --git a/brush-core/src/shell/expansion.rs b/brush-core/src/shell/expansion.rs index cc48b0a60..93bd4b9fd 100644 --- a/brush-core/src/shell/expansion.rs +++ b/brush-core/src/shell/expansion.rs @@ -2,7 +2,9 @@ use std::borrow::Cow; -use crate::{error, expansion, extensions, interp::ExecutionParameters}; +use brush_parser::ast; + +use crate::{error, expansion, extensions, interp::ExecutionParameters, variables::ArrayKind}; impl crate::Shell { /// Returns the current value of the IFS variable, or the default value if it is not set. @@ -43,4 +45,55 @@ impl crate::Shell { let result = expansion::full_expand_and_split_word(self, params, s.as_ref()).await?; Ok(result) } + + /// Expands a raw parsed assignment and resolves its subscripts against `target`. See + /// `expansion::expand_assignment`. + /// + /// # Arguments + /// + /// * `params` - The execution parameters to use during expansion. + /// * `assignment` - The parsed assignment to expand. + /// * `target` - The target array type controlling subscript expansion. + pub async fn expand_assignment( + &mut self, + params: &ExecutionParameters, + assignment: &ast::Assignment, + target: ArrayKind, + ) -> Result { + expansion::expand_assignment(self, params, assignment, target).await + } + + /// Resolves one array subscript against the kind of the array it names. See + /// `expansion::resolve_array_subscript`. + /// + /// # Arguments + /// + /// * `params` - The execution parameters to use during expansion. + /// * `index` - The subscript, as written after the operand's own word expansion. + /// * `kind` - The target array type controlling subscript expansion. + pub async fn resolve_array_subscript( + &mut self, + params: &ExecutionParameters, + index: &str, + kind: ArrayKind, + ) -> Result { + expansion::resolve_array_subscript(self, params, index, kind).await + } + + /// Resolves the subscripts of an assignment whose words were already expanded, leaving its + /// values untouched. See `expansion::resolve_assignment_subscripts`. + /// + /// # Arguments + /// + /// * `params` - The execution parameters to use during expansion. + /// * `assignment` - The already-word-expanded assignment to resolve. + /// * `target` - The target array type controlling subscript expansion. + pub async fn resolve_assignment_subscripts( + &mut self, + params: &ExecutionParameters, + assignment: ast::Assignment, + target: ArrayKind, + ) -> Result { + expansion::resolve_assignment_subscripts(self, params, assignment, target).await + } } diff --git a/brush-core/src/shell/funcs.rs b/brush-core/src/shell/funcs.rs index f3fd06c12..936b6318a 100644 --- a/brush-core/src/shell/funcs.rs +++ b/brush-core/src/shell/funcs.rs @@ -17,17 +17,22 @@ impl crate::Shell { } /// Tries to undefine a function in the shell's environment. Returns whether or - /// not a definition was removed. + /// not a definition was removed. A readonly function is refused. /// /// # Arguments /// /// * `name` - The name of the function to undefine. - pub fn undefine_func(&mut self, name: &str) -> bool { - self.funcs.remove(name).is_some() + pub fn undefine_func(&mut self, name: &str) -> Result { + if self.funcs.get(name).is_some_and(|reg| reg.is_readonly()) { + return Err(error::ErrorKind::ReadonlyFunction(name.to_owned()).into()); + } + + Ok(self.funcs.remove(name).is_some()) } /// Defines a function in the shell's environment. If a function already exists - /// with the given name, it is replaced with the new definition. + /// with the given name, it is replaced with the new definition -- unless it is + /// readonly, in which case the definition is refused. /// /// # Arguments /// @@ -39,9 +44,15 @@ impl crate::Shell { name: impl Into, definition: brush_parser::ast::FunctionDefinition, source_info: &crate::SourceInfo, - ) { + ) -> Result<(), error::Error> { + let name = name.into(); + if self.funcs.get(&name).is_some_and(|reg| reg.is_readonly()) { + return Err(error::ErrorKind::ReadonlyFunction(name).into()); + } + let reg = functions::Registration::new(definition, source_info); - self.funcs.update(name.into(), reg); + self.funcs.update(name, reg); + Ok(()) } /// Tries to return a mutable reference to the registration for a named function. @@ -79,9 +90,7 @@ impl crate::Shell { body: func_body, }; - self.define_func(name, def, &crate::SourceInfo::default()); - - Ok(()) + self.define_func(name, def, &crate::SourceInfo::default()) } /// Invokes a function defined in this shell, returning its execution result. diff --git a/brush-core/src/variables.rs b/brush-core/src/variables.rs index d8302be8e..d6a50c69d 100644 --- a/brush-core/src/variables.rs +++ b/brush-core/src/variables.rs @@ -187,41 +187,79 @@ impl ShellVariable { self } - /// Converts the variable to an indexed array. - pub fn convert_to_indexed_array(&mut self) -> Result<(), error::Error> { - match self.value() { - ShellValue::IndexedArray(_) => Ok(()), - ShellValue::AssociativeArray(_) => { - Err(error::ErrorKind::ConvertingAssociativeArrayToIndexedArray.into()) + /// Converts the variable to the given array kind. A readonly variable refuses the request + /// outright -- even a re-selection of the kind it already has -- and that refusal takes + /// precedence over a kind conflict, as in a shell. Otherwise, selecting the kind the + /// variable already has is a no-op and converting between the two array kinds is an error. + /// Anything else retypes the variable: a declared-but-unset variable stays + /// declared-but-unset with the new kind, and a set scalar becomes an array per the given + /// policy. + pub fn convert_to_array_kind( + &mut self, + kind: ArrayKind, + scalar_policy: ScalarConversionPolicy, + ) -> Result<(), error::Error> { + if self.is_readonly() { + return Err(error::ErrorKind::ReadonlyVariable.into()); + } + + match self.value().array_kind() { + Some(existing) if existing == kind => Ok(()), + Some(_) => Err(match kind { + ArrayKind::Indexed => error::ErrorKind::ConvertingAssociativeArrayToIndexedArray, + ArrayKind::Associative => { + error::ErrorKind::ConvertingIndexedArrayToAssociativeArray + } } - _ => { - let mut new_values = BTreeMap::new(); - new_values.insert( - 0, - self.value.to_cow_str_without_dynamic_support().to_string(), - ); - self.value = ShellValue::IndexedArray(new_values); + .into()), + None => { + self.value = if matches!(self.value, ShellValue::Unset(_)) { + ShellValue::Unset(kind.into()) + } else { + let promoted = + matches!(scalar_policy, ScalarConversionPolicy::PromoteToElementZero) + .then(|| self.value.to_cow_str_without_dynamic_support().to_string()); + match kind { + ArrayKind::Indexed => { + ShellValue::IndexedArray(promoted.map(|v| (0, v)).into_iter().collect()) + } + ArrayKind::Associative => ShellValue::AssociativeArray( + promoted + .map(|v| (String::from("0"), v)) + .into_iter() + .collect(), + ), + } + }; + Ok(()) } } } - /// Converts the variable to an associative array. - pub fn convert_to_associative_array(&mut self) -> Result<(), error::Error> { - match self.value() { - ShellValue::AssociativeArray(_) => Ok(()), - ShellValue::IndexedArray(_) => { - Err(error::ErrorKind::ConvertingIndexedArrayToAssociativeArray.into()) + /// Assigns the given value to the variable, targeting one array element when an + /// (already-resolved) subscript is given. Assigning a list to a single element is an error, + /// as in a shell. + /// + /// # Arguments + /// + /// * `index` - The resolved subscript of the element to assign to, if any. + /// * `value` - The value to assign. + /// * `append` - Whether or not to append the value to the preexisting value. + pub fn assign_at( + &mut self, + index: Option, + value: ShellValueLiteral, + append: bool, + ) -> Result<(), error::Error> { + match (index, value) { + (Some(index), ShellValueLiteral::Scalar(value)) => { + self.assign_at_index(index, value, append) } - _ => { - let mut new_values: BTreeMap = BTreeMap::new(); - new_values.insert( - String::from("0"), - self.value.to_cow_str_without_dynamic_support().to_string(), - ); - self.value = ShellValue::AssociativeArray(new_values); - Ok(()) + (Some(_), ShellValueLiteral::Array(_)) => { + Err(error::ErrorKind::AssigningListToArrayMember.into()) } + (None, value) => self.assign(value, append), } } @@ -261,7 +299,10 @@ 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_array_kind( + ArrayKind::Indexed, + ScalarConversionPolicy::PromoteToElementZero, + )?; } _ => (), } @@ -380,12 +421,22 @@ impl ShellVariable { value: String, append: bool, ) -> Result<(), error::Error> { + // Readonly is enforced here and not only in `assign`, so that every path reaching an + // element -- a subscripted assignment, a declaration builtin, arithmetic, or an + // assignment expansion -- is blocked, not just whole-variable assignment. + if self.is_readonly() { + return Err(error::ErrorKind::ReadonlyVariable.into()); + } + match &self.value { ShellValue::Unset(_) => { self.assign(ShellValueLiteral::Array(ArrayLiteral(vec![])), false)?; } ShellValue::String(_) => { - self.convert_to_indexed_array()?; + self.convert_to_array_kind( + ArrayKind::Indexed, + ScalarConversionPolicy::PromoteToElementZero, + )?; } _ => (), } @@ -499,6 +550,12 @@ impl ShellVariable { /// /// * `index` - The index at which to unset the value. pub fn unset_index(&mut self, index: &str) -> Result { + // As with assignment, readonly is enforced here so that every path reaching an element is + // blocked, not just whole-variable unset. + if self.is_readonly() { + return Err(error::ErrorKind::ReadonlyVariable.into()); + } + match &mut self.value { ShellValue::Unset(ty) => match ty { ShellValueUnsetType::Untyped => Err(error::ErrorKind::NotArray.into()), @@ -516,6 +573,33 @@ impl ShellVariable { } } + /// Unsets every element of the array, leaving the variable itself declared. Returns whether + /// any element was removed. A variable that is not an array is an error; a declared-but-unset + /// array has nothing to remove and stays unset. An associative array is left alone -- a shell + /// quirk: `*` and `@` name no key. + pub fn unset_all_indices(&mut self) -> Result { + // As with `unset_index`, readonly is enforced here so that every path reaching an + // element is blocked. + if self.is_readonly() { + return Err(error::ErrorKind::ReadonlyVariable.into()); + } + + match &mut self.value { + ShellValue::Unset(ShellValueUnsetType::Untyped) | ShellValue::String(_) => { + Err(error::ErrorKind::NotArray.into()) + } + ShellValue::Unset( + ShellValueUnsetType::IndexedArray | ShellValueUnsetType::AssociativeArray, + ) => Ok(false), + ShellValue::IndexedArray(values) => { + let removed_any = !values.is_empty(); + values.clear(); + Ok(removed_any) + } + ShellValue::AssociativeArray(_) | ShellValue::Dynamic { .. } => Ok(false), + } + } + /// Returns the variable's value; for dynamic values, this will resolve the value. /// /// # Arguments @@ -529,6 +613,14 @@ impl ShellVariable { } } + /// Replaces a dynamic value with the value it currently resolves to; other values are left + /// untouched. + pub fn resolve_dynamic(&mut self, shell: &Shell) { + if matches!(self.value, ShellValue::Dynamic { .. }) { + self.value = self.resolve_value(shell); + } + } + /// Returns the canonical attribute flag string for this variable. pub fn attribute_flags(&self, shell: &Shell) -> String { let value = self.resolve_value(shell); @@ -635,6 +727,34 @@ pub enum ShellValueUnsetType { IndexedArray, } +/// The kind of an array variable. +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub enum ArrayKind { + /// An array indexed by integers. + Indexed, + /// An array keyed by arbitrary strings. + Associative, +} + +impl From for ShellValueUnsetType { + fn from(kind: ArrayKind) -> Self { + match kind { + ArrayKind::Indexed => Self::IndexedArray, + ArrayKind::Associative => Self::AssociativeArray, + } + } +} + +/// What happens to a set scalar value when its variable converts to an array kind. +#[derive(Clone, Copy, Debug)] +pub enum ScalarConversionPolicy { + /// The scalar becomes element 0 of the new array. + PromoteToElementZero, + /// The scalar is discarded; the variable becomes a set, empty array. This is how a shell + /// converts a function-local variable. + Discard, +} + /// A shell value literal; used for assignment. #[derive(Clone, Debug)] pub enum ShellValueLiteral { @@ -698,6 +818,22 @@ impl From> for ShellValueLiteral { } } +/// Takes an assignment's value as the literal to assign. The words are used as they are, so the +/// value must already have been expanded. +impl From for ShellValueLiteral { + fn from(value: brush_parser::ast::AssignmentValue) -> Self { + match value { + brush_parser::ast::AssignmentValue::Scalar(value) => Self::Scalar(value.value), + brush_parser::ast::AssignmentValue::Array(elements) => Self::Array(ArrayLiteral( + elements + .into_iter() + .map(|(key, value)| (key.map(|key| key.value), value.value)) + .collect(), + )), + } + } +} + /// An array literal. #[derive(Clone, Debug)] pub struct ArrayLiteral(pub Vec<(Option, String)>); @@ -712,26 +848,34 @@ pub enum FormatStyle { } impl ShellValue { + /// Returns the kind of array this value is, or `None` if it is not an array. A declared but + /// unset array still has a kind. + pub const fn array_kind(&self) -> Option { + match self { + Self::IndexedArray(_) | Self::Unset(ShellValueUnsetType::IndexedArray) => { + Some(ArrayKind::Indexed) + } + Self::AssociativeArray(_) | Self::Unset(ShellValueUnsetType::AssociativeArray) => { + Some(ArrayKind::Associative) + } + _ => None, + } + } + /// Returns whether or not the value is an indexed array, including a declared but unset one. pub const fn is_indexed_array(&self) -> bool { - matches!( - self, - Self::IndexedArray(_) | Self::Unset(ShellValueUnsetType::IndexedArray) - ) + matches!(self.array_kind(), Some(ArrayKind::Indexed)) } /// Returns whether or not the value is an associative array, including a declared but unset /// one. pub const fn is_associative_array(&self) -> bool { - matches!( - self, - Self::AssociativeArray(_) | Self::Unset(ShellValueUnsetType::AssociativeArray) - ) + matches!(self.array_kind(), Some(ArrayKind::Associative)) } /// Returns whether or not the value is an array. pub const fn is_array(&self) -> bool { - self.is_indexed_array() || self.is_associative_array() + self.array_kind().is_some() } /// Returns whether or not the value is set. diff --git a/brush-parser/src/arithmetic.rs b/brush-parser/src/arithmetic.rs index b0991ca32..5dcebdb98 100644 --- a/brush-parser/src/arithmetic.rs +++ b/brush-parser/src/arithmetic.rs @@ -21,8 +21,10 @@ fn cacheable_parse(input: &str) -> Result ast::ArithmeticExpr = - ![_] { ast::ArithmeticExpr::Literal(0) } / + _ ![_] { ast::ArithmeticExpr::Literal(0) } / _ e:expression() _ { e } pub(crate) rule expression() -> ast::ArithmeticExpr = precedence!{ @@ -169,3 +171,23 @@ fn parse_shell_literal_number(s: &str, radix: u64) -> Result Ok(result) } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn parse_blank_expression_yields_zero() { + for input in ["", " ", "\t", " \n "] { + assert!( + matches!(parse(input), Ok(ast::ArithmeticExpr::Literal(0))), + "expected 0 for {input:?}" + ); + } + } + + #[test] + fn parse_rejects_an_incomplete_expression() { + assert!(parse("1 +").is_err()); + } +} diff --git a/brush-shell/benches/shell.rs b/brush-shell/benches/shell.rs index 4352b6e69..b20aa8365 100644 --- a/brush-shell/benches/shell.rs +++ b/brush-shell/benches/shell.rs @@ -119,22 +119,24 @@ mod unix { // Benchmark: function invocation. let mut shell = rt.block_on(instantiate_shell()); - shell.define_func( - String::from("testfunc"), - brush_parser::ast::FunctionDefinition { - fname: String::from("testfunc").into(), - body: brush_parser::ast::FunctionBody( - brush_parser::ast::CompoundCommand::BraceGroup( - brush_parser::ast::BraceGroupCommand { - list: brush_parser::ast::CompoundList(vec![]), - loc: SourceSpan::default(), - }, + shell + .define_func( + String::from("testfunc"), + brush_parser::ast::FunctionDefinition { + fname: String::from("testfunc").into(), + body: brush_parser::ast::FunctionBody( + brush_parser::ast::CompoundCommand::BraceGroup( + brush_parser::ast::BraceGroupCommand { + list: brush_parser::ast::CompoundList(vec![]), + loc: SourceSpan::default(), + }, + ), + None, ), - None, - ), - }, - &brush_core::SourceInfo::default(), - ); + }, + &brush_core::SourceInfo::default(), + ) + .unwrap(); c.bench_function("function_call", |b| { b.iter_batched_ref( || shell.clone(), diff --git a/brush-shell/tests/cases/compat/arithmetic.yaml b/brush-shell/tests/cases/compat/arithmetic.yaml index b3a4df4cf..c32501d8b 100644 --- a/brush-shell/tests/cases/compat/arithmetic.yaml +++ b/brush-shell/tests/cases/compat/arithmetic.yaml @@ -360,3 +360,69 @@ cases: (( ref += 5 )) echo "after +=5: $counter" echo "expr: $(( ref * 2 ))" + + # + # A failed arithmetic evaluation is a command that returns non-zero, not a fatal error: a shell + # reports it and carries on with the rest of the list. brush abandons the remaining commands in + # the list, so the status line never prints. Each of these passes when the commands are written + # on separate lines; only sequencing within one list diverges. + # + - name: "Division by zero yields a non-zero status without aborting the list" + known_failure: true # TODO: arithmetic errors should not abandon the rest of a command list + stdin: | + (( 1 / 0 )); echo "status: $?" + + - name: "Arithmetic assignment to a readonly variable does not abort the list" + known_failure: true # TODO: arithmetic errors should not abandon the rest of a command list + stdin: | + a=(one); readonly a; (( a[0] = 9 )); echo "status: $?" + + - name: "Arithmetic error on its own line leaves the following command reachable" + ignore_stderr: true + stdin: | + (( 1 / 0 )) + echo "status: $?" + + - name: "An empty arithmetic expression evaluates to 0" + stdin: | + echo "[$(( ))]" + x=" " + echo "[$(( x ))]" + echo "[$(( $x ))]" + + # + # An expression that is empty or holds nothing but whitespace evaluates to 0, whichever entry + # point reaches the arithmetic parser: an expansion, a `((...))` command, `let`, or an array + # subscript. + # + - name: "A whitespace-only arithmetic expression evaluates to 0" + stdin: | + echo "[$(( ))]" + echo "[$(( ))]" + s=" " + echo "[$(( s ))]" + echo "[$(( s + 1 ))]" + + - name: "let with an empty or blank expression fails without a diagnostic" + stdin: | + let "" + echo "empty: $?" + let " " + echo "blank: $?" + let "" 1 + echo "trailing: $?" + + - name: "(( )) with a blank expression is false" + stdin: | + (( )) + echo "empty: $?" + (( )) + echo "blank: $?" + + - name: "A blank array subscript is element 0" + stdin: | + declare 'zz_a[ ]=one' + declare -p zz_a + zz_b=(x y z) + unset 'zz_b[ ]' + declare -p zz_b diff --git a/brush-shell/tests/cases/compat/arrays.yaml b/brush-shell/tests/cases/compat/arrays.yaml index fcb1a6f94..d69e1e59b 100644 --- a/brush-shell/tests/cases/compat/arrays.yaml +++ b/brush-shell/tests/cases/compat/arrays.yaml @@ -387,7 +387,228 @@ cases: echo "\${myarray[@]}:" ${myarray[@]} echo "\${myarray[*]}:" ${myarray[*]} + # + # Field splitting applies to what an expansion produced, not to the literal text around it. A + # compound value's elements are separated by the parser on whitespace; an IFS character sitting + # in literal text is just a character. brush splits the whole element after expanding it, so a + # literal separator splits it too. + # + - name: "Literal text in a compound value is not field-split" + known_failure: true # TODO: compound elements field-split literal text, not just expansion results + stdin: | + IFS=, + arr=(a,b) + declare -p arr + + - name: "A compound value field-splits an expansion but not the literal beside it" + known_failure: true # TODO: compound elements field-split literal text, not just expansion results + stdin: | + IFS=, + v="p,q" + arr=(a,b $v keep,me) + declare -p arr + + - name: "A quoted compound operand does not field-split its literal text" + known_failure: true # TODO: compound elements field-split literal text, not just expansion results + stdin: | + IFS=, + declare -a "arr=(a,b)" + declare -p arr + - name: "Empty array with newline" stdin: | declare -a myarray=( ) + + # + # Ordering between value expansion and subscript evaluation. A shell expands an + # assignment's words before it evaluates any arithmetic subscript, so side effects + # in a subscript are not visible to the value being assigned. + # + - name: "Index arithmetic is evaluated after the value is expanded" + stdin: | + i=0 + a[i++]=$i + declare -p a + echo "i=$i" + + - name: "Compound array keys are evaluated after every value is expanded" + stdin: | + i=0 + a=([i++]=$i [i++]=$i) + declare -p a + echo "i=$i" + + - name: "Append with an arithmetic subscript" + stdin: | + i=0 + a[i++]=$i + a[i++]+=$i + declare -p a + echo "i=$i" + + - name: "Expansion order of a subscript relative to its value" + stdin: | + a[$(echo "sub" >&2; echo 0)]=$(echo "val" >&2; echo v) + declare -p a + + - name: "Associative subscripts are not arithmetically evaluated" + stdin: | + i=0 + declare -A m + m[i++]=$i + declare -p m + echo "i=$i" + + - name: "Associative subscript is expanded once in a plain assignment" + stdin: | + declare -A m + k='$(echo expanded-twice)' + m[$k]=v + declare -p m + + # + # The same subscript rules reached through a declaration builtin, which resolves + # subscripts itself once its options reveal the target array type. + # + - name: "declare resolves an arithmetic subscript" + stdin: | + i=2 + declare -a a[i]=x + declare -p a + + - name: "declare evaluates subscript arithmetic after the value expands" + stdin: | + i=0 + declare -a a[i++]=$i + declare -p a + echo "i=$i" + + - name: "declare expands an associative subscript a second time" + stdin: | + declare -A n + k='$(echo expanded-twice)' + declare n[$k]=v + declare -p n + + # + # Assigning a list to a single array element is an error; the shell reports it and + # continues with a failure status. + # + - name: "Assigning a list to an element of an existing array fails" + ignore_stderr: true + stdin: | + a=(1 2) + a[0]=(9 9) + echo "status: $?" + declare -p a + + # + # A shell binds the variable, with the kind the operand implies, before it tries the element + # assignment; the failure then abandons the remaining operands. The diagnostic is pinned through + # the pipeline; the second invocation's copy of it carries a line prefix, so stderr is ignored. + # + - name: "declare refuses a list for an element, leaving the variable declared" + ignore_stderr: true + stdin: | + declare a[1]=(x y) later=1 + echo "status: $?" + declare -p a + declare -p later 2>&1 | strip + echo after + + - name: "Assigning a list to an element of an unset variable fails" + ignore_stderr: true + stdin: | + b[0]=(9 9) + echo "status: $?" + declare -p b 2>/dev/null + echo "status: $?" + + # + # A subscript no element can have -- empty, or `*`/`@` on an indexed array -- is a bad array + # subscript: the assignment fails and, as an assignment error, abandons the rest of the list. + # An empty associative key is checked after expansion; an empty indexed subscript before it + # (`a[$empty]` is element 0). + # + - name: "Empty and star subscripts are bad array subscripts" + ignore_stderr: true + stdin: | + a=(1) + a[]=x + echo "status: $?" + a[*]=x + echo "status: $?" + a[@]=x + echo "status: $?" + declare -p a + declare -A m + m[]=1 + echo "status: $?" + e= + m[$e]=1 + echo "status: $?" + declare -p m + + # + # A bad key inside a compound value stops the assignment at that element and is an assignment + # error: the elements before it are assigned, the rest of the command list is abandoned. + # + - name: "A bad compound key stops the assignment and abandons the list" + min_oracle_version: "5.3" # bash 5.3 makes a bad compound key an assignment error; 5.2 skipped the element + ignore_stderr: true + stdin: | + a=(1 []=2 3); echo "same list" + echo "status: $?" + declare -p a + b=(1 [*]=2 3); echo "same list" + echo "status: $?" + declare -p b + c=(1 [@]=2 3); echo "same list" + echo "status: $?" + declare -p c + declare -A m=([z]=9) + m=([a]=1 []=2 [b]=3); echo "same list" + echo "status: $?" + declare -p m + m+=([c]=3 []=4 [d]=5); echo "same list" + echo "status: $?" + echo "${#m[@]} ${m[c]}" + e= + declare -A n + n=([$e]=1); echo "same list" + echo "status: $?" + declare -p n + + # + # A compound value's keys are word-expanded along with the rest of the value and then, for an + # indexed array, evaluated arithmetically -- which expands them a second time. An associative + # key is taken as the once-expanded text. The same holds through a declaration builtin, quoted + # or not. + # + - name: "Compound keys expand twice for an indexed array and once for an associative one" + stdin: | + x=5 + k='$x' + a=([$k]=v) + declare -p a + declare -A m + m=([$k]=v) + declare -p m + declare -a b=([$k]=v) + declare -A n=([$k]=v) + declare -a "c=([$k]=v)" + declare -A "o=([$k]=v)" + declare -p b n c o + + # + # An empty subscript in a parameter reference is rejected while the word is parsed, before any + # array is consulted. + # + - name: "An empty subscript is a bad substitution" + known_failure: true # TODO: the word parser accepts an empty subscript and treats it as element 0 + ignore_stderr: true + stdin: | + zz_a=(1 2 3) + echo "[${zz_a[]}]" + echo "status: $?" diff --git a/brush-shell/tests/cases/compat/builtins/alias.yaml b/brush-shell/tests/cases/compat/builtins/alias.yaml index 15cb75d08..4000a336d 100644 --- a/brush-shell/tests/cases/compat/builtins/alias.yaml +++ b/brush-shell/tests/cases/compat/builtins/alias.yaml @@ -268,3 +268,31 @@ cases: alias alias -p alias q1 q3 + + # + # An alias body is expanded before the resulting words are inspected, so an alias can supply + # the name of a declaration builtin and have the operands that follow it still be treated as + # assignments rather than ordinary strings. + # + - name: "alias supplying a declaration builtin still splits its compound operand" + stdin: | + shopt -s expand_aliases + alias mydecl='declare -a' + v="a b" + mydecl arr=($v) + declare -p arr + + - name: "alias ending mid-assignment joins with the following word" + stdin: | + shopt -s expand_aliases + alias mkvar='declare x=' + mkvar hello + declare -p x + + - name: "alias supplying export assigns and exports" + stdin: | + shopt -s expand_aliases + alias exp='export' + exp E1=val + echo "E1=$E1" + declare -p E1 diff --git a/brush-shell/tests/cases/compat/builtins/command.yaml b/brush-shell/tests/cases/compat/builtins/command.yaml index 6519818ac..3e348b72b 100644 --- a/brush-shell/tests/cases/compat/builtins/command.yaml +++ b/brush-shell/tests/cases/compat/builtins/command.yaml @@ -110,7 +110,6 @@ cases: command ls --help - name: "command forwards expanded declaration assignments" - known_failure: true # TODO: command builtin improvements required stdin: | value=world command declare declared=$value @@ -118,7 +117,6 @@ cases: declare -p declared exported - name: "nested wrappers resolve ordinary and declaration targets" - known_failure: true # TODO: command builtin improvements required stdin: | value=world command builtin echo ordinary=$value @@ -158,7 +156,6 @@ cases: # syntax that survives. # - name: "command does not preserve assignment splitting semantics" - known_failure: true # TODO: command builtin improvements required ignore_stderr: true stdin: | v="x y" @@ -166,7 +163,6 @@ cases: declare -p b y - name: "command forwards a value with no field separators intact" - known_failure: true # TODO: command builtin improvements required stdin: | value=world command declare declared=$value diff --git a/brush-shell/tests/cases/compat/builtins/complete.yaml b/brush-shell/tests/cases/compat/builtins/complete.yaml index 3d6d13b43..4a9e053eb 100644 --- a/brush-shell/tests/cases/compat/builtins/complete.yaml +++ b/brush-shell/tests/cases/compat/builtins/complete.yaml @@ -34,7 +34,7 @@ cases: complete -Fmyfunc mycmd complete -p mycmd - - name: "Roundtrip: complete -F" + - name: "Roundtrip: complete -G" stdin: | complete -G pattern mycmd complete -p mycmd diff --git a/brush-shell/tests/cases/compat/builtins/declare.yaml b/brush-shell/tests/cases/compat/builtins/declare.yaml index f5881ba70..c1abe7d47 100644 --- a/brush-shell/tests/cases/compat/builtins/declare.yaml +++ b/brush-shell/tests/cases/compat/builtins/declare.yaml @@ -105,7 +105,7 @@ cases: var=value declare -p var - - name: "Update integer array with non-integer string" + - name: "Update integer indexed array with non-integer string" stdin: | declare -ai arr=() declare -p arr @@ -113,7 +113,7 @@ cases: arr[0]="value" declare -p arr - - name: "Update integer array with non-integer string" + - name: "Update integer associative array with non-integer string" stdin: | declare -Ai arr=() declare -p arr @@ -574,34 +574,29 @@ cases: declare -p -n 2>/dev/null | sort - name: "Single-quoted scalar assignment" - known_failure: true # TODO: declare improvements required stdin: | declare 'x=hello' echo "$x" - name: "Single-quoted scalar with dollar sign preserved" - known_failure: true # TODO: declare improvements required stdin: | Y=world declare 'x=$Y' echo "$x" - name: "Double-quoted scalar assignment expands" - known_failure: true # TODO: declare improvements required stdin: | Y=world declare "x=$Y" echo "$x" - name: "Single-quoted array assignment with -a" - known_failure: true # TODO: declare improvements required stdin: | declare -a 'arr=(1 2 3)' echo "${arr[@]}" echo "${#arr[@]}" - name: "Single-quoted array assignment with deferred expansion" - known_failure: true # TODO: declare improvements required stdin: | X="a b" declare -a 'arr=(${X})' @@ -609,7 +604,6 @@ cases: echo "${#arr[@]}" - name: "Single-quoted array assignment with IFS splitting" - known_failure: true # TODO: declare improvements required stdin: | X="a,b" IFS=, declare -a 'arr=(${X})' @@ -617,7 +611,6 @@ cases: echo "${#arr[@]}" - name: "Double-quoted compound assignment with escaped expansion" - known_failure: true # TODO: declare improvements required stdin: | X="a b" declare -a "arr=(\${X})" @@ -625,7 +618,6 @@ cases: echo "${#arr[@]}" - name: "Variable-generated compound assignment syntax" - known_failure: true # TODO: declare improvements required stdin: | X="a b" value='(${X})' @@ -634,7 +626,6 @@ cases: echo "${#arr[@]}" - name: "Command-substitution-generated compound assignment syntax" - known_failure: true # TODO: declare improvements required stdin: | X="a b" declare -a "arr=$(printf '(%s)' '${X}')" @@ -642,7 +633,6 @@ cases: echo "${#arr[@]}" - name: "Single-quoted compound value in parsed assignment is reparsed" - known_failure: true # TODO: declare improvements required stdin: | X="a b" declare -a arr='(${X})' @@ -650,13 +640,11 @@ cases: echo "${#arr[@]}" - name: "Single-quoted array assignment without -a is scalar" - known_failure: true # TODO: declare improvements required stdin: | declare 'arr=(1 2 3)' declare -p arr - name: "Single-quoted array assignment Gentoo rpm.eclass pattern" - known_failure: true # TODO: declare improvements required stdin: | RPM_COMPRESS_TYPE="lzma zstd" IFS=, declare -a 'types=(${RPM_COMPRESS_TYPE})' @@ -664,7 +652,6 @@ cases: echo "${#types[@]}" - name: "local -a single-quoted array assignment" - known_failure: true # TODO: declare improvements required stdin: | f() { X="a b" @@ -674,13 +661,11 @@ cases: f - name: "Single-quoted indexed element assignment" - known_failure: true # TODO: declare improvements required stdin: | declare 'arr[0]=hello' declare -p arr - name: "Quoted array assignment does not execute value as shell code" - known_failure: true # TODO: declare improvements required stdin: | payload='(x); printf "INJECTED\n"; #' declare -a "arr=$payload" @@ -695,7 +680,6 @@ cases: declare -p arr - name: "Quoted array assignment applies uppercase attribute" - known_failure: true # TODO: declare improvements required stdin: | declare -au 'arr=(ab Cd)' declare -p arr @@ -708,21 +692,18 @@ cases: declare -p arr - name: "Quoted assignment uses existing indexed array type" - known_failure: true # TODO: declare improvements required stdin: | arr=(old) declare 'arr=(new values)' declare -p arr - name: "Quoted assignment uses existing associative array type" - known_failure: true # TODO: declare improvements required stdin: | declare -A map=([old]=value) declare 'map=([new]=value [other]=two)' declare -p map - name: "Quoted assignment uses declaration target scope" - known_failure: true # TODO: declare improvements required stdin: | arr=(global) f() { @@ -734,7 +715,6 @@ cases: declare -p arr - name: "Quoted global assignment ignores local array type" - known_failure: true # TODO: declare improvements required stdin: | arr=global f() { @@ -746,7 +726,6 @@ cases: declare -p arr - name: "Quoted local declaration does not inherit global array type" - known_failure: true # TODO: declare improvements required stdin: | arr=(global) f() { @@ -757,61 +736,52 @@ cases: declare -p arr - name: "Single-quoted nonzero indexed element assignment" - known_failure: true # TODO: declare improvements required stdin: | declare 'arr[5]=hello' declare -p arr - name: "Single-quoted indexed element append assignment" - known_failure: true # TODO: declare improvements required stdin: | declare -a arr=([0]=one) declare 'arr[0]+=two' declare -p arr - name: "Single-quoted associative element append assignment" - known_failure: true # TODO: declare improvements required stdin: | declare -A map=([key]=one) declare 'map[key]+=two' declare -p map - name: "Single-quoted arithmetic indexed element assignment" - known_failure: true # TODO: declare improvements required stdin: | declare 'arr[2+2]=value' declare -p arr - name: "Single-quoted variable arithmetic indexed element assignment" - known_failure: true # TODO: declare improvements required stdin: | index=3 declare 'arr[index+1]=value' declare -p arr - name: "Single-quoted parameter-expanded indexed element assignment" - known_failure: true # TODO: declare improvements required stdin: | index=3 declare 'arr[$index+1]=value' declare -p arr - name: "Single-quoted command-substitution indexed element assignment" - known_failure: true # TODO: declare improvements required stdin: | index=3 declare 'arr[$(echo 2)+index]=value' declare -p arr - name: "Single-quoted parameter-expanded associative element assignment" - known_failure: true # TODO: declare improvements required stdin: | key="space key" declare -A 'map[$key]=value' declare -p map - name: "Single-quoted indexed subscript expands without re-expanding value" - known_failure: true # TODO: declare improvements required stdin: | index=2 value=expanded @@ -819,7 +789,6 @@ cases: declare -p arr - name: "Single-quoted associative subscript expands without re-expanding value" - known_failure: true # TODO: declare improvements required stdin: | key="space key" value=expanded @@ -827,7 +796,6 @@ cases: declare -p map - name: "Single-quoted append assignment" - known_failure: true # TODO: declare improvements required stdin: | declare 'text=one' declare 'text+=two' 2>/dev/null @@ -835,14 +803,12 @@ cases: declare -p text - name: "Single-quoted array-like scalar assignment with +a" - known_failure: true # TODO: declare improvements required stdin: | declare +a 'arr=(1 2)' echo "status: $?" declare -p arr - name: "Cannot remove indexed array attribute with +a" - known_failure: true # TODO: declare improvements required stdin: | declare -a arr=(1 2) declare +a arr 2>/dev/null @@ -857,7 +823,6 @@ cases: declare -p arr - name: "Single-quoted associative array assignment" - known_failure: true # TODO: declare improvements required stdin: | declare -A 'map=([alpha]=one [beta]=two)' 2>/dev/null echo "status: $?" @@ -887,7 +852,6 @@ cases: f - name: "Function shadowing declare is reached through command" - known_failure: true # TODO: declare improvements required stdin: | declare() { echo "shadow declare: [$*]"; } command declare cx=1 @@ -910,7 +874,6 @@ cases: declare -p arr - name: "Temporary IFS does split a quoted compound declare operand" - known_failure: true # TODO: declare improvements required stdin: | X="a,b" IFS=, declare -a 'arr=(${X})' @@ -969,14 +932,12 @@ cases: # like a subscript. # - name: "Assignment value ending in a bracket is not a subscripted name" - known_failure: true # TODO: declare improvements required stdin: | v='[a]' declare "x=$v" declare -p x - name: "Assignment value containing brackets is preserved" - known_failure: true # TODO: declare improvements required stdin: | declare 'a[1]=v[2]' declare -p a @@ -990,7 +951,6 @@ cases: # Regression coverage: `+a`/`+A` report why they refuse to remove an array attribute. # - name: "Removing array attribute reports an error" - known_failure: true # TODO: declare improvements required ignore_stderr: true stdin: | declare -a arr=(1 2) @@ -999,7 +959,6 @@ cases: declare -p arr - name: "Removing array attribute from a declared but unset array fails" - known_failure: true # TODO: declare improvements required ignore_stderr: true stdin: | declare -a arr @@ -1008,7 +967,6 @@ cases: declare -p arr - name: "Removing associative array attribute fails" - known_failure: true # TODO: declare improvements required ignore_stderr: true stdin: | declare -A map=([k]=v) @@ -1016,6 +974,29 @@ cases: echo "status: $?" declare -p map + - name: "Converting a declared but unset array to the other kind fails" + stdin: | + strip() { sed -E 's/^[^ ]*: line [0-9]+: //'; } + exec 2>err.txt + declare -a arr + declare -A arr + echo "status: $?" + declare -p arr + declare -A map + declare -a map + echo "status: $?" + declare -p map + strip /dev/null echo "status: $?" - declare -p b 2>&1 + declare -p b 2>/dev/null + echo "declare_status: $?" # # Attribute-filtered display. # - name: "Display filtered by readonly and trace attributes" - known_failure: true # TODO: declare improvements required stdin: | declare -r zzro=1 declare -t zztr=2 @@ -1105,8 +1078,7 @@ cases: - name: "Display filtered by several attributes lists any that match" # A shell combines attribute filters as a union: `declare -rt` lists variables that are - # readonly or traced. brush intersects them, listing only variables that are both. - known_failure: true + # readonly or traced. stdin: | declare -r zzro=1 declare -t zztr=2 @@ -1114,26 +1086,36 @@ cases: declare -prt | grep zz | sort - name: "Display is not filtered by a plus attribute option" - # brush treats `+x` as a request for variables lacking the attribute; a shell applies no - # filter at all in that case. This affects every attribute, not just the one shown here. - known_failure: true + # A plus attribute option (`+x`) applies no display filter at all. stdin: | declare -x zzex=1 zzplain=2 declare +x | grep zz | sort - name: "Combining -a and -A is rejected" - # A shell treats `-a` and `-A` together as an invalid option combination. brush accepts it and - # lets `-A` win, which lands on the same variable type but without the diagnostic or status. + # A shell treats `-a` and `-A` together as an invalid option combination. + min_oracle_version: "5.3" # bash 5.3 rejects the combination as an invalid option; 5.2 tried the conversion + stdin: | + strip() { sed -E 's/^[^ ]*: line [0-9]+: //'; } + exec 2>err.txt + declare -aA both + echo "status: $?" + declare -p both 2>/dev/null + echo "declare_status: $?" + strip &1 + declare -p both 2>/dev/null + echo "declare_status: $?" - name: "Display diagnostics name the builtin as invoked" - known_failure: true # TODO: declare improvements required stdin: | check() { case $2 in @@ -1146,8 +1128,15 @@ cases: f() { check "local: nosuch: not found" "$(local -p nosuch 2>&1)"; } f + - name: "Display quotes an associative key of @" + # bash quotes both `*` and `@` when displaying an associative key; brush quotes only `*`. + # Pre-existing; the key quoting predates this branch. + known_failure: true + stdin: | + declare -A zzat=(['@']=at ['*']=star [k]=v) + declare -p zzat + - name: "Display filtered by array attributes includes unassigned arrays" - known_failure: true # TODO: declare improvements required stdin: | declare -a zzempty declare -A zzemptyA @@ -1157,7 +1146,6 @@ cases: echo "-A:"; declare -pA | grep zz | sort - name: "Declaration diagnostics name the builtin and quote the operand" - known_failure: true # TODO: declare improvements required # Covers every diagnostic the declaration builtins emit, so a new one cannot quietly omit the # builtin's name. The leading "file: line N:" a shell prefixes is stripped. stdin: | @@ -1194,8 +1182,563 @@ cases: declare -fx nosuchfn 2>&1; echo "rc=$?" - name: "declare -ft records the trace attribute" - known_failure: true # TODO(declare): brush accepts -t on functions but doesn't track it stdin: | f() { :; } declare -ft f declare -F + + # + # Operand ordering, kind conversion, readonly, and attribute rules. + # + # Operands are processed one at a time, each against the environment its predecessors + # left behind: a later operand's subscript or reinterpreted compound value sees what an + # earlier one assigned. (Values the interpreter expanded before the command ran do not; see + # "Multiple readonly operands expand before assignments are applied".) + # + - name: "each operand sees what the preceding operands assigned" + stdin: | + declare -a 'x=(1 2)' 'y=(${x[@]})' + declare -p y + i=5 + declare 'a[i]=1' i=9 'c[i]=2' + declare -p a i c + unset i + declare 'i=3' 'd[i]=x' 'e[i+1]=y' + declare -p d e + + # A failed kind conversion reports the one operand and keeps going. + # + - name: "a failed conversion on one operand does not abort sibling operands" + stdin: | + strip() { sed -E 's/^[^ ]*: line [0-9]+: //'; } + exec 2>err.txt + declare -a q=(1) + declare -A q r=v + echo "status=$?" + declare -p r q + strip err.txt + declare q2 + readonly q2 + declare -a q2 + echo "status=$?" + declare -p q2 + s3=1 + readonly s3 + declare -A s3 + echo "status2=$?" + declare -p s3 + readonly -a arr=(1) + declare -a arr + echo "status3=$?" + declare arr + echo "status4=$?" + declare -p arr + declare -a ra=(1) + readonly ra + declare -a ra + echo "status3=$?" + strip /dev/null + echo "$flag=$?" + done + declare -p n + + # + # `set -a` exports a variable a declaration modifies, but a bare re-declaration with no + # assignment leaves the export attribute alone. + # + - name: "set -a exports a variable modified via declare" + stdin: | + w=1 + u2=1 + set -a + declare w=2 + declare -p w u2 + declare u2 + declare -p u2 + + # + # `+I` is accepted and behaves like `-I`. + # + - name: "+I is accepted and inherits like -I" + stdin: | + v=outer + f() { local +I v; echo "inner=${v-UNSET}"; v=changed; } + f + echo "global=$v" + g() { local -I +I v; echo "both=${v-UNSET}"; } + g + declare +I newvar + echo "declare_status=$?" + + - name: "declare -pF shows function attributes" + stdin: | + f() { :; } + declare -ft f + declare -fx f + declare -pF f + declare +t -f f + declare -pF f + declare -F f + declare -fr f + declare -pF f + + - name: "attribute options list in declare -p form without -p" + stdin: | + zz_x=1 + export zz_e=2 + declare -i zz_i=3 + declare -a zz_a=(1) + declare -t zz_t=4 + declare -r zz_r=5 + zz_f() { :; } + declare -ft zz_f + declare -x | grep zz_ + declare -i | grep zz_ + declare -a | grep zz_ + declare -t + declare -xi | grep zz_ + declare -r | grep zz_ + readonly | grep zz_ + readonly -a | grep -c zz_ + declare +x | grep -c '^declare' + declare | grep zz_f + declare -x | grep -c zz_f + + - name: "option attributes are not granted when the assignment is refused" + stdin: | + readonly zz_ro=1 + declare -x zz_ro=2 2>/dev/null + echo "status: $?" + declare -p zz_ro + export zz_ro=3 2>/dev/null + echo "status: $?" + declare -p zz_ro + + - name: "the integer attribute evaluates an arithmetic value on assignment" + known_failure: true # TODO: integer-attributed assignments are not evaluated arithmetically + stdin: | + declare -i x=1+1 + declare -p x + declare -i y + y=2*3 + declare -p y + declare -ai a=(1+1 2*3) + declare -p a + + # + # `declare -g` names a global, but a shell resolves a subscripted operand's kind against the + # local that shadows it and then assigns there -- so the global is left untouched. brush honors + # `-g` for both halves and writes the global. + # + - name: "declare -g with a subscripted operand under a shadowing local" + known_failure: true # TODO: -g resolves the subscript against the global, not the shadowing local + stdin: | + declare -A g=([x]=1) + f() { + local -a g=(9) + declare -g g[y]=2 + declare -p g + } + f + declare -p g + + - name: "a local cannot shadow a readonly global" + stdin: | + strip() { sed -E 's/^[^ ]*: line [0-9]+: //'; } + readonly zz_ro=1 + f() { local zz_ro=2 other=3; echo "status: $?"; declare -p zz_ro other; } + { f; } 2>&1 | strip + g() { declare zz_ro; echo "status: $?"; } + { g; } 2>&1 | strip + h() { local -a zz_ro; echo "status: $?"; } + { h; } 2>&1 | strip + outer() { local inner=1; readonly inner; middle; } + middle() { local inner=2; echo "status: $?"; declare -p inner; } + outer + + # + # A parenthesized value in a subscripted operand is reinterpreted as a compound value (and + # assigned to the whole array) only under an explicit -a/-A; without one, an existing array's + # element receives the text literally. + # + - name: "a parenthesized value in a subscripted string operand stays literal without -a" + stdin: | + declare -a sub=(1 2) + declare 'sub[0]=(a b)' + declare -p sub + declare -A sm=([k]=v) + declare 'sm[k]=(a b)' + declare -p sm + f() { local -a la=(1); local 'la[0]=(x y)'; declare -p la; } + f + declare -a sub2=(1 2) + declare -a 'sub2[0]=(a b)' + declare -p sub2 + + # + # With no operands, -f/-F list only the functions matching the attribute options given, as a + # union; a plus option applies no filter. When definitions are listed under a filter, each is + # followed by its attribute line. + # + - name: "declare -F with attribute options lists only matching functions" + stdin: | + zf_a() { echo a; } + zf_b() { echo b; } + zf_c() { echo c; } + zf_d() { echo d; } + declare -fx zf_a + readonly -f zf_b + declare -ft zf_c + echo "[Fx]"; declare -Fx + echo "[Fr]"; declare -Fr + echo "[Ft]"; declare -Ft + echo "[Frx]"; declare -Frx + echo "[fx]"; declare -fx + echo "[F+x]"; declare -F +x | grep -c zf_ + echo "[typeset -Fx]"; typeset -Fx + + # + # A subscript no element can have -- empty, or `*`/`@` on an indexed array -- is a bad array + # subscript. The operand fails, but a shell has already bound the target with the kind the + # operand implies, and it goes on to the remaining operands. The diagnostic is pinned through + # the pipeline, where it carries no line prefix. + # + - name: "a bad subscript fails the operand but still declares the target" + stdin: | + strip() { sed -E 's/^[^ ]*: line [0-9]+: //'; } + { declare 'b[]=1' later=1; } 2>&1 | strip + declare 'b[]=1' later=1 2>/dev/null + echo "status: $?" + declare -p b later + { declare 'c[*]=1'; declare 'd[@]=1'; } 2>&1 | strip + declare 'c[*]=1' 'd[@]=1' 2>/dev/null + declare -p c d 2>/dev/null + declare -A e + { declare 'e[]=1'; } 2>&1 | strip + declare 'e[]=1' 2>/dev/null + declare -p e + + # + # Which target a bad subscript binds depends on what was there before. A variable that does not + # exist yet, or holds a scalar, gets bound; one that is *already* an array is left exactly as it + # was, so a declared-but-unset array stays unset rather than becoming a set, empty one. + # + - name: "a bad subscript binds only a target that is not already an array" + stdin: | + declare 'zz_new[]=1' 2>/dev/null + declare -p zz_new + declare -a zz_unset_a + declare 'zz_unset_a[]=1' 2>/dev/null + declare -p zz_unset_a + declare -A zz_unset_A + declare 'zz_unset_A[]=1' 2>/dev/null + declare -p zz_unset_A + declare -a zz_set=(9) + declare 'zz_set[]=1' 2>/dev/null + declare -p zz_set + zz_scalar=scalar + declare 'zz_scalar[]=1' 2>/dev/null + declare -p zz_scalar + declare 'zz_star[*]=1' 2>/dev/null + declare -p zz_star + declare -A zz_assoc + declare 'zz_assoc[*]=1' 2>/dev/null + declare -p zz_assoc + + - name: "an expansion that leaves an indexed subscript empty is element 0" + stdin: | + e= + declare "b[$e]=1" 2>/dev/null + echo "status: $?" + a[$e]=x + declare -p a + declare 'w[ ]=2' + declare -p w + + # + # A bad key inside a compound value stops the assignment at that element: the elements before + # it are assigned, the rest are not. For a quoted operand -- whose compound syntax the builtin + # recognizes only after expansion -- that is a warning and the operand still succeeds. For an + # unquoted operand it is an assignment error: attributes are not granted, the status is 1, and + # the rest of the command list is abandoned. + # + - name: "a bad key in a quoted compound operand stops the value with a warning" + min_oracle_version: "5.3" # bash 5.3 stops the value at a bad key; 5.2 skipped the element and continued + stdin: | + strip() { sed -E 's/^[^ ]*: line [0-9]+: //'; } + { declare -a 'c=([2]=x []=1 [3]=y)'; } 2>&1 | strip + declare -a 'c=([2]=x []=1 [3]=y)' 2>/dev/null + echo "status: $?" + declare -p c + { declare -A 'F=([a]=1 []=2 [b]=3)'; } 2>&1 | strip + declare -A 'F=([a]=1 []=2 [b]=3)' 2>/dev/null + echo "status: $?" + declare -p F + { declare -a 'G=(1 [*]=2 3)'; } 2>&1 | strip + { declare -a 'H=(1 [@]=2 3)'; } 2>&1 | strip + declare -a 'G=(1 [*]=2 3)' 'H=(1 [@]=2 3)' 2>/dev/null + echo "status: $?" + declare -p G H + + - name: "a bad subscript on the name still grants option attributes" + stdin: | + strip() { sed -E 's/^[^ ]*: line [0-9]+: //'; } + { declare -rx 'b[*]=1'; } 2>&1 | strip + declare -rx 'b[*]=1' 2>/dev/null + echo "status: $?" + declare -p b + n=s + declare -x 'n[@]=1' 2>/dev/null + echo "status: $?" + declare -p n + + - name: "a bad key in an unquoted compound operand is an assignment error" + min_oracle_version: "5.3" # bash 5.3 makes a bad compound key an assignment error; 5.2 skipped the element + ignore_stderr: true + stdin: | + declare -a c=(1 []=2 3); echo "same list" + echo "status: $?" + declare -p c + declare -a d=(1 [*]=2 3); echo "same list" + echo "status: $?" + declare -p d + declare -a e=(1 [@]=2 3); echo "same list" + echo "status: $?" + declare -p e + declare -A F=([a]=1 []=2 [b]=3); echo "same list" + echo "status: $?" + declare -p F + s=scalar + declare -a s=(1 []=2) + declare -p s + f() { local -a l=(1 []=2); echo "in function after"; } + f + echo "caller next line" + + - name: "an assignment error on a compound operand grants no attributes" + min_oracle_version: "5.3" # bash 5.3 makes a bad compound key an assignment error; 5.2 assigned and granted + ignore_stderr: true + stdin: | + declare -rx r=(1 []=2) + echo "status: $?" + declare -p r + r[5]=x + echo "status: $?" + declare -p r + declare -i i=(1 []=2) + declare -p i + declare -t t=(1 []=2) + declare -p t + + - name: "a conversion failure on an unquoted compound operand is an assignment error" + ignore_stderr: true + stdin: | + declare -A x=([k]=v) + declare -a x=(1); echo "same list" + echo "status: $?" + declare -p x + declare -a y=(1) + declare -A y=([k]=v); echo "same list" + echo "status: $?" + declare -p y + + - name: "a readonly variable assigned an unquoted compound operand is an assignment error" + min_oracle_version: "5.3" # bash 5.3 grants no attributes on the refusal; 5.2 still applied -a + ignore_stderr: true + stdin: | + readonly r=1 + declare r=(1 2); echo "same list" + echo "status: $?" + declare -p r + declare -a r=(1 2); echo "same list" + echo "status: $?" + declare -p r + + # + # Only a shell's own refusal to assign is an assignment error. brush failing to carry an + # assignment out is an ordinary per-operand failure, so the caller's command list survives it. + # The literal below mixes keyed and unkeyed elements in an associative array, which brush does + # not yet assign the way bash pairs them up; what this case pins down is that hitting that gap + # does not abandon the rest of the line. + # + - name: "an unimplemented compound value fails the operand without abandoning the list" + known_failure: true # TODO: misaligned keys/values in an associative array literal + ignore_stderr: true + stdin: | + declare -A y=(a [k]=b [""]=c d) + echo "status: $?" + declare -p y + declare -A z=(a [k]=b); echo "same list" + echo "reached" + + # + # A list assigned to one element abandons not just the remaining operands but, as an + # assignment error, the rest of the command list. brush continues the list. + # + - name: "a list assigned to an element abandons the rest of the list" + ignore_stderr: true + stdin: | + declare m[1]=(a b); echo "status: $?" + echo after + + # + # A declaration naming a nameref acts on the variable it references. + # + - name: "declare assigns through a nameref" + known_failure: true # TODO: declaration builtins do not follow namerefs + stdin: | + declare -n nr=tgt + declare nr=hello + declare -p tgt nr + + - name: "a subscripted assignment through a nameref targets the referenced array" + known_failure: true # TODO: subscripted assignments do not follow namerefs + stdin: | + declare -A m + declare -n r=m + r[key]=v + declare -p m + declare r[other]=w + declare -p m + + # + # A bare operand's subscript must end the word: text after the closing bracket, or a second + # subscript, makes the whole operand an invalid identifier. (An assignment form is rejected by + # the parser the same way.) + # + - name: "a bare operand with a nested or trailing subscript is not a valid identifier" + stdin: | + strip() { sed -E 's/^[^ ]*: line [0-9]+: //'; } + { declare 'nb[a][b]'; declare 'nt[a]x]'; declare 'nb[a][b]=1'; } 2>&1 | strip + declare 'nb[a][b]' 2>/dev/null + echo "status: $?" + declare -p nb nt 2>/dev/null + echo "declare_status: $?" + + # + # The diagnostic for a bad key in an unquoted compound operand. brush surfaces it through the + # interpreter's error display, which adds an `error:` prefix and the builtin's name; a shell + # prints the bare `[]=2: bad array subscript`. + # + - name: "a bad key in an unquoted compound operand is reported as written" + known_failure: true # TODO: interpreter-displayed assignment errors carry an `error:` prefix and the builtin name + stdin: | + strip() { sed -E 's/^[^ ]*: line [0-9]+: //'; } + { declare -a c=(1 []=2 3); } 2>&1 | strip + { declare -A m=([a]=1 [""]=2); } 2>&1 | strip + + - name: "a function cannot be declared by assignment" + stdin: | + strip() { sed -E 's/^[^ ]*: line [0-9]+: //'; } + { declare -f f=1; echo "status: $?"; } 2>&1 | strip + { declare -Fx f=1; echo "status: $?"; } 2>&1 | strip + declare -F f; echo "status: $?" + + # + # Every one of `declare`'s options has a `+X` form, including the four that select a mode rather + # than an attribute. For those four `+X` means "off", which is already the default, so they are + # accepted and (except for `+p`) do nothing: `+f` and `+F` drop the function restriction, `+g` + # drops global creation, and `+p` prints just as `-p` does. brush models those four as plain + # boolean flags with no plus form, so clap rejects the word outright and the command exits 2. + # This also takes down any combined form that includes one of them, such as `+ft`. + # + - name: "declare +f drops the function restriction" + known_failure: true # TODO(declare): -f/-F/-g/-p have no `+X` form + stdin: | + strip() { sed -E 's/^[^ ]*: line [0-9]+: //'; } + f() { :; } + { declare +fp f; echo "status: $?"; } 2>&1 | strip + + - name: "declare +F drops names-only display and declares a variable instead" + known_failure: true # TODO(declare): -f/-F/-g/-p have no `+X` form + stdin: | + f() { :; } + declare -F f + declare +F f + echo "status: $?" + declare -p f + + - name: "declare +g declares locally inside a function" + known_failure: true # TODO(declare): -f/-F/-g/-p have no `+X` form + stdin: | + g=outer + f() { declare +g g=inner; echo "inside: $g"; } + f + echo "outside: $g" + + - name: "declare +p displays just as -p does" + known_failure: true # TODO(declare): -f/-F/-g/-p have no `+X` form + stdin: | + x=1 + declare -p x + declare +p x + echo "status: $?" + + - name: "declare +ft combines a mode option and an attribute option" + known_failure: true # TODO(declare): -f/-F/-g/-p have no `+X` form + stdin: | + f() { :; } + declare -ft f + declare +ft f + echo "status: $?" + declare -pF f + declare -p f diff --git a/brush-shell/tests/cases/compat/builtins/export.yaml b/brush-shell/tests/cases/compat/builtins/export.yaml index 038954ed5..d6d59711f 100644 --- a/brush-shell/tests/cases/compat/builtins/export.yaml +++ b/brush-shell/tests/cases/compat/builtins/export.yaml @@ -37,14 +37,12 @@ cases: declare -p result - name: "Export generated assignment does not expand its value again" - known_failure: true # TODO: export improvements required stdin: | assignment='result=$(printf expanded)' export "$assignment" declare -p result - name: "Export indexed array with arithmetic key" - known_failure: true # TODO: export improvements required stdin: | export arr=([2+2]=value) declare -p arr @@ -151,15 +149,259 @@ cases: unset -f export - name: "Export rejects an array element target" - known_failure: true # TODO: export improvements required ignore_stderr: true stdin: | export 'ea[0]=1' echo "status: $?" + # A rejected element target is rejected as written: its subscript is never evaluated, so it + # has no arithmetic side effects and cannot raise an arithmetic error in place of the + # identifier diagnostic. + - name: "Export does not evaluate the subscript of a rejected element target" + stdin: | + strip() { sed -E 's/^[^ ]*: line [0-9]+: //'; } + i=0 + export "ea2[i++]=1" 2>/dev/null + echo "status: $? i=$i" + export "ea3[1/0]=1" 2>/dev/null + echo "status: $?" + { export "ea2[i++]=1"; export "ea3[1/0]=1"; } 2>&1 | strip + + - name: "export -f with an assignment operand reports not a function" + stdin: | + f() { :; } + export -f "f=1" 2>/dev/null + echo "status: $?" + declare -p f 2>/dev/null + echo "declare_status: $?" + + - name: "Export identifier failure inside a pipeline exits with bash's status" + # A shell's declaration builtins report an invalid identifier with status 4 when run + # directly as a pipeline element; only at top level is it status 1. + known_failure: true # TODO: pipeline-element status for an invalid identifier is 1, not 4 + stdin: | + export "e[0]=1" 2>/dev/null | cat + echo "pipeline: ${PIPESTATUS[0]}" + export "e[0]=1" 2>/dev/null + echo "direct: $?" + - name: "Export recognizes an assignment produced through command" - known_failure: true # TODO: export improvements required stdin: | value=world command export wrapped=$value declare -p wrapped + + # + # `export` accepts the array attribute options of its declaration siblings (issue #1132: + # `mise activate bash` emits `export -a chpwd_functions` on every startup). + # + # Note what a shell actually does with `-a`, since it is easy to over-apply: the array + # attribute is set only when the operand carries an assignment. A bare name gets the export + # attribute alone, which is why the scalar assignment below stays a scalar rather than becoming + # element 0 of an array. + # + - name: "export -a on a bare name marks it exported without making it an array" + stdin: | + export -a foo + declare -p foo + + - name: "export -a on a bare name leaves a later scalar assignment scalar" + stdin: | + export -a bar + bar=val + declare -p bar + + - name: "export -a with an assignment creates an exported indexed array" + stdin: | + export -a baz=hello + declare -p baz + + - name: "export -A is accepted but does not make the variable associative" + stdin: | + export -A m + m[k]=v + declare -p m + + - name: "export -a then array append keeps the export attribute" + stdin: | + export -a hooks + hooks+=(h1) + declare -p hooks + + - name: "export -a with an assignment converts an existing scalar" + stdin: | + s=old + export -a s=new + declare -p s + + - name: "export -a with an assignment refuses an associative array without aborting siblings" + stdin: | + strip() { sed -E 's/^[^ ]*: line [0-9]+: //'; } + declare -A aa=([k]=v) + { export -a aa=x other=1; } 2>&1 | strip + export -a aa=x other=1 2>/dev/null + echo "status: $?" + declare -p aa other + + - name: "export -A with a quoted compound value creates an associative array" + stdin: | + export -A "am=([k]=v)" + export -a "ia=(1 2)" + declare -p am ia + + - name: "export -aA on a bare name only exports it" + stdin: | + export -aA both + echo "status: $?" + declare -p both + + - name: "export treats a plus option as an invalid identifier operand" + stdin: | + strip() { sed -E 's/^[^ ]*: line [0-9]+: //'; } + { export +x zz_plus=1; } 2>&1 | strip + export +x zz_plus=1 2>/dev/null + echo "status=$?" + declare -p zz_plus + + - name: "export -a and -A together with an assignment makes an indexed array" + # Given both, a shell lets -a win, whichever order they come in. + stdin: | + export -aA both_a=1 + export -Aa both_b=2 + export -aA 'both_c=([k]=v)' + declare -p both_a both_b both_c + + - name: "export rejects attribute options other than -a and -A" + stdin: | + for opt in -i -r -u -x -t -g; do + export $opt v=1 2>/dev/null + echo "$opt: $?" + done + declare -p v 2>/dev/null + echo "declare_status: $?" + + - name: "export -p prints declarations in declare -p form" + stdin: | + export zz_plain="a b" + export zz_quote="it's \$1" + export zz_arr=(x y) + export -p | grep "zz_" + + - name: "export -n on a missing variable creates nothing" + stdin: | + export -n nosuch_n + echo "status: $?" + declare -p nosuch_n 2>/dev/null + echo "declare_status: $?" + + - name: "export on a missing variable creates it exported and unset" + stdin: | + export nosuch_v + declare -p nosuch_v + + - name: "export -f lists exported functions with their attributes" + stdin: | + zz_a() { echo a; } + zz_b() { echo b; } + export -f zz_b + export -f | grep zz_ + export -pf | grep zz_ + export -nf zz_b + export -f | grep -c zz_ + + - name: "export -f with a missing function reports it" + stdin: | + strip() { sed -E 's/^[^ ]*: line [0-9]+: //'; } + { export -f nosuchfn; } 2>&1 | strip + export -f nosuchfn 2>/dev/null + echo "status: $?" + + - name: "export -a on a readonly variable is refused with the readonly diagnostic" + stdin: | + strip() { sed -E 's/^[^ ]*: line [0-9]+: //'; } + declare -r ro=1 + { export -a ro=2; } 2>&1 | strip + export ro 2>/dev/null + echo "bare_status: $?" + declare -p ro + + - name: "export -n with an assignment creates the variable unexported" + stdin: | + export -n zz_n=5 + echo "status: $?" + declare -p zz_n + export -n zz_n2+=x + declare -p zz_n2 + + - name: "set -a does not export what export -n assigns" + # `set -a` outranks `declare +x`, but not `export -n`. + stdin: | + set -a + export -n zz_na=1 + declare +x zz_nb=2 + set +a + declare -p zz_na zz_nb + + - name: "export -n with no operands lists exported variables but no functions" + stdin: | + export zz_v=1 + zz_f() { :; } + export -f zz_f + export -n | grep -c '^declare -x zz_v' + export -n | grep -c zz_f + export -nf | grep -c zz_f + + - name: "export with no operands lists in declare -p form" + stdin: | + export zz_v=1 + export | grep zz_ + + - name: "plain export and readonly report a readonly variable without naming the builtin" + stdin: | + strip() { sed -E 's/^[^ ]*: line [0-9]+: //'; } + readonly zz_ro=1 + { export zz_ro=5; } 2>&1 | strip + { export -n zz_ro=5; } 2>&1 | strip + { readonly zz_ro=5; } 2>&1 | strip + { export -a zz_ro=5; } 2>&1 | strip + { declare -x zz_ro=5; } 2>&1 | strip + declare -p zz_ro + + # + # A conversion failure is a builtin error for a scalar or quoted operand -- reported under the + # builtin's name, with the export attribute still granted -- but an assignment error for an + # unquoted compound operand: reported bare, nothing granted, and the command list abandoned. + # + - name: "export -a conversion failure on a scalar or quoted operand still exports" + stdin: | + strip() { sed -E 's/^[^ ]*: line [0-9]+: //'; } + declare -A s=([k]=v) q=([k]=v) + { export -a s=x; } 2>&1 | strip + { export -a 'q=(1)'; } 2>&1 | strip + export -a s=x 2>/dev/null + echo "status: $?" + declare -p s q + + - name: "export -a conversion failure on an unquoted compound operand is an assignment error" + ignore_stderr: true + stdin: | + declare -A u=([k]=v) + export -a u=(1); echo "same list" + echo "status: $?" + declare -p u + readonly r=1 + export r=(1); echo "same list" + echo "status: $?" + declare -p r + + - name: "export -n outranks set -a" + stdin: | + set -a + v=1 + export -n v=2 + declare -p v + w=1 + export -n w + declare -p w + export -n nv=3 + declare -p nv diff --git a/brush-shell/tests/cases/compat/builtins/pushd_popd_dirs.yaml b/brush-shell/tests/cases/compat/builtins/pushd_popd_dirs.yaml index 928127b67..409365f52 100644 --- a/brush-shell/tests/cases/compat/builtins/pushd_popd_dirs.yaml +++ b/brush-shell/tests/cases/compat/builtins/pushd_popd_dirs.yaml @@ -63,3 +63,15 @@ cases: pushd / dirs -c dirs + + # + # DIRSTACK mirrors `dirs`: element 0 is the current directory, so one pushd yields two + # elements. brush's dynamic DIRSTACK omits the current directory. + # + - name: "DIRSTACK includes the current directory" + known_failure: true # TODO: the DIRSTACK getter omits the current directory + stdin: | + cd /usr + echo "empty=${#DIRSTACK[@]}" + pushd / > /dev/null + echo "after_pushd=${#DIRSTACK[@]}" diff --git a/brush-shell/tests/cases/compat/builtins/readonly.yaml b/brush-shell/tests/cases/compat/builtins/readonly.yaml index e0f9c6c45..1b5d2ecc4 100644 --- a/brush-shell/tests/cases/compat/builtins/readonly.yaml +++ b/brush-shell/tests/cases/compat/builtins/readonly.yaml @@ -59,10 +59,8 @@ cases: echo "target: $target" - name: "readonly -f marks a function readonly rather than displaying it" - # brush routes `readonly -f` through the same display path as `declare -f`, so it prints the - # function's definition and never makes it readonly. A shell prints nothing, rejects a later - # redefinition, and reports a missing name as "readonly: NAME: not a function". - known_failure: true + # A shell prints nothing, rejects a later redefinition, and reports a missing name as + # "readonly: NAME: not a function". ignore_stderr: true stdin: | g() { echo original; } @@ -71,3 +69,351 @@ cases: g() { echo redefined; } g readonly -f nosuchfn + + # + # Readonly is enforced for assignments that target a single array element, not just for + # assignments that replace a whole variable. + # + - name: "readonly blocks a plain subscripted assignment" + ignore_stderr: true + stdin: | + arr=(one) + readonly arr + arr[0]=mutated + echo "status: $?" + declare -p arr + + - name: "readonly blocks a subscripted assignment through declare" + stdin: | + strip() { sed -E 's/^[^ ]*: line [0-9]+: //'; } + exec 2>err.txt + declare -ar arr=(one) + declare 'arr[0]=mutated' + echo "status: $?" + declare -p arr + strip err.txt + arr=(1 2) + readonly 'arr[0]=v' + echo "assign_status=$?" + declare -p arr + arr[0]=okay + echo "write_status=$?" + declare -p arr + arr2=(1) + readonly 'arr2[0]' + echo "bare_status=$?" + strip /dev/null + echo "assign_status=$? i=$i" + readonly "arr4[1/0]=v" 2>/dev/null + echo "error_status=$?" + readonly "arr5[i++]" 2>/dev/null + echo "bare_status=$? i=$i" + { readonly "arr3[i++]=v"; readonly "arr4[1/0]=v"; readonly "arr5[i++]"; } 2>&1 | strip + + - name: "a failed readonly assignment inside a pipeline reports its status" + known_failure: true # TODO: a readonly assignment failing as a pipeline element reports status 0 + stdin: | + readonly r=1 + r=2 | cat + echo "pipeline: ${PIPESTATUS[0]}" + + # + # readonly's -a/-A act only on an operand that assigns a value; on a bare operand they are + # ignored outright: a scalar stays a scalar and an associative array stays associative, both + # simply becoming readonly with status 0. + # + - name: "readonly -a on a scalar marks it readonly without conversion" + stdin: | + e=x + readonly -a e + echo "status=$?" + declare -p e + + - name: "readonly -a on an associative array is a selector not a conversion" + ignore_stderr: true + stdin: | + declare -A m2=([k]=v) + readonly -a m2 + echo "status=$?" + declare -p m2 + m2[k]=w + echo "write_status=$?" + + - name: "readonly -a with an assignment converts and types the variable" + stdin: | + e5=x + readonly -a e5=y + echo "value_status=$?" + declare -p e5 + readonly -A na=([k]=v) + echo "assoc_status=$?" + declare -p na + + # + # readonly takes only -a, -A, -f, and -p. Any other option is a usage error (status 2) that + # touches nothing, and a plus option is taken as an operand and rejected as an invalid + # identifier (status 1). + # + - name: "readonly rejects the options it does not take" + stdin: | + for opt in -g -i -x -t -l -u -c -F -I; do + readonly $opt zz_opt=1 2>/dev/null + echo "$opt: $?" + done + declare -p zz_opt 2>/dev/null + echo "declare_status: $?" + zz_opt=stillwritable + echo "write_status: $?" + + - name: "readonly treats a plus option as an invalid identifier operand" + stdin: | + strip() { sed -E 's/^[^ ]*: line [0-9]+: //'; } + { readonly +I; readonly +a zz_plus; } 2>&1 | strip + readonly +a zz_plus 2>/dev/null + echo "status=$?" + declare -p zz_plus 2>/dev/null + echo "declare_status: $?" + + - name: "readonly -a and -A together with an assignment makes an indexed array" + # Given both, a shell lets -a win, whichever order they come in; a bare operand gets neither. + stdin: | + readonly -aA zz_ra=1 + echo "status=$?" + readonly -Aa zz_rb=2 + readonly -aA zz_rc + declare -p zz_ra zz_rb zz_rc + + - name: "readonly -n is accepted and ignored" + known_failure: true # TODO: bash accepts `readonly -n` and assigns a plain scalar; brush rejects the option + stdin: | + t=1 + readonly -n r=t + echo "status=$?" + declare -p r + + - name: "the readonly attribute cannot be removed from a function" + stdin: | + strip() { sed -E 's/^[^ ]*: line [0-9]+: //'; } + g() { echo original; } + readonly -f g + { declare +r -f g; } 2>&1 | strip + declare +r -f g 2>/dev/null + echo "status: $?" + declare -F g + g + + - name: "a readonly function cannot be unset" + stdin: | + strip() { sed -E 's/^[^ ]*: line [0-9]+: //'; } + g() { echo original; } + h() { echo h; } + readonly -f g + { unset -f g; } 2>&1 | strip + unset -f g h 2>/dev/null + echo "status: $?" + g + declare -F h + echo "h_status: $?" + + - name: "redefining a readonly function fails without abandoning the list" + stdin: | + strip() { sed -E 's/^[^ ]*: line [0-9]+: //'; } + g() { echo original; } + readonly -f g + { g() { echo redefined; }; } 2>&1 | strip + { g() { echo redefined; }; echo "status: $?"; g; } 2>/dev/null + declare -pF g + + - name: "a readonly assignment failing inside a function abandons the caller's list" + known_failure: true # TODO: the error abandons only the function body, not the list that called it + stdin: | + readonly z=1 + g() { z=3; echo "in g after"; } + g; echo "caller same list" + echo "caller next line" + + - name: "readonly -f with no operands lists readonly functions with their attributes" + stdin: | + zz_ro() { echo ro; } + zz_rw() { echo rw; } + readonly -f zz_ro + readonly -f + echo "[p]" + readonly -pf + + # + # A conversion failure is a builtin error for a scalar or quoted operand -- reported under the + # builtin's name, with the readonly attribute still granted -- but an assignment error for an + # unquoted compound operand: reported bare, nothing granted, and the command list abandoned. + # + - name: "readonly -a conversion failure on a scalar or quoted operand still marks readonly" + stdin: | + strip() { sed -E 's/^[^ ]*: line [0-9]+: //'; } + declare -A s=([k]=v) q=([k]=v) + { readonly -a s=x; } 2>&1 | strip + { readonly -a 'q=(1)'; } 2>&1 | strip + echo "status: $?" + declare -p s q + + - name: "readonly -a conversion failure on an unquoted compound operand grants nothing" + min_oracle_version: "5.3" # bash 5.3 makes a bad compound key an assignment error; 5.2 assigned and granted + ignore_stderr: true + stdin: | + declare -A u=([k]=v) + readonly -a u=(1); echo "same list" + echo "status: $?" + declare -p u + u[z]=1 + echo "status: $?" + readonly -a b=(1 []=2); echo "same list" + echo "status: $?" + declare -p b + b[5]=x + echo "status: $?" + declare -p b + + # + # `-p` alongside operands is a no-op for readonly and export: the operands are applied as if + # `-p` were absent, and nothing is displayed. + # + - name: "readonly -p and export -p with operands apply them" + ignore_stderr: true + # stderr carries the failed write's diagnostic, which brush prints through the interpreter's + # error display rather than the assignment's own redirection. + stdin: | + readonly -p zz_rp=1 + echo "status: $?" + declare -p zz_rp + zz_rp=2 2>/dev/null + echo "write_status: $?" + export -p zz_ep=2 + echo "status: $?" + declare -p zz_ep + readonly -p zz_rq + export -p zz_eq + declare -p zz_rq zz_eq + + # + # A refusal is reported against the variable it is about, not against the command that tried to + # write it. The check matches on a suffix, so the shell's own `file: line N:` prefix is not part + # of the comparison. + # + - name: "Readonly refusals name the variable, not the command" + stdin: | + check() { + case $2 in + *"$1") echo "$1: ok";; + *) echo "$1: unexpected: $2";; + esac + } + readonly -a a=(1) + readonly r=1 + check "declare: a: readonly variable" "$( { declare a=9; } 2>&1 )" + check "r: readonly variable" "$( { export r=9; } 2>&1 )" + check "r: readonly variable" "$( { readonly r=9; } 2>&1 )" + check "local: r: readonly variable" "$( f() { local -g r=9; }; f 2>&1 )" + check "a: readonly variable" "$( { read -a a <<< 'p q'; } 2>&1 )" + declare -p a r + + # + # The same refusal raised by an assignment statement, by a builtin that writes an array, or by + # arithmetic names the variable too, but brush writes it to the shell's stderr through the + # interpreter's error display rather than to the command's own, so a redirection on the command + # does not capture it. + # + - name: "Readonly refusals from writes outside a declaration builtin are capturable" + known_failure: true # TODO: these diagnostics go to the shell's stderr, not the command's + stdin: | + check() { + case $2 in + *"$1") echo "$1: ok";; + *) echo "$1: unexpected: $2";; + esac + } + readonly -a a=(1) + readonly r=1 + check "a: readonly variable" "$( { a[0]=9; } 2>&1 )" + check "a: readonly variable" "$( { printf 'x\n' | mapfile -t a; } 2>&1 )" + check "a: readonly variable" "$( { (( a[0]++ )); } 2>&1 )" + check "r: readonly variable" "$( { (( r++ )); } 2>&1 )" + declare -p a r diff --git a/brush-shell/tests/cases/compat/builtins/trap.yaml b/brush-shell/tests/cases/compat/builtins/trap.yaml index da2a87042..656834cfa 100644 --- a/brush-shell/tests/cases/compat/builtins/trap.yaml +++ b/brush-shell/tests/cases/compat/builtins/trap.yaml @@ -161,7 +161,6 @@ cases: echo "after" - name: "trap ERR - has access to BASH_COMMAND" - known_failure: true # TODO(traps): ERR trap basic execution not implemented stdin: | trap 'echo "ERR for: $BASH_COMMAND"' ERR nonexistent_command_xyz 2>/dev/null @@ -618,6 +617,13 @@ cases: x=1 echo "x is $x" + - name: "DEBUG trap - output is not captured by the command's own redirection" + known_failure: true # TODO(traps): the DEBUG trap runs after the command's redirections are applied + stdin: | + trap 'echo "[debug: $BASH_COMMAND]"' DEBUG + echo hidden >/dev/null + echo visible + - name: "DEBUG trap - sees LINENO" known_failure: true # TODO(traps): LINENO in DEBUG trap not accurate stdin: | @@ -725,7 +731,6 @@ cases: echo "done" - name: "DEBUG trap - BASH_COMMAND reports commands before expansion" - known_failure: true # TODO: BASH_COMMAND improvements required stdin: | value=world trap 'case $BASH_COMMAND in echo*|declare*|printf*) echo "cmd: <$BASH_COMMAND>";; esac' DEBUG diff --git a/brush-shell/tests/cases/compat/builtins/unset.yaml b/brush-shell/tests/cases/compat/builtins/unset.yaml index 91fe630a6..0ffc1dfbf 100644 --- a/brush-shell/tests/cases/compat/builtins/unset.yaml +++ b/brush-shell/tests/cases/compat/builtins/unset.yaml @@ -259,3 +259,137 @@ cases: echo "[Checking after unset]" type [ + + # + # A readonly variable refuses to be unset. The failure is reported and fails the builtin's + # status, but the remaining names are still processed. The diagnostic is pinned through a + # pipeline, where it carries no line prefix. + # + - name: "unset refuses a readonly variable and continues with the rest" + stdin: | + strip() { sed -E 's/^[^ ]*: line [0-9]+: //'; } + readonly zz_ro=1 + zz_other=2 + { unset zz_ro zz_other; } 2>&1 | strip + unset zz_ro zz_other 2>/dev/null + echo "status: $?" + declare -p zz_ro + declare -p zz_other 2>/dev/null + echo "other_status: $?" + + - name: "unset refuses an element of a readonly array" + stdin: | + strip() { sed -E 's/^[^ ]*: line [0-9]+: //'; } + # One key only: bash and brush enumerate an associative array in different orders. + declare -A zz_m=([k]=1) + readonly zz_m + { unset 'zz_m[k]'; } 2>&1 | strip + unset 'zz_m[k]' 2>/dev/null + echo "status: $?" + declare -p zz_m + declare -a zz_a=(1 2) + readonly zz_a + { unset 'zz_a[0]'; } 2>&1 | strip + unset 'zz_a[0]' 2>/dev/null + echo "status: $?" + declare -p zz_a + + # + # An element operand's subscript is resolved the way every other subscript is: arithmetically + # for an indexed array (so a blank one is element 0), literally for an associative one -- after + # one round of word expansion in both cases. + # + - name: "unset resolves an element subscript against the array's kind" + stdin: | + declare -a zz_a=(1 2 3) + i=1 + unset 'zz_a[i]' + declare -p zz_a + declare -a zz_b=(1 2 3) + unset 'zz_b[1+1]' + declare -p zz_b + declare -a zz_c=(1 2 3) + j=2 + unset 'zz_c[$j]' + declare -p zz_c + + - name: "unset does not evaluate an associative key arithmetically" + stdin: | + declare -A zz_m + zz_m[1+1]=literal + zz_m[2]=numeric + unset 'zz_m[1+1]' + declare -p zz_m + + - name: "unset expands an associative key once" + stdin: | + declare -A zz_m + zz_m[k]=v + k=k + unset 'zz_m[$k]' + declare -p zz_m + echo "status: $?" + + # + # A subscript no element can have is not an error: an empty one names nothing and is quietly + # ignored, while `*` and `@` name every element of an indexed array (clearing it, but leaving it + # declared) and -- a shell quirk -- nothing at all in an associative one. + # + - name: "unset with an empty subscript does nothing" + stdin: | + declare -a zz_a=(1 2 3) + unset 'zz_a[]' + echo "status: $?" + declare -p zz_a + declare -A zz_m=([k]=v) + unset 'zz_m[]' + echo "status: $?" + declare -p zz_m + + - name: "unset with a star or at subscript clears an indexed array" + stdin: | + declare -a zz_a=(1 2 3) + unset 'zz_a[*]' + echo "status: $?" + declare -p zz_a + declare -a zz_b=(1 2 3) + unset 'zz_b[@]' + echo "status: $?" + declare -p zz_b + + - name: "unset with a star or at subscript leaves an associative array alone" + stdin: | + declare -A zz_m=([k]=v) + unset 'zz_m[*]' + echo "status: $?" + declare -p zz_m + unset 'zz_m[@]' + echo "status: $?" + declare -p zz_m + + - name: "unset with a star or at subscript removes that key from an associative array" + # `*` and `@` clear an indexed array, but an associative array can hold them as ordinary + # keys, so there they name one element like any other key. + stdin: | + declare -A zz_m=(['*']=star [k]=v) + echo "before: ${#zz_m[@]}" + unset 'zz_m[*]' + echo "status: $?" + declare -p zz_m + declare -A zz_n=(['@']=at [k]=v) + unset 'zz_n[@]' + echo "status: $?" + echo "after: ${#zz_n[@]} ${zz_n[k]}" + + - name: "unset refuses every element subscript of a readonly array" + stdin: | + strip() { sed -E 's/^[^ ]*: line [0-9]+: //'; } + declare -a zz_a=(1 2) + readonly zz_a + { unset 'zz_a[*]'; } 2>&1 | strip + unset 'zz_a[*]' 2>/dev/null + echo "star: $?" + { unset 'zz_a[]'; } 2>&1 | strip + unset 'zz_a[]' 2>/dev/null + echo "empty: $?" + declare -p zz_a diff --git a/brush-shell/tests/cases/compat/compound_cmds/for.yaml b/brush-shell/tests/cases/compat/compound_cmds/for.yaml index 0eedf9321..10e929f7c 100644 --- a/brush-shell/tests/cases/compat/compound_cmds/for.yaml +++ b/brush-shell/tests/cases/compat/compound_cmds/for.yaml @@ -140,7 +140,7 @@ cases: echo "Left inner loop" done - - name: "Multi-line for loop" + - name: "Multi-line for loop in a script" test_files: - path: "script.sh" contents: | diff --git a/brush-shell/tests/cases/compat/here.yaml b/brush-shell/tests/cases/compat/here.yaml index 596c95477..15b0de01c 100644 --- a/brush-shell/tests/cases/compat/here.yaml +++ b/brush-shell/tests/cases/compat/here.yaml @@ -66,7 +66,7 @@ cases: wc -l <&2' DEBUG @@ -143,8 +139,11 @@ cases: # # Function-specific trace attribute (declare -t) # + # N.B. The cases immediately below name a function but write `declare -t name`, which without + # `-f` gives the trace attribute to a *variable* of that name. The function attribute is + # `declare -ft name`; it is covered separately further down. + # - name: "declare -t - enables trace for specific function" - known_failure: true # TODO(BASH_COMMAND): quotes differ stdin: | trap 'echo "[debug: $BASH_COMMAND]"' DEBUG traced_func() { @@ -312,3 +311,42 @@ cases: a echo "debug count: $count" + # + # The function trace attribute proper (`declare -ft`). A traced function inherits the DEBUG and + # RETURN traps from the calling shell, the way `set -T` does for every function. brush records + # and displays the attribute, but does not yet consult it when deciding whether a trap fires + # inside a function. + # + - name: "declare -ft - records and displays the function trace attribute" + stdin: | + f() { :; } + g() { :; } + declare -ft f + declare -pF f + declare -pF g + declare -Ft + + - name: "declare -ft - a traced function inherits the DEBUG trap" + known_failure: true # TODO(functrace): the function trace attribute is recorded but not honored + stdin: | + trap 'echo "[debug: $BASH_COMMAND]"' DEBUG + traced() { + echo "in traced" + } + untraced() { + echo "in untraced" + } + declare -ft traced + untraced + traced + + - name: "declare -ft - a traced function inherits the RETURN trap" + known_failure: true # TODO(functrace): the function trace attribute is recorded but not honored + stdin: | + trap 'echo "[return from ${FUNCNAME[0]}]"' RETURN + traced() { echo "in traced"; } + untraced() { echo "in untraced"; } + declare -ft traced + untraced + traced + echo done diff --git a/brush-shell/tests/cases/compat/options/set-x.yaml b/brush-shell/tests/cases/compat/options/set-x.yaml index 1019c73ca..54a5d3bee 100644 --- a/brush-shell/tests/cases/compat/options/set-x.yaml +++ b/brush-shell/tests/cases/compat/options/set-x.yaml @@ -64,7 +64,6 @@ cases: echo "After function call" - name: "declaration assignments show expanded values" - known_failure: true # TODO: xtrace improvements required stdin: | value=world set -x @@ -134,7 +133,6 @@ cases: set +x - name: "wrappers trace the full invocation for declaration targets" - known_failure: true # TODO: xtrace improvements required stdin: | value=world set -x @@ -146,7 +144,6 @@ cases: set +x - name: "export and readonly echo assignments but not bare names" - known_failure: true # TODO: xtrace improvements required stdin: | set -x export a=1 b=2 @@ -158,7 +155,6 @@ cases: set +x - name: "export and readonly echo assignments recognized from strings" - known_failure: true # TODO: xtrace improvements required stdin: | set -x readonly 'q=1' @@ -166,7 +162,6 @@ cases: set +x - name: "assignment operands are traced as single quoted words" - known_failure: true # TODO: xtrace improvements required stdin: | v="a b" set -x @@ -191,8 +186,7 @@ cases: - name: "compound assignment operands are traced as two lines" # A shell traces a compound assignment operand as a standalone assignment line followed by the - # declaration command carrying only the variable name; brush emits a single combined line. - known_failure: true + # declaration command carrying only the variable name. stdin: | set -x declare -a b=(1 2) @@ -210,3 +204,34 @@ cases: x=1 echo done trap - DEBUG + + - name: "compound operand traces quote every element and key" + stdin: | + set -x + declare -a k1=([2]=a [5]="b c") + declare -a k2=() + declare -a k1+=(z "it's") + declare -A k3=([k]=v ['x y']=1 [n]="") + X=1 declare -a k4=(1) k5=2 + declare -a 'k6=(1 2)' k7=(3) + f() { local l=(1 2) m=v; } + f + set +x + + - name: "compound assignment statements are traced as written" + stdin: | + x=5 + set -x + a=($x "$x y" [1+1]=z) + a+=("$x") + b[1+1]=$x + declare -A c + c=([k]="$x") + set +x + + - name: "compound operand traces escape single quotes in keys and values" + stdin: | + set -x + declare -a q=("a'b" [3]="c'd") + declare -A r=(["it's"]="x'y" [k]="") + set +x diff --git a/brush-shell/tests/cases/compat/variables.yaml b/brush-shell/tests/cases/compat/variables.yaml index c9ee70e4a..a0b0a2005 100644 --- a/brush-shell/tests/cases/compat/variables.yaml +++ b/brush-shell/tests/cases/compat/variables.yaml @@ -79,38 +79,269 @@ cases: } f - - name: "local -I inherits from enclosing scope" + # + # An inheriting local reaches past the local scope it creates in, so the array kind governing + # its subscripts has to come from the variable it inherits -- not from the not-yet-created + # local, which would default to indexed and arithmetically evaluate an associative key to 0. + # + - name: "local -I resolves an associative subscript against the inherited variable" stdin: | - g=global-value + declare -A m=([foo]=x) + f() { local -I 'm[foo]+=y'; declare -p m; } + f + declare -p m + + - name: "local -I assigns an associative key without arithmetic evaluation" + stdin: | + declare -A n=([bar]=1) + g() { local -I 'n[bar]=2'; declare -p n; } + g + + - name: "local -I resolves an associative subscript in a parsed operand" + stdin: | + declare -A p=([k]=a) + h() { local -I p[k]+=b; declare -p p; } + h + + - name: "local -I still evaluates arithmetic for an inherited indexed array" + stdin: | + declare -a a=(zero one two) + i=1 + f() { local -I "a[i+1]=X"; declare -p a; } + f + + - name: "local -I with a matching explicit array kind inherits value and attributes" + stdin: | + declare -ax a=(one two) + declare -A m=([k]=v) f() { - local -I g - echo "[$g]" + local -I -a a; declare -p a + local -I -A m; declare -p m } f - echo "[$g]" - - name: "local -I with append" + - name: "local -I with a conflicting array kind fails" stdin: | - v=base + strip() { sed -E 's/^[^ ]*: line [0-9]+: //'; } + exec 2>err.txt + declare -a x=(one two) + f() { local -I -A x; echo "status=$?"; } + f + declare -A y=([k]=v) + g() { local -I -a y; echo "status=$?"; } + g + declare -p x y + strip err.txt + f() { local -a w=(one); local -I -A w; echo "status=$?"; declare -p w; } + f + strip err.txt + declare -A z=([k]=v) + f() { local -I -a z; echo "status=$?"; z[0]=inner; } + f + declare -p z + strip err.txt + declare -a a1=(1 2) + f() { local -I +a a1; echo "status=$?"; } + f + strip /dev/null + before=${#DIRSTACK[@]} + f() { local -I -a DIRSTACK; echo "inherited_all=$(( ${#DIRSTACK[@]} == before ))"; } + f + + # + # A readonly inheritee refuses `local -I` outright: status 1 and no local is created, whatever + # the kinds involved. + # + - name: "local -I over a readonly enclosing variable is refused" + stdin: | + strip() { sed -E 's/^[^ ]*: line [0-9]+: //'; } + exec 2>err.txt + readonly -a arr=(1 2) + f() { local -I -a arr; echo "status=$?"; } + f + readonly r=x + g() { local -I r; echo "scalar_status=$?"; } + g + strip err.txt + declare -a cx=(one) + f() { local -I -A cx cy=z; echo "status=$?"; declare -p cy; } + f + strip