reject unexpected kwargs

src/beatmap.rs

@@ -22,36 +22,110 @@ impl PyBeatmap {
             }
         };
 
-        if let Some(arg) = kwargs.get_item("path") {
+        let mut map = None;
-            let path = arg
+        let mut ar = None;
-                .extract::<&str>()
+        let mut cs = None;
-                .map_err(|_| PyTypeError::new_err("kwarg 'path': must be a string"))?;
+        let mut hp = None;
+        let mut od = None;
 
-            let map = Beatmap::from_path(path)
+        for (key, value) in kwargs.iter() {
-                .map_err(|e| ParseError::new_err(e.unwind("Failed to parse beatmap")))?;
+            match key.extract()? {
+                "path" => {
+                    let path = value
+                        .extract::<&str>()
+                        .map_err(|_| PyTypeError::new_err("kwarg 'path': must be a string"))?;
 
-            Self::new_with_attrs(map, kwargs)
+                    let parsed = Beatmap::from_path(path)
-        } else if let Some(arg) = kwargs.get_item("content") {
+                        .map_err(|e| ParseError::new_err(e.unwind("Failed to parse beatmap")))?;
-            if let Ok(content) = arg.extract::<&str>() {
+
-                Self::new_from_bytes(content.as_bytes(), kwargs)
+                    map = Some(parsed);
-            } else if let Ok(bytes) = arg.extract::<&[u8]>() {
+                }
-                Self::new_from_bytes(bytes, kwargs)
+                "content" => {
-            } else {
+                    let bytes = if let Ok(content) = value.extract::<&str>() {
-                Err(PyTypeError::new_err(
+                        content.as_bytes()
-                    "kwarg 'content': must be a string or a bytearray",
+                    } else if let Ok(bytes) = value.extract::<&[u8]>() {
-                ))
+                        bytes
+                    } else {
+                        return Err(PyTypeError::new_err(
+                            "kwarg 'content': must be a string or a bytearray",
+                        ));
+                    };
+
+                    let parsed = Beatmap::from_bytes(bytes)
+                        .map_err(|e| ParseError::new_err(e.unwind("Failed to parse beatmap")))?;
+
+                    map = Some(parsed);
+                }
+                "bytes" => {
+                    let bytes = value
+                        .extract::<&[u8]>()
+                        .map_err(|_| PyTypeError::new_err("kwarg 'bytes': must be a bytearray"))?;
+
+                    let parsed = Beatmap::from_bytes(bytes)
+                        .map_err(|e| ParseError::new_err(e.unwind("Failed to parse beatmap")))?;
+
+                    map = Some(parsed);
+                }
+                "ar" => {
+                    let value = value
+                        .extract()
+                        .map_err(|_| PyTypeError::new_err("kwarg 'ar': must be a real number"))?;
+
+                    ar = Some(value);
+                }
+                "cs" => {
+                    let value = value
+                        .extract()
+                        .map_err(|_| PyTypeError::new_err("kwarg 'cs': must be a real number"))?;
+
+                    cs = Some(value);
+                }
+                "hp" => {
+                    let value = value
+                        .extract()
+                        .map_err(|_| PyTypeError::new_err("kwarg 'hp': must be a real number"))?;
+
+                    hp = Some(value);
+                }
+                "od" => {
+                    let value = value
+                        .extract()
+                        .map_err(|_| PyTypeError::new_err("kwarg 'od': must be a real number"))?;
+
+                    od = Some(value);
+                }
+                kwarg => {
+                    let err = format!(
+                        "unexpected kwarg '{kwarg}': expected 'path', \n\
+                        'content', 'bytes', 'ar', 'cs', 'hp', or 'od'"
+                    );
+
+                    return Err(KwargsError::new_err(err));
+                }
             }
-        } else if let Some(arg) = kwargs.get_item("bytes") {
-            let bytes = arg
-                .extract::<&[u8]>()
-                .map_err(|_| PyTypeError::new_err("kwarg 'bytes': must be a bytearray"))?;
-
-            Self::new_from_bytes(bytes, kwargs)
-        } else {
-            Err(KwargsError::new_err(
-                "kwargs must include 'path', 'content', or 'bytes'",
-            ))
         }
+
+        let mut map = map.ok_or_else(|| {
+            KwargsError::new_err("kwargs must include 'path', 'content', or 'bytes'")
+        })?;
+
+        if let Some(ar) = ar {
+            map.ar = ar;
+        }
+
+        if let Some(cs) = cs {
+            map.cs = cs;
+        }
+
+        if let Some(hp) = hp {
+            map.hp = hp;
+        }
+
+        if let Some(od) = od {
+            map.od = od;
+        }
+
+        Ok(Self { inner: map })
     }
 
     fn set_ar(&mut self, ar: f32) {
@@ -70,36 +144,3 @@ impl PyBeatmap {
         self.inner.od = od;
     }
 }
-
-impl PyBeatmap {
-    fn new_from_bytes(bytes: &[u8], kwargs: &PyDict) -> PyResult<Self> {
-        let map = Beatmap::from_bytes(bytes)
-            .map_err(|e| ParseError::new_err(e.unwind("Failed to parse beatmap")))?;
-
-        Self::new_with_attrs(map, kwargs)
-    }
-
-    fn new_with_attrs(mut map: Beatmap, kwargs: &PyDict) -> PyResult<Self> {
-        macro_rules! parse_attr {
-            ( $( $name:ident ),*) => {
-                $(
-                    if let Some(arg) = kwargs.get_item(stringify!($name)) {
-                        let value = arg.extract::<f32>().map_err(|_| {
-                            PyTypeError::new_err(concat!(
-                                "kwarg '",
-                                stringify!($name),
-                                "': must be a real number"
-                            ))
-                        })?;
-
-                        map.$name = value;
-                    }
-                )*
-            };
-        }
-
-        parse_attr!(ar, cs, hp, od);
-
-        Ok(Self { inner: map })
-    }
-}

src/calculator.rs

@@ -1,4 +1,9 @@
-use pyo3::{exceptions::PyValueError, pyclass, pymethods, types::PyDict, PyResult};
+use pyo3::{
+    exceptions::{PyTypeError, PyValueError},
+    pyclass, pymethods,
+    types::PyDict,
+    PyResult,
+};
 use rosu_pp::{AnyPP, AnyStars, DifficultyAttributes, GameMode};
 
 use crate::{
@@ -49,7 +54,11 @@ impl Calculator {
         for (key, value) in kwargs.iter() {
             match key.extract()? {
                 "mode" => {
-                    this.mode = match value.extract::<u8>()? {
+                    let int = value
+                        .extract::<u8>()
+                        .map_err(|_| PyTypeError::new_err("kwarg 'mode': must be an int"))?;
+
+                    this.mode = match int {
                         0 => Some(GameMode::Osu),
                         1 => Some(GameMode::Taiko),
                         2 => Some(GameMode::Catch),
@@ -57,19 +66,66 @@ impl Calculator {
                         _ => return Err(PyValueError::new_err("invalid mode integer")),
                     }
                 }
-                "mods" => this.mods = value.extract()?,
+                "mods" => {
-                "n300" => this.n300 = value.extract()?,
+                    this.mods = value
-                "n100" => this.n100 = value.extract()?,
+                        .extract()
-                "n50" => this.n50 = value.extract()?,
+                        .map_err(|_| PyTypeError::new_err("kwarg 'mods': must be an int"))?;
-                "n_misses" => this.n_misses = value.extract()?,
+                }
-                "n_geki" => this.n_geki = value.extract()?,
+                "n300" => {
-                "n_katu" => this.n_katu = value.extract()?,
+                    this.n300 = value
-                "acc" | "accuracy" => this.acc = value.extract()?,
+                        .extract()
-                "combo" => this.combo = value.extract()?,
+                        .map_err(|_| PyTypeError::new_err("kwarg 'n300': must be an int"))?;
-                "passed_objects" => this.passed_objects = value.extract()?,
+                }
-                "clock_rate" => this.clock_rate = value.extract()?,
+                "n100" => {
+                    this.n100 = value
+                        .extract()
+                        .map_err(|_| PyTypeError::new_err("kwarg 'n100': must be an int"))?;
+                }
+                "n50" => {
+                    this.n50 = value
+                        .extract()
+                        .map_err(|_| PyTypeError::new_err("kwarg 'n50': must be an int"))?;
+                }
+                "n_misses" => {
+                    this.n_misses = value
+                        .extract()
+                        .map_err(|_| PyTypeError::new_err("kwarg 'n_misses': must be an int"))?;
+                }
+                "n_geki" => {
+                    this.n_geki = value
+                        .extract()
+                        .map_err(|_| PyTypeError::new_err("kwarg 'n_geki': must be an int"))?;
+                }
+                "n_katu" => {
+                    this.n_katu = value
+                        .extract()
+                        .map_err(|_| PyTypeError::new_err("kwarg 'n_katu': must be an int"))?;
+                }
+                "acc" | "accuracy" => {
+                    this.acc = value
+                        .extract()
+                        .map_err(|_| PyTypeError::new_err("kwarg 'acc': must be a real number"))?;
+                }
+                "combo" => {
+                    this.combo = value
+                        .extract()
+                        .map_err(|_| PyTypeError::new_err("kwarg 'combo': must be an int"))?;
+                }
+                "passed_objects" => {
+                    this.passed_objects = value.extract().map_err(|_| {
+                        PyTypeError::new_err("kwarg 'passed_objects': must be an int")
+                    })?;
+                }
+                "clock_rate" => {
+                    this.clock_rate = value.extract().map_err(|_| {
+                        PyTypeError::new_err("kwarg 'clock_rate': must be a real number")
+                    })?;
+                }
                 "difficulty" | "attributes" => {
-                    let attrs = value.extract::<PyDifficultyAttributes>()?;
+                    let attrs = value.extract::<PyDifficultyAttributes>().map_err(|_| {
+                        PyTypeError::new_err("kwarg 'difficulty': must be DifficultyAttributes")
+                    })?;
+
                     this.attributes = Some(attrs.inner);
                 }
                 kwarg => {

src/map_attrs.rs

@@ -1,10 +1,9 @@
-use std::fmt::{Debug, Display, Formatter, Result as FmtResult};
+use std::fmt::{Display, Formatter, Result as FmtResult};
 
 use pyo3::{pyclass, pymethods};
 use rosu_pp::{beatmap::BeatmapAttributes, Beatmap};
 
 #[pyclass(name = "BeatmapAttributes")]
-#[derive(Debug)]
 pub struct PyBeatmapAttributes {
     #[pyo3(get)]
     ar: f64,
@@ -57,7 +56,45 @@ impl PyBeatmapAttributes {
 impl Display for PyBeatmapAttributes {
     #[inline]
     fn fmt(&self, f: &mut Formatter<'_>) -> FmtResult {
-        <Self as Debug>::fmt(self, f)
+        let Self {
+            ar,
+            cs,
+            hp,
+            od,
+            ar_hit_window,
+            od_hit_window,
+            clock_rate,
+            bpm,
+            mode,
+            version,
+            n_circles,
+            n_sliders,
+            n_spinners,
+        } = self;
+
+        macro_rules! debug {
+            ( $( $field:ident ,)* ) => {
+                f.debug_struct("BeatmapAttributes")
+                    $( .field(stringify!($field), $field) )*
+                    .finish()
+            };
+        }
+
+        debug! {
+            ar,
+            cs,
+            hp,
+            od,
+            ar_hit_window,
+            od_hit_window,
+            clock_rate,
+            bpm,
+            mode,
+            version,
+            n_circles,
+            n_sliders,
+            n_spinners,
+        }
     }
 }