Skip to content

Commit

Permalink
refactor: use custom error instead of anyhow
Browse files Browse the repository at this point in the history
  • Loading branch information
weihanglo committed Nov 26, 2023
1 parent 35ed69c commit c5bbef9
Show file tree
Hide file tree
Showing 5 changed files with 76 additions and 63 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion crates/rustfix/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
21 changes: 21 additions & 0 deletions crates/rustfix/src/error.rs
Original file line number Diff line number Diff line change
@@ -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<usize>),

#[error("invalid range {0:?}, original data is only {1} byte long")]
DataLengthExceeded(Range<usize>, usize),

#[error("could not replace range {0:?}, maybe parts of it were already replaced?")]
MaybeAlreadyReplaced(Range<usize>),

#[error("cannot replace slice of data that was already replaced")]
AlreadyReplaced,

#[error(transparent)]
Utf8(#[from] std::string::FromUtf8Error),
}
8 changes: 5 additions & 3 deletions crates/rustfix/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
106 changes: 47 additions & 59 deletions crates/rustfix/src/replace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -77,22 +78,13 @@ impl Data {
range: std::ops::Range<usize>,
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;

Expand All @@ -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::<Vec<_>>();
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::<Vec<_>>();
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];

Expand All @@ -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);

Expand Down Expand Up @@ -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]
Expand Down

0 comments on commit c5bbef9

Please sign in to comment.