Skip to content

Commit aa6fa37

Browse files
sbilligsbillig
committed
Gate tests on dependency diagnostics
1 parent b30f625 commit aa6fa37

10 files changed

Lines changed: 212 additions & 76 deletions

File tree

‎crates/fe/src/check.rs‎

Lines changed: 5 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ use mir::build_runtime_package;
1313
use salsa::Setter;
1414
use url::Url;
1515

16+
use crate::dependency_diagnostics::DependencyIssues;
1617
use crate::report::{
1718
copy_input_into_report, create_dir_all_utf8, create_report_staging_dir, enable_panic_report,
1819
normalize_report_out_path, tar_gz_dir, write_report_meta,
@@ -403,64 +404,15 @@ fn check_ingot_and_dependencies(
403404
}
404405
}
405406

406-
let mut dependency_errors = Vec::new();
407-
for dependency_url in db.dependency_graph().dependency_urls(db, ingot_url) {
408-
if !seen.insert(dependency_url.clone()) {
409-
continue;
410-
}
411-
let Some(ingot) = db.workspace().containing_ingot(db, dependency_url.clone()) else {
412-
continue;
413-
};
414-
if !ingot_has_source_files(db, ingot) {
415-
eprintln!("Error: Could not find source files for ingot {dependency_url}");
416-
has_errors = true;
417-
continue;
418-
}
419-
let hir_diags = db.run_on_ingot(ingot);
420-
let mir_diags = if hir_diags.has_errors(db) {
421-
Vec::new()
422-
} else {
423-
db.mir_diagnostics_for_ingot(ingot)
424-
};
425-
if !hir_diags.is_empty() || !mir_diags.is_empty() {
426-
dependency_errors.push((dependency_url, hir_diags, mir_diags));
427-
}
428-
}
407+
let dependency_errors = DependencyIssues::collect(db, ingot_url, seen);
429408

430409
if !dependency_errors.is_empty() {
431410
has_errors = true;
432-
if dependency_errors.len() == 1 {
433-
eprintln!("Error: Downstream ingot has errors");
434-
} else {
435-
eprintln!("Error: Downstream ingots have errors");
436-
}
411+
let formatted = dependency_errors.format(db);
412+
eprint!("{formatted}");
437413

438414
if let Some(report) = report {
439-
let mut out = String::new();
440-
for (dependency_url, hir_diags, mir_diags) in &dependency_errors {
441-
out.push_str(&format!("dependency: {dependency_url}\n"));
442-
if !hir_diags.is_empty() {
443-
out.push_str(&hir_diags.format_diags(db));
444-
}
445-
if !mir_diags.is_empty() {
446-
out.push_str(&format!(
447-
"MIR diagnostics: {} emitted to stderr\n",
448-
mir_diags.len()
449-
));
450-
}
451-
out.push('\n');
452-
}
453-
write_report_file(report, "errors/dependency_diagnostics.txt", &out);
454-
}
455-
456-
for (dependency_url, hir_diags, mir_diags) in dependency_errors {
457-
print_dependency_info(db, &dependency_url);
458-
if !hir_diags.is_empty() {
459-
hir_diags.emit(db);
460-
}
461-
if !mir_diags.is_empty() {
462-
db.emit_complete_diagnostics(&mir_diags);
463-
}
415+
write_report_file(report, "errors/dependency_diagnostics.txt", &formatted);
464416
}
465417
}
466418

@@ -558,29 +510,6 @@ fn check_single_file(
558510
false
559511
}
560512

561-
fn print_dependency_info(db: &DriverDataBase, dependency_url: &Url) {
562-
eprintln!();
563-
564-
// Get the ingot for this dependency URL to access its config
565-
if let Some(ingot) = db.workspace().containing_ingot(db, dependency_url.clone()) {
566-
if let Some(config) = ingot.config(db) {
567-
let name = config.metadata.name.as_deref().unwrap_or("unknown");
568-
if let Some(version) = &config.metadata.version {
569-
eprintln!("Dependency: {name} (version: {version})");
570-
} else {
571-
eprintln!("Dependency: {name}");
572-
}
573-
} else {
574-
eprintln!("Dependency: <unknown>");
575-
}
576-
} else {
577-
eprintln!("Dependency: <unknown>");
578-
}
579-
580-
eprintln!("URL: {dependency_url}");
581-
eprintln!();
582-
}
583-
584513
fn dump_module_mir(db: &DriverDataBase, top_mod: TopLevelMod<'_>) {
585514
match build_runtime_package(db, top_mod) {
586515
Ok(package) => {

‎crates/fe/src/dependency_diagnostics.rs‎

Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,125 @@
1+
use std::{collections::HashSet, fmt::Write as _};
2+
3+
use common::{InputDb, diagnostics::CompleteDiagnostic, file::IngotFileKind};
4+
use driver::{DriverDataBase, db::DiagnosticsCollection};
5+
use url::Url;
6+
7+
pub(crate) struct DependencyIssues<'db> {
8+
issues: Vec<DependencyIssue<'db>>,
9+
}
10+
11+
enum DependencyIssue<'db> {
12+
MissingSourceFiles(Url),
13+
Diagnostics {
14+
url: Url,
15+
hir: DiagnosticsCollection<'db>,
16+
mir: Vec<CompleteDiagnostic>,
17+
},
18+
}
19+
20+
impl DependencyIssue<'_> {
21+
fn format(&self, db: &DriverDataBase, out: &mut String) {
22+
let url = match self {
23+
Self::MissingSourceFiles(url) | Self::Diagnostics { url, .. } => url,
24+
};
25+
append_dependency_header(db, url, out);
26+
match self {
27+
DependencyIssue::MissingSourceFiles(url) => {
28+
let _ = writeln!(out, "Error: Could not find source files for ingot {url}");
29+
}
30+
DependencyIssue::Diagnostics { hir, mir, .. } => {
31+
if !hir.is_empty() {
32+
out.push_str(&hir.format_diags(db));
33+
}
34+
if !mir.is_empty() {
35+
out.push_str(&db.format_complete_diagnostics(mir));
36+
}
37+
}
38+
}
39+
if !out.ends_with('\n') {
40+
out.push('\n');
41+
}
42+
}
43+
}
44+
45+
impl<'db> DependencyIssues<'db> {
46+
pub(crate) fn collect(
47+
db: &'db DriverDataBase,
48+
ingot_url: &Url,
49+
seen: &mut HashSet<Url>,
50+
) -> Self {
51+
let mut issues = Vec::new();
52+
for dependency_url in db.dependency_graph().dependency_urls(db, ingot_url) {
53+
if !seen.insert(dependency_url.clone()) {
54+
continue;
55+
}
56+
let Some(ingot) = db.workspace().containing_ingot(db, dependency_url.clone()) else {
57+
continue;
58+
};
59+
if !ingot_has_source_files(db, ingot) {
60+
issues.push(DependencyIssue::MissingSourceFiles(dependency_url));
61+
continue;
62+
}
63+
let hir = db.run_on_ingot(ingot);
64+
let mir = if hir.has_errors(db) {
65+
Vec::new()
66+
} else {
67+
db.mir_diagnostics_for_ingot(ingot)
68+
};
69+
if !hir.is_empty() || !mir.is_empty() {
70+
issues.push(DependencyIssue::Diagnostics {
71+
url: dependency_url,
72+
hir,
73+
mir,
74+
});
75+
}
76+
}
77+
Self { issues }
78+
}
79+
80+
pub(crate) fn is_empty(&self) -> bool {
81+
self.issues.is_empty()
82+
}
83+
84+
pub(crate) fn message(&self) -> &'static str {
85+
if self.issues.len() == 1 {
86+
"Errors in dependency"
87+
} else {
88+
"Errors in dependencies"
89+
}
90+
}
91+
92+
pub(crate) fn format(&self, db: &DriverDataBase) -> String {
93+
let mut out = String::new();
94+
let _ = writeln!(out, "Error: {}", self.message());
95+
for issue in &self.issues {
96+
issue.format(db, &mut out);
97+
out.push('\n');
98+
}
99+
out
100+
}
101+
}
102+
103+
fn ingot_has_source_files(db: &DriverDataBase, ingot: hir::Ingot<'_>) -> bool {
104+
ingot
105+
.files(db)
106+
.iter()
107+
.any(|(_, file)| matches!(file.kind(db), Some(IngotFileKind::Source)))
108+
}
109+
110+
fn append_dependency_header(db: &DriverDataBase, dependency_url: &Url, out: &mut String) {
111+
let dependency = if let Some(ingot) =
112+
db.workspace().containing_ingot(db, dependency_url.clone())
113+
&& let Some(config) = ingot.config(db)
114+
{
115+
let name = config.metadata.name.as_deref().unwrap_or("unknown");
116+
if let Some(version) = &config.metadata.version {
117+
format!("Dependency: {name} (version: {version})")
118+
} else {
119+
format!("Dependency: {name}")
120+
}
121+
} else {
122+
"Dependency: <unknown>".to_string()
123+
};
124+
let _ = writeln!(out, "\n{dependency}\nURL: {dependency_url}\n");
125+
}

‎crates/fe/src/main.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ mod abi;
33
mod build;
44
mod check;
55
mod cli;
6+
mod dependency_diagnostics;
67
mod doc;
78
#[cfg(feature = "doc-server")]
89
mod doc_serve;

‎crates/fe/src/test/mod.rs‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
//! executes them using revm.
55
66
use crate::TestEmit;
7+
use crate::dependency_diagnostics::DependencyIssues;
78
use crate::report::{
89
PanicReportGuard, ReportStaging, copy_input_into_report, create_dir_all_utf8,
910
create_report_staging_root, enable_panic_report, is_verifier_error_text,
@@ -34,6 +35,7 @@ use rustc_hash::{FxHashMap, FxHashSet};
3435
use salsa::Setter;
3536
use solc_runner::compile_single_contract_with_solc;
3637
use std::{
38+
collections::HashSet,
3739
fmt::Write as _,
3840
sync::Arc,
3941
time::{Duration, Instant},
@@ -2250,6 +2252,21 @@ fn prepare_tests_ingot(
22502252
};
22512253
}
22522254

2255+
let mut seen = HashSet::from([ingot_url.clone()]);
2256+
let dependency_errors = DependencyIssues::collect(db, &ingot_url, &mut seen);
2257+
if !dependency_errors.is_empty() {
2258+
let formatted = dependency_errors.format(db);
2259+
let _ = write!(output, "{formatted}");
2260+
if let Some(report) = report {
2261+
write_report_error(report, "compilation_errors.txt", &formatted);
2262+
}
2263+
return SuitePreparation {
2264+
results: suite_error_result(suite, "compile", dependency_errors.message().to_string()),
2265+
single_jobs: Vec::new(),
2266+
gas_comparison_cases: None,
2267+
};
2268+
}
2269+
22532270
let root_mod = ingot.root_mod(db);
22542271
if !ingot_has_test_functions(db, ingot) {
22552272
return SuitePreparation {

‎crates/fe/tests/cli_output.rs‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1612,6 +1612,30 @@ fn test_cli_test_workspace_ingot_missing_member_is_error() {
16121612
);
16131613
}
16141614

1615+
#[test]
1616+
fn test_cli_test_dependency_diagnostics_block_codegen() {
1617+
let fixture_dir = fe_test_runner_fixture_dir("dependency_diagnostic_gating");
1618+
let fixture_dir = fixture_dir.to_str().expect("fixture path should be utf-8");
1619+
let (output, exit_code) = run_fe_main(&["test", "--jobs", "1", "--ingot", "app", fixture_dir]);
1620+
assert_ne!(
1621+
exit_code, 0,
1622+
"expected dependency diagnostic failure:\n{output}"
1623+
);
1624+
assert!(
1625+
output.contains("Error: Errors in dependency"),
1626+
"expected dependency error:\n{output}"
1627+
);
1628+
assert!(
1629+
output.contains("associated const not defined in trait")
1630+
&& output.contains("missing associated const `HEAD_SIZE`"),
1631+
"expected ABI trait diagnostics:\n{output}"
1632+
);
1633+
assert!(
1634+
!output.contains("backend panicked") && !output.contains("panicked at"),
1635+
"dependency diagnostics should block codegen before panic:\n{output}"
1636+
);
1637+
}
1638+
16151639
/// Regression test: `create2` of a contract defined in another ingot within
16161640
/// the same workspace must compile and run correctly.
16171641
#[test]

‎crates/fe/tests/fixtures/fe_test_runner/dependency_diagnostic_gating/fe.toml‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
[workspace]
2+
name = "dependency_diagnostic_gating"
3+
version = "0.1.0"
4+
members = [
5+
{ path = "ingots/app", name = "app" },
6+
{ path = "ingots/dep", name = "dep" },
7+
]

‎crates/fe/tests/fixtures/fe_test_runner/dependency_diagnostic_gating/ingots/app/fe.toml‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
[ingot]
2+
name = "app"
3+
version = "0.1.0"
4+
5+
[dependencies]
6+
dep = true

‎crates/fe/tests/fixtures/fe_test_runner/dependency_diagnostic_gating/ingots/app/src/lib.fe‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
use std::evm::assert
2+
3+
#[test]
4+
fn dependency_errors_block_codegen() {
5+
assert(true)
6+
}

‎crates/fe/tests/fixtures/fe_test_runner/dependency_diagnostic_gating/ingots/dep/fe.toml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
[ingot]
2+
name = "dep"
3+
version = "0.1.0"

‎crates/fe/tests/fixtures/fe_test_runner/dependency_diagnostic_gating/ingots/dep/src/lib.fe‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
use core::abi::{Abi, AbiEncoder, AbiSize, Encode}
2+
3+
pub enum Error {
4+
Oops,
5+
}
6+
7+
impl AbiSize for Error {
8+
const ENCODED_SIZE: u256 = 32
9+
const IS_DYNAMIC: bool = false
10+
}
11+
12+
impl<A: Abi> Encode<A> for Error {
13+
const DIRECT_ENCODE: bool = true
14+
15+
fn encode<E: AbiEncoder<A>>(own self, _ e: mut E) {}
16+
17+
fn encode_to_ptr(own self, _ ptr: u256) {}
18+
}

0 commit comments

Comments
 (0)