diff --git a/crates/ty/docs/cli.md b/crates/ty/docs/cli.md index 56a6ae32c6..efe141813e 100644 --- a/crates/ty/docs/cli.md +++ b/crates/ty/docs/cli.md @@ -84,7 +84,8 @@ over all configuration files.

  • 3.11
  • 3.12
  • 3.13
    • -
    --respect-ignore-files

    Respect file exclusions via .gitignore and other standard ignore files. Use --no-respect-gitignore to disable

    +
    --quiet

    Use quiet output

    +
    --respect-ignore-files

    Respect file exclusions via .gitignore and other standard ignore files. Use --no-respect-gitignore to disable

    --typeshed, --custom-typeshed-dir path

    Custom directory to use for stdlib typeshed stubs

    --verbose, -v

    Use verbose output (or -vv and -vvv for more verbose output)

    --warn rule

    Treat the given rule as having severity 'warn'. Can be specified multiple times.

    diff --git a/crates/ty/src/lib.rs b/crates/ty/src/lib.rs index 6da471e2ad..1286bae8c8 100644 --- a/crates/ty/src/lib.rs +++ b/crates/ty/src/lib.rs @@ -1,12 +1,13 @@ mod args; mod logging; +mod printer; mod python_version; mod version; pub use args::Cli; use ty_static::EnvVars; -use std::io::{self, BufWriter, Write, stdout}; +use std::fmt::Write; use std::process::{ExitCode, Termination}; use anyhow::Result; @@ -14,6 +15,7 @@ use std::sync::Mutex; use crate::args::{CheckCommand, Command, TerminalColor}; use crate::logging::setup_tracing; +use crate::printer::Printer; use anyhow::{Context, anyhow}; use clap::{CommandFactory, Parser}; use colored::Colorize; @@ -25,7 +27,7 @@ use ruff_db::system::{OsSystem, SystemPath, SystemPathBuf}; use salsa::plumbing::ZalsaDatabase; use ty_project::metadata::options::ProjectOptionsOverrides; use ty_project::watch::ProjectWatcher; -use ty_project::{Db, DummyReporter, Reporter, watch}; +use ty_project::{Db, watch}; use ty_project::{ProjectDatabase, ProjectMetadata}; use ty_server::run_server; @@ -42,6 +44,8 @@ pub fn run() -> anyhow::Result { Command::Check(check_args) => run_check(check_args), Command::Version => version().map(|()| ExitStatus::Success), Command::GenerateShellCompletion { shell } => { + use std::io::stdout; + shell.generate(&mut Cli::command(), &mut stdout()); Ok(ExitStatus::Success) } @@ -49,7 +53,7 @@ pub fn run() -> anyhow::Result { } pub(crate) fn version() -> Result<()> { - let mut stdout = BufWriter::new(io::stdout().lock()); + let mut stdout = Printer::default().stream_for_requested_summary().lock(); let version_info = crate::version::version(); writeln!(stdout, "ty {}", &version_info)?; Ok(()) @@ -61,6 +65,8 @@ fn run_check(args: CheckCommand) -> anyhow::Result { let verbosity = args.verbosity.level(); let _guard = setup_tracing(verbosity, args.color.unwrap_or_default())?; + let printer = Printer::default().with_verbosity(verbosity); + tracing::warn!( "ty is pre-release software and not ready for production use. \ Expect to encounter bugs, missing features, and fatal errors.", @@ -125,7 +131,8 @@ fn run_check(args: CheckCommand) -> anyhow::Result { } let project_options_overrides = ProjectOptionsOverrides::new(config_file, options); - let (main_loop, main_loop_cancellation_token) = MainLoop::new(project_options_overrides); + let (main_loop, main_loop_cancellation_token) = + MainLoop::new(project_options_overrides, printer); // Listen to Ctrl+C and abort the watch mode. let main_loop_cancellation_token = Mutex::new(Some(main_loop_cancellation_token)); @@ -143,7 +150,7 @@ fn run_check(args: CheckCommand) -> anyhow::Result { main_loop.run(&mut db)? }; - let mut stdout = stdout().lock(); + let mut stdout = printer.stream_for_requested_summary().lock(); match std::env::var(EnvVars::TY_MEMORY_REPORT).as_deref() { Ok("short") => write!(stdout, "{}", db.salsa_memory_dump().display_short())?, Ok("mypy_primer") => write!(stdout, "{}", db.salsa_memory_dump().display_mypy_primer())?, @@ -192,12 +199,16 @@ struct MainLoop { /// The file system watcher, if running in watch mode. watcher: Option, + /// Interface for displaying information to the user. + printer: Printer, + project_options_overrides: ProjectOptionsOverrides, } impl MainLoop { fn new( project_options_overrides: ProjectOptionsOverrides, + printer: Printer, ) -> (Self, MainLoopCancellationToken) { let (sender, receiver) = crossbeam_channel::bounded(10); @@ -207,6 +218,7 @@ impl MainLoop { receiver, watcher: None, project_options_overrides, + printer, }, MainLoopCancellationToken { sender }, ) @@ -223,32 +235,24 @@ impl MainLoop { // Do not show progress bars with `--watch`, indicatif does not seem to // handle cancelling independent progress bars very well. - self.run_with_progress::(db)?; + // TODO(zanieb): We can probably use `MultiProgress` to handle this case in the future. + self.printer = self.printer.with_no_progress(); + self.run(db)?; Ok(ExitStatus::Success) } fn run(self, db: &mut ProjectDatabase) -> Result { - self.run_with_progress::(db) - } - - fn run_with_progress(mut self, db: &mut ProjectDatabase) -> Result - where - R: Reporter + Default + 'static, - { self.sender.send(MainLoopMessage::CheckWorkspace).unwrap(); - let result = self.main_loop::(db); + let result = self.main_loop(db); tracing::debug!("Exiting main loop"); result } - fn main_loop(&mut self, db: &mut ProjectDatabase) -> Result - where - R: Reporter + Default + 'static, - { + fn main_loop(mut self, db: &mut ProjectDatabase) -> Result { // Schedule the first check. tracing::debug!("Starting main loop"); @@ -264,7 +268,7 @@ impl MainLoop { // to prevent blocking the main loop here. rayon::spawn(move || { match salsa::Cancelled::catch(|| { - let mut reporter = R::default(); + let mut reporter = IndicatifReporter::from(self.printer); db.check_with_reporter(&mut reporter) }) { Ok(result) => { @@ -299,10 +303,12 @@ impl MainLoop { return Ok(ExitStatus::Success); } - let mut stdout = stdout().lock(); - if result.is_empty() { - writeln!(stdout, "{}", "All checks passed!".green().bold())?; + writeln!( + self.printer.stream_for_success_summary(), + "{}", + "All checks passed!".green().bold() + )?; if self.watcher.is_none() { return Ok(ExitStatus::Success); @@ -311,14 +317,19 @@ impl MainLoop { let mut max_severity = Severity::Info; let diagnostics_count = result.len(); + let mut stdout = self.printer.stream_for_details().lock(); for diagnostic in result { - write!(stdout, "{}", diagnostic.display(db, &display_config))?; + // Only render diagnostics if they're going to be displayed, since doing + // so is expensive. + if stdout.is_enabled() { + write!(stdout, "{}", diagnostic.display(db, &display_config))?; + } max_severity = max_severity.max(diagnostic.severity()); } writeln!( - stdout, + self.printer.stream_for_failure_summary(), "Found {} diagnostic{}", diagnostics_count, if diagnostics_count > 1 { "s" } else { "" } @@ -378,27 +389,53 @@ impl MainLoop { } /// A progress reporter for `ty check`. -#[derive(Default)] -struct IndicatifReporter(Option); +enum IndicatifReporter { + /// A constructed reporter that is not yet ready, contains the target for the progress bar. + Pending(indicatif::ProgressDrawTarget), + /// A reporter that is ready, containing a progress bar to report to. + /// + /// Initialization of the bar is deferred to [`ty_project::ProgressReporter::set_files`] so we + /// do not initialize the bar too early as it may take a while to collect the number of files to + /// process and we don't want to display an empty "0/0" bar. + Initialized(indicatif::ProgressBar), +} -impl ty_project::Reporter for IndicatifReporter { +impl From for IndicatifReporter { + fn from(printer: Printer) -> Self { + Self::Pending(printer.progress_target()) + } +} + +impl ty_project::ProgressReporter for IndicatifReporter { fn set_files(&mut self, files: usize) { - let progress = indicatif::ProgressBar::new(files as u64); - progress.set_style( + let target = match std::mem::replace( + self, + IndicatifReporter::Pending(indicatif::ProgressDrawTarget::hidden()), + ) { + Self::Pending(target) => target, + Self::Initialized(_) => panic!("The progress reporter should only be initialized once"), + }; + + let bar = indicatif::ProgressBar::with_draw_target(Some(files as u64), target); + bar.set_style( indicatif::ProgressStyle::with_template( "{msg:8.dim} {bar:60.green/dim} {pos}/{len} files", ) .unwrap() .progress_chars("--"), ); - progress.set_message("Checking"); - - self.0 = Some(progress); + bar.set_message("Checking"); + *self = Self::Initialized(bar); } fn report_file(&self, _file: &ruff_db::files::File) { - if let Some(ref progress_bar) = self.0 { - progress_bar.inc(1); + match self { + IndicatifReporter::Initialized(progress_bar) => { + progress_bar.inc(1); + } + IndicatifReporter::Pending(_) => { + panic!("`report_file` called before `set_files`") + } } } } diff --git a/crates/ty/src/logging.rs b/crates/ty/src/logging.rs index 4ba5da3f74..a85d527a47 100644 --- a/crates/ty/src/logging.rs +++ b/crates/ty/src/logging.rs @@ -24,15 +24,32 @@ pub(crate) struct Verbosity { help = "Use verbose output (or `-vv` and `-vvv` for more verbose output)", action = clap::ArgAction::Count, global = true, + overrides_with = "quiet", )] verbose: u8, + + #[arg( + long, + help = "Use quiet output", + action = clap::ArgAction::Count, + global = true, + overrides_with = "verbose", + )] + quiet: u8, } impl Verbosity { - /// Returns the verbosity level based on the number of `-v` flags. + /// Returns the verbosity level based on the number of `-v` and `-q` flags. /// /// Returns `None` if the user did not specify any verbosity flags. pub(crate) fn level(&self) -> VerbosityLevel { + // `--quiet` and `--verbose` are mutually exclusive in Clap, so we can just check one first. + match self.quiet { + 0 => {} + _ => return VerbosityLevel::Quiet, + // TODO(zanieb): Add support for `-qq` with a "silent" mode + } + match self.verbose { 0 => VerbosityLevel::Default, 1 => VerbosityLevel::Verbose, @@ -42,9 +59,14 @@ impl Verbosity { } } -#[derive(Debug, Copy, Clone, Eq, PartialEq, Ord, PartialOrd)] +#[derive(Debug, Copy, Clone, Eq, PartialEq, Ord, PartialOrd, Default)] pub(crate) enum VerbosityLevel { + /// Quiet output. Only shows Ruff and ty events up to the [`ERROR`](tracing::Level::ERROR). + /// Silences output except for summary information. + Quiet, + /// Default output level. Only shows Ruff and ty events up to the [`WARN`](tracing::Level::WARN). + #[default] Default, /// Enables verbose output. Emits Ruff and ty events up to the [`INFO`](tracing::Level::INFO). @@ -62,6 +84,7 @@ pub(crate) enum VerbosityLevel { impl VerbosityLevel { const fn level_filter(self) -> LevelFilter { match self { + VerbosityLevel::Quiet => LevelFilter::ERROR, VerbosityLevel::Default => LevelFilter::WARN, VerbosityLevel::Verbose => LevelFilter::INFO, VerbosityLevel::ExtraVerbose => LevelFilter::DEBUG, diff --git a/crates/ty/src/printer.rs b/crates/ty/src/printer.rs new file mode 100644 index 0000000000..669786a90f --- /dev/null +++ b/crates/ty/src/printer.rs @@ -0,0 +1,172 @@ +use std::io::StdoutLock; + +use indicatif::ProgressDrawTarget; + +use crate::logging::VerbosityLevel; + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] +pub(crate) struct Printer { + verbosity: VerbosityLevel, + no_progress: bool, +} + +impl Printer { + #[must_use] + pub(crate) fn with_no_progress(self) -> Self { + Self { + verbosity: self.verbosity, + no_progress: true, + } + } + + #[must_use] + pub(crate) fn with_verbosity(self, verbosity: VerbosityLevel) -> Self { + Self { + verbosity, + no_progress: self.no_progress, + } + } + + /// Return the [`ProgressDrawTarget`] for this printer. + pub(crate) fn progress_target(self) -> ProgressDrawTarget { + if self.no_progress { + return ProgressDrawTarget::hidden(); + } + + match self.verbosity { + VerbosityLevel::Quiet => ProgressDrawTarget::hidden(), + VerbosityLevel::Default => ProgressDrawTarget::stderr(), + // Hide the progress bar when in verbose mode. + // Otherwise, it gets interleaved with log messages. + VerbosityLevel::Verbose => ProgressDrawTarget::hidden(), + VerbosityLevel::ExtraVerbose => ProgressDrawTarget::hidden(), + VerbosityLevel::Trace => ProgressDrawTarget::hidden(), + } + } + + /// Return the [`Stdout`] stream for important messages. + /// + /// Unlike [`Self::stdout_general`], the returned stream will be enabled when + /// [`VerbosityLevel::Quiet`] is used. + fn stdout_important(self) -> Stdout { + match self.verbosity { + VerbosityLevel::Quiet => Stdout::enabled(), + VerbosityLevel::Default => Stdout::enabled(), + VerbosityLevel::Verbose => Stdout::enabled(), + VerbosityLevel::ExtraVerbose => Stdout::enabled(), + VerbosityLevel::Trace => Stdout::enabled(), + } + } + + /// Return the [`Stdout`] stream for general messages. + /// + /// The returned stream will be disabled when [`VerbosityLevel::Quiet`] is used. + fn stdout_general(self) -> Stdout { + match self.verbosity { + VerbosityLevel::Quiet => Stdout::disabled(), + VerbosityLevel::Default => Stdout::enabled(), + VerbosityLevel::Verbose => Stdout::enabled(), + VerbosityLevel::ExtraVerbose => Stdout::enabled(), + VerbosityLevel::Trace => Stdout::enabled(), + } + } + + /// Return the [`Stdout`] stream for a summary message that was explicitly requested by the + /// user. + /// + /// For example, in `ty version` the user has requested the version information and we should + /// display it even if [`VerbosityLevel::Quiet`] is used. Or, in `ty check`, if the + /// `TY_MEMORY_REPORT` variable has been set, we should display the memory report because the + /// user has opted-in to display. + pub(crate) fn stream_for_requested_summary(self) -> Stdout { + self.stdout_important() + } + + /// Return the [`Stdout`] stream for a summary message on failure. + /// + /// For example, in `ty check`, this would be used for the message indicating the number of + /// diagnostics found. The failure summary should capture information that is not reflected in + /// the exit code. + pub(crate) fn stream_for_failure_summary(self) -> Stdout { + self.stdout_important() + } + + /// Return the [`Stdout`] stream for a summary message on success. + /// + /// For example, in `ty check`, this would be used for the message indicating that no diagnostic + /// were found. The success summary does not capture important information for users that have + /// opted-in to [`VerbosityLevel::Quiet`]. + pub(crate) fn stream_for_success_summary(self) -> Stdout { + self.stdout_general() + } + + /// Return the [`Stdout`] stream for detailed messages. + /// + /// For example, in `ty check`, this would be used for the diagnostic output. + pub(crate) fn stream_for_details(self) -> Stdout { + self.stdout_general() + } +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum StreamStatus { + Enabled, + Disabled, +} + +#[derive(Debug)] +pub(crate) struct Stdout { + status: StreamStatus, + lock: Option>, +} + +impl Stdout { + fn enabled() -> Self { + Self { + status: StreamStatus::Enabled, + lock: None, + } + } + + fn disabled() -> Self { + Self { + status: StreamStatus::Disabled, + lock: None, + } + } + + pub(crate) fn lock(mut self) -> Self { + match self.status { + StreamStatus::Enabled => { + // Drop the previous lock first, to avoid deadlocking + self.lock.take(); + self.lock = Some(std::io::stdout().lock()); + } + StreamStatus::Disabled => self.lock = None, + } + self + } + + fn handle(&mut self) -> Box { + match self.lock.as_mut() { + Some(lock) => Box::new(lock), + None => Box::new(std::io::stdout()), + } + } + + pub(crate) fn is_enabled(&self) -> bool { + matches!(self.status, StreamStatus::Enabled) + } +} + +impl std::fmt::Write for Stdout { + fn write_str(&mut self, s: &str) -> std::fmt::Result { + match self.status { + StreamStatus::Enabled => { + let _ = write!(self.handle(), "{s}"); + Ok(()) + } + StreamStatus::Disabled => Ok(()), + } + } +} diff --git a/crates/ty/tests/cli/main.rs b/crates/ty/tests/cli/main.rs index a8b8b4bd55..14c237a729 100644 --- a/crates/ty/tests/cli/main.rs +++ b/crates/ty/tests/cli/main.rs @@ -14,6 +14,64 @@ use std::{ }; use tempfile::TempDir; +#[test] +fn test_quiet_output() -> anyhow::Result<()> { + let case = CliTest::with_file("test.py", "x: int = 1")?; + + // By default, we emit an "all checks passed" message + assert_cmd_snapshot!(case.command(), @r" + success: true + exit_code: 0 + ----- stdout ----- + All checks passed! + + ----- stderr ----- + WARN ty is pre-release software and not ready for production use. Expect to encounter bugs, missing features, and fatal errors. + "); + + // With `quiet`, the message is not displayed + assert_cmd_snapshot!(case.command().arg("--quiet"), @r" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + "); + + let case = CliTest::with_file("test.py", "x: int = 'foo'")?; + + // By default, we emit a diagnostic + assert_cmd_snapshot!(case.command(), @r#" + success: false + exit_code: 1 + ----- stdout ----- + error[invalid-assignment]: Object of type `Literal["foo"]` is not assignable to `int` + --> test.py:1:1 + | + 1 | x: int = 'foo' + | ^ + | + info: rule `invalid-assignment` is enabled by default + + Found 1 diagnostic + + ----- stderr ----- + WARN ty is pre-release software and not ready for production use. Expect to encounter bugs, missing features, and fatal errors. + "#); + + // With `quiet`, the diagnostic is not displayed, just the summary message + assert_cmd_snapshot!(case.command().arg("--quiet"), @r" + success: false + exit_code: 1 + ----- stdout ----- + Found 1 diagnostic + + ----- stderr ----- + "); + + Ok(()) +} + #[test] fn test_run_in_sub_directory() -> anyhow::Result<()> { let case = CliTest::with_files([("test.py", "~"), ("subdir/nothing", "")])?; diff --git a/crates/ty_project/src/db.rs b/crates/ty_project/src/db.rs index 9f15d612dd..76f001b3b8 100644 --- a/crates/ty_project/src/db.rs +++ b/crates/ty_project/src/db.rs @@ -5,7 +5,7 @@ use std::{cmp, fmt}; use crate::metadata::settings::file_settings; use crate::{DEFAULT_LINT_REGISTRY, DummyReporter}; -use crate::{Project, ProjectMetadata, Reporter}; +use crate::{ProgressReporter, Project, ProjectMetadata}; use ruff_db::Db as SourceDb; use ruff_db::diagnostic::Diagnostic; use ruff_db::files::{File, Files}; @@ -87,7 +87,7 @@ impl ProjectDatabase { } /// Checks all open files in the project and its dependencies, using the given reporter. - pub fn check_with_reporter(&self, reporter: &mut dyn Reporter) -> Vec { + pub fn check_with_reporter(&self, reporter: &mut dyn ProgressReporter) -> Vec { let reporter = AssertUnwindSafe(reporter); self.project().check(self, CheckMode::OpenFiles, reporter) } @@ -95,7 +95,7 @@ impl ProjectDatabase { /// Check the project with the given mode. pub fn check_with_mode(&self, mode: CheckMode) -> Vec { let mut reporter = DummyReporter; - let reporter = AssertUnwindSafe(&mut reporter as &mut dyn Reporter); + let reporter = AssertUnwindSafe(&mut reporter as &mut dyn ProgressReporter); self.project().check(self, mode, reporter) } diff --git a/crates/ty_project/src/lib.rs b/crates/ty_project/src/lib.rs index e486f8c239..79778d93fa 100644 --- a/crates/ty_project/src/lib.rs +++ b/crates/ty_project/src/lib.rs @@ -113,7 +113,7 @@ pub struct Project { } /// A progress reporter. -pub trait Reporter: Send + Sync { +pub trait ProgressReporter: Send + Sync { /// Initialize the reporter with the number of files. fn set_files(&mut self, files: usize); @@ -121,11 +121,11 @@ pub trait Reporter: Send + Sync { fn report_file(&self, file: &File); } -/// A no-op implementation of [`Reporter`]. +/// A no-op implementation of [`ProgressReporter`]. #[derive(Default)] pub struct DummyReporter; -impl Reporter for DummyReporter { +impl ProgressReporter for DummyReporter { fn set_files(&mut self, _files: usize) {} fn report_file(&self, _file: &File) {} } @@ -212,7 +212,7 @@ impl Project { self, db: &ProjectDatabase, mode: CheckMode, - mut reporter: AssertUnwindSafe<&mut dyn Reporter>, + mut reporter: AssertUnwindSafe<&mut dyn ProgressReporter>, ) -> Vec { let project_span = tracing::debug_span!("Project::check"); let _span = project_span.enter();