mirror of
https://github.com/astral-sh/ruff.git
synced 2025-10-01 06:11:21 +00:00
Add progress bar for ty check
(#17965)
## Summary Adds a simple progress bar for the `ty check` CLI command. The style is taken from uv, and like uv the bar is always shown - for smaller projects it is fast enough that it isn't noticeable. We could alternatively hide it completely based on some heuristic for the number of files, or only show it after some amount of time. I also disabled it when `--watch` is passed, cancelling inflight checks was leading to zombie progress bars. I think we can fix this by using [`MultiProgress`](https://docs.rs/indicatif/latest/indicatif/struct.MultiProgress.html) and managing all the bars globally, but I left that out for now. Resolves https://github.com/astral-sh/ty/issues/98.
This commit is contained in:
parent
25e13debc0
commit
e9da1750a1
9 changed files with 103 additions and 35 deletions
1
Cargo.lock
generated
1
Cargo.lock
generated
|
@ -3984,6 +3984,7 @@ dependencies = [
|
||||||
"crossbeam",
|
"crossbeam",
|
||||||
"ctrlc",
|
"ctrlc",
|
||||||
"filetime",
|
"filetime",
|
||||||
|
"indicatif",
|
||||||
"insta",
|
"insta",
|
||||||
"insta-cmd",
|
"insta-cmd",
|
||||||
"jiff",
|
"jiff",
|
||||||
|
|
|
@ -16,7 +16,7 @@ use ruff_python_ast::PythonVersion;
|
||||||
use ty_project::metadata::options::{EnvironmentOptions, Options};
|
use ty_project::metadata::options::{EnvironmentOptions, Options};
|
||||||
use ty_project::metadata::value::RangedValue;
|
use ty_project::metadata::value::RangedValue;
|
||||||
use ty_project::watch::{ChangeEvent, ChangedKind};
|
use ty_project::watch::{ChangeEvent, ChangedKind};
|
||||||
use ty_project::{Db, ProjectDatabase, ProjectMetadata};
|
use ty_project::{Db, DummyReporter, ProjectDatabase, ProjectMetadata};
|
||||||
|
|
||||||
struct Case {
|
struct Case {
|
||||||
db: ProjectDatabase,
|
db: ProjectDatabase,
|
||||||
|
@ -153,7 +153,7 @@ fn benchmark_incremental(criterion: &mut Criterion) {
|
||||||
fn setup() -> Case {
|
fn setup() -> Case {
|
||||||
let case = setup_tomllib_case();
|
let case = setup_tomllib_case();
|
||||||
|
|
||||||
let result: Vec<_> = case.db.check().unwrap();
|
let result: Vec<_> = case.db.check(&DummyReporter).unwrap();
|
||||||
|
|
||||||
assert_diagnostics(&case.db, &result, EXPECTED_TOMLLIB_DIAGNOSTICS);
|
assert_diagnostics(&case.db, &result, EXPECTED_TOMLLIB_DIAGNOSTICS);
|
||||||
|
|
||||||
|
@ -181,7 +181,7 @@ fn benchmark_incremental(criterion: &mut Criterion) {
|
||||||
None,
|
None,
|
||||||
);
|
);
|
||||||
|
|
||||||
let result = db.check().unwrap();
|
let result = db.check(&DummyReporter).unwrap();
|
||||||
|
|
||||||
assert_eq!(result.len(), EXPECTED_TOMLLIB_DIAGNOSTICS.len());
|
assert_eq!(result.len(), EXPECTED_TOMLLIB_DIAGNOSTICS.len());
|
||||||
}
|
}
|
||||||
|
@ -201,7 +201,7 @@ fn benchmark_cold(criterion: &mut Criterion) {
|
||||||
setup_tomllib_case,
|
setup_tomllib_case,
|
||||||
|case| {
|
|case| {
|
||||||
let Case { db, .. } = case;
|
let Case { db, .. } = case;
|
||||||
let result: Vec<_> = db.check().unwrap();
|
let result: Vec<_> = db.check(&DummyReporter).unwrap();
|
||||||
|
|
||||||
assert_diagnostics(db, &result, EXPECTED_TOMLLIB_DIAGNOSTICS);
|
assert_diagnostics(db, &result, EXPECTED_TOMLLIB_DIAGNOSTICS);
|
||||||
},
|
},
|
||||||
|
@ -315,7 +315,7 @@ fn benchmark_many_string_assignments(criterion: &mut Criterion) {
|
||||||
},
|
},
|
||||||
|case| {
|
|case| {
|
||||||
let Case { db, .. } = case;
|
let Case { db, .. } = case;
|
||||||
let result = db.check().unwrap();
|
let result = db.check(&DummyReporter).unwrap();
|
||||||
assert_eq!(result.len(), 0);
|
assert_eq!(result.len(), 0);
|
||||||
},
|
},
|
||||||
BatchSize::SmallInput,
|
BatchSize::SmallInput,
|
||||||
|
|
|
@ -28,6 +28,7 @@ colored = { workspace = true }
|
||||||
countme = { workspace = true, features = ["enable"] }
|
countme = { workspace = true, features = ["enable"] }
|
||||||
crossbeam = { workspace = true }
|
crossbeam = { workspace = true }
|
||||||
ctrlc = { version = "3.4.4" }
|
ctrlc = { version = "3.4.4" }
|
||||||
|
indicatif = { workspace = true }
|
||||||
jiff = { workspace = true }
|
jiff = { workspace = true }
|
||||||
rayon = { workspace = true }
|
rayon = { workspace = true }
|
||||||
salsa = { workspace = true }
|
salsa = { workspace = true }
|
||||||
|
|
|
@ -25,7 +25,7 @@ use ruff_db::Upcast;
|
||||||
use salsa::plumbing::ZalsaDatabase;
|
use salsa::plumbing::ZalsaDatabase;
|
||||||
use ty_project::metadata::options::Options;
|
use ty_project::metadata::options::Options;
|
||||||
use ty_project::watch::ProjectWatcher;
|
use ty_project::watch::ProjectWatcher;
|
||||||
use ty_project::{watch, Db};
|
use ty_project::{watch, Db, DummyReporter, Reporter};
|
||||||
use ty_project::{ProjectDatabase, ProjectMetadata};
|
use ty_project::{ProjectDatabase, ProjectMetadata};
|
||||||
use ty_server::run_server;
|
use ty_server::run_server;
|
||||||
|
|
||||||
|
@ -200,22 +200,28 @@ impl MainLoop {
|
||||||
|
|
||||||
self.watcher = Some(ProjectWatcher::new(watcher, db));
|
self.watcher = Some(ProjectWatcher::new(watcher, db));
|
||||||
|
|
||||||
self.run(db)?;
|
// Do not show progress bars with `--watch`, indicatif does not seem to
|
||||||
|
// handle cancelling independent progress bars very well.
|
||||||
|
self.run_with_progress::<DummyReporter>(db)?;
|
||||||
|
|
||||||
Ok(ExitStatus::Success)
|
Ok(ExitStatus::Success)
|
||||||
}
|
}
|
||||||
|
|
||||||
fn run(mut self, db: &mut ProjectDatabase) -> Result<ExitStatus> {
|
fn run(self, db: &mut ProjectDatabase) -> Result<ExitStatus> {
|
||||||
|
self.run_with_progress::<IndicatifReporter>(db)
|
||||||
|
}
|
||||||
|
|
||||||
|
fn run_with_progress<R: Reporter>(mut self, db: &mut ProjectDatabase) -> Result<ExitStatus> {
|
||||||
self.sender.send(MainLoopMessage::CheckWorkspace).unwrap();
|
self.sender.send(MainLoopMessage::CheckWorkspace).unwrap();
|
||||||
|
|
||||||
let result = self.main_loop(db);
|
let result = self.main_loop::<R>(db);
|
||||||
|
|
||||||
tracing::debug!("Exiting main loop");
|
tracing::debug!("Exiting main loop");
|
||||||
|
|
||||||
result
|
result
|
||||||
}
|
}
|
||||||
|
|
||||||
fn main_loop(&mut self, db: &mut ProjectDatabase) -> Result<ExitStatus> {
|
fn main_loop<R: Reporter>(&mut self, db: &mut ProjectDatabase) -> Result<ExitStatus> {
|
||||||
// Schedule the first check.
|
// Schedule the first check.
|
||||||
tracing::debug!("Starting main loop");
|
tracing::debug!("Starting main loop");
|
||||||
|
|
||||||
|
@ -226,11 +232,12 @@ impl MainLoop {
|
||||||
MainLoopMessage::CheckWorkspace => {
|
MainLoopMessage::CheckWorkspace => {
|
||||||
let db = db.clone();
|
let db = db.clone();
|
||||||
let sender = self.sender.clone();
|
let sender = self.sender.clone();
|
||||||
|
let reporter = R::default();
|
||||||
|
|
||||||
// Spawn a new task that checks the project. This needs to be done in a separate thread
|
// Spawn a new task that checks the project. This needs to be done in a separate thread
|
||||||
// to prevent blocking the main loop here.
|
// to prevent blocking the main loop here.
|
||||||
rayon::spawn(move || {
|
rayon::spawn(move || {
|
||||||
match db.check() {
|
match db.check(&reporter) {
|
||||||
Ok(result) => {
|
Ok(result) => {
|
||||||
// Send the result back to the main loop for printing.
|
// Send the result back to the main loop for printing.
|
||||||
sender
|
sender
|
||||||
|
@ -340,6 +347,34 @@ impl MainLoop {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// A progress reporter for `ty check`.
|
||||||
|
struct IndicatifReporter(indicatif::ProgressBar);
|
||||||
|
|
||||||
|
impl Default for IndicatifReporter {
|
||||||
|
fn default() -> IndicatifReporter {
|
||||||
|
let progress = indicatif::ProgressBar::new(0);
|
||||||
|
progress.set_style(
|
||||||
|
indicatif::ProgressStyle::with_template(
|
||||||
|
"{msg:8.dim} {bar:60.green/dim} {pos}/{len} files",
|
||||||
|
)
|
||||||
|
.unwrap()
|
||||||
|
.progress_chars("--"),
|
||||||
|
);
|
||||||
|
progress.set_message("Checking");
|
||||||
|
IndicatifReporter(progress)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
impl ty_project::Reporter for IndicatifReporter {
|
||||||
|
fn set_files(&self, files: usize) {
|
||||||
|
self.0.set_length(files as u64);
|
||||||
|
}
|
||||||
|
|
||||||
|
fn report_file(&self, _file: &ruff_db::files::File) {
|
||||||
|
self.0.inc(1);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
#[derive(Debug)]
|
#[derive(Debug)]
|
||||||
struct MainLoopCancellationToken {
|
struct MainLoopCancellationToken {
|
||||||
sender: crossbeam_channel::Sender<MainLoopMessage>,
|
sender: crossbeam_channel::Sender<MainLoopMessage>,
|
||||||
|
|
|
@ -14,7 +14,7 @@ use ty_project::metadata::options::{EnvironmentOptions, Options};
|
||||||
use ty_project::metadata::pyproject::{PyProject, Tool};
|
use ty_project::metadata::pyproject::{PyProject, Tool};
|
||||||
use ty_project::metadata::value::{RangedValue, RelativePathBuf};
|
use ty_project::metadata::value::{RangedValue, RelativePathBuf};
|
||||||
use ty_project::watch::{directory_watcher, ChangeEvent, ProjectWatcher};
|
use ty_project::watch::{directory_watcher, ChangeEvent, ProjectWatcher};
|
||||||
use ty_project::{Db, ProjectDatabase, ProjectMetadata};
|
use ty_project::{Db, DummyReporter, ProjectDatabase, ProjectMetadata};
|
||||||
use ty_python_semantic::{resolve_module, ModuleName, PythonPlatform};
|
use ty_python_semantic::{resolve_module, ModuleName, PythonPlatform};
|
||||||
|
|
||||||
struct TestCase {
|
struct TestCase {
|
||||||
|
@ -1117,7 +1117,10 @@ print(sys.last_exc, os.getegid())
|
||||||
Ok(())
|
Ok(())
|
||||||
})?;
|
})?;
|
||||||
|
|
||||||
let diagnostics = case.db.check().context("Failed to check project.")?;
|
let diagnostics = case
|
||||||
|
.db
|
||||||
|
.check(&DummyReporter)
|
||||||
|
.context("Failed to check project.")?;
|
||||||
|
|
||||||
assert_eq!(diagnostics.len(), 2);
|
assert_eq!(diagnostics.len(), 2);
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
|
@ -1142,7 +1145,10 @@ print(sys.last_exc, os.getegid())
|
||||||
})
|
})
|
||||||
.expect("Search path settings to be valid");
|
.expect("Search path settings to be valid");
|
||||||
|
|
||||||
let diagnostics = case.db.check().context("Failed to check project.")?;
|
let diagnostics = case
|
||||||
|
.db
|
||||||
|
.check(&DummyReporter)
|
||||||
|
.context("Failed to check project.")?;
|
||||||
assert!(diagnostics.is_empty());
|
assert!(diagnostics.is_empty());
|
||||||
|
|
||||||
Ok(())
|
Ok(())
|
||||||
|
|
|
@ -2,7 +2,7 @@ use std::panic::RefUnwindSafe;
|
||||||
use std::sync::Arc;
|
use std::sync::Arc;
|
||||||
|
|
||||||
use crate::DEFAULT_LINT_REGISTRY;
|
use crate::DEFAULT_LINT_REGISTRY;
|
||||||
use crate::{Project, ProjectMetadata};
|
use crate::{Project, ProjectMetadata, Reporter};
|
||||||
use ruff_db::diagnostic::Diagnostic;
|
use ruff_db::diagnostic::Diagnostic;
|
||||||
use ruff_db::files::{File, Files};
|
use ruff_db::files::{File, Files};
|
||||||
use ruff_db::system::System;
|
use ruff_db::system::System;
|
||||||
|
@ -68,8 +68,8 @@ impl ProjectDatabase {
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Checks all open files in the project and its dependencies.
|
/// Checks all open files in the project and its dependencies.
|
||||||
pub fn check(&self) -> Result<Vec<Diagnostic>, Cancelled> {
|
pub fn check(&self, reporter: &impl Reporter) -> Result<Vec<Diagnostic>, Cancelled> {
|
||||||
self.with_db(|db| db.project().check(db))
|
self.with_db(|db| db.project().check(db, reporter))
|
||||||
}
|
}
|
||||||
|
|
||||||
#[tracing::instrument(level = "debug", skip(self))]
|
#[tracing::instrument(level = "debug", skip(self))]
|
||||||
|
|
|
@ -161,6 +161,10 @@ impl Indexed<'_> {
|
||||||
pub(super) fn diagnostics(&self) -> &[IOErrorDiagnostic] {
|
pub(super) fn diagnostics(&self) -> &[IOErrorDiagnostic] {
|
||||||
&self.inner.diagnostics
|
&self.inner.diagnostics
|
||||||
}
|
}
|
||||||
|
|
||||||
|
pub(super) fn len(&self) -> usize {
|
||||||
|
self.inner.files.len()
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
impl Deref for Indexed<'_> {
|
impl Deref for Indexed<'_> {
|
||||||
|
|
|
@ -18,7 +18,7 @@ use rustc_hash::FxHashSet;
|
||||||
use salsa::Durability;
|
use salsa::Durability;
|
||||||
use salsa::Setter;
|
use salsa::Setter;
|
||||||
use std::backtrace::BacktraceStatus;
|
use std::backtrace::BacktraceStatus;
|
||||||
use std::panic::{AssertUnwindSafe, UnwindSafe};
|
use std::panic::{AssertUnwindSafe, RefUnwindSafe, UnwindSafe};
|
||||||
use std::sync::Arc;
|
use std::sync::Arc;
|
||||||
use thiserror::Error;
|
use thiserror::Error;
|
||||||
use tracing::error;
|
use tracing::error;
|
||||||
|
@ -106,6 +106,24 @@ pub struct Project {
|
||||||
settings_diagnostics: Vec<OptionDiagnostic>,
|
settings_diagnostics: Vec<OptionDiagnostic>,
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// A progress reporter.
|
||||||
|
pub trait Reporter: Default + Send + Sync + RefUnwindSafe + 'static {
|
||||||
|
/// Initialize the reporter with the number of files.
|
||||||
|
fn set_files(&self, files: usize);
|
||||||
|
|
||||||
|
/// Report the completion of a given file.
|
||||||
|
fn report_file(&self, file: &File);
|
||||||
|
}
|
||||||
|
|
||||||
|
/// A no-op implementation of [`Reporter`].
|
||||||
|
#[derive(Default)]
|
||||||
|
pub struct DummyReporter;
|
||||||
|
|
||||||
|
impl Reporter for DummyReporter {
|
||||||
|
fn set_files(&self, _files: usize) {}
|
||||||
|
fn report_file(&self, _file: &File) {}
|
||||||
|
}
|
||||||
|
|
||||||
#[salsa::tracked]
|
#[salsa::tracked]
|
||||||
impl Project {
|
impl Project {
|
||||||
pub fn from_metadata(db: &dyn Db, metadata: ProjectMetadata) -> Self {
|
pub fn from_metadata(db: &dyn Db, metadata: ProjectMetadata) -> Self {
|
||||||
|
@ -168,7 +186,7 @@ impl Project {
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Checks all open files in the project and its dependencies.
|
/// Checks all open files in the project and its dependencies.
|
||||||
pub(crate) fn check(self, db: &ProjectDatabase) -> Vec<Diagnostic> {
|
pub(crate) fn check(self, db: &ProjectDatabase, reporter: &impl Reporter) -> Vec<Diagnostic> {
|
||||||
let project_span = tracing::debug_span!("Project::check");
|
let project_span = tracing::debug_span!("Project::check");
|
||||||
let _span = project_span.enter();
|
let _span = project_span.enter();
|
||||||
|
|
||||||
|
@ -182,6 +200,7 @@ impl Project {
|
||||||
);
|
);
|
||||||
|
|
||||||
let files = ProjectFiles::new(db, self);
|
let files = ProjectFiles::new(db, self);
|
||||||
|
reporter.set_files(files.len());
|
||||||
|
|
||||||
diagnostics.extend(
|
diagnostics.extend(
|
||||||
files
|
files
|
||||||
|
@ -190,36 +209,31 @@ impl Project {
|
||||||
.map(IOErrorDiagnostic::to_diagnostic),
|
.map(IOErrorDiagnostic::to_diagnostic),
|
||||||
);
|
);
|
||||||
|
|
||||||
let file_diagnostics = Arc::new(std::sync::Mutex::new(vec![]));
|
let file_diagnostics = std::sync::Mutex::new(vec![]);
|
||||||
|
|
||||||
{
|
{
|
||||||
let file_diagnostics = Arc::clone(&file_diagnostics);
|
|
||||||
let db = db.clone();
|
let db = db.clone();
|
||||||
let project_span = project_span.clone();
|
let file_diagnostics = &file_diagnostics;
|
||||||
|
let project_span = &project_span;
|
||||||
|
|
||||||
rayon::scope(move |scope| {
|
rayon::scope(move |scope| {
|
||||||
for file in &files {
|
for file in &files {
|
||||||
let result = Arc::clone(&file_diagnostics);
|
|
||||||
let db = db.clone();
|
let db = db.clone();
|
||||||
let project_span = project_span.clone();
|
|
||||||
|
|
||||||
scope.spawn(move |_| {
|
scope.spawn(move |_| {
|
||||||
let check_file_span =
|
let check_file_span =
|
||||||
tracing::debug_span!(parent: &project_span, "check_file", ?file);
|
tracing::debug_span!(parent: project_span, "check_file", ?file);
|
||||||
let _entered = check_file_span.entered();
|
let _entered = check_file_span.entered();
|
||||||
|
|
||||||
let file_diagnostics = check_file_impl(&db, file);
|
let result = check_file_impl(&db, file);
|
||||||
result.lock().unwrap().extend(file_diagnostics);
|
file_diagnostics.lock().unwrap().extend(result);
|
||||||
|
|
||||||
|
reporter.report_file(&file);
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
let mut file_diagnostics = Arc::into_inner(file_diagnostics)
|
let mut file_diagnostics = file_diagnostics.into_inner().unwrap();
|
||||||
.unwrap()
|
|
||||||
.into_inner()
|
|
||||||
.unwrap();
|
|
||||||
|
|
||||||
file_diagnostics.sort_by(|left, right| {
|
file_diagnostics.sort_by(|left, right| {
|
||||||
left.rendering_sort_key(db)
|
left.rendering_sort_key(db)
|
||||||
.cmp(&right.rendering_sort_key(db))
|
.cmp(&right.rendering_sort_key(db))
|
||||||
|
@ -493,6 +507,13 @@ impl<'a> ProjectFiles<'a> {
|
||||||
ProjectFiles::Indexed(indexed) => indexed.diagnostics(),
|
ProjectFiles::Indexed(indexed) => indexed.diagnostics(),
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn len(&self) -> usize {
|
||||||
|
match self {
|
||||||
|
ProjectFiles::OpenFiles(open_files) => open_files.len(),
|
||||||
|
ProjectFiles::Indexed(indexed) => indexed.len(),
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
impl<'a> IntoIterator for &'a ProjectFiles<'a> {
|
impl<'a> IntoIterator for &'a ProjectFiles<'a> {
|
||||||
|
|
|
@ -18,8 +18,8 @@ use ty_ide::{goto_type_definition, hover, inlay_hints, MarkupKind};
|
||||||
use ty_project::metadata::options::Options;
|
use ty_project::metadata::options::Options;
|
||||||
use ty_project::metadata::value::ValueSource;
|
use ty_project::metadata::value::ValueSource;
|
||||||
use ty_project::watch::{ChangeEvent, ChangedKind, CreatedKind, DeletedKind};
|
use ty_project::watch::{ChangeEvent, ChangedKind, CreatedKind, DeletedKind};
|
||||||
use ty_project::ProjectMetadata;
|
|
||||||
use ty_project::{Db, ProjectDatabase};
|
use ty_project::{Db, ProjectDatabase};
|
||||||
|
use ty_project::{DummyReporter, ProjectMetadata};
|
||||||
use ty_python_semantic::Program;
|
use ty_python_semantic::Program;
|
||||||
use wasm_bindgen::prelude::*;
|
use wasm_bindgen::prelude::*;
|
||||||
|
|
||||||
|
@ -186,7 +186,7 @@ impl Workspace {
|
||||||
|
|
||||||
/// Checks all open files
|
/// Checks all open files
|
||||||
pub fn check(&self) -> Result<Vec<Diagnostic>, Error> {
|
pub fn check(&self) -> Result<Vec<Diagnostic>, Error> {
|
||||||
let result = self.db.check().map_err(into_error)?;
|
let result = self.db.check(&DummyReporter).map_err(into_error)?;
|
||||||
|
|
||||||
Ok(result.into_iter().map(Diagnostic::wrap).collect())
|
Ok(result.into_iter().map(Diagnostic::wrap).collect())
|
||||||
}
|
}
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue