Skip to content

Commit

Permalink
GEN-38: Minor improvements of code quality (#2885)
Browse files Browse the repository at this point in the history
  • Loading branch information
TimDiekmann authored Aug 10, 2023
1 parent 3f626e0 commit f190a08
Show file tree
Hide file tree
Showing 16 changed files with 55 additions and 101 deletions.
3 changes: 0 additions & 3 deletions libs/error-stack/src/compat.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
//! Compatibility module to convert errors from other libraries into [`Report`].
//!
//! In order to convert these error types, use [`IntoReportCompat::into_report()`].
//!
//! [`Report`]: crate::Report
use crate::Report;

Expand All @@ -19,7 +17,6 @@ mod eyre;
/// would imply an implementation for [`Context`]. This also implies, that it's not possible to
/// implement [`ResultExt`] from within `error-stack`.
///
/// [`Report`]: Report
/// [`ResultExt`]: crate::ResultExt
/// [`Context`]: crate::Context
/// [`Error`]: core::error::Error
Expand Down
15 changes: 3 additions & 12 deletions libs/error-stack/src/context.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
#[cfg(nightly)]
use core::any::Demand;
#[cfg(nightly)]
use core::error::Error;
use core::fmt;
#[cfg(nightly)]
use core::{any::Demand, error::Error};
#[cfg(all(not(nightly), feature = "std"))]
use std::error::Error;

Expand All @@ -13,14 +11,11 @@ use crate::Report;
/// When in a `std` environment or on a nightly toolchain, every [`Error`] is a valid `Context`.
/// This trait is not limited to [`Error`]s and can also be manually implemented on a type.
///
/// [`Error`]: core::error::Error
///
/// ## Example
///
/// Used for creating a [`Report`] or for switching the [`Report`]'s context:
///
/// ```rust
/// # #![cfg_attr(any(not(feature = "std"), miri), allow(unused_imports))]
/// use std::{fmt, fs, io};
///
/// use error_stack::{Context, Result, ResultExt, Report};
Expand All @@ -32,7 +27,7 @@ use crate::Report;
/// }
///
/// impl fmt::Display for ConfigError {
/// # #[allow(unused_variables)]
/// # #[allow(unused_variables)] // `fmt` is not used in this example
/// fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
/// # const _: &str = stringify! {
/// ...
Expand All @@ -44,9 +39,6 @@ use crate::Report;
/// // `Context` manually.
/// impl Context for ConfigError {}
///
/// # #[cfg(any(not(feature = "std"), miri))]
/// # pub fn read_file(_: &str) -> Result<String, ConfigError> { error_stack::bail!(ConfigError::ParseError) }
/// # #[cfg(all(feature = "std", not(miri)))]
/// pub fn read_file(path: &str) -> Result<String, io::Error> {
/// // Creates a `Report` from `io::Error`, the current context is `io::Error`
/// fs::read_to_string(path).map_err(Report::from)
Expand All @@ -62,7 +54,6 @@ use crate::Report;
/// # }; Ok(())
/// }
/// # let err = parse_config("invalid-path").unwrap_err();
/// # #[cfg(all(feature = "std", not(miri)))]
/// # assert!(err.contains::<io::Error>());
/// # assert!(err.contains::<ConfigError>());
/// ```
Expand Down
4 changes: 2 additions & 2 deletions libs/error-stack/src/fmt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,7 @@ pub use hook::HookContext;
#[cfg(any(feature = "std", feature = "hooks"))]
pub(crate) use hook::{install_builtin_hooks, Format, Hooks};
#[cfg(not(any(feature = "std", feature = "hooks")))]
use location::LocationDisplay;
use location::LocationAttachment;

use crate::{
fmt::{
Expand Down Expand Up @@ -864,7 +864,7 @@ fn debug_attachments_invoke<'a>(
FrameKind::Attachment(AttachmentKind::Opaque(_)) => frame
.downcast_ref::<core::panic::Location<'static>>()
.map(|location| {
vec![LocationDisplay::new(location, config.color_mode()).to_string()]
vec![LocationAttachment::new(location, config.color_mode()).to_string()]
}),
})
.flat_map(|body| {
Expand Down
2 changes: 1 addition & 1 deletion libs/error-stack/src/fmt/charset.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ impl Report<()> {
///
/// # Example
///
/// ```
/// ```rust
/// # // we only test the snapshot on nightly, therefore report is unused (so is render)
/// # #![cfg_attr(not(nightly), allow(dead_code, unused_variables, unused_imports))]
/// use std::io::{Error, ErrorKind};
Expand Down
2 changes: 1 addition & 1 deletion libs/error-stack/src/fmt/color.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ impl Report<()> {
///
/// # Example
///
/// ```
/// ```rust
/// # // we only test the snapshot on nightly, therefore report is unused (so is render)
/// # #![cfg_attr(not(nightly), allow(dead_code, unused_variables, unused_imports))]
/// use std::io::{Error, ErrorKind};
Expand Down
4 changes: 1 addition & 3 deletions libs/error-stack/src/fmt/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,7 @@ pub(crate) struct Config {

#[cfg(not(any(feature = "std", feature = "hooks")))]
impl Config {
// alternate is still provided, as it is used in the hook counterpart
#[allow(unused)]
pub(crate) const fn new(color_mode: ColorMode, charset: Charset, alternate: bool) -> Self {
pub(crate) const fn new(color_mode: ColorMode, charset: Charset, _alternate: bool) -> Self {
Self {
color_mode,
charset,
Expand Down
4 changes: 2 additions & 2 deletions libs/error-stack/src/fmt/hook.rs
Original file line number Diff line number Diff line change
Expand Up @@ -468,7 +468,7 @@ mod default {
use tracing_error::SpanTrace;

use crate::{
fmt::{hook::HookContext, location::LocationDisplay},
fmt::{hook::HookContext, location::LocationAttachment},
Report,
};

Expand Down Expand Up @@ -507,7 +507,7 @@ mod default {
}

fn location(location: &Location<'static>, context: &mut HookContext<Location<'static>>) {
context.push_body(LocationDisplay::new(location, context.color_mode()).to_string());
context.push_body(LocationAttachment::new(location, context.color_mode()).to_string());
}

#[cfg(all(feature = "std", rust_1_65))]
Expand Down
27 changes: 10 additions & 17 deletions libs/error-stack/src/fmt/location.rs
Original file line number Diff line number Diff line change
@@ -1,28 +1,21 @@
use core::{
fmt,
fmt::{Display, Formatter},
panic::Location,
};

use crate::fmt::{
color::{Color, DisplayStyle, Style},
ColorMode,
};

pub(super) struct LocationDisplay<'a> {
location: &'a Location<'static>,
use core::{fmt, panic::Location};

use crate::fmt::color::{Color, ColorMode, DisplayStyle, Style};

pub(super) struct LocationAttachment<'a, 'loc> {
location: &'a Location<'loc>,
mode: ColorMode,
}

impl<'a> LocationDisplay<'a> {
impl<'a, 'loc> LocationAttachment<'a, 'loc> {
#[must_use]
pub(super) const fn new(location: &'a Location<'static>, mode: ColorMode) -> Self {
pub(super) const fn new(location: &'a Location<'loc>, mode: ColorMode) -> Self {
Self { location, mode }
}
}

impl<'a> Display for LocationDisplay<'a> {
fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
impl fmt::Display for LocationAttachment<'_, '_> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
let location = self.location;

let mut style = Style::new();
Expand Down
31 changes: 14 additions & 17 deletions libs/error-stack/src/frame/frame_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ pub(super) trait FrameImpl: Send + Sync + 'static {
fn provide<'a>(&'a self, demand: &mut Demand<'a>);
}

#[repr(C)]
struct ContextFrame<C> {
context: C,
}
Expand All @@ -42,7 +41,6 @@ impl<C: Context> FrameImpl for ContextFrame<C> {
}
}

#[repr(C)]
struct AttachmentFrame<A> {
attachment: A,
}
Expand All @@ -66,7 +64,6 @@ impl<A: 'static + Send + Sync> FrameImpl for AttachmentFrame<A> {
}
}

#[repr(C)]
struct PrintableAttachmentFrame<A> {
attachment: A,
}
Expand Down Expand Up @@ -210,6 +207,20 @@ impl Frame {
}
}

/// Creates a frame from an attachment which implements [`Debug`] and [`Display`].
///
/// [`Debug`]: core::fmt::Debug
/// [`Display`]: core::fmt::Display
pub(crate) fn from_printable_attachment<A>(attachment: A, sources: Box<[Self]>) -> Self
where
A: fmt::Display + fmt::Debug + Send + Sync + 'static,
{
Self {
frame: Box::new(PrintableAttachmentFrame { attachment }),
sources,
}
}

/// Creates a frame from an [`anyhow::Error`].
#[cfg(feature = "anyhow")]
pub(crate) fn from_anyhow(error: anyhow::Error, sources: Box<[Self]>) -> Self {
Expand All @@ -227,18 +238,4 @@ impl Frame {
sources,
}
}

/// Creates a frame from an attachment which implements [`Debug`] and [`Display`].
///
/// [`Debug`]: core::fmt::Debug
/// [`Display`]: core::fmt::Display
pub(crate) fn from_printable_attachment<A>(attachment: A, sources: Box<[Self]>) -> Self
where
A: fmt::Display + fmt::Debug + Send + Sync + 'static,
{
Self {
frame: Box::new(PrintableAttachmentFrame { attachment }),
sources,
}
}
}
4 changes: 2 additions & 2 deletions libs/error-stack/src/hook.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ impl Report<()> {
///
/// # Examples
///
/// ```
/// ```rust
/// # // we only test the snapshot on nightly, therefore report is unused (so is render)
/// # #![cfg_attr(not(nightly), allow(dead_code, unused_variables, unused_imports))]
/// use std::io::{Error, ErrorKind};
Expand Down Expand Up @@ -71,7 +71,7 @@ impl Report<()> {
/// This example showcases the ability of hooks to be invoked for values provided via the
/// Provider API using [`Error::provide`].
///
/// ```
/// ```rust
/// # // this is a lot of boilerplate, if you find a better way, please change this!
/// # // with #![cfg(nightly)] docsrs will complain that there's no main in non-nightly
/// # #![cfg_attr(nightly, feature(error_generic_member_access, provide_any))]
Expand Down
7 changes: 3 additions & 4 deletions libs/error-stack/src/iter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ use alloc::{vec, vec::Vec};
use core::marker::PhantomData;
use core::{
fmt,
fmt::Formatter,
iter::FusedIterator,
slice::{Iter, IterMut},
};
Expand Down Expand Up @@ -104,7 +103,7 @@ impl<'r> Iterator for Frames<'r> {
impl<'r> FusedIterator for Frames<'r> {}

impl fmt::Debug for Frames<'_> {
fn fmt(&self, fmt: &mut Formatter<'_>) -> fmt::Result {
fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
fmt.debug_list().entries(self.clone()).finish()
}
}
Expand Down Expand Up @@ -205,7 +204,7 @@ impl<'r, T> fmt::Debug for RequestRef<'r, T>
where
T: ?Sized + fmt::Debug + 'static,
{
fn fmt(&self, fmt: &mut Formatter<'_>) -> fmt::Result {
fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
fmt.debug_list().entries(self.clone()).finish()
}
}
Expand Down Expand Up @@ -263,7 +262,7 @@ impl<'r, T> fmt::Debug for RequestValue<'r, T>
where
T: fmt::Debug + 'static,
{
fn fmt(&self, fmt: &mut Formatter<'_>) -> fmt::Result {
fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
fmt.debug_list().entries(self.clone()).finish()
}
}
6 changes: 0 additions & 6 deletions libs/error-stack/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@
//! existing [`Error`]:
//!
//! ```rust
//! # #[cfg(all(not(miri), feature = "std"))] {
//! use std::{fs, io, path::Path};
//!
//! use error_stack::Report;
Expand All @@ -85,7 +84,6 @@
//! }
//! # let report = read_file("test.txt").unwrap_err();
//! # assert!(report.contains::<io::Error>());
//! # }
//! ```
//!
//! ## Using and Expanding the Report
Expand All @@ -105,7 +103,6 @@
//! (Again, for convenience, using [`ResultExt`] will do that on the [`Err`] variant)
//!
//! ```rust
//! # #![cfg_attr(not(feature = "std"), allow(dead_code, unused_variables, unused_imports))]
//! # use std::{fmt, fs, io, path::Path};
//! use error_stack::{Context, Result, ResultExt};
//! # pub type Config = String;
Expand All @@ -122,7 +119,6 @@
//! // It's also possible to implement `Error` instead.
//! impl Context for ParseConfigError {}
//!
//! # #[cfg(all(not(miri), feature = "std"))] {
//! // For clarification, this example is not using `error_stack::Result`.
//! fn parse_config(path: impl AsRef<Path>) -> Result<Config, ParseConfigError> {
//! let content = fs::read_to_string(path.as_ref())
Expand All @@ -135,7 +131,6 @@
//! # let report = parse_config("test.txt").unwrap_err();
//! # assert!(report.contains::<io::Error>());
//! # assert!(report.contains::<ParseConfigError>());
//! # }
//! ```
//!
//! ### Building up the Report - Attachments
Expand Down Expand Up @@ -226,7 +221,6 @@
//! [`extend_one()`]: Report::extend_one
//!
//! ```rust
//! # #![cfg_attr(not(feature = "std"), allow(dead_code, unused_variables, unused_imports))]
//! # use std::{fs, path::Path};
//! # use error_stack::Report;
//! # pub type Config = String;
Expand Down
14 changes: 6 additions & 8 deletions libs/error-stack/src/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,7 @@ pub mod __private {
///
/// Create a [`Report`] from [`Error`]:
///
/// ```
/// # #[cfg(all(not(miri), feature = "std"))] {
/// ```rust
/// use std::fs;
///
/// use error_stack::report;
Expand All @@ -91,12 +90,12 @@ pub mod __private {
/// Err(err) => return Err(report!(err)),
/// }
/// # Ok(()) }
/// # assert!(wrapper().unwrap_err().contains::<std::io::Error>()); }
/// # assert!(wrapper().unwrap_err().contains::<std::io::Error>());
/// ```
///
/// Create a [`Report`] from [`Context`]:
///
/// ```
/// ```rust
/// # fn has_permission(_: &u32, _: &u32) -> bool { true }
/// # type User = u32;
/// # let user = 0;
Expand Down Expand Up @@ -147,7 +146,6 @@ macro_rules! report {
/// [`Error`]: core::error::Error
///
/// ```
/// # #[cfg(all(not(miri), feature = "std"))] {
/// use std::fs;
///
/// use error_stack::bail;
Expand All @@ -157,14 +155,14 @@ macro_rules! report {
/// Err(err) => bail!(err),
/// }
/// # Ok(()) }
/// # assert!(wrapper().unwrap_err().contains::<std::io::Error>()); }
/// # assert!(wrapper().unwrap_err().contains::<std::io::Error>());
/// ```
///
/// Create a [`Report`] from [`Context`]:
///
/// [`Context`]: crate::Context
///
/// ```
/// ```rust
/// # fn has_permission(_: &u32, _: &u32) -> bool { true }
/// # type User = u32;
/// # let user = 0;
Expand Down Expand Up @@ -213,7 +211,7 @@ macro_rules! bail {
/// [`Report`]: crate::Report
/// [`Context`]: crate::Context
///
/// ```
/// ```rust
/// # fn has_permission(_: &u32, _: &u32) -> bool { true }
/// # type User = u32;
/// # let user = 0;
Expand Down
Loading

0 comments on commit f190a08

Please sign in to comment.