Merge pull request #951 from Likio3000/codex/reject-negative-parameter-steps

Reject negative parameter steps before expanding ranges

doc/hyperfine.1

@@ -180,7 +180,7 @@ The exact syntax may vary depending on your shell and OS.
 \fB\-D\fR, \fB\-\-parameter\-step\-size\fR \fIDELTA\fP
 .IP
 This argument requires \fB\-\-parameter\-scan\fR to be specified as well. Traverse the
-range \fIMIN..MAX\fP in steps of \fIDELTA\fP.
+range \fIMIN..MAX\fP in steps of \fIDELTA\fP, which must be positive.
 .IP
 .RS
 Example:

src/cli.rs

@@ -182,7 +182,7 @@ fn build_command() -> Command {
                 .requires("parameter-scan")
                 .help(
                     "This argument requires --parameter-scan to be specified as well. \
-                     Traverse the range MIN..MAX in steps of DELTA.\n\n  \
+                     Traverse the range MIN..MAX in steps of DELTA, which must be positive.\n\n  \
                      Example:  hyperfine -P delay 0.3 0.7 -D 0.2 'sleep {delay}'\n\n\
                      This performs benchmarks for 'sleep 0.3', 'sleep 0.5' and 'sleep 0.7'.",
                 ),

src/error.rs

@@ -19,6 +19,8 @@ pub enum ParameterScanError {
     TooLarge,
     #[error("Zero is not a valid parameter step")]
     ZeroStep,
+    #[error("Parameter step size must be positive")]
+    NegativeStep,
     #[error("A step size is required when the range bounds are floating point numbers. The step size can be specified with the '-D/--parameter-step-size <DELTA>' parameter")]
     StepRequired,
     #[error("'--command-name' has been specified {0} times. It has to appear exactly once, or exactly {1} times (number of benchmarks)")]

src/parameter/range_step.rs

@@ -47,6 +47,10 @@ impl<T: Numeric> RangeStep<T> {
             return Err(ParameterScanError::ZeroStep);
         }
 
+        if step < T::from(0) {
+            return Err(ParameterScanError::NegativeStep);
+        }
+
         const MAX_PARAMETERS: usize = 100_000;
         match range_step_size_hint(start, end, step) {
             (_, Some(size)) if size <= MAX_PARAMETERS => Ok(Self {
@@ -120,6 +124,36 @@ mod tests {
         assert_eq!(param_range[10], Decimal::from(1));
     }
 
+    #[test]
+    fn test_negative_integer_step() {
+        for step in [-2, -3, i32::MIN, -1] {
+            for end in [0, 1, 10] {
+                let result = RangeStep::new(0, end, step);
+                assert_eq!(
+                    result.unwrap_err().to_string(),
+                    "Parameter step size must be positive"
+                );
+            }
+        }
+    }
+
+    #[test]
+    fn test_negative_decimal_step() {
+        for step in ["-2.0", "-0.1"] {
+            for end in [0, 1, 10] {
+                let result = RangeStep::new(
+                    Decimal::ZERO,
+                    Decimal::from(end),
+                    Decimal::from_str(step).unwrap(),
+                );
+                assert_eq!(
+                    result.unwrap_err().to_string(),
+                    "Parameter step size must be positive"
+                );
+            }
+        }
+    }
+
     #[test]
     fn test_range_step_validate() {
         let result = RangeStep::new(0, 10, 3);

tests/integration_tests.rs

@@ -779,6 +779,22 @@ fn performs_all_benchmarks_in_parameter_scan() {
     ");
 }
 
+#[test]
+fn rejects_negative_parameter_steps() {
+    for step in ["-1", "-1.0"] {
+        hyperfine()
+            .args(["--parameter-scan", "n", "0", "1"])
+            .arg(format!("--parameter-step-size={step}"))
+            .arg("echo {n}")
+            .assert()
+            .code(1)
+            .stdout(predicate::str::is_empty())
+            .stderr(predicate::str::contains(
+                "Parameter step size must be positive",
+            ));
+    }
+}
+
 #[test]
 fn performs_reference_and_all_benchmarks_in_parameter_scan() {
     let _settings = snapshot_settings().bind_to_scope();