From 8da70370340401ebc7e82076780d5186a939118c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E4=BA=8C=E6=89=8B=E6=8E=89=E5=8C=85=E5=B7=A5=E7=A8=8B?= =?UTF-8?q?=E5=B8=88?= Date: Wed, 14 Feb 2024 00:40:43 +0800 Subject: [PATCH] feat(console): add `--allow` flag (#513) ## Description This pull request adds a new flag to `tokio-console` that allows warning lints to be selectively disabled. Additionally, you can use the value `all` to disable all warnings. For example, you can use it like this: ```sh cargo run -- --warn "self-wakes,lost-waker,never-yielded" --allow "self-wakes,never-yielded" ``` Which will warn on all three of the current lints, but then allow the `self-wakes` and `never-yielded` lints. The end result will be that only the `lost-wakers` lint will be "active". This PR follows from [comments in #493]. ## Explanation of Changes Added an `AllowedWarnings` enum to the command line interface to represent the allowed warnings. `All` for disable all warnings. `Explicit` explicitly indicates which warning can be ignored. [comments in #493]: https://github.com/tokio-rs/console/pull/493#discussion_r141061048 --- README.md | 20 +++++++ tokio-console/src/config.rs | 87 ++++++++++++++++++++++++++++++- tokio-console/src/main.rs | 12 ++++- tokio-console/tests/cli-ui.stdout | 20 +++++++ 4 files changed, 137 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index b5a4db4d8..fe3f3d945 100644 --- a/README.md +++ b/README.md @@ -229,6 +229,26 @@ Options: [default: self-wakes lost-waker never-yielded] [possible values: self-wakes, lost-waker, never-yielded] + -A, --allow ... + Allow lint warnings. + + This is a comma-separated list of warnings to allow. + + Each warning is specified by its name, which is one of: + + * `self-wakes` -- Warns when a task wakes itself more than a + certain percentage of its total wakeups. Default percentage is + 50%. + + * `lost-waker` -- Warns when a task is dropped without being + woken. + + * `never-yielded` -- Warns when a task has never yielded. + + If this is set to `all`, all warnings are allowed. + + [possible values: all, self-wakes, lost-waker, never-yielded] + --log-dir Path to a directory to write the console's internal logs to. diff --git a/tokio-console/src/config.rs b/tokio-console/src/config.rs index c470a22e9..5da8ae0c1 100644 --- a/tokio-console/src/config.rs +++ b/tokio-console/src/config.rs @@ -6,6 +6,7 @@ use clap::{ArgAction, ArgGroup, CommandFactory, Parser as Clap, Subcommand, Valu use clap_complete::Shell; use color_eyre::eyre::WrapErr; use serde::{Deserialize, Serialize}; +use std::collections::BTreeSet; use std::fmt; use std::fs; use std::ops::Not; @@ -62,11 +63,29 @@ pub struct Config { /// * `lost-waker` -- Warns when a task is dropped without being woken. /// /// * `never-yielded` -- Warns when a task has never yielded. - /// #[clap(long = "warn", short = 'W', value_delimiter = ',', num_args = 1..)] #[clap(default_values_t = KnownWarnings::default_enabled_warnings())] pub(crate) warnings: Vec, + /// Allow lint warnings. + /// + /// This is a comma-separated list of warnings to allow. + /// + /// Each warning is specified by its name, which is one of: + /// + /// * `self-wakes` -- Warns when a task wakes itself more than a certain percentage of its total wakeups. + /// Default percentage is 50%. + /// + /// * `lost-waker` -- Warns when a task is dropped without being woken. + /// + /// * `never-yielded` -- Warns when a task has never yielded. + /// + /// If this is set to `all`, all warnings are allowed. + /// + /// [possible values: all, self-wakes, lost-waker, never-yielded] + #[clap(long = "allow", short = 'A', num_args = 1..)] + pub(crate) allow_warnings: Option, + /// Path to a directory to write the console's internal logs to. /// /// [default: /tmp/tokio-console/logs] @@ -126,6 +145,19 @@ pub(crate) enum KnownWarnings { NeverYielded, } +impl FromStr for KnownWarnings { + type Err = String; + + fn from_str(s: &str) -> Result { + match s { + "self-wakes" => Ok(KnownWarnings::SelfWakes), + "lost-waker" => Ok(KnownWarnings::LostWaker), + "never-yielded" => Ok(KnownWarnings::NeverYielded), + _ => Err(format!("unknown warning: {}", s)), + } + } +} + impl From<&KnownWarnings> for warnings::Linter { fn from(warning: &KnownWarnings) -> Self { match warning { @@ -155,6 +187,49 @@ impl KnownWarnings { ] } } +/// An enum representing the types of warnings that are allowed. +// Note: ValueEnum only supports unit variants, so we have to use a custom +// parser for this. +#[derive(Debug, Clone, PartialEq, Eq, Deserialize, Serialize)] +pub(crate) enum AllowedWarnings { + /// Represents the case where all warnings are allowed. + All, + /// Represents the case where only some specific warnings are allowed. + /// The allowed warnings are stored in a `BTreeSet` of `KnownWarnings`. + Explicit(BTreeSet), +} + +impl FromStr for AllowedWarnings { + type Err = String; + + fn from_str(s: &str) -> Result { + match s { + "all" => Ok(AllowedWarnings::All), + _ => { + let warnings = s + .split(',') + .map(|s| s.parse::()) + .collect::, _>>() + .map_err(|err| format!("failed to parse warning: {}", err))?; + Ok(AllowedWarnings::Explicit(warnings)) + } + } + } +} + +impl AllowedWarnings { + fn merge(&self, allowed: &Self) -> Self { + match (self, allowed) { + (AllowedWarnings::All, _) => AllowedWarnings::All, + (_, AllowedWarnings::All) => AllowedWarnings::All, + (AllowedWarnings::Explicit(a), AllowedWarnings::Explicit(b)) => { + let mut warnings = a.clone(); + warnings.extend(b.clone()); + AllowedWarnings::Explicit(warnings) + } + } + } +} #[derive(Debug, Subcommand, PartialEq, Eq)] pub enum OptionalCmd { @@ -299,6 +374,7 @@ struct ConfigFile { default_target_addr: Option, log: Option, warnings: Vec, + allow_warnings: Option, log_directory: Option, retention: Option, charset: Option, @@ -494,6 +570,12 @@ impl Config { warns.dedup(); warns }, + allow_warnings: { + match (self.allow_warnings, other.allow_warnings) { + (Some(a), Some(b)) => Some(a.merge(&b)), + (a, b) => a.or(b), + } + }, retain_for: other.retain_for.or(self.retain_for), view_options: self.view_options.merge_with(other.view_options), subcmd: other.subcmd.or(self.subcmd), @@ -509,6 +591,7 @@ impl Default for Config { filter::Targets::new().with_default(filter::LevelFilter::OFF), )), warnings: KnownWarnings::default_enabled_warnings(), + allow_warnings: None, log_directory: Some(default_log_directory()), retain_for: Some(RetainFor::default()), view_options: ViewOptions::default(), @@ -745,6 +828,7 @@ impl From for ConfigFile { log: config.log_filter.map(|filter| filter.to_string()), log_directory: config.log_directory, warnings: config.warnings, + allow_warnings: config.allow_warnings, retention: config.retain_for, charset: Some(CharsetConfig { lang: config.view_options.lang, @@ -768,6 +852,7 @@ impl TryFrom for Config { target_addr: value.target_addr()?, log_filter: value.log_filter()?, warnings: value.warnings.clone(), + allow_warnings: value.allow_warnings.clone(), log_directory: value.log_directory.take(), retain_for: value.retain_for(), view_options: ViewOptions { diff --git a/tokio-console/src/main.rs b/tokio-console/src/main.rs index 4f99ee71a..421c660a7 100644 --- a/tokio-console/src/main.rs +++ b/tokio-console/src/main.rs @@ -15,6 +15,7 @@ use ratatui::{ use tokio::sync::{mpsc, watch}; use crate::{ + config::AllowedWarnings, input::{Event, KeyEvent, KeyEventKind}, view::{bold, UpdateKind}, }; @@ -64,9 +65,18 @@ async fn main() -> color_eyre::Result<()> { let (update_tx, update_rx) = watch::channel(UpdateKind::Other); // A channel to send the task details update stream (no need to keep outdated details in the memory) let (details_tx, mut details_rx) = mpsc::channel::(2); + let warnings = match args.allow_warnings { + Some(AllowedWarnings::All) => vec![], + Some(AllowedWarnings::Explicit(allow_warnings)) => args + .warnings + .iter() + .filter(|lint| !allow_warnings.contains(lint)) + .collect::>(), + None => args.warnings.iter().collect::>(), + }; let mut state = State::default() - .with_task_linters(args.warnings.iter().map(|lint| lint.into())) + .with_task_linters(warnings.into_iter().map(|lint| lint.into())) .with_retain_for(retain_for); let mut input = Box::pin(input::EventStream::new().try_filter(|event| { future::ready(!matches!( diff --git a/tokio-console/tests/cli-ui.stdout b/tokio-console/tests/cli-ui.stdout index 519a82dbd..1db3373a2 100644 --- a/tokio-console/tests/cli-ui.stdout +++ b/tokio-console/tests/cli-ui.stdout @@ -58,6 +58,26 @@ Options: [default: self-wakes lost-waker never-yielded] [possible values: self-wakes, lost-waker, never-yielded] + -A, --allow ... + Allow lint warnings. + + This is a comma-separated list of warnings to allow. + + Each warning is specified by its name, which is one of: + + * `self-wakes` -- Warns when a task wakes itself more than a + certain percentage of its total wakeups. Default percentage is + 50%. + + * `lost-waker` -- Warns when a task is dropped without being + woken. + + * `never-yielded` -- Warns when a task has never yielded. + + If this is set to `all`, all warnings are allowed. + + [possible values: all, self-wakes, lost-waker, never-yielded] + --log-dir Path to a directory to write the console's internal logs to.