From 0439f5daaf05774a9fba847a234b710333a33d68 Mon Sep 17 00:00:00 2001 From: ev3nvy <12035264+ev3nvy@users.noreply.github.com> Date: Sat, 11 Feb 2023 19:20:03 +0100 Subject: [PATCH 1/4] conform to C-COMMON-TRAITS Some structs were missing `Copy` implementations. --- src/entries/chunk_entry.rs | 2 +- src/entries/key_entry.rs | 2 +- src/entries/param_entry.rs | 2 +- src/lib.rs | 5 +++-- 4 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/entries/chunk_entry.rs b/src/entries/chunk_entry.rs index 5f14ae9..6b4a646 100644 --- a/src/entries/chunk_entry.rs +++ b/src/entries/chunk_entry.rs @@ -7,7 +7,7 @@ use crate::generated::rman::Chunk; /// /// [rman-schema]: https://github.com/ev3nvy/rman-schema #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] -#[derive(Debug, Default, Clone, PartialEq, Eq)] +#[derive(Debug, Default, Clone, Copy, PartialEq, Eq)] pub struct ChunkEntry { /// Id of the chunk entry. pub id: i64, diff --git a/src/entries/key_entry.rs b/src/entries/key_entry.rs index 3d4da92..cd370de 100644 --- a/src/entries/key_entry.rs +++ b/src/entries/key_entry.rs @@ -7,7 +7,7 @@ use crate::generated::rman::Key; /// /// [rman-schema]: https://github.com/ev3nvy/rman-schema #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] -#[derive(Debug, Default, Clone, PartialEq, Eq)] +#[derive(Debug, Default, Clone, Copy, PartialEq, Eq)] pub struct KeyEntry { /// Field with an unknown function and type (it might also be an [`i16`]). pub unk0: u16, diff --git a/src/entries/param_entry.rs b/src/entries/param_entry.rs index ad8aea0..e001b22 100644 --- a/src/entries/param_entry.rs +++ b/src/entries/param_entry.rs @@ -7,7 +7,7 @@ use crate::generated::rman::Param; /// /// [rman-schema]: https://github.com/ev3nvy/rman-schema #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] -#[derive(Debug, Default, Clone, PartialEq, Eq)] +#[derive(Debug, Default, Clone, Copy, PartialEq, Eq)] pub struct ParamEntry { /// Field with an unknown function and type (it might also be an [`i16`]). pub unk0: u16, diff --git a/src/lib.rs b/src/lib.rs index 4abdec3..c3f5ed3 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,5 +1,5 @@ #![deny(missing_docs)] -#![deny(missing_debug_implementations)] +#![deny(missing_debug_implementations, missing_copy_implementations)] #![deny(clippy::all, clippy::pedantic, clippy::nursery, clippy::unwrap_used)] #![allow(clippy::module_name_repetitions, clippy::similar_names)] @@ -168,8 +168,9 @@ mod file; mod parser; mod generated { + #![allow(warnings)] + #![allow(missing_debug_implementations, missing_copy_implementations)] #![allow(clippy::all, clippy::pedantic, clippy::nursery, clippy::unwrap_used)] - #![allow(missing_debug_implementations, unused_imports)] include!(concat!(env!("OUT_DIR"), "/schema_generated.rs")); } From c59575f746295608ae0bc2c447991663b5ebc20e Mon Sep 17 00:00:00 2001 From: ev3nvy <12035264+ev3nvy@users.noreply.github.com> Date: Sat, 11 Feb 2023 20:07:16 +0100 Subject: [PATCH 2/4] conform to C-GOOD-ERR Specifically to "The error message given by the Display representation of an error type should be lowercase without trailing punctuation, and typically concise." --- src/error.rs | 18 +++++++++--------- src/file.rs | 4 ++-- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/error.rs b/src/error.rs index 53e7e13..1e362c1 100644 --- a/src/error.rs +++ b/src/error.rs @@ -9,7 +9,7 @@ pub enum ManifestError { /// The error was caused by a failure to seek to a desired offset. /// /// This error occurs when [`Seek`][std::io::Seek] fails. - #[error("Could not seek to desired position. Error: {0}")] + #[error("could not seek to desired position, error: {0}")] SeekError(std::io::Error), /// The error was caused by invalid magic bytes. /// @@ -17,7 +17,7 @@ pub enum ManifestError { /// `0x41` and `0x4E` (or `R`, `M`, `A`, `N` in ascii) respectively. /// /// Usually caused by providing a file that is not a Riot Manifest file. - #[error("Invalid magic bytes (expected: \"0x4E414D52\", was: \"{0:#010x}\").")] + #[error("invalid magic bytes (expected: \"0x4E414D52\", was: \"{0:#010x}\")")] InvalidMagicBytes(u32), /// The error was caused by invalid major version. /// @@ -27,8 +27,8 @@ pub enum ManifestError { /// may no longer function if this happens. /// /// NOTE: The feature `version_error` must be enabled for this error to occur. - #[error("Unsupported major version (expected: \"2\", was: \"{0}\").")] - #[cfg(feature = "version_error")] + #[error("unsupported major version (expected: \"2\", was: \"{0}\")")] + #[cfg(any(feature = "version_error", doc))] InvalidMajor(u8), /// The error was caused by invalid minor version. /// @@ -38,22 +38,22 @@ pub enum ManifestError { /// should still be functional if this happens, /// /// NOTE: The feature `version_error` must be enabled for this error to occur. - #[error("Unsupported minor version (expected: \"0\", was: \"{0}\").")] - #[cfg(feature = "version_error")] + #[error("unsupported minor version (expected: \"0\", was: \"{0}\")")] + #[cfg(any(feature = "version_error", doc))] InvalidMinor(u8), /// The error was caused by an invalid offset. /// /// This error occurs when the offset is smaller or larger than the file itself. /// /// Should never happen for official, Riot-made manifests. - #[error("Offset ({0}) is larger than the total file size.")] + #[error("offset ({0}) is larger than the total file size")] InvalidOffset(u32), /// The error was caused by compressed size being too large. /// /// This error occurs when the compressed size is larger than the file itself. /// /// Should never happen for official, Riot-made manifests. - #[error("Compressed size ({0}) is larger than the total file size.")] + #[error("compressed size ({0}) is larger than the total file size")] CompressedSizeTooLarge(u32), /// The error was caused by a failure to read or write bytes on an IO stream. /// @@ -68,7 +68,7 @@ pub enum ManifestError { /// This error occurs when the conversion from or into [`usize`] fails. /// /// Should never fail on 32-bit (or higher) targets. - #[error("Conversion failed. Error: {0}")] + #[error("conversion failed, error: {0}")] ConversionFailure(#[from] std::num::TryFromIntError), /// The error was caused by a failure to decompress zstd data. /// diff --git a/src/file.rs b/src/file.rs index bf609c8..08e9162 100644 --- a/src/file.rs +++ b/src/file.rs @@ -87,7 +87,7 @@ impl File { while directory_id != 0 { let Some((dir_name, parent_id)) = directories.get(&directory_id) else { - let message = format!("Could not find a directory with the following id: \"{directory_id}\"."); + let message = format!("could not find a directory with the following id: \"{directory_id}\""); return Err(ManifestError::FileParseError(message)); }; path = format!("{dir_name}/{path}"); @@ -112,7 +112,7 @@ impl File { for chunk_id in chunk_ids { let Some(chunk) = chunk_entries.get(chunk_id) else { - let message = format!("Could not find a chunk with the following id: \"{chunk_id}\"."); + let message = format!("could not find a chunk with the following id: \"{chunk_id}\""); return Err(ManifestError::FileParseError(message)); }; chunks.push(chunk.to_owned()); From cd1c93df45f374d60a36f56101c33c0db7bb5685 Mon Sep 17 00:00:00 2001 From: ev3nvy <12035264+ev3nvy@users.noreply.github.com> Date: Sat, 11 Feb 2023 20:11:49 +0100 Subject: [PATCH 3/4] conform to C-RW-VALUE --- src/parser.rs | 12 +++--------- src/parser/header.rs | 8 +++----- 2 files changed, 6 insertions(+), 14 deletions(-) diff --git a/src/parser.rs b/src/parser.rs index 1600c8b..a2adf3b 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -49,10 +49,7 @@ impl RiotManifest { /// [parsing a manifest file from path](index.html#example-parsing-a-manifest-file-from-path). /// /// [`RiotManifest::from_reader`]: crate::RiotManifest::from_reader - pub fn from_path

(path: P) -> Result - where - P: AsRef, - { + pub fn from_path>(path: P) -> Result { let file = fs::File::open(path)?; let mut reader = BufReader::new(file); Self::from_reader(&mut reader) @@ -90,11 +87,8 @@ impl RiotManifest { /// [`ManifestData::parse`][crate::ManifestData::parse]. /// /// [flatbuffer binary]: https://github.com/ev3nvy/rman-schema - pub fn from_reader(reader: &mut R) -> Result - where - R: Read + Seek, - { - let header = Header::from_reader(reader)?; + pub fn from_reader(mut reader: R) -> Result { + let header = Header::from_reader(&mut reader)?; if let Err(error) = reader.seek(SeekFrom::Start(header.offset.into())) { return Err(ManifestError::SeekError(error)); diff --git a/src/parser/header.rs b/src/parser/header.rs index 8f191d2..3f8b417 100644 --- a/src/parser/header.rs +++ b/src/parser/header.rs @@ -1,3 +1,4 @@ +use std::borrow::BorrowMut; use std::io::{Read, Seek}; use byteorder::{ReadBytesExt, LE}; @@ -74,12 +75,9 @@ impl Header { /// If [`compressed_size`](Header::compressed_size) is smaller or larger than the file, the /// error [`CompressedSizeTooLarge`][crate::ManifestError::CompressedSizeTooLarge] is /// returned. - pub fn from_reader(reader: &mut R) -> Result - where - R: Read + Seek, - { + pub fn from_reader(mut reader: R) -> Result { debug!("Attempting to convert \"reader.bytes().count()\" into \"u32\"."); - let size: u32 = reader.bytes().count().try_into()?; + let size: u32 = reader.borrow_mut().bytes().count().try_into()?; debug!("Successfully converted \"reader.bytes().count()\" into \"u32\"."); debug!("The file is {size} bytes in size"); From ade7af69383b6507f31608cba1ba3fcc076488e5 Mon Sep 17 00:00:00 2001 From: ev3nvy <12035264+ev3nvy@users.noreply.github.com> Date: Sat, 11 Feb 2023 20:17:00 +0100 Subject: [PATCH 4/4] conform to C-QUESTION-MARK Although this doesn't seem as broadly adapted, seems like a good idea, because I do agree with "Like it or not, example code is often copied verbatim by users. Unwrapping an error should be a conscious decision that the user needs to make." --- src/lib.rs | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index c3f5ed3..cb4f238 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -30,15 +30,19 @@ //! internally, so everything from [`&str`][str] to [`PathBuf`][std::path::PathBuf] is a valid //! argument. //! -//! ``` +//! ```rust +//! # use rman::Result; //! use rman::RiotManifest; //! +//! # fn main() -> Result<()> { //! let path = "file.manifest"; -//! # let path = concat!(env!("OUT_DIR"), "/valid.manifest"); +//! # let path = concat!(env!("OUT_DIR"), "/valid.manifest"); //! -//! let manifest = RiotManifest::from_path(path).unwrap(); +//! let manifest = RiotManifest::from_path(path)?; //! //! assert_eq!(manifest.data.files.len(), 1); +//! # Ok(()) +//! # } //! ``` //! //! # Example: parsing a manifest file from reader @@ -49,20 +53,24 @@ //! Internally, this is what [`from_path`](crate::RiotManifest::from_path) calls, so the response //! of the two functions should be identical. //! -//! ``` +//! ```rust //! use std::fs; //! use std::io::BufReader; //! +//! # use rman::Result; //! use rman::RiotManifest; //! +//! # fn main() -> Result<()> { //! let path = "file.manifest"; -//! # let path = concat!(env!("OUT_DIR"), "/valid.manifest"); -//! let file = fs::File::open(path).unwrap(); +//! # let path = concat!(env!("OUT_DIR"), "/valid.manifest"); +//! let file = fs::File::open(path)?; //! let mut reader = BufReader::new(file); //! -//! let manifest = RiotManifest::from_reader(&mut reader).unwrap(); +//! let manifest = RiotManifest::from_reader(&mut reader)?; //! //! assert_eq!(manifest.data.files.len(), 1); +//! # Ok(()) +//! # } //! ``` //! //! # Example: downloading a file @@ -70,7 +78,7 @@ //! To download a specific file from a parsed manifest, you can invoke the //! [download function](crate::File::download) on that [file][crate::File]. //! -//! ``` +//! ```rust //! use std::fs; //! //! # use httptest::{matchers::*, responders::*, Expectation, Server};