Skip to content

Commit

Permalink
chore: remove unneeded result wrappings (#367)
Browse files Browse the repository at this point in the history
  • Loading branch information
reubeno authored Jan 28, 2025
1 parent 42d75a7 commit 696733b
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 36 deletions.
2 changes: 0 additions & 2 deletions brush-core/src/arithmetic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,6 @@ fn deref_lvalue(shell: &mut Shell, lvalue: &ast::ArithmeticTarget) -> Result<i64
parsed_value.eval(shell)
}

#[allow(clippy::unnecessary_wraps)]
fn apply_unary_op(
shell: &mut Shell,
op: ast::UnaryOperator,
Expand Down Expand Up @@ -292,7 +291,6 @@ fn apply_unary_assignment_op(
}
}

#[allow(clippy::unnecessary_wraps)]
fn assign(shell: &mut Shell, lvalue: &ast::ArithmeticTarget, value: i64) -> Result<i64, EvalError> {
match lvalue {
ast::ArithmeticTarget::Variable(name) => {
Expand Down
43 changes: 21 additions & 22 deletions brush-core/src/expansion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1086,12 +1086,12 @@ impl<'a> WordExpander<'a> {
)?;

transform_expansion(expanded_parameter, |s| {
Self::replace_substring(
Ok(Self::replace_substring(
s.as_str(),
&regex,
expanded_replacement.as_str(),
&match_kind,
)
))
})
}
brush_parser::word::ParameterExpr::VariableNames {
Expand Down Expand Up @@ -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)
{
Expand All @@ -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)
Expand Down Expand Up @@ -1360,51 +1361,50 @@ 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, error::Error> {
) -> 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())]))
.collect(),
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()),
)),
),
}
}

Expand Down Expand Up @@ -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, error::Error> {
) -> 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()
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions brush-core/src/interp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand All @@ -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),
}
};

Expand Down
17 changes: 7 additions & 10 deletions brush-core/src/variables.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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(())
}

Expand Down Expand Up @@ -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<ShellValue, error::Error> {
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<u64, String>,
literal_values: ArrayLiteral,
) -> Result<(), error::Error> {
) {
let mut new_key = if let Some((largest_index, _)) = existing_values.last_key_value() {
largest_index + 1
} else {
Expand All @@ -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.
Expand Down Expand Up @@ -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<Option<Cow<'_, str>>, error::Error> {
match self {
ShellValue::Unset(_) => Ok(None),
Expand Down

0 comments on commit 696733b

Please sign in to comment.