red_knot_project: sort diagnostics from checking files

Previously, we could iterate over files in an unspecified order (via
`HashSet` iteration) and we could accumulate diagnostics from files in
an unspecified order (via parallelism).

Here, we change the status quo so that diagnostics collected from files
are sorted after checking is complete. For now, we sort by severity
(with higher severity diagnostics appearing first) and then by
diagnostic ID to give a stable ordering.

I'm not sure if this is the best ordering.
This commit is contained in:
Andrew Gallant 2025-04-25 10:11:25 -04:00 committed by Andrew Gallant
parent fa88989ef0
commit 049280a3bc
2 changed files with 68 additions and 32 deletions

View file

@ -935,13 +935,6 @@ fn check_specific_paths() -> anyhow::Result<()> {
success: false
exit_code: 1
----- stdout -----
error: lint:unresolved-import: Cannot resolve import `does_not_exist`
--> <temp_dir>/project/tests/test_main.py:2:8
|
2 | import does_not_exist # error: unresolved-import
| ^^^^^^^^^^^^^^
|
error: lint:division-by-zero: Cannot divide object of type `Literal[4]` by zero
--> <temp_dir>/project/main.py:2:5
|
@ -958,6 +951,13 @@ fn check_specific_paths() -> anyhow::Result<()> {
4 | print(z)
|
error: lint:unresolved-import: Cannot resolve import `does_not_exist`
--> <temp_dir>/project/tests/test_main.py:2:8
|
2 | import does_not_exist # error: unresolved-import
| ^^^^^^^^^^^^^^
|
Found 3 diagnostics
----- stderr -----
@ -972,13 +972,6 @@ fn check_specific_paths() -> anyhow::Result<()> {
success: false
exit_code: 1
----- stdout -----
error: lint:unresolved-import: Cannot resolve import `does_not_exist`
--> <temp_dir>/project/tests/test_main.py:2:8
|
2 | import does_not_exist # error: unresolved-import
| ^^^^^^^^^^^^^^
|
error: lint:unresolved-import: Cannot resolve import `main2`
--> <temp_dir>/project/other.py:2:6
|
@ -988,6 +981,13 @@ fn check_specific_paths() -> anyhow::Result<()> {
4 | print(z)
|
error: lint:unresolved-import: Cannot resolve import `does_not_exist`
--> <temp_dir>/project/tests/test_main.py:2:8
|
2 | import does_not_exist # error: unresolved-import
| ^^^^^^^^^^^^^^
|
Found 2 diagnostics
----- stderr -----

View file

@ -187,30 +187,66 @@ impl Project {
.map(IOErrorDiagnostic::to_diagnostic),
);
let result = Arc::new(std::sync::Mutex::new(diagnostics));
let inner_result = Arc::clone(&result);
let file_diagnostics = Arc::new(std::sync::Mutex::new(vec![]));
let db = db.clone();
let project_span = project_span.clone();
{
let file_diagnostics = Arc::clone(&file_diagnostics);
let db = db.clone();
let project_span = project_span.clone();
rayon::scope(move |scope| {
for file in &files {
let result = inner_result.clone();
let db = db.clone();
let project_span = project_span.clone();
rayon::scope(move |scope| {
for file in &files {
let result = Arc::clone(&file_diagnostics);
let db = db.clone();
let project_span = project_span.clone();
scope.spawn(move |_| {
let check_file_span =
tracing::debug_span!(parent: &project_span, "check_file", ?file);
let _entered = check_file_span.entered();
scope.spawn(move |_| {
let check_file_span =
tracing::debug_span!(parent: &project_span, "check_file", ?file);
let _entered = check_file_span.entered();
let file_diagnostics = check_file_impl(&db, file);
result.lock().unwrap().extend(file_diagnostics);
});
let file_diagnostics = check_file_impl(&db, file);
result.lock().unwrap().extend(file_diagnostics);
});
}
});
}
let mut file_diagnostics = Arc::into_inner(file_diagnostics)
.unwrap()
.into_inner()
.unwrap();
// We sort diagnostics in a way that keeps them in source order
// and grouped by file. After that, we fall back to severity
// (with fatal messages sorting before info messages) and then
// finally the diagnostic ID.
file_diagnostics.sort_by(|d1, d2| {
if let (Some(span1), Some(span2)) = (d1.primary_span(), d2.primary_span()) {
let order = span1
.file()
.path(db)
.as_str()
.cmp(span2.file().path(db).as_str());
if order.is_ne() {
return order;
}
if let (Some(range1), Some(range2)) = (span1.range(), span2.range()) {
let order = range1.start().cmp(&range2.start());
if order.is_ne() {
return order;
}
}
}
// Reverse so that, e.g., Fatal sorts before Info.
let order = d1.severity().cmp(&d2.severity()).reverse();
if order.is_ne() {
return order;
}
d1.id().cmp(&d2.id())
});
Arc::into_inner(result).unwrap().into_inner().unwrap()
diagnostics.extend(file_diagnostics);
diagnostics
}
pub(crate) fn check_file(self, db: &dyn Db, file: File) -> Vec<Diagnostic> {