refactor: ensure range is bounded (#15429)

No linked issue, it's a follow-up of 2 PRs I recently made to improve
some math commands. (#15319)

# Description
Small refactor to simplify the code. It was suggested in the comments of
my previous PR.

# User-Facing Changes
None

# Tests + Formatting
Tests, fmt and clippy OK

# After Submitting
Nothing more required
This commit is contained in:
Loïc Riegel 2025-03-27 14:25:55 +01:00 committed by GitHub
parent 07be33c119
commit 5d32cd2c40
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
8 changed files with 30 additions and 71 deletions

View File

@ -1,6 +1,5 @@
use crate::math::utils::ensure_bounded; use crate::math::utils::ensure_bounded;
use nu_engine::command_prelude::*; use nu_engine::command_prelude::*;
use nu_protocol::Range;
#[derive(Clone)] #[derive(Clone)]
pub struct MathAbs; pub struct MathAbs;
@ -57,10 +56,7 @@ impl Command for MathAbs {
.., ..,
) = input ) = input
{ {
match &**val { ensure_bounded(val.as_ref(), internal_span, head)?;
Range::IntRange(range) => ensure_bounded(range.end(), internal_span, head)?,
Range::FloatRange(range) => ensure_bounded(range.end(), internal_span, head)?,
}
} }
input.map(move |value| abs_helper(value, head), engine_state.signals()) input.map(move |value| abs_helper(value, head), engine_state.signals())
} }
@ -80,10 +76,7 @@ impl Command for MathAbs {
.., ..,
) = input ) = input
{ {
match &**val { ensure_bounded(val.as_ref(), internal_span, head)?;
Range::IntRange(range) => ensure_bounded(range.end(), internal_span, head)?,
Range::FloatRange(range) => ensure_bounded(range.end(), internal_span, head)?,
}
} }
input.map( input.map(
move |value| abs_helper(value, head), move |value| abs_helper(value, head),

View File

@ -1,6 +1,5 @@
use crate::math::utils::ensure_bounded; use crate::math::utils::ensure_bounded;
use nu_engine::command_prelude::*; use nu_engine::command_prelude::*;
use nu_protocol::Range;
#[derive(Clone)] #[derive(Clone)]
pub struct MathCeil; pub struct MathCeil;
@ -56,10 +55,7 @@ impl Command for MathCeil {
.., ..,
) = input ) = input
{ {
match &**val { ensure_bounded(val.as_ref(), internal_span, head)?;
Range::IntRange(range) => ensure_bounded(range.end(), internal_span, head)?,
Range::FloatRange(range) => ensure_bounded(range.end(), internal_span, head)?,
}
} }
input.map(move |value| operate(value, head), engine_state.signals()) input.map(move |value| operate(value, head), engine_state.signals())
} }
@ -83,10 +79,7 @@ impl Command for MathCeil {
.., ..,
) = input ) = input
{ {
match &**val { ensure_bounded(val.as_ref(), internal_span, head)?;
Range::IntRange(range) => ensure_bounded(range.end(), internal_span, head)?,
Range::FloatRange(range) => ensure_bounded(range.end(), internal_span, head)?,
}
} }
input.map( input.map(
move |value| operate(value, head), move |value| operate(value, head),

View File

@ -1,6 +1,5 @@
use crate::math::utils::ensure_bounded; use crate::math::utils::ensure_bounded;
use nu_engine::command_prelude::*; use nu_engine::command_prelude::*;
use nu_protocol::Range;
#[derive(Clone)] #[derive(Clone)]
pub struct MathFloor; pub struct MathFloor;
@ -56,10 +55,7 @@ impl Command for MathFloor {
.., ..,
) = input ) = input
{ {
match &**val { ensure_bounded(val.as_ref(), internal_span, head)?;
Range::IntRange(range) => ensure_bounded(range.end(), internal_span, head)?,
Range::FloatRange(range) => ensure_bounded(range.end(), internal_span, head)?,
}
} }
input.map(move |value| operate(value, head), engine_state.signals()) input.map(move |value| operate(value, head), engine_state.signals())
} }
@ -83,10 +79,7 @@ impl Command for MathFloor {
.., ..,
) = input ) = input
{ {
match &**val { ensure_bounded(val.as_ref(), internal_span, head)?;
Range::IntRange(range) => ensure_bounded(range.end(), internal_span, head)?,
Range::FloatRange(range) => ensure_bounded(range.end(), internal_span, head)?,
}
} }
input.map( input.map(
move |value| operate(value, head), move |value| operate(value, head),

View File

@ -1,6 +1,5 @@
use crate::math::utils::ensure_bounded; use crate::math::utils::ensure_bounded;
use nu_engine::command_prelude::*; use nu_engine::command_prelude::*;
use nu_protocol::Range;
use nu_protocol::Signals; use nu_protocol::Signals;
#[derive(Clone)] #[derive(Clone)]
@ -59,10 +58,7 @@ impl Command for MathLog {
.., ..,
) = input ) = input
{ {
match &**val { ensure_bounded(val.as_ref(), internal_span, head)?;
Range::IntRange(range) => ensure_bounded(range.end(), internal_span, head)?,
Range::FloatRange(range) => ensure_bounded(range.end(), internal_span, head)?,
}
} }
log(base, call.head, input, engine_state.signals()) log(base, call.head, input, engine_state.signals())
} }
@ -83,10 +79,7 @@ impl Command for MathLog {
.., ..,
) = input ) = input
{ {
match &**val { ensure_bounded(val.as_ref(), internal_span, head)?;
Range::IntRange(range) => ensure_bounded(range.end(), internal_span, head)?,
Range::FloatRange(range) => ensure_bounded(range.end(), internal_span, head)?,
}
} }
log(base, call.head, input, working_set.permanent().signals()) log(base, call.head, input, working_set.permanent().signals())
} }

View File

@ -1,6 +1,5 @@
use crate::math::utils::ensure_bounded; use crate::math::utils::ensure_bounded;
use nu_engine::command_prelude::*; use nu_engine::command_prelude::*;
use nu_protocol::Range;
#[derive(Clone)] #[derive(Clone)]
pub struct MathRound; pub struct MathRound;
@ -63,10 +62,7 @@ impl Command for MathRound {
.., ..,
) = input ) = input
{ {
match &**val { ensure_bounded(val.as_ref(), internal_span, head)?;
Range::IntRange(range) => ensure_bounded(range.end(), internal_span, head)?,
Range::FloatRange(range) => ensure_bounded(range.end(), internal_span, head)?,
}
} }
input.map( input.map(
move |value| operate(value, head, precision_param), move |value| operate(value, head, precision_param),
@ -94,10 +90,7 @@ impl Command for MathRound {
.., ..,
) = input ) = input
{ {
match &**val { ensure_bounded(val.as_ref(), internal_span, head)?;
Range::IntRange(range) => ensure_bounded(range.end(), internal_span, head)?,
Range::FloatRange(range) => ensure_bounded(range.end(), internal_span, head)?,
}
} }
input.map( input.map(
move |value| operate(value, head, precision_param), move |value| operate(value, head, precision_param),

View File

@ -1,6 +1,5 @@
use crate::math::utils::ensure_bounded; use crate::math::utils::ensure_bounded;
use nu_engine::command_prelude::*; use nu_engine::command_prelude::*;
use nu_protocol::Range;
#[derive(Clone)] #[derive(Clone)]
pub struct MathSqrt; pub struct MathSqrt;
@ -56,10 +55,7 @@ impl Command for MathSqrt {
.., ..,
) = input ) = input
{ {
match &**val { ensure_bounded(val.as_ref(), internal_span, head)?;
Range::IntRange(range) => ensure_bounded(range.end(), internal_span, head)?,
Range::FloatRange(range) => ensure_bounded(range.end(), internal_span, head)?,
}
} }
input.map(move |value| operate(value, head), engine_state.signals()) input.map(move |value| operate(value, head), engine_state.signals())
} }
@ -83,10 +79,7 @@ impl Command for MathSqrt {
.., ..,
) = input ) = input
{ {
match &**val { ensure_bounded(val.as_ref(), internal_span, head)?;
Range::IntRange(range) => ensure_bounded(range.end(), internal_span, head)?,
Range::FloatRange(range) => ensure_bounded(range.end(), internal_span, head)?,
}
} }
input.map( input.map(
move |value| operate(value, head), move |value| operate(value, head),

View File

@ -1,4 +1,4 @@
use core::{ops::Bound, slice}; use core::slice;
use indexmap::IndexMap; use indexmap::IndexMap;
use nu_protocol::{ use nu_protocol::{
engine::Call, IntoPipelineData, PipelineData, Range, ShellError, Signals, Span, Value, engine::Call, IntoPipelineData, PipelineData, Range, ShellError, Signals, Span, Value,
@ -93,10 +93,7 @@ pub fn calculate(
Ok(Value::record(record, span)) Ok(Value::record(record, span))
} }
PipelineData::Value(Value::Range { val, .. }, ..) => { PipelineData::Value(Value::Range { val, .. }, ..) => {
match *val { ensure_bounded(val.as_ref(), span, name)?;
Range::IntRange(range) => ensure_bounded(range.end(), span, name)?,
Range::FloatRange(range) => ensure_bounded(range.end(), span, name)?,
}
let new_vals: Result<Vec<Value>, ShellError> = val let new_vals: Result<Vec<Value>, ShellError> = val
.into_range_iter(span, Signals::empty()) .into_range_iter(span, Signals::empty())
.map(|val| mf(&[val], span, name)) .map(|val| mf(&[val], span, name))
@ -117,17 +114,13 @@ pub fn calculate(
} }
} }
pub fn ensure_bounded<T>( pub fn ensure_bounded(range: &Range, val_span: Span, call_span: Span) -> Result<(), ShellError> {
bound: Bound<T>, if range.is_bounded() {
val_span: Span, return Ok(());
call_span: Span,
) -> Result<(), ShellError> {
match bound {
Bound::<T>::Unbounded => Err(ShellError::IncorrectValue {
msg: "Range must be bounded".to_string(),
val_span,
call_span,
}),
_ => Ok(()),
} }
Err(ShellError::IncorrectValue {
msg: "Range must be bounded".to_string(),
val_span,
call_span,
})
} }

View File

@ -1,6 +1,7 @@
//! A Range is an iterator over integers or floats. //! A Range is an iterator over integers or floats.
use crate::{ast::RangeInclusion, ShellError, Signals, Span, Value}; use crate::{ast::RangeInclusion, ShellError, Signals, Span, Value};
use core::ops::Bound;
use serde::{Deserialize, Serialize}; use serde::{Deserialize, Serialize};
use std::{cmp::Ordering, fmt::Display}; use std::{cmp::Ordering, fmt::Display};
@ -631,6 +632,13 @@ impl Range {
} }
} }
pub fn is_bounded(&self) -> bool {
match self {
Range::IntRange(range) => range.end() != Bound::<i64>::Unbounded,
Range::FloatRange(range) => range.end() != Bound::<f64>::Unbounded,
}
}
pub fn into_range_iter(self, span: Span, signals: Signals) -> Iter { pub fn into_range_iter(self, span: Span, signals: Signals) -> Iter {
match self { match self {
Range::IntRange(range) => Iter::IntIter(range.into_range_iter(signals), span), Range::IntRange(range) => Iter::IntIter(range.into_range_iter(signals), span),