From 696733b88f743022de685e807d0e5907abe62732 Mon Sep 17 00:00:00 2001 From: reuben olinsky Date: Mon, 27 Jan 2025 21:38:14 -0800 Subject: [PATCH] chore: remove unneeded result wrappings (#367) --- brush-core/src/arithmetic.rs | 2 -- brush-core/src/expansion.rs | 43 ++++++++++++++++++------------------ brush-core/src/interp.rs | 4 ++-- brush-core/src/variables.rs | 17 ++++++-------- 4 files changed, 30 insertions(+), 36 deletions(-) diff --git a/brush-core/src/arithmetic.rs b/brush-core/src/arithmetic.rs index 59f51dd9..0dfe413f 100644 --- a/brush-core/src/arithmetic.rs +++ b/brush-core/src/arithmetic.rs @@ -165,7 +165,6 @@ fn deref_lvalue(shell: &mut Shell, lvalue: &ast::ArithmeticTarget) -> Result Result { match lvalue { ast::ArithmeticTarget::Variable(name) => { diff --git a/brush-core/src/expansion.rs b/brush-core/src/expansion.rs index d4b6012b..78e565da 100644 --- a/brush-core/src/expansion.rs +++ b/brush-core/src/expansion.rs @@ -1086,12 +1086,12 @@ impl<'a> WordExpander<'a> { )?; transform_expansion(expanded_parameter, |s| { - Self::replace_substring( + Ok(Self::replace_substring( s.as_str(), ®ex, expanded_replacement.as_str(), &match_kind, - ) + )) }) } brush_parser::word::ParameterExpr::VariableNames { @@ -1266,7 +1266,8 @@ impl<'a> WordExpander<'a> { match parameter { brush_parser::word::Parameter::Positional(p) => { if *p == 0 { - self.expand_special_parameter(&brush_parser::word::SpecialParameter::ShellName) + Ok(self + .expand_special_parameter(&brush_parser::word::SpecialParameter::ShellName)) } else if let Some(parameter) = self.shell.positional_parameters.get((p - 1) as usize) { @@ -1275,7 +1276,7 @@ impl<'a> WordExpander<'a> { Ok(Expansion::undefined()) } } - brush_parser::word::Parameter::Special(s) => self.expand_special_parameter(s), + brush_parser::word::Parameter::Special(s) => Ok(self.expand_special_parameter(s)), brush_parser::word::Parameter::Named(n) => { if !valid_variable_name(n.as_str()) { Err(error::Error::BadSubstitution) @@ -1360,16 +1361,15 @@ impl<'a> WordExpander<'a> { Ok(index_to_use) } - #[allow(clippy::unnecessary_wraps)] fn expand_special_parameter( &mut self, parameter: &brush_parser::word::SpecialParameter, - ) -> Result { + ) -> Expansion { match parameter { brush_parser::word::SpecialParameter::AllPositionalParameters { concatenate } => { let positional_params = self.shell.positional_parameters.iter(); - Ok(Expansion { + Expansion { fields: positional_params .into_iter() .map(|param| WordField(vec![ExpansionPiece::Splittable(param.to_owned())])) @@ -1377,34 +1377,34 @@ impl<'a> WordExpander<'a> { concatenate: *concatenate, from_array: true, undefined: false, - }) + } + } + brush_parser::word::SpecialParameter::PositionalParameterCount => { + Expansion::from(self.shell.positional_parameters.len().to_string()) } - brush_parser::word::SpecialParameter::PositionalParameterCount => Ok(Expansion::from( - self.shell.positional_parameters.len().to_string(), - )), brush_parser::word::SpecialParameter::LastExitStatus => { - Ok(Expansion::from(self.shell.last_exit_status.to_string())) + Expansion::from(self.shell.last_exit_status.to_string()) } brush_parser::word::SpecialParameter::CurrentOptionFlags => { - Ok(Expansion::from(self.shell.options.get_option_flags())) + Expansion::from(self.shell.options.get_option_flags()) } brush_parser::word::SpecialParameter::ProcessId => { - Ok(Expansion::from(std::process::id().to_string())) + Expansion::from(std::process::id().to_string()) } brush_parser::word::SpecialParameter::LastBackgroundProcessId => { if let Some(job) = self.shell.jobs.current_job() { if let Some(pid) = job.get_representative_pid() { - return Ok(Expansion::from(pid.to_string())); + return Expansion::from(pid.to_string()); } } - Ok(Expansion::from(String::new())) + Expansion::from(String::new()) } - brush_parser::word::SpecialParameter::ShellName => Ok(Expansion::from( + brush_parser::word::SpecialParameter::ShellName => Expansion::from( self.shell .shell_name .as_ref() .map_or_else(String::new, |name| name.clone()), - )), + ), } } @@ -1508,22 +1508,21 @@ impl<'a> WordExpander<'a> { } } - #[allow(clippy::unnecessary_wraps)] fn replace_substring( s: &str, regex: &fancy_regex::Regex, replacement: &str, match_kind: &SubstringMatchKind, - ) -> Result { + ) -> String { match match_kind { brush_parser::word::SubstringMatchKind::Prefix | brush_parser::word::SubstringMatchKind::Suffix | brush_parser::word::SubstringMatchKind::FirstOccurrence => { - Ok(regex.replace(s, replacement).into_owned()) + regex.replace(s, replacement).into_owned() } brush_parser::word::SubstringMatchKind::Anywhere => { - Ok(regex.replace_all(s, replacement).into_owned()) + regex.replace_all(s, replacement).into_owned() } } } diff --git a/brush-core/src/interp.rs b/brush-core/src/interp.rs index c1205fd9..d2c38007 100644 --- a/brush-core/src/interp.rs +++ b/brush-core/src/interp.rs @@ -1288,7 +1288,7 @@ async fn apply_assignment( let new_value = if let Some(array_index) = array_index { match new_value { ShellValueLiteral::Scalar(s) => { - ShellValue::indexed_array_from_literals(ArrayLiteral(vec![(Some(array_index), s)]))? + ShellValue::indexed_array_from_literals(ArrayLiteral(vec![(Some(array_index), s)])) } ShellValueLiteral::Array(_) => { return error::unimp("cannot assign list to array member"); @@ -1300,7 +1300,7 @@ async fn apply_assignment( export = export || shell.options.export_variables_on_modification; ShellValue::String(s) } - ShellValueLiteral::Array(values) => ShellValue::indexed_array_from_literals(values)?, + ShellValueLiteral::Array(values) => ShellValue::indexed_array_from_literals(values), } }; diff --git a/brush-core/src/variables.rs b/brush-core/src/variables.rs index d1ddf76a..89eb3f28 100644 --- a/brush-core/src/variables.rs +++ b/brush-core/src/variables.rs @@ -284,7 +284,8 @@ impl ShellVariable { self.assign_at_index(String::from("0"), new_value, append) } ShellValueLiteral::Array(new_values) => { - ShellValue::update_indexed_array_from_literals(existing_values, new_values) + ShellValue::update_indexed_array_from_literals(existing_values, new_values); + Ok(()) } }, ShellValue::AssociativeArray(existing_values) => match value { @@ -327,7 +328,7 @@ impl ShellVariable { | ShellValue::Dynamic { .. }, ShellValueLiteral::Array(literal_values), ) => { - self.value = ShellValue::indexed_array_from_literals(literal_values)?; + self.value = ShellValue::indexed_array_from_literals(literal_values); Ok(()) } @@ -701,18 +702,17 @@ impl ShellValue { /// # Arguments /// /// * `literals` - The literals to construct the indexed array from. - pub fn indexed_array_from_literals(literals: ArrayLiteral) -> Result { + pub fn indexed_array_from_literals(literals: ArrayLiteral) -> ShellValue { let mut values = BTreeMap::new(); - ShellValue::update_indexed_array_from_literals(&mut values, literals)?; + ShellValue::update_indexed_array_from_literals(&mut values, literals); - Ok(ShellValue::IndexedArray(values)) + ShellValue::IndexedArray(values) } - #[allow(clippy::unnecessary_wraps)] fn update_indexed_array_from_literals( existing_values: &mut BTreeMap, literal_values: ArrayLiteral, - ) -> Result<(), error::Error> { + ) { let mut new_key = if let Some((largest_index, _)) = existing_values.last_key_value() { largest_index + 1 } else { @@ -727,8 +727,6 @@ impl ShellValue { existing_values.insert(new_key, value); new_key += 1; } - - Ok(()) } /// Returns a new associative array value constructed from the given literals. @@ -830,7 +828,6 @@ impl ShellValue { /// # Arguments /// /// * `index` - The index at which to retrieve the value. - #[allow(clippy::unnecessary_wraps)] pub fn get_at(&self, index: &str, shell: &Shell) -> Result>, error::Error> { match self { ShellValue::Unset(_) => Ok(None),