From ed347ce4dc3d4cc53a545a16bb792be892f32bd0 Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Sun, 26 Nov 2023 00:55:33 -0500 Subject: [PATCH 1/2] refactor: use custom error instead of anyhow --- Cargo.lock | 1 + crates/rustfix/Cargo.toml | 3 +- crates/rustfix/src/error.rs | 21 +++++++ crates/rustfix/src/lib.rs | 8 ++- crates/rustfix/src/replace.rs | 106 +++++++++++++++------------------- 5 files changed, 76 insertions(+), 63 deletions(-) create mode 100644 crates/rustfix/src/error.rs diff --git a/Cargo.lock b/Cargo.lock index 748d3497ecf..3708ad587a2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2893,6 +2893,7 @@ dependencies = [ "serde_json", "similar", "tempfile", + "thiserror", "tracing", "tracing-subscriber", ] diff --git a/crates/rustfix/Cargo.toml b/crates/rustfix/Cargo.toml index 0acaf4f8e80..ebac1f8f921 100644 --- a/crates/rustfix/Cargo.toml +++ b/crates/rustfix/Cargo.toml @@ -18,12 +18,13 @@ exclude = [ ] [dependencies] -anyhow.workspace = true serde = { workspace = true, features = ["derive"] } serde_json.workspace = true +thiserror.workspace = true tracing.workspace = true [dev-dependencies] +anyhow.workspace = true proptest.workspace = true similar = "2.2.1" tempfile.workspace = true diff --git a/crates/rustfix/src/error.rs b/crates/rustfix/src/error.rs new file mode 100644 index 00000000000..171864504ed --- /dev/null +++ b/crates/rustfix/src/error.rs @@ -0,0 +1,21 @@ +//! Error types. + +use std::ops::Range; + +#[derive(Debug, thiserror::Error)] +pub enum Error { + #[error("invalid range {0:?}, start is larger than end")] + InvalidRange(Range), + + #[error("invalid range {0:?}, original data is only {1} byte long")] + DataLengthExceeded(Range, usize), + + #[error("could not replace range {0:?}, maybe parts of it were already replaced?")] + MaybeAlreadyReplaced(Range), + + #[error("cannot replace slice of data that was already replaced")] + AlreadyReplaced, + + #[error(transparent)] + Utf8(#[from] std::string::FromUtf8Error), +} diff --git a/crates/rustfix/src/lib.rs b/crates/rustfix/src/lib.rs index 789ca6534e6..5fdb37b56fa 100644 --- a/crates/rustfix/src/lib.rs +++ b/crates/rustfix/src/lib.rs @@ -22,12 +22,14 @@ use std::collections::HashSet; use std::ops::Range; -use anyhow::Error; - pub mod diagnostics; -use crate::diagnostics::{Diagnostic, DiagnosticSpan}; +mod error; mod replace; +use diagnostics::Diagnostic; +use diagnostics::DiagnosticSpan; +pub use error::Error; + /// A filter to control which suggestion should be applied. #[derive(Debug, Clone, Copy)] pub enum Filter { diff --git a/crates/rustfix/src/replace.rs b/crates/rustfix/src/replace.rs index c6e6f69f7d9..ed467dcbac1 100644 --- a/crates/rustfix/src/replace.rs +++ b/crates/rustfix/src/replace.rs @@ -2,9 +2,10 @@ //! replacement of parts of its content, with the ability to prevent changing //! the same parts multiple times. -use anyhow::{anyhow, ensure, Error}; use std::rc::Rc; +use crate::error::Error; + /// Indicates the change state of a [`Span`]. #[derive(Debug, Clone, PartialEq, Eq)] enum State { @@ -77,22 +78,13 @@ impl Data { range: std::ops::Range, data: &[u8], ) -> Result<(), Error> { - let exclusive_end = range.end; - - ensure!( - range.start <= exclusive_end, - "Invalid range {}..{}, start is larger than end", - range.start, - range.end - ); - - ensure!( - exclusive_end <= self.original.len(), - "Invalid range {}..{} given, original data is only {} byte long", - range.start, - range.end, - self.original.len() - ); + if range.start > range.end { + return Err(Error::InvalidRange(range)); + } + + if range.end > self.original.len() { + return Err(Error::DataLengthExceeded(range, self.original.len())); + } let insert_only = range.start == range.end; @@ -106,42 +98,35 @@ impl Data { // the whole chunk. As an optimization and without loss of generality we // don't add empty parts. let new_parts = { - let index_of_part_to_split = self - .parts - .iter() - .position(|p| !p.data.is_inserted() && p.start <= range.start && p.end >= range.end) - .ok_or_else(|| { - if tracing::enabled!(tracing::Level::DEBUG) { - let slices = self - .parts - .iter() - .map(|p| { - ( - p.start, - p.end, - match p.data { - State::Initial => "initial", - State::Replaced(..) => "replaced", - State::Inserted(..) => "inserted", - }, - ) - }) - .collect::>(); - tracing::debug!( - "no single slice covering {}..{}, current slices: {:?}", - range.start, - range.end, - slices, - ); - } - - anyhow!( - "Could not replace range {}..{} in file \ - -- maybe parts of it were already replaced?", + let Some(index_of_part_to_split) = self.parts.iter().position(|p| { + !p.data.is_inserted() && p.start <= range.start && p.end >= range.end + }) else { + if tracing::enabled!(tracing::Level::DEBUG) { + let slices = self + .parts + .iter() + .map(|p| { + ( + p.start, + p.end, + match p.data { + State::Initial => "initial", + State::Replaced(..) => "replaced", + State::Inserted(..) => "inserted", + }, + ) + }) + .collect::>(); + tracing::debug!( + "no single slice covering {}..{}, current slices: {:?}", range.start, range.end, - ) - })?; + slices, + ); + } + + return Err(Error::MaybeAlreadyReplaced(range)); + }; let part_to_split = &self.parts[index_of_part_to_split]; @@ -161,10 +146,9 @@ impl Data { } } - ensure!( - part_to_split.data == State::Initial, - "Cannot replace slice of data that was already replaced" - ); + if part_to_split.data != State::Initial { + return Err(Error::AlreadyReplaced); + } let mut new_parts = Vec::with_capacity(self.parts.len() + 2); @@ -293,21 +277,25 @@ mod tests { } #[test] - #[should_panic(expected = "Cannot replace slice of data that was already replaced")] fn replace_overlapping_stuff_errs() { let mut d = Data::new(b"foo bar baz"); d.replace_range(4..7, b"lol").unwrap(); assert_eq!("foo lol baz", str(&d.to_vec())); - d.replace_range(4..7, b"lol2").unwrap(); + assert!(matches!( + d.replace_range(4..7, b"lol2").unwrap_err(), + Error::AlreadyReplaced, + )); } #[test] - #[should_panic(expected = "original data is only 3 byte long")] fn broken_replacements() { let mut d = Data::new(b"foo"); - d.replace_range(4..8, b"lol").unwrap(); + assert!(matches!( + d.replace_range(4..8, b"lol").unwrap_err(), + Error::DataLengthExceeded(std::ops::Range { start: 4, end: 8 }, 3), + )); } #[test] From 21274da65b651abe2d4d613a23e43121bc9021c7 Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Sun, 26 Nov 2023 00:59:16 -0500 Subject: [PATCH 2/2] chore: bump `rustfix` to 0.7.0 --- Cargo.lock | 2 +- Cargo.toml | 2 +- crates/rustfix/Cargo.toml | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 3708ad587a2..9569d8ce706 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2885,7 +2885,7 @@ checksum = "08d43f7aa6b08d49f382cde6a7982047c3426db949b1424bc4b7ec9ae12c6ce2" [[package]] name = "rustfix" -version = "0.6.2" +version = "0.7.0" dependencies = [ "anyhow", "proptest", diff --git a/Cargo.toml b/Cargo.toml index 0af9f6f9a5a..5812826615d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -75,7 +75,7 @@ pulldown-cmark = { version = "0.9.3", default-features = false } rand = "0.8.5" regex = "1.9.3" rusqlite = { version = "0.29.0", features = ["bundled"] } -rustfix = { version = "0.6.2", path = "crates/rustfix" } +rustfix = { version = "0.7.0", path = "crates/rustfix" } same-file = "1.0.6" security-framework = "2.9.2" semver = { version = "1.0.20", features = ["serde"] } diff --git a/crates/rustfix/Cargo.toml b/crates/rustfix/Cargo.toml index ebac1f8f921..6529e3f3acd 100644 --- a/crates/rustfix/Cargo.toml +++ b/crates/rustfix/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "rustfix" -version = "0.6.2" +version = "0.7.0" authors = [ "Pascal Hertleif ", "Oliver Schneider ",