From 5d32cd2c404dbdd45500712c197e9a74759e6f0f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Riegel?= <96702577+LoicRiegel@users.noreply.github.com> Date: Thu, 27 Mar 2025 14:25:55 +0100 Subject: [PATCH] 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 --- crates/nu-command/src/math/abs.rs | 11 ++--------- crates/nu-command/src/math/ceil.rs | 11 ++--------- crates/nu-command/src/math/floor.rs | 11 ++--------- crates/nu-command/src/math/log.rs | 11 ++--------- crates/nu-command/src/math/round.rs | 11 ++--------- crates/nu-command/src/math/sqrt.rs | 11 ++--------- crates/nu-command/src/math/utils.rs | 27 ++++++++++----------------- crates/nu-protocol/src/value/range.rs | 8 ++++++++ 8 files changed, 30 insertions(+), 71 deletions(-) diff --git a/crates/nu-command/src/math/abs.rs b/crates/nu-command/src/math/abs.rs index 5ebae80e2f..ec86c5a3c3 100644 --- a/crates/nu-command/src/math/abs.rs +++ b/crates/nu-command/src/math/abs.rs @@ -1,6 +1,5 @@ use crate::math::utils::ensure_bounded; use nu_engine::command_prelude::*; -use nu_protocol::Range; #[derive(Clone)] pub struct MathAbs; @@ -57,10 +56,7 @@ impl Command for MathAbs { .., ) = input { - match &**val { - Range::IntRange(range) => ensure_bounded(range.end(), internal_span, head)?, - Range::FloatRange(range) => ensure_bounded(range.end(), internal_span, head)?, - } + ensure_bounded(val.as_ref(), internal_span, head)?; } input.map(move |value| abs_helper(value, head), engine_state.signals()) } @@ -80,10 +76,7 @@ impl Command for MathAbs { .., ) = input { - match &**val { - Range::IntRange(range) => ensure_bounded(range.end(), internal_span, head)?, - Range::FloatRange(range) => ensure_bounded(range.end(), internal_span, head)?, - } + ensure_bounded(val.as_ref(), internal_span, head)?; } input.map( move |value| abs_helper(value, head), diff --git a/crates/nu-command/src/math/ceil.rs b/crates/nu-command/src/math/ceil.rs index 585b12aa62..996e862bc8 100644 --- a/crates/nu-command/src/math/ceil.rs +++ b/crates/nu-command/src/math/ceil.rs @@ -1,6 +1,5 @@ use crate::math::utils::ensure_bounded; use nu_engine::command_prelude::*; -use nu_protocol::Range; #[derive(Clone)] pub struct MathCeil; @@ -56,10 +55,7 @@ impl Command for MathCeil { .., ) = input { - match &**val { - Range::IntRange(range) => ensure_bounded(range.end(), internal_span, head)?, - Range::FloatRange(range) => ensure_bounded(range.end(), internal_span, head)?, - } + ensure_bounded(val.as_ref(), internal_span, head)?; } input.map(move |value| operate(value, head), engine_state.signals()) } @@ -83,10 +79,7 @@ impl Command for MathCeil { .., ) = input { - match &**val { - Range::IntRange(range) => ensure_bounded(range.end(), internal_span, head)?, - Range::FloatRange(range) => ensure_bounded(range.end(), internal_span, head)?, - } + ensure_bounded(val.as_ref(), internal_span, head)?; } input.map( move |value| operate(value, head), diff --git a/crates/nu-command/src/math/floor.rs b/crates/nu-command/src/math/floor.rs index 73760a390a..0510cf6534 100644 --- a/crates/nu-command/src/math/floor.rs +++ b/crates/nu-command/src/math/floor.rs @@ -1,6 +1,5 @@ use crate::math::utils::ensure_bounded; use nu_engine::command_prelude::*; -use nu_protocol::Range; #[derive(Clone)] pub struct MathFloor; @@ -56,10 +55,7 @@ impl Command for MathFloor { .., ) = input { - match &**val { - Range::IntRange(range) => ensure_bounded(range.end(), internal_span, head)?, - Range::FloatRange(range) => ensure_bounded(range.end(), internal_span, head)?, - } + ensure_bounded(val.as_ref(), internal_span, head)?; } input.map(move |value| operate(value, head), engine_state.signals()) } @@ -83,10 +79,7 @@ impl Command for MathFloor { .., ) = input { - match &**val { - Range::IntRange(range) => ensure_bounded(range.end(), internal_span, head)?, - Range::FloatRange(range) => ensure_bounded(range.end(), internal_span, head)?, - } + ensure_bounded(val.as_ref(), internal_span, head)?; } input.map( move |value| operate(value, head), diff --git a/crates/nu-command/src/math/log.rs b/crates/nu-command/src/math/log.rs index eff8c583df..5f31daa24f 100644 --- a/crates/nu-command/src/math/log.rs +++ b/crates/nu-command/src/math/log.rs @@ -1,6 +1,5 @@ use crate::math::utils::ensure_bounded; use nu_engine::command_prelude::*; -use nu_protocol::Range; use nu_protocol::Signals; #[derive(Clone)] @@ -59,10 +58,7 @@ impl Command for MathLog { .., ) = input { - match &**val { - Range::IntRange(range) => ensure_bounded(range.end(), internal_span, head)?, - Range::FloatRange(range) => ensure_bounded(range.end(), internal_span, head)?, - } + ensure_bounded(val.as_ref(), internal_span, head)?; } log(base, call.head, input, engine_state.signals()) } @@ -83,10 +79,7 @@ impl Command for MathLog { .., ) = input { - match &**val { - Range::IntRange(range) => ensure_bounded(range.end(), internal_span, head)?, - Range::FloatRange(range) => ensure_bounded(range.end(), internal_span, head)?, - } + ensure_bounded(val.as_ref(), internal_span, head)?; } log(base, call.head, input, working_set.permanent().signals()) } diff --git a/crates/nu-command/src/math/round.rs b/crates/nu-command/src/math/round.rs index aa04839e32..847fe51337 100644 --- a/crates/nu-command/src/math/round.rs +++ b/crates/nu-command/src/math/round.rs @@ -1,6 +1,5 @@ use crate::math::utils::ensure_bounded; use nu_engine::command_prelude::*; -use nu_protocol::Range; #[derive(Clone)] pub struct MathRound; @@ -63,10 +62,7 @@ impl Command for MathRound { .., ) = input { - match &**val { - Range::IntRange(range) => ensure_bounded(range.end(), internal_span, head)?, - Range::FloatRange(range) => ensure_bounded(range.end(), internal_span, head)?, - } + ensure_bounded(val.as_ref(), internal_span, head)?; } input.map( move |value| operate(value, head, precision_param), @@ -94,10 +90,7 @@ impl Command for MathRound { .., ) = input { - match &**val { - Range::IntRange(range) => ensure_bounded(range.end(), internal_span, head)?, - Range::FloatRange(range) => ensure_bounded(range.end(), internal_span, head)?, - } + ensure_bounded(val.as_ref(), internal_span, head)?; } input.map( move |value| operate(value, head, precision_param), diff --git a/crates/nu-command/src/math/sqrt.rs b/crates/nu-command/src/math/sqrt.rs index 7bb5ccc78e..cc16c60494 100644 --- a/crates/nu-command/src/math/sqrt.rs +++ b/crates/nu-command/src/math/sqrt.rs @@ -1,6 +1,5 @@ use crate::math::utils::ensure_bounded; use nu_engine::command_prelude::*; -use nu_protocol::Range; #[derive(Clone)] pub struct MathSqrt; @@ -56,10 +55,7 @@ impl Command for MathSqrt { .., ) = input { - match &**val { - Range::IntRange(range) => ensure_bounded(range.end(), internal_span, head)?, - Range::FloatRange(range) => ensure_bounded(range.end(), internal_span, head)?, - } + ensure_bounded(val.as_ref(), internal_span, head)?; } input.map(move |value| operate(value, head), engine_state.signals()) } @@ -83,10 +79,7 @@ impl Command for MathSqrt { .., ) = input { - match &**val { - Range::IntRange(range) => ensure_bounded(range.end(), internal_span, head)?, - Range::FloatRange(range) => ensure_bounded(range.end(), internal_span, head)?, - } + ensure_bounded(val.as_ref(), internal_span, head)?; } input.map( move |value| operate(value, head), diff --git a/crates/nu-command/src/math/utils.rs b/crates/nu-command/src/math/utils.rs index e9b87b2df1..3035e78e02 100644 --- a/crates/nu-command/src/math/utils.rs +++ b/crates/nu-command/src/math/utils.rs @@ -1,4 +1,4 @@ -use core::{ops::Bound, slice}; +use core::slice; use indexmap::IndexMap; use nu_protocol::{ engine::Call, IntoPipelineData, PipelineData, Range, ShellError, Signals, Span, Value, @@ -93,10 +93,7 @@ pub fn calculate( Ok(Value::record(record, span)) } PipelineData::Value(Value::Range { val, .. }, ..) => { - match *val { - Range::IntRange(range) => ensure_bounded(range.end(), span, name)?, - Range::FloatRange(range) => ensure_bounded(range.end(), span, name)?, - } + ensure_bounded(val.as_ref(), span, name)?; let new_vals: Result, ShellError> = val .into_range_iter(span, Signals::empty()) .map(|val| mf(&[val], span, name)) @@ -117,17 +114,13 @@ pub fn calculate( } } -pub fn ensure_bounded( - bound: Bound, - val_span: Span, - call_span: Span, -) -> Result<(), ShellError> { - match bound { - Bound::::Unbounded => Err(ShellError::IncorrectValue { - msg: "Range must be bounded".to_string(), - val_span, - call_span, - }), - _ => Ok(()), +pub fn ensure_bounded(range: &Range, val_span: Span, call_span: Span) -> Result<(), ShellError> { + if range.is_bounded() { + return Ok(()); } + Err(ShellError::IncorrectValue { + msg: "Range must be bounded".to_string(), + val_span, + call_span, + }) } diff --git a/crates/nu-protocol/src/value/range.rs b/crates/nu-protocol/src/value/range.rs index 5524a79fe3..c07bfbe78e 100644 --- a/crates/nu-protocol/src/value/range.rs +++ b/crates/nu-protocol/src/value/range.rs @@ -1,6 +1,7 @@ //! A Range is an iterator over integers or floats. use crate::{ast::RangeInclusion, ShellError, Signals, Span, Value}; +use core::ops::Bound; use serde::{Deserialize, Serialize}; 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::::Unbounded, + Range::FloatRange(range) => range.end() != Bound::::Unbounded, + } + } + pub fn into_range_iter(self, span: Span, signals: Signals) -> Iter { match self { Range::IntRange(range) => Iter::IntIter(range.into_range_iter(signals), span),