many clippy lints, FixedString inner value now private

Author: briandorsey

Cargo.toml

@@ -53,6 +53,8 @@ missing_crate_level_docs = "warn"
 unwrap_used = "warn"
 needless_pass_by_value = "warn"
 
+indexing_slicing = "warn"
+
 # no_std helpers
 std_instead_of_core = "warn"
 alloc_instead_of_core = "warn"
@@ -96,7 +98,6 @@ manual_ok_or = "warn"
 # map_err_ignore = "warn"   # broken on br(count = size) attributes
 map_flatten = "warn"
 map_unwrap_or = "warn"
-match_on_vec_items = "warn"
 match_same_arms = "warn"
 match_wild_err_arm = "warn"
 match_wildcard_for_single_variants = "warn"
@@ -119,7 +120,6 @@ single_match_else = "warn"
 string_add_assign = "warn"
 string_add = "warn"
 string_lit_as_bytes = "warn"
-string_to_string = "warn"
 todo = "warn"
 trait_duplication_in_bounds = "warn"
 unimplemented = "warn"

clippy.toml

@@ -1 +1,5 @@
 allow-unwrap-in-tests = true
+
+# Has to be specified here, Cargo.toml can't handle these configs at workspace level. :/ 
+#  https://github.com/rust-lang/cargo/issues/12917#issuecomment-1795069197
+doc-valid-idents = ["BWFMetaEdit", ".."] 

wavrw/src/chunk/adtl.rs

@@ -125,7 +125,7 @@ pub struct Ltxt {
     /// Specifies the cue point name. This value must match one of the names listed in the `cue` chunk's [CuePoint][super::cue::CuePoint] table.
     pub name: u32,
 
-    /// Specifies the number of samples in the segment of waveform data. 	...>sample_length
+    /// Specifies the number of samples in the segment of waveform data.
     pub sample_length: u32,
 
     /// Specifies the type or purpose of the text. For example, dwPurpose can specify a FOURCC code like `scrp` for script text or `capt` for close-caption text. `rgn ` is commonly used for "region notes"

wavrw/src/chunk/cset.rs

@@ -31,14 +31,14 @@ pub struct Cset {
 
     /// Specify the language and dialect used for file elements.
     ///
-    /// See cset_ld_map, for a list of language and dialect codes. If the CSET
+    /// See `cset_ld_map`, for a list of language and dialect codes. If the CSET
     /// chunk is not present, or if these fields have value zero, assume US
     /// English (language code 9, dialect code 1).
     pub language: u16,
 
     /// Specify the language and dialect used for file elements.
     ///
-    /// See cset_ld_map, for a list of language and dialect codes. If the CSET
+    /// See `cset_ld_map`, for a list of language and dialect codes. If the CSET
     /// chunk is not present, or if these fields have value zero, assume US
     /// English (language code 9, dialect code 1).
     pub dialect: u16,

wavrw/src/chunk/fmt.rs

@@ -13,7 +13,7 @@ use crate::{FourCC, KnownChunk, KnownChunkID, Summarizable};
 
 /// A number indicating the WAVE format category of the file.
 ///
-/// The content of the format-specific-fields [ed: everything after block_align]
+/// The content of the format-specific-fields [ed: everything after `block_align`]
 /// portion of the fmt chunk, and the interpretation of the waveform data, depend on
 /// this value. [RIFF1991](https://wavref.til.cafe/chunk/fmt/)
 #[allow(dead_code, missing_docs)]
@@ -592,7 +592,7 @@ pub trait Tag {
 }
 
 //---------------------------
-/// Format of PCM audio samples in `data`. (WAVE_FORMAT_PCM) [RIFF1991](https://wavref.til.cafe/chunk/fmt/)
+/// Format of PCM audio samples in `data`. (`WAVE_FORMAT_PCM`) [RIFF1991](https://wavref.til.cafe/chunk/fmt/)
 #[binrw]
 #[brw(little)]
 #[br(import(_size: u32))]
@@ -729,7 +729,7 @@ impl Display for AdpcmCoefficients {
     }
 }
 
-/// Format of ADPCM audio samples in `data`. (WAVE_FORMAT_ADPCM) [RIFF1994](https://wavref.til.cafe/chunk/fmt/)
+/// Format of ADPCM audio samples in `data`. (`WAVE_FORMAT_ADPCM`) [RIFF1994](https://wavref.til.cafe/chunk/fmt/)
 #[binrw]
 #[brw(little)]
 #[br(import(_size: u32))]
@@ -907,7 +907,7 @@ impl Iterator for FmtAdpcmIterator<'_> {
 
 //---------------------------
 
-/// Format of DVI ADPCM audio samples in `data`. (WAVE_FORMAT_DVI_ADPCM) [RIFF1994](https://wavref.til.cafe/chunk/fmt/)
+/// Format of DVI ADPCM audio samples in `data`. (`WAVE_FORMAT_DVI_ADPCM`) [RIFF1994](https://wavref.til.cafe/chunk/fmt/)
 #[binrw]
 #[brw(little)]
 #[br(import(_size: u32))]
@@ -939,7 +939,7 @@ pub struct FmtDviAdpcm {
     /// of data at a time, so the value of `block_align` can be used for
     /// buffer alignment.
     ///
-    /// |bits_per_sample | block_align |
+    /// |`bits_per_sample` | `block_align `|
     /// |-|-|
     /// |3 | (( N * 3 ) + 1 ) * 4 * channels |
     /// |4 | (N + 1) * 4 * channels |
@@ -1105,8 +1105,8 @@ pub struct FmtExtended {
     ///
     /// The size in bytes of the extra information in the WAVE format header not
     /// including the size of the `FmtExtended` structure. (size of fields from
-    /// format_tag through extra_size inclusive (all fields except id, size and
-    /// the extra_bytes))
+    /// `format_tag` through `extra_size` inclusive (all fields except id, size and
+    /// the `extra_bytes`))
     #[br()]
     #[bw(map = |_| self.extra_bytes.len() as u16)]
     pub extra_size: u16,

wavrw/src/chunk/inst.rs

@@ -19,7 +19,7 @@ pub struct Inst {
     /// Pitch shift adjustment in cents.
     ///
     /// Pitch shift adjustment in cents. (or 100ths of a semitone) needed to
-    /// hit `unshifted_note` value exactly.  fine_tune can be used to compensate
+    /// hit `unshifted_note` value exactly.  `fine_tune` can be used to compensate
     /// for tuning errors in the sampling process. Valid values range from -50
     /// to 50.
     pub fine_tune: i8,

wavrw/src/chunk/ixml/aswg.rs

@@ -127,7 +127,7 @@ pub struct Aswg {
     /// UCS compliant SFX creator/publisher. category:Sound Effects
     pub creator_id: Option<String>,
 
-    /// UCS compliant SFX SourceID. category:Sound Effects
+    /// UCS compliant SFX `SourceID`. category:Sound Effects
     pub source_id: Option<String>,
 
     /// RMS power of file. category:Audio Features

wavrw/src/chunk/ixml/mod.rs

@@ -183,7 +183,7 @@ impl Ixml {
     /// assert_eq!("My Project", ixml.project.unwrap());
     /// ```
     pub fn set(&mut self, path: &[String], value: String) {
-        if let Some(first) = path.first() {
+        if let Some((first, remaining_path)) = path.split_first() {
             match first.as_str() {
                 "IXML_VERSION" => self.ixml_version = Some(value),
                 "PROJECT" => self.project = Some(value),
@@ -212,7 +212,7 @@ impl Ixml {
                 "NOTE" => self.note = Some(value),
                 "ASWG" => {
                     let aswg = self.aswg.get_or_insert(Aswg::new());
-                    aswg.set(&path[1..], value);
+                    aswg.set(remaining_path, value);
                 }
                 &_ => {
                     self.extra.insert(path.join("/"), value);
@@ -233,7 +233,13 @@ impl Ixml {
                 }
                 Ok(XmlEvent::Characters(chars)) => {
                     // explicitly strip the `BWFXML` root node before passing on.
-                    ixml.set(&path[1..], chars);
+                    if let Some(remaining_path) = &path.get(1..) {
+                        ixml.set(remaining_path, chars);
+                    } else {
+                        // TODO: actual error handling
+                        eprintln!("Error: XmlEvent::Characters, no remaining path: {path:?}");
+                        break;
+                    }
                 }
                 Ok(XmlEvent::EndElement { name: _ }) => {
                     path.pop();

wavrw/src/chunk/smpl.rs

@@ -73,7 +73,7 @@ pub struct Smpl {
     /// Period of one sample in nanoseconds.
     ///
     /// Specifies the period of one sample in nanoseconds (normally 1/
-    /// samples_per_second from the WAVEFORMAT structure for the RIFF WAVE file
+    /// `samples_per_second` from the WAVEFORMAT structure for the RIFF WAVE file
     /// -- however, this field allows fine tuning). For example, 44.1 kHz would
     /// be specified as 22675 (0x00005893).
     pub sample_period: u32,
@@ -88,15 +88,15 @@ pub struct Smpl {
     /// Fine tune pitch as fraction of a semitone.
     ///
     /// Specifies the fraction of a semitone up from the specified
-    /// midi_unity_note. A value of 0x80000000 is 1/2 semitone (50 cents); a
+    /// `midi_unity_note. A value of 0x80000000 is 1/2 semitone (50 cents); a
     /// value of 0x00000000 represents no fine tuning between semitones.
     pub midi_pitch_fraction: u32,
 
     /// SMPTE time format.
     ///
-    /// Specifies the SMPTE time format used in the smpte_offset field. Possible
+    /// Specifies the SMPTE time format used in the `smpte_offset` field. Possible
     /// values are (unrecognized formats should be ignored): 0 - specifies
-    /// no SMPTE offset (smpte_offset should also be zero). 24 - specifies 24
+    /// no SMPTE offset (`smpte_offset` should also be zero). 24 - specifies 24
     /// frames per second. 25 - specifies 25 frames per second. 29 - specifies
     /// 30 frames per second with frame dropping ('30 drop'). 30 - specifies 30
     /// frames per second.
@@ -108,7 +108,7 @@ pub struct Smpl {
     /// calibrated according to a start time other than 0. The format of this
     /// value is 0xhhmmssff. hh is a signed Hours value [-23..23]. mm is an
     /// unsigned Minutes value [0..59]. ss is unsigned Seconds value [0..59]. ff
-    /// is an unsigned value [0..(smpte_format - 1)].
+    /// is an unsigned value [0..(`smpte_format` - 1)].
     pub smpte_offset: u32,
 
     /// Count of sample loops (for serialization/deserialization)

wavrw/src/fixedstring.rs

@@ -1,7 +1,6 @@
 #![doc = include_str!("fixedstring.md")]
 
 use alloc::string::FromUtf8Error;
-use core::cmp::min;
 use core::error::Error;
 use core::fmt::{Debug, Display, Formatter};
 use core::str::FromStr;
@@ -65,7 +64,9 @@ impl From<FromUtf8Error> for FixedStringError {
 
 #[doc = include_str!("fixedstring.md")]
 #[derive(Clone, PartialEq, Eq, Hash)]
-pub struct FixedString<const N: usize>(String);
+pub struct FixedString<const N: usize> {
+    inner: String,
+}
 // This is only immutable because it would be a lot of work to correctly DeRef
 // to the inner string while still enforcing the length constraint. Design
 // quesion: is it worth the work? Maybe if it turns out to be annoying to work
@@ -81,7 +82,7 @@ impl<const N: usize> Debug for FixedString<N> {
 
 impl<const N: usize> Display for FixedString<N> {
     fn fmt(&self, f: &mut Formatter<'_>) -> Result<(), core::fmt::Error> {
-        write!(f, "{}", &self.0)
+        write!(f, "{}", &self.inner)
     }
 }
 
@@ -140,7 +141,7 @@ impl<const N: usize> FixedString<N> {
         }
         let s = alloc::string::String::from_utf8(vec)?;
         let s = s.trim_end_matches('\0').to_string();
-        Ok(Self(s))
+        Ok(Self { inner: s })
     }
 
     /// Create a new [u8; N] from &self
@@ -163,16 +164,20 @@ impl<const N: usize> FixedString<N> {
     /// ```
     pub fn to_bytes(&self) -> [u8; N] {
         let mut array_tmp = [0u8; N];
-        let bytes = self.0.as_bytes();
-        let l = min(bytes.len(), N);
-        array_tmp[..l].copy_from_slice(&bytes[..l]);
+        let bytes = self.inner.as_bytes();
+        // copy_from_slice requires slices of same length or it panics
+        // self.inner.len() <= N bytes because it's also defined by N (enforced by
+        // constructors and From impls - inner is not public.
+        array_tmp[..bytes.len()].copy_from_slice(bytes);
         array_tmp
     }
 }
 
 impl<const N: usize> Default for FixedString<N> {
     fn default() -> Self {
-        FixedString::<N>(String::new())
+        FixedString::<N> {
+            inner: String::new(),
+        }
     }
 }
 
@@ -186,7 +191,9 @@ impl<const N: usize> FromStr for FixedString<N> {
                 len: s.len(),
             });
         }
-        Ok(FixedString(s.to_string()))
+        Ok(FixedString {
+            inner: s.to_string(),
+        })
     }
 }
 
@@ -201,6 +208,7 @@ impl<const N: usize> BinRead for FixedString<N> {
         let mut values: [u8; N] = [0; N];
         let mut index = 0;
 
+        #[allow(clippy::indexing_slicing)]
         loop {
             if index >= N {
                 return match Self::from_utf8(values.to_vec()) {
@@ -231,6 +239,9 @@ impl<const N: usize> BinRead for FixedString<N> {
                     }),
                 };
             }
+            // #[allow(clippy::indexing_slicing)] set above because attributes on
+            // expressions are experimental (at the time of writing).
+            // index >= N checked at top of loop
             values[index] = val;
             index += 1;
         }
@@ -246,7 +257,7 @@ impl<const N: usize> BinWrite for FixedString<N> {
         endian: Endian,
         args: Self::Args<'_>,
     ) -> BinResult<()> {
-        let bytes = self.0.as_bytes();
+        let bytes = self.inner.as_bytes();
         let padding_size = N - bytes.len();
         bytes.write_options(writer, endian, args)?;
         for _ in 0..padding_size {
@@ -266,7 +277,7 @@ mod test {
 
     #[test]
     fn fixed_string() {
-        let fs = FixedString::<6>("abc".to_string());
+        let fs = FixedString::<6>::from_str("abc").unwrap();
         assert_eq!(6, fs.len());
         let s = fs.to_string();
         assert_eq!("abc".to_string(), s);
@@ -289,10 +300,10 @@ mod test {
     fn fixed_string_long() {
         // strings longer than fixed size should get truncated
 
-        // initializing with ::new() truncates without error
-        // let long_str = "this is a longer str";
-        // let fs = FixedString::<6>::new(long_str);
-        // assert_eq!(fs.to_string(), "this i");
+        // initializing with ::from_utf8() truncates without error
+        let long_str = "this is a longer str";
+        let err = FixedString::<6>::from_utf8(long_str.to_string().into());
+        assert_eq!(err, Err(FixedStringError::Truncated { limit: 6, len: 20 }));
 
         // via FromStr returns an error
         let long_str = "this is a longer str";