From d4a0e9afb640c80ab1f5481f2ce8de07fec173d3 Mon Sep 17 00:00:00 2001 From: Steven Gu Date: Thu, 13 May 2021 05:54:25 +0000 Subject: [PATCH] Fixes to limit `--command-name` count appear exactly once or same count with parameters. --- src/hyperfine/error.rs | 16 +++++ src/hyperfine/parameter_range.rs | 118 ++++++++++++++++++++++++------- src/hyperfine/types.rs | 16 +++++ src/main.rs | 21 ++++-- 4 files changed, 137 insertions(+), 34 deletions(-) diff --git a/src/hyperfine/error.rs b/src/hyperfine/error.rs index 994afbc..b8995d5 100644 --- a/src/hyperfine/error.rs +++ b/src/hyperfine/error.rs @@ -12,6 +12,7 @@ pub enum ParameterScanError { TooLarge, ZeroStep, StepRequired, + DifferentCommandNameCountWithParameters(usize, usize), } impl From for ParameterScanError { @@ -40,6 +41,13 @@ impl fmt::Display for ParameterScanError { floating point numbers. The step size can be specified \ with the '--parameter-step-size' parameter" ), + ParameterScanError::DifferentCommandNameCountWithParameters(real, expected) => { + write!( + f, + "Current --command-name count is {}: expected {} as parameters", + real, expected + ) + } } } } @@ -51,6 +59,7 @@ pub enum OptionsError<'a> { RunsBelowTwo, EmptyRunsRange, TooManyCommandNames(usize), + DifferentCommandNameCountWithParameters(usize, usize), NumericParsingError(&'a str, ParseIntError), } @@ -62,6 +71,13 @@ impl<'a> fmt::Display for OptionsError<'a> { OptionsError::TooManyCommandNames(n) => { write!(f, "Too many --command-name options: expected {} at most", n) } + OptionsError::DifferentCommandNameCountWithParameters(real, expected) => { + write!( + f, + "Current --command-name count is {}: expected {} as parameters", + real, expected + ) + } OptionsError::NumericParsingError(cmd, ref err) => write!( f, "Could not read numeric argument to '--{cmd}': {err}", diff --git a/src/hyperfine/parameter_range.rs b/src/hyperfine/parameter_range.rs index c08e257..dd0db44 100644 --- a/src/hyperfine/parameter_range.rs +++ b/src/hyperfine/parameter_range.rs @@ -2,6 +2,7 @@ use crate::hyperfine::error::ParameterScanError; use crate::hyperfine::types::{Command, NumericType, ParameterValue}; use clap::Values; use rust_decimal::Decimal; +use std::convert::TryInto; use std::ops::{Add, AddAssign, Div, Sub}; use std::str::FromStr; @@ -45,6 +46,22 @@ impl RangeStep { step, } } + + fn validate(&self) -> Result<(), ParameterScanError> { + if self.end < self.state { + return Err(ParameterScanError::EmptyRange); + } + + if self.step == T::from(0) { + return Err(ParameterScanError::ZeroStep); + } + + const MAX_PARAMETERS: usize = 100_000; + match self.size_hint() { + (_, Some(size)) if size <= MAX_PARAMETERS => Ok(()), + _ => Err(ParameterScanError::TooLarge), + } + } } impl Iterator for RangeStep { @@ -59,24 +76,18 @@ impl Iterator for RangeStep { Some(return_val) } -} -fn validate_params(start: T, end: T, step: T) -> Result<(), ParameterScanError> { - if end < start { - return Err(ParameterScanError::EmptyRange); + fn size_hint(&self) -> (usize, Option) { + if self.step == T::from(0) { + return (usize::MAX, None); + } + + let steps = (self.end - self.state + T::from(1)) / self.step; + steps + .into() + .try_into() + .map_or((usize::MAX, None), |u| (u, Some(u))) } - - if step == T::from(0) { - return Err(ParameterScanError::ZeroStep); - } - - const MAX_PARAMETERS: i32 = 100_000; - let steps = (end - start + T::from(1)) / step; - if steps > T::from(MAX_PARAMETERS) { - return Err(ParameterScanError::TooLarge); - } - - Ok(()) } fn build_parameterized_commands<'a, T: Numeric>( @@ -87,27 +98,33 @@ fn build_parameterized_commands<'a, T: Numeric>( command_strings: Vec<&'a str>, param_name: &'a str, ) -> Result>, ParameterScanError> { - validate_params(param_min, param_max, step)?; let param_range = RangeStep::new(param_min, param_max, step); - let mut commands = vec![]; + param_range.validate()?; + let param_count = param_range.size_hint().1.unwrap(); + let command_name_count = command_names.len(); + + // `--command-name` should appear exactly once or same count with parameters. + if command_name_count > 1 && command_name_count != param_count { + return Err(ParameterScanError::DifferentCommandNameCountWithParameters( + command_name_count, + param_count, + )); + } let mut i = 0; - let name_count = command_names.len(); + let mut commands = vec![]; for value in param_range { for cmd in &command_strings { - // Sets the command name by index (remainder) if exists. - let name = if name_count > 0 { - Some(command_names[i % name_count]) - } else { - None - }; - i += 1; - + let name = command_names + .get(i) + .or_else(|| command_names.get(0)) + .map(|s| *s); commands.push(Command::new_parametrized( name, cmd, vec![(param_name, ParameterValue::Numeric(value.into()))], )); + i += 1; } } Ok(commands) @@ -182,6 +199,37 @@ fn test_decimal_range() { assert_eq!(param_range[10], Decimal::from(1)); } +#[test] +fn test_range_step_validate() { + let range_step = RangeStep::new(0, 10, 3); + assert!(range_step.validate().is_ok()); + + let range_step = RangeStep::new( + Decimal::from(0), + Decimal::from(1), + Decimal::from_str("0.1").unwrap(), + ); + assert!(range_step.validate().is_ok()); + + let range_step = RangeStep::new(11, 10, 1); + assert_eq!( + format!("{}", range_step.validate().unwrap_err()), + "Empty parameter range" + ); + + let range_step = RangeStep::new(0, 10, 0); + assert_eq!( + format!("{}", range_step.validate().unwrap_err()), + "Zero is not a valid parameter step" + ); + + let range_step = RangeStep::new(0, 100_001, 1); + assert_eq!( + format!("{}", range_step.validate().unwrap_err()), + "Parameter range is too large" + ); +} + #[test] fn test_get_parameterized_commands_int() { let commands = @@ -248,3 +296,19 @@ fn test_get_specified_command_names() { .collect::>(); assert_eq!(command_names, vec!["name-a", "name-b", "name-c"]); } + +#[test] +fn test_different_command_name_count_with_parameters() { + let result = build_parameterized_commands( + 1i32, + 3i32, + 1i32, + vec!["name-1", "name-2"], + vec!["echo {val}"], + "val", + ); + assert_eq!( + format!("{}", result.unwrap_err()), + "Current --command-name count is 2: expected 3 as parameters" + ); +} diff --git a/src/hyperfine/types.rs b/src/hyperfine/types.rs index a5e3613..7a0c4f4 100644 --- a/src/hyperfine/types.rs +++ b/src/hyperfine/types.rs @@ -1,7 +1,9 @@ +use rust_decimal::prelude::ToPrimitive; use rust_decimal::Decimal; /// This module contains common internal types. use serde::*; use std::collections::BTreeMap; +use std::convert::TryFrom; use std::fmt; use crate::hyperfine::units::{Second, Unit}; @@ -40,6 +42,20 @@ impl Into for Decimal { } } +impl TryFrom for usize { + type Error = (); + + fn try_from(numeric: NumericType) -> Result { + match numeric { + NumericType::Int(i) => usize::try_from(i).map_err(|_| ()), + NumericType::Decimal(d) => match d.to_u64() { + Some(u) => usize::try_from(u).map_err(|_| ()), + None => Err(()), + }, + } + } +} + #[derive(Debug, Clone, PartialEq, Eq)] pub enum ParameterValue { Text(String), diff --git a/src/main.rs b/src/main.rs index d699f4a..677eb42 100644 --- a/src/main.rs +++ b/src/main.rs @@ -255,17 +255,24 @@ fn build_commands<'a>(matches: &'a ArgMatches<'_>) -> Vec> { return Vec::new(); } + // `--command-name` should appear exactly once or same count with parameters. + let command_name_count = command_names.len(); + if command_name_count > 1 && command_name_count != param_space_size { + let err = OptionsError::DifferentCommandNameCountWithParameters( + command_name_count, + param_space_size, + ); + error(&err.to_string()); + } + let mut i = 0; - let name_count = command_names.len(); let mut commands = Vec::with_capacity(param_space_size); let mut index = vec![0usize; dimensions.len()]; 'outer: loop { - // Sets the command name by index (remainder) if exists. - let name = if name_count > 0 { - Some(command_names[i % name_count]) - } else { - None - }; + let name = command_names + .get(i) + .or_else(|| command_names.get(0)) + .map(|s| *s); i += 1; let (command_index, params_indices) = index.split_first().unwrap();