chore: fix clippy errors by Noah-Kennedy · Pull Request #375 · tokio-rs/console

Noah-Kennedy · 2022-09-04T20:31:54Z

Needed for #374.

Noah-Kennedy · 2022-09-04T20:39:06Z

Oh these issues are with generated files!

Noah-Kennedy · 2022-09-04T20:40:46Z

@hawkw should I have clippy ignore the generated files?

hawkw · 2022-09-04T20:43:50Z

@hawkw should I have clippy ignore the generated files?

yeah, probably...that's annoying. :|

Noah-Kennedy · 2022-09-04T20:43:58Z

Done.

Noah-Kennedy · 2022-09-04T20:46:42Z

Alright, it looks to be passing CI. Is squashing enabled for this repo?

hawkw

i'm fine with merging this as-is in order to fix CI, but if you don't mind, it might be nice to change the blanket allow for the whole console-api crate so that it doesn't include the human-written code in console_api::common.

hawkw · 2022-09-04T20:46:58Z

console-api/src/lib.rs

@@ -1,4 +1,5 @@
 #![doc = include_str!("../README.md")]
+#![allow(warnings)]


hmm, could we, instead, add the attribute only to the modules that are generated by prost? the common module also contains some handwritten code:

console/console-api/src/common.rs

Lines 5 to 240 in ff6d048

impl From<tracing_core::Level> for metadata::Level {

fn from(level: tracing_core::Level) -> Self {

match level {

tracing_core::Level::ERROR => metadata::Level::Error,

tracing_core::Level::WARN => metadata::Level::Warn,

tracing_core::Level::INFO => metadata::Level::Info,

tracing_core::Level::DEBUG => metadata::Level::Debug,

tracing_core::Level::TRACE => metadata::Level::Trace,

}

}

}

impl From<tracing_core::metadata::Kind> for metadata::Kind {

fn from(kind: tracing_core::metadata::Kind) -> Self {

// /!\ Note that this is intentionally *not* implemented using match.

// The `metadata::Kind` struct in `tracing_core` was written not

// intending to allow exhaustive matches, but accidentally did.

//

// Therefore, we shouldn't be able to write a match against both

// variants without a wildcard arm. However, on versions of

// `tracing_core` where the type was exhaustively matchable, a wildcard

// arm will result in a warning. Thus we must write this rather

// tortured-looking `if` statement to get non-exhaustive matching

// behavior.

if kind == tracing_core::metadata::Kind::SPAN {

metadata::Kind::Span

} else {

metadata::Kind::Event

}

}

}

impl<'a> From<&'a tracing_core::Metadata<'a>> for Metadata {

fn from(meta: &'a tracing_core::Metadata<'a>) -> Self {

let kind = if meta.is_span() {

metadata::Kind::Span

} else {

debug_assert!(meta.is_event());

metadata::Kind::Event

};

let field_names = meta.fields().iter().map(|f| f.name().to_string()).collect();

Metadata {

name: meta.name().to_string(),

target: meta.target().to_string(),

location: Some(meta.into()),

kind: kind as i32,

level: metadata::Level::from(*meta.level()) as i32,

field_names,

..Default::default()

}

}

}

impl<'a> From<&'a tracing_core::Metadata<'a>> for Location {

fn from(meta: &'a tracing_core::Metadata<'a>) -> Self {

Location {

file: meta.file().map(String::from),

module_path: meta.module_path().map(String::from),

line: meta.line(),

column: None, // tracing doesn't support columns yet

}

}

}

impl<'a> From<&'a std::panic::Location<'a>> for Location {

fn from(loc: &'a std::panic::Location<'a>) -> Self {

Location {

file: Some(loc.file().to_string()),

line: Some(loc.line()),

column: Some(loc.column()),

..Default::default()

}

}

}

impl fmt::Display for field::Value {

fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {

match self {

field::Value::BoolVal(v) => fmt::Display::fmt(v, f)?,

field::Value::StrVal(v) => fmt::Display::fmt(v, f)?,

field::Value::U64Val(v) => fmt::Display::fmt(v, f)?,

field::Value::DebugVal(v) => fmt::Display::fmt(v, f)?,

field::Value::I64Val(v) => fmt::Display::fmt(v, f)?,

}

Ok(())

}

}

impl fmt::Display for Field {

fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {

let name_val = (self.name.as_ref(), self.value.as_ref());

if let (Some(field::Name::StrName(name)), Some(val)) = name_val {

write!(f, "{}={}", name, val)?;

}

Ok(())

}

}

impl fmt::Display for Location {

fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {

match (self.module_path.as_ref(), self.file.as_ref()) {

// Module paths take precedence because they're shorter...

(Some(module), _) => f.write_str(module.as_ref())?,

(None, Some(file)) => f.write_str(file.as_ref())?,

// If there's no file or module path, then printing the line and

// column makes no sense...

(None, None) => return f.write_str("<unknown location>"),

};

if let Some(line) = self.line {

write!(f, ":{}", line)?;

// Printing the column only makes sense if there's a line...

if let Some(column) = self.column {

write!(f, ":{}", column)?;

}

}

Ok(())

}

}

// === IDs ===

impl From<&'static tracing_core::Metadata<'static>> for MetaId {

fn from(meta: &'static tracing_core::Metadata) -> Self {

MetaId {

id: meta as *const _ as u64,

}

}

}

impl From<tracing_core::span::Id> for SpanId {

fn from(id: tracing_core::span::Id) -> Self {

SpanId { id: id.into_u64() }

}

}

impl From<SpanId> for tracing_core::span::Id {

fn from(span_id: SpanId) -> Self {

tracing_core::span::Id::from_u64(span_id.id)

}

}

impl From<u64> for SpanId {

fn from(id: u64) -> Self {

SpanId { id }

}

}

impl From<&'static tracing_core::Metadata<'static>> for register_metadata::NewMetadata {

fn from(meta: &'static tracing_core::Metadata) -> Self {

register_metadata::NewMetadata {

id: Some(meta.into()),

metadata: Some(meta.into()),

}

}

}

impl From<i64> for field::Value {

fn from(val: i64) -> Self {

field::Value::I64Val(val)

}

}

impl From<u64> for field::Value {

fn from(val: u64) -> Self {

field::Value::U64Val(val)

}

}

impl From<bool> for field::Value {

fn from(val: bool) -> Self {

field::Value::BoolVal(val)

}

}

impl From<&str> for field::Value {

fn from(val: &str) -> Self {

field::Value::StrVal(val.into())

}

}

impl From<&str> for field::Name {

fn from(val: &str) -> Self {

field::Name::StrName(val.into())

}

}

impl From<&dyn std::fmt::Debug> for field::Value {

fn from(val: &dyn std::fmt::Debug) -> Self {

field::Value::DebugVal(format!("{:?}", val))

}

}

// Clippy warns when a type derives `PartialEq` but has a manual `Hash` impl,

// or vice versa. However, this is unavoidable here, because `prost` generates

// a struct with `#[derive(PartialEq)]`, but we cannot add`#[derive(Hash)]` to the

// generated code.

#[allow(clippy::derive_hash_xor_eq)]

impl Hash for field::Name {

fn hash<H: Hasher>(&self, state: &mut H) {

match self {

field::Name::NameIdx(idx) => idx.hash(state),

field::Name::StrName(s) => s.hash(state),

}

}

}

impl Eq for field::Name {}

// === IDs ===

impl From<u64> for Id {

fn from(id: u64) -> Self {

Id { id }

}

}

impl From<Id> for u64 {

fn from(id: Id) -> Self {

id.id

}

}

impl Copy for Id {}

impl From<tracing_core::span::Id> for Id {

fn from(id: tracing_core::span::Id) -> Self {

Id { id: id.into_u64() }

}

}

and it would be nice if clippy could lint that. we might have to put the generated code in a submodule of common, like this:

// in `common.rs` mod generated { #![allow(warnings)] include!("generated/rs.tokio.console.common.rs"); } pub use self::generated::*; // ...

for the other modules, we can just put the #[allow(warnings)] attribute on the declaration of those modules.

Noah-Kennedy · 2022-09-04T20:52:10Z

squashed

Noah-Kennedy · 2022-09-04T20:55:23Z

@hawkw I don't have merge permissions for this repo, so you'll have to do it.

Noah-Kennedy · 2022-09-04T20:57:47Z

Thanks!

Needed for #374. This configures clippy to ignore most of the generated code in `console-api`.

Noah-Kennedy requested a review from a team as a code owner September 4, 2022 20:31

Noah-Kennedy mentioned this pull request Sep 4, 2022

chore: fix console-subscriber on tokio 1.21.0 #374

Merged

hawkw approved these changes Sep 4, 2022

View reviewed changes

chore: fix clippy errors

cb6f387

Noah-Kennedy force-pushed the noah/clippy branch from 03d4e8f to cb6f387 Compare September 4, 2022 20:51

hawkw merged commit faaf808 into tokio-rs:main Sep 4, 2022

Noah-Kennedy deleted the noah/clippy branch September 4, 2022 20:57

hawkw pushed a commit that referenced this pull request Sep 29, 2023

chore(api): fix clippy errors (#375)

c903b33

Needed for #374. This configures clippy to ignore most of the generated code in `console-api`.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

chore: fix clippy errors#375

chore: fix clippy errors#375
hawkw merged 1 commit intotokio-rs:mainfrom
Noah-Kennedy:noah/clippy

Noah-Kennedy commented Sep 4, 2022

Uh oh!

Noah-Kennedy commented Sep 4, 2022

Uh oh!

Noah-Kennedy commented Sep 4, 2022

Uh oh!

hawkw commented Sep 4, 2022

Uh oh!

Noah-Kennedy commented Sep 4, 2022

Uh oh!

Noah-Kennedy commented Sep 4, 2022

Uh oh!

hawkw left a comment

Uh oh!

hawkw Sep 4, 2022

Uh oh!

Noah-Kennedy Sep 4, 2022

Uh oh!

Noah-Kennedy Sep 4, 2022

Uh oh!

Noah-Kennedy commented Sep 4, 2022

Uh oh!

Noah-Kennedy commented Sep 4, 2022

Uh oh!

Noah-Kennedy commented Sep 4, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		@@ -1,4 +1,5 @@
		#![doc = include_str!("../README.md")]
		#![allow(warnings)]


	impl From<tracing_core::Level> for metadata::Level {
	fn from(level: tracing_core::Level) -> Self {
	match level {
	tracing_core::Level::ERROR => metadata::Level::Error,
	tracing_core::Level::WARN => metadata::Level::Warn,
	tracing_core::Level::INFO => metadata::Level::Info,
	tracing_core::Level::DEBUG => metadata::Level::Debug,
	tracing_core::Level::TRACE => metadata::Level::Trace,
	}
	}
	}

	impl From<tracing_core::metadata::Kind> for metadata::Kind {
	fn from(kind: tracing_core::metadata::Kind) -> Self {
	// /!\ Note that this is intentionally not implemented using match.
	// The `metadata::Kind` struct in `tracing_core` was written not
	// intending to allow exhaustive matches, but accidentally did.
	//
	// Therefore, we shouldn't be able to write a match against both
	// variants without a wildcard arm. However, on versions of
	// `tracing_core` where the type was exhaustively matchable, a wildcard
	// arm will result in a warning. Thus we must write this rather
	// tortured-looking `if` statement to get non-exhaustive matching
	// behavior.
	if kind == tracing_core::metadata::Kind::SPAN {
	metadata::Kind::Span
	} else {
	metadata::Kind::Event
	}
	}
	}

	impl<'a> From<&'a tracing_core::Metadata<'a>> for Metadata {
	fn from(meta: &'a tracing_core::Metadata<'a>) -> Self {
	let kind = if meta.is_span() {
	metadata::Kind::Span
	} else {
	debug_assert!(meta.is_event());
	metadata::Kind::Event
	};

	let field_names = meta.fields().iter().map(\|f\| f.name().to_string()).collect();
	Metadata {
	name: meta.name().to_string(),
	target: meta.target().to_string(),
	location: Some(meta.into()),
	kind: kind as i32,
	level: metadata::Level::from(*meta.level()) as i32,
	field_names,
	..Default::default()
	}
	}
	}

	impl<'a> From<&'a tracing_core::Metadata<'a>> for Location {
	fn from(meta: &'a tracing_core::Metadata<'a>) -> Self {
	Location {
	file: meta.file().map(String::from),
	module_path: meta.module_path().map(String::from),
	line: meta.line(),
	column: None, // tracing doesn't support columns yet
	}
	}
	}

	impl<'a> From<&'a std::panic::Location<'a>> for Location {
	fn from(loc: &'a std::panic::Location<'a>) -> Self {
	Location {
	file: Some(loc.file().to_string()),
	line: Some(loc.line()),
	column: Some(loc.column()),
	..Default::default()
	}
	}
	}

	impl fmt::Display for field::Value {
	fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
	match self {
	field::Value::BoolVal(v) => fmt::Display::fmt(v, f)?,
	field::Value::StrVal(v) => fmt::Display::fmt(v, f)?,
	field::Value::U64Val(v) => fmt::Display::fmt(v, f)?,
	field::Value::DebugVal(v) => fmt::Display::fmt(v, f)?,
	field::Value::I64Val(v) => fmt::Display::fmt(v, f)?,
	}

	Ok(())
	}
	}

	impl fmt::Display for Field {
	fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
	let name_val = (self.name.as_ref(), self.value.as_ref());
	if let (Some(field::Name::StrName(name)), Some(val)) = name_val {
	write!(f, "{}={}", name, val)?;
	}

	Ok(())
	}
	}

	impl fmt::Display for Location {
	fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
	match (self.module_path.as_ref(), self.file.as_ref()) {
	// Module paths take precedence because they're shorter...
	(Some(module), _) => f.write_str(module.as_ref())?,
	(None, Some(file)) => f.write_str(file.as_ref())?,
	// If there's no file or module path, then printing the line and
	// column makes no sense...
	(None, None) => return f.write_str("<unknown location>"),
	};

	if let Some(line) = self.line {
	write!(f, ":{}", line)?;

	// Printing the column only makes sense if there's a line...
	if let Some(column) = self.column {
	write!(f, ":{}", column)?;
	}
	}

	Ok(())
	}
	}

	// === IDs ===

	impl From<&'static tracing_core::Metadata<'static>> for MetaId {
	fn from(meta: &'static tracing_core::Metadata) -> Self {
	MetaId {
	id: meta as *const _ as u64,
	}
	}
	}

	impl From<tracing_core::span::Id> for SpanId {
	fn from(id: tracing_core::span::Id) -> Self {
	SpanId { id: id.into_u64() }
	}
	}

	impl From<SpanId> for tracing_core::span::Id {
	fn from(span_id: SpanId) -> Self {
	tracing_core::span::Id::from_u64(span_id.id)
	}
	}

	impl From<u64> for SpanId {
	fn from(id: u64) -> Self {
	SpanId { id }
	}
	}

	impl From<&'static tracing_core::Metadata<'static>> for register_metadata::NewMetadata {
	fn from(meta: &'static tracing_core::Metadata) -> Self {
	register_metadata::NewMetadata {
	id: Some(meta.into()),
	metadata: Some(meta.into()),
	}
	}
	}

	impl From<i64> for field::Value {
	fn from(val: i64) -> Self {
	field::Value::I64Val(val)
	}
	}

	impl From<u64> for field::Value {
	fn from(val: u64) -> Self {
	field::Value::U64Val(val)
	}
	}

	impl From<bool> for field::Value {
	fn from(val: bool) -> Self {
	field::Value::BoolVal(val)
	}
	}

	impl From<&str> for field::Value {
	fn from(val: &str) -> Self {
	field::Value::StrVal(val.into())
	}
	}

	impl From<&str> for field::Name {
	fn from(val: &str) -> Self {
	field::Name::StrName(val.into())
	}
	}

	impl From<&dyn std::fmt::Debug> for field::Value {
	fn from(val: &dyn std::fmt::Debug) -> Self {
	field::Value::DebugVal(format!("{:?}", val))
	}
	}

	// Clippy warns when a type derives `PartialEq` but has a manual `Hash` impl,
	// or vice versa. However, this is unavoidable here, because `prost` generates
	// a struct with `#[derive(PartialEq)]`, but we cannot add`#[derive(Hash)]` to the
	// generated code.
	#[allow(clippy::derive_hash_xor_eq)]
	impl Hash for field::Name {
	fn hash<H: Hasher>(&self, state: &mut H) {
	match self {
	field::Name::NameIdx(idx) => idx.hash(state),
	field::Name::StrName(s) => s.hash(state),
	}
	}
	}

	impl Eq for field::Name {}

	// === IDs ===

	impl From<u64> for Id {
	fn from(id: u64) -> Self {
	Id { id }
	}
	}

	impl From<Id> for u64 {
	fn from(id: Id) -> Self {
	id.id
	}
	}

	impl Copy for Id {}

	impl From<tracing_core::span::Id> for Id {
	fn from(id: tracing_core::span::Id) -> Self {
	Id { id: id.into_u64() }
	}
	}

Uh oh!

Conversation

Noah-Kennedy commented Sep 4, 2022

Uh oh!

Noah-Kennedy commented Sep 4, 2022

Uh oh!

Noah-Kennedy commented Sep 4, 2022

Uh oh!

hawkw commented Sep 4, 2022

Uh oh!

Noah-Kennedy commented Sep 4, 2022

Uh oh!

Noah-Kennedy commented Sep 4, 2022

Uh oh!

hawkw left a comment

Choose a reason for hiding this comment

Uh oh!

hawkw Sep 4, 2022

Choose a reason for hiding this comment

Uh oh!

Noah-Kennedy Sep 4, 2022

Choose a reason for hiding this comment

Uh oh!

Noah-Kennedy Sep 4, 2022

Choose a reason for hiding this comment

Uh oh!

Noah-Kennedy commented Sep 4, 2022

Uh oh!

Noah-Kennedy commented Sep 4, 2022

Uh oh!

Noah-Kennedy commented Sep 4, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants