Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: use custom error instead of anyhow #13050

Merged
merged 2 commits into from
Nov 27, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion Cargo.lock

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

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"] }
Expand Down
5 changes: 3 additions & 2 deletions crates/rustfix/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "rustfix"
version = "0.6.2"
version = "0.7.0"
authors = [
"Pascal Hertleif <killercup@gmail.com>",
"Oliver Schneider <oli-obk@users.noreply.github.com>",
Expand All @@ -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