Skip to content

Commit 550b8be

Browse files
authored
Avoid initializing progress bars early (astral-sh#18049)
## Summary Resolves astral-sh/ty#324.
1 parent bdccb37 commit 550b8be

File tree

6 files changed

+53
-39
lines changed

6 files changed

+53
-39
lines changed

crates/ruff_benchmark/benches/ty.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ use ruff_python_ast::PythonVersion;
1616
use ty_project::metadata::options::{EnvironmentOptions, Options};
1717
use ty_project::metadata::value::RangedValue;
1818
use ty_project::watch::{ChangeEvent, ChangedKind};
19-
use ty_project::{Db, DummyReporter, ProjectDatabase, ProjectMetadata};
19+
use ty_project::{Db, ProjectDatabase, ProjectMetadata};
2020

2121
struct Case {
2222
db: ProjectDatabase,
@@ -164,7 +164,7 @@ fn benchmark_incremental(criterion: &mut Criterion) {
164164
fn setup() -> Case {
165165
let case = setup_tomllib_case();
166166

167-
let result: Vec<_> = case.db.check(&DummyReporter).unwrap();
167+
let result: Vec<_> = case.db.check().unwrap();
168168

169169
assert_diagnostics(&case.db, &result, EXPECTED_TOMLLIB_DIAGNOSTICS);
170170

@@ -192,7 +192,7 @@ fn benchmark_incremental(criterion: &mut Criterion) {
192192
None,
193193
);
194194

195-
let result = db.check(&DummyReporter).unwrap();
195+
let result = db.check().unwrap();
196196

197197
assert_eq!(result.len(), EXPECTED_TOMLLIB_DIAGNOSTICS.len());
198198
}
@@ -212,7 +212,7 @@ fn benchmark_cold(criterion: &mut Criterion) {
212212
setup_tomllib_case,
213213
|case| {
214214
let Case { db, .. } = case;
215-
let result: Vec<_> = db.check(&DummyReporter).unwrap();
215+
let result: Vec<_> = db.check().unwrap();
216216

217217
assert_diagnostics(db, &result, EXPECTED_TOMLLIB_DIAGNOSTICS);
218218
},
@@ -326,7 +326,7 @@ fn benchmark_many_string_assignments(criterion: &mut Criterion) {
326326
},
327327
|case| {
328328
let Case { db, .. } = case;
329-
let result = db.check(&DummyReporter).unwrap();
329+
let result = db.check().unwrap();
330330
assert_eq!(result.len(), 0);
331331
},
332332
BatchSize::SmallInput,

crates/ty/src/lib.rs

Lines changed: 19 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -216,7 +216,10 @@ impl MainLoop {
216216
self.run_with_progress::<IndicatifReporter>(db)
217217
}
218218

219-
fn run_with_progress<R: Reporter>(mut self, db: &mut ProjectDatabase) -> Result<ExitStatus> {
219+
fn run_with_progress<R>(mut self, db: &mut ProjectDatabase) -> Result<ExitStatus>
220+
where
221+
R: Reporter + Default + 'static,
222+
{
220223
self.sender.send(MainLoopMessage::CheckWorkspace).unwrap();
221224

222225
let result = self.main_loop::<R>(db);
@@ -226,7 +229,10 @@ impl MainLoop {
226229
result
227230
}
228231

229-
fn main_loop<R: Reporter>(&mut self, db: &mut ProjectDatabase) -> Result<ExitStatus> {
232+
fn main_loop<R>(&mut self, db: &mut ProjectDatabase) -> Result<ExitStatus>
233+
where
234+
R: Reporter + Default + 'static,
235+
{
230236
// Schedule the first check.
231237
tracing::debug!("Starting main loop");
232238

@@ -237,12 +243,12 @@ impl MainLoop {
237243
MainLoopMessage::CheckWorkspace => {
238244
let db = db.clone();
239245
let sender = self.sender.clone();
240-
let reporter = R::default();
246+
let mut reporter = R::default();
241247

242248
// Spawn a new task that checks the project. This needs to be done in a separate thread
243249
// to prevent blocking the main loop here.
244250
rayon::spawn(move || {
245-
match db.check(&reporter) {
251+
match db.check_with_reporter(&mut reporter) {
246252
Ok(result) => {
247253
// Send the result back to the main loop for printing.
248254
sender
@@ -353,11 +359,12 @@ impl MainLoop {
353359
}
354360

355361
/// A progress reporter for `ty check`.
356-
struct IndicatifReporter(indicatif::ProgressBar);
362+
#[derive(Default)]
363+
struct IndicatifReporter(Option<indicatif::ProgressBar>);
357364

358-
impl Default for IndicatifReporter {
359-
fn default() -> IndicatifReporter {
360-
let progress = indicatif::ProgressBar::new(0);
365+
impl ty_project::Reporter for IndicatifReporter {
366+
fn set_files(&mut self, files: usize) {
367+
let progress = indicatif::ProgressBar::new(files as u64);
361368
progress.set_style(
362369
indicatif::ProgressStyle::with_template(
363370
"{msg:8.dim} {bar:60.green/dim} {pos}/{len} files",
@@ -366,17 +373,14 @@ impl Default for IndicatifReporter {
366373
.progress_chars("--"),
367374
);
368375
progress.set_message("Checking");
369-
IndicatifReporter(progress)
370-
}
371-
}
372376

373-
impl ty_project::Reporter for IndicatifReporter {
374-
fn set_files(&self, files: usize) {
375-
self.0.set_length(files as u64);
377+
self.0 = Some(progress);
376378
}
377379

378380
fn report_file(&self, _file: &ruff_db::files::File) {
379-
self.0.inc(1);
381+
if let Some(ref progress_bar) = self.0 {
382+
progress_bar.inc(1);
383+
}
380384
}
381385
}
382386

crates/ty/tests/file_watching.rs

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ use ty_project::metadata::options::{EnvironmentOptions, Options};
1414
use ty_project::metadata::pyproject::{PyProject, Tool};
1515
use ty_project::metadata::value::{RangedValue, RelativePathBuf};
1616
use ty_project::watch::{directory_watcher, ChangeEvent, ProjectWatcher};
17-
use ty_project::{Db, DummyReporter, ProjectDatabase, ProjectMetadata};
17+
use ty_project::{Db, ProjectDatabase, ProjectMetadata};
1818
use ty_python_semantic::{resolve_module, ModuleName, PythonPlatform};
1919

2020
struct TestCase {
@@ -1117,10 +1117,7 @@ print(sys.last_exc, os.getegid())
11171117
Ok(())
11181118
})?;
11191119

1120-
let diagnostics = case
1121-
.db
1122-
.check(&DummyReporter)
1123-
.context("Failed to check project.")?;
1120+
let diagnostics = case.db.check().context("Failed to check project.")?;
11241121

11251122
assert_eq!(diagnostics.len(), 2);
11261123
assert_eq!(
@@ -1145,10 +1142,7 @@ print(sys.last_exc, os.getegid())
11451142
})
11461143
.expect("Search path settings to be valid");
11471144

1148-
let diagnostics = case
1149-
.db
1150-
.check(&DummyReporter)
1151-
.context("Failed to check project.")?;
1145+
let diagnostics = case.db.check().context("Failed to check project.")?;
11521146
assert!(diagnostics.is_empty());
11531147

11541148
Ok(())

crates/ty_project/src/db.rs

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
1-
use std::panic::RefUnwindSafe;
1+
use std::panic::{AssertUnwindSafe, RefUnwindSafe};
22
use std::sync::Arc;
33

4-
use crate::DEFAULT_LINT_REGISTRY;
4+
use crate::{DummyReporter, DEFAULT_LINT_REGISTRY};
55
use crate::{Project, ProjectMetadata, Reporter};
66
use ruff_db::diagnostic::Diagnostic;
77
use ruff_db::files::{File, Files};
@@ -68,7 +68,18 @@ impl ProjectDatabase {
6868
}
6969

7070
/// Checks all open files in the project and its dependencies.
71-
pub fn check(&self, reporter: &impl Reporter) -> Result<Vec<Diagnostic>, Cancelled> {
71+
pub fn check(&self) -> Result<Vec<Diagnostic>, Cancelled> {
72+
let mut reporter = DummyReporter;
73+
let reporter = AssertUnwindSafe(&mut reporter as &mut dyn Reporter);
74+
self.with_db(|db| db.project().check(db, reporter))
75+
}
76+
77+
/// Checks all open files in the project and its dependencies, using the given reporter.
78+
pub fn check_with_reporter(
79+
&self,
80+
reporter: &mut dyn Reporter,
81+
) -> Result<Vec<Diagnostic>, Cancelled> {
82+
let reporter = AssertUnwindSafe(reporter);
7283
self.with_db(|db| db.project().check(db, reporter))
7384
}
7485

crates/ty_project/src/lib.rs

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ use rustc_hash::FxHashSet;
1818
use salsa::Durability;
1919
use salsa::Setter;
2020
use std::backtrace::BacktraceStatus;
21-
use std::panic::{AssertUnwindSafe, RefUnwindSafe, UnwindSafe};
21+
use std::panic::{AssertUnwindSafe, UnwindSafe};
2222
use std::sync::Arc;
2323
use thiserror::Error;
2424
use tracing::error;
@@ -107,9 +107,9 @@ pub struct Project {
107107
}
108108

109109
/// A progress reporter.
110-
pub trait Reporter: Default + Send + Sync + RefUnwindSafe + 'static {
110+
pub trait Reporter: Send + Sync {
111111
/// Initialize the reporter with the number of files.
112-
fn set_files(&self, files: usize);
112+
fn set_files(&mut self, files: usize);
113113

114114
/// Report the completion of a given file.
115115
fn report_file(&self, file: &File);
@@ -120,7 +120,7 @@ pub trait Reporter: Default + Send + Sync + RefUnwindSafe + 'static {
120120
pub struct DummyReporter;
121121

122122
impl Reporter for DummyReporter {
123-
fn set_files(&self, _files: usize) {}
123+
fn set_files(&mut self, _files: usize) {}
124124
fn report_file(&self, _file: &File) {}
125125
}
126126

@@ -186,7 +186,11 @@ impl Project {
186186
}
187187

188188
/// Checks all open files in the project and its dependencies.
189-
pub(crate) fn check(self, db: &ProjectDatabase, reporter: &impl Reporter) -> Vec<Diagnostic> {
189+
pub(crate) fn check(
190+
self,
191+
db: &ProjectDatabase,
192+
mut reporter: AssertUnwindSafe<&mut dyn Reporter>,
193+
) -> Vec<Diagnostic> {
190194
let project_span = tracing::debug_span!("Project::check");
191195
let _span = project_span.enter();
192196

@@ -215,6 +219,7 @@ impl Project {
215219
let db = db.clone();
216220
let file_diagnostics = &file_diagnostics;
217221
let project_span = &project_span;
222+
let reporter = &reporter;
218223

219224
rayon::scope(move |scope| {
220225
for file in &files {

crates/ty_wasm/src/lib.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,8 @@ use ty_ide::{goto_type_definition, hover, inlay_hints, MarkupKind};
1818
use ty_project::metadata::options::Options;
1919
use ty_project::metadata::value::ValueSource;
2020
use ty_project::watch::{ChangeEvent, ChangedKind, CreatedKind, DeletedKind};
21+
use ty_project::ProjectMetadata;
2122
use ty_project::{Db, ProjectDatabase};
22-
use ty_project::{DummyReporter, ProjectMetadata};
2323
use ty_python_semantic::Program;
2424
use wasm_bindgen::prelude::*;
2525

@@ -186,7 +186,7 @@ impl Workspace {
186186

187187
/// Checks all open files
188188
pub fn check(&self) -> Result<Vec<Diagnostic>, Error> {
189-
let result = self.db.check(&DummyReporter).map_err(into_error)?;
189+
let result = self.db.check().map_err(into_error)?;
190190

191191
Ok(result.into_iter().map(Diagnostic::wrap).collect())
192192
}

0 commit comments

Comments
 (0)