Skip to content

Commit

Permalink
chore: fix new warnings in Rust 1.72.0 (#2700)
Browse files Browse the repository at this point in the history
This branch fixes a handful of new warnings which have shown up after
updating to Rust 1.72.0.

This includes:

* `clippy::redundant_closure_call` in macros --- allowed because the
  macro sometimes calls a function that isn't a closure, and the closure
  is just used in the case where it's not a function.
* Unnecessary uses of `#` in raw string literals that don't contain `"`
  characters.
* Dead code warnings with specific feature flag combinations in
  `tracing-subscriber`.

In addition, I've fixed a broken RustDoc link that was making the
Netlify build sad.
  • Loading branch information
hawkw committed Oct 1, 2023
1 parent 72bbf0f commit e03a831
Show file tree
Hide file tree
Showing 9 changed files with 51 additions and 28 deletions.
1 change: 1 addition & 0 deletions netlify.toml
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
[build.environment]
RUSTDOCFLAGS="""
-D warnings \
--force-warn rustdoc::redundant-explicit-links \
--force-warn renamed-and-removed-lints \
--cfg docsrs \
--cfg tracing_unstable
Expand Down
8 changes: 8 additions & 0 deletions tracing-core/src/field.rs
Original file line number Diff line number Diff line change
Expand Up @@ -470,6 +470,10 @@ macro_rules! impl_one_value {
impl $crate::sealed::Sealed for $value_ty {}
impl $crate::field::Value for $value_ty {
fn record(&self, key: &$crate::field::Field, visitor: &mut dyn $crate::field::Visit) {
// `op` is always a function; the closure is used because
// sometimes there isn't a real function corresponding to that
// operation. the clippy warning is not that useful here.
#[allow(clippy::redundant_closure_call)]
visitor.$record(key, $op(*self))
}
}
Expand All @@ -485,6 +489,10 @@ macro_rules! impl_one_value {
impl $crate::sealed::Sealed for ty_to_nonzero!($value_ty) {}
impl $crate::field::Value for ty_to_nonzero!($value_ty) {
fn record(&self, key: &$crate::field::Field, visitor: &mut dyn $crate::field::Visit) {
// `op` is always a function; the closure is used because
// sometimes there isn't a real function corresponding to that
// operation. the clippy warning is not that useful here.
#[allow(clippy::redundant_closure_call)]
visitor.$record(key, $op(self.get()))
}
}
Expand Down
3 changes: 1 addition & 2 deletions tracing-subscriber/src/field/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ pub trait VisitOutput<Out>: Visit {
/// Extension trait implemented by types which can be recorded by a [visitor].
///
/// This allows writing code that is generic over `tracing_core`'s
/// [`span::Attributes`][attr], [`span::Record`][rec], and [`Event`][event]
/// [`span::Attributes`][attr], [`span::Record`][rec], and [`Event`]
/// types. These types all provide inherent `record` methods that allow a
/// visitor to record their fields, but there is no common trait representing this.
///
Expand Down Expand Up @@ -85,7 +85,6 @@ pub trait VisitOutput<Out>: Visit {
/// [visitor]: tracing_core::field::Visit
/// [attr]: tracing_core::span::Attributes
/// [rec]: tracing_core::span::Record
/// [event]: tracing_core::event::Event
pub trait RecordFields: crate::sealed::Sealed<RecordFieldsMarker> {
/// Record all the fields in `self` with the provided `visitor`.
fn record(&self, visitor: &mut dyn Visit);
Expand Down
5 changes: 3 additions & 2 deletions tracing-subscriber/src/filter/directive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,8 @@ enum ParseErrorKind {
// === impl DirectiveSet ===

impl<T> DirectiveSet<T> {
#[cfg(feature = "env-filter")]
// this is only used by `env-filter`.
#[cfg(all(feature = "std", feature = "env-filter"))]
pub(crate) fn is_empty(&self) -> bool {
self.directives.is_empty()
}
Expand Down Expand Up @@ -397,7 +398,7 @@ impl FromStr for StaticDirective {
// === impl ParseError ===

impl ParseError {
#[cfg(feature = "env-filter")]
#[cfg(all(feature = "std", feature = "env-filter"))]
pub(crate) fn new() -> Self {
ParseError {
kind: ParseErrorKind::Other(None),
Expand Down
23 changes: 15 additions & 8 deletions tracing-subscriber/src/filter/env/directive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,8 +120,9 @@ impl Directive {
}

pub(super) fn parse(from: &str, regex: bool) -> Result<Self, ParseError> {
static DIRECTIVE_RE: Lazy<Regex> = Lazy::new(|| Regex::new(
r"(?x)
static DIRECTIVE_RE: Lazy<Regex> = Lazy::new(|| {
Regex::new(
r"(?x)
^(?P<global_level>(?i:trace|debug|info|warn|error|off|[0-5]))$ |
# ^^^.
# `note: we match log level names case-insensitively
Expand All @@ -135,15 +136,18 @@ impl Directive {
# `note: we match log level names case-insensitively
)?
$
"
)
.unwrap());
",
)
.unwrap()
});
static SPAN_PART_RE: Lazy<Regex> =
Lazy::new(|| Regex::new(r#"(?P<name>[^\]\{]+)?(?:\{(?P<fields>[^\}]*)\})?"#).unwrap());
Lazy::new(|| Regex::new(r"(?P<name>[^\]\{]+)?(?:\{(?P<fields>[^\}]*)\})?").unwrap());
static FIELD_FILTER_RE: Lazy<Regex> =
// TODO(eliza): this doesn't _currently_ handle value matchers that include comma
// characters. We should fix that.
Lazy::new(|| Regex::new(r#"(?x)
Lazy::new(|| {
Regex::new(
r"(?x)
(
# field name
[[:word:]][[[:word:]]\.]*
Expand All @@ -152,7 +156,10 @@ impl Directive {
)
# trailing comma or EOS
(?:,\s?|$)
"#).unwrap());
",
)
.unwrap()
});

let caps = DIRECTIVE_RE.captures(from).ok_or_else(ParseError::new)?;

Expand Down
13 changes: 10 additions & 3 deletions tracing-subscriber/src/fmt/format/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -326,8 +326,10 @@ pub struct FieldFnVisitor<'a, F> {
/// Marker for [`Format`] that indicates that the compact log format should be used.
///
/// The compact format includes fields from all currently entered spans, after
/// the event's fields. Span names are listed in order before fields are
/// displayed.
/// the event's fields. Span fields are ordered (but not grouped) by
/// span, and span names are not shown. A more compact representation of the
/// event's [`Level`] is used, and additional information—such as the event's
/// target—is disabled by default.
///
/// # Example Output
///
Expand Down Expand Up @@ -1086,7 +1088,12 @@ where

let mut needs_space = false;
if self.display_target {
write!(writer, "{}{}", dimmed.paint(meta.target()), dimmed.paint(":"))?;
write!(
writer,
"{}{}",
dimmed.paint(meta.target()),
dimmed.paint(":")
)?;
needs_space = true;
}

Expand Down
11 changes: 7 additions & 4 deletions tracing-subscriber/src/fmt/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -762,7 +762,7 @@ where

/// Sets the subscriber being built to use a JSON formatter.
///
/// See [`format::Json`][super::fmt::format::Json]
/// See [`format::Json`] for details.
#[cfg(feature = "json")]
#[cfg_attr(docsrs, doc(cfg(feature = "json")))]
pub fn json(
Expand All @@ -783,7 +783,7 @@ where
impl<T, F, W> SubscriberBuilder<format::JsonFields, format::Format<format::Json, T>, F, W> {
/// Sets the json subscriber being built to flatten event metadata.
///
/// See [`format::Json`][super::fmt::format::Json]
/// See [`format::Json`] for details.
pub fn flatten_event(
self,
flatten_event: bool,
Expand All @@ -797,7 +797,7 @@ impl<T, F, W> SubscriberBuilder<format::JsonFields, format::Format<format::Json,
/// Sets whether or not the JSON subscriber being built will include the current span
/// in formatted events.
///
/// See [`format::Json`][super::fmt::format::Json]
/// See [`format::Json`] for details.
pub fn with_current_span(
self,
display_current_span: bool,
Expand All @@ -811,7 +811,7 @@ impl<T, F, W> SubscriberBuilder<format::JsonFields, format::Format<format::Json,
/// Sets whether or not the JSON subscriber being built will include a list (from
/// root to leaf) of all currently entered spans in formatted events.
///
/// See [`format::Json`][super::fmt::format::Json]
/// See [`format::Json`] for details.
pub fn with_span_list(
self,
display_span_list: bool,
Expand Down Expand Up @@ -1304,6 +1304,9 @@ mod test {
Self { buf }
}

// this is currently only used by the JSON formatter tests. if we need
// it elsewhere in the future, feel free to remove the `#[cfg]`
// attribute!
#[cfg(feature = "json")]
pub(crate) fn buf(&self) -> MutexGuard<'_, Vec<u8>> {
self.buf.lock().unwrap()
Expand Down
9 changes: 4 additions & 5 deletions tracing-subscriber/src/fmt/writer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use tracing_core::Metadata;
/// This trait is already implemented for function pointers and
/// immutably-borrowing closures that return an instance of [`io::Write`], such
/// as [`io::stdout`] and [`io::stderr`]. Additionally, it is implemented for
/// [`std::sync::Mutex`][mutex] when the type inside the mutex implements
/// [`std::sync::Mutex`] when the type inside the mutex implements
/// [`io::Write`].
///
/// # Examples
Expand Down Expand Up @@ -66,7 +66,7 @@ use tracing_core::Metadata;
/// ```
///
/// A single instance of a type implementing [`io::Write`] may be used as a
/// `MakeWriter` by wrapping it in a [`Mutex`][mutex]. For example, we could
/// `MakeWriter` by wrapping it in a [`Mutex`]. For example, we could
/// write to a file like so:
///
/// ```
Expand All @@ -88,7 +88,6 @@ use tracing_core::Metadata;
/// [`Event`]: tracing_core::event::Event
/// [`io::stdout`]: std::io::stdout()
/// [`io::stderr`]: std::io::stderr()
/// [mutex]: std::sync::Mutex
/// [`MakeWriter::make_writer_for`]: MakeWriter::make_writer_for
/// [`Metadata`]: tracing_core::Metadata
/// [levels]: tracing_core::Level
Expand Down Expand Up @@ -325,7 +324,7 @@ pub trait MakeWriterExt<'a>: MakeWriter<'a> {

/// Wraps `self` with a predicate that takes a span or event's [`Metadata`]
/// and returns a `bool`. The returned [`MakeWriter`]'s
/// [`MakeWriter::make_writer_for`][mwf] method will check the predicate to
/// [`MakeWriter::make_writer_for`] method will check the predicate to
/// determine if a writer should be produced for a given span or event.
///
/// If the predicate returns `false`, the wrapped [`MakeWriter`]'s
Expand Down Expand Up @@ -544,7 +543,7 @@ pub struct BoxMakeWriter {
name: &'static str,
}

/// A [writer] that is one of two types implementing [`io::Write`][writer].
/// A [writer] that is one of two types implementing [`io::Write`].
///
/// This may be used by [`MakeWriter`] implementations that may conditionally
/// return one of two writers.
Expand Down
6 changes: 2 additions & 4 deletions tracing-subscriber/src/registry/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -200,11 +200,9 @@ pub trait SpanData<'a> {

/// A reference to [span data] and the associated [registry].
///
/// This type implements all the same methods as [`SpanData`][span data], and
/// provides additional methods for querying the registry based on values from
/// the span.
/// This type implements all the same methods as [`SpanData`], and provides
/// additional methods for querying the registry based on values from the span.
///
/// [span data]: SpanData
/// [registry]: LookupSpan
#[derive(Debug)]
pub struct SpanRef<'a, R: LookupSpan<'a>> {
Expand Down

0 comments on commit e03a831

Please sign in to comment.