Skip to content

feat: Don't stop at first error when emitting lints and warnings #15889

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 1, 2025
+15 −3
Merged

feat: Don't stop at first error when emitting lints and warnings #15889

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 15 additions & 3 deletions src/cargo/core/workspace.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1205,10 +1205,15 @@ impl<'gctx> Workspace<'gctx> {
}

pub fn emit_warnings(&self) -> CargoResult<()> {
let mut first_emitted_error = None;
for (path, maybe_pkg) in &self.packages.packages {
if let MaybePackage::Package(pkg) = maybe_pkg {
if self.gctx.cli_unstable().cargo_lints {
self.emit_lints(pkg, &path)?
if let Err(e) = self.emit_lints(pkg, &path)
&& first_emitted_error.is_none()
{
first_emitted_error = Some(e);
}
}
}
let warnings = match maybe_pkg {
Expand All @@ -1220,7 +1225,9 @@ impl<'gctx> Workspace<'gctx> {
let err = anyhow::format_err!("{}", warning.message);
let cx =
anyhow::format_err!("failed to parse manifest at `{}`", path.display());
return Err(err.context(cx));
if first_emitted_error.is_none() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually the unstable build.warnings has the same behavior, so fine with that and also this is still unstable.

However, what if there is error critical enough and cargo should stop analyzing? Or at least stop the current package and continue with the next one?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the reason we we show all warnings but only first error encountered? Should we show all errors as well?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see this as an incremental improvement. Showing all errors has a bit more policy to figure out (how to render, what error to report back up)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That makes sense!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

However it still changed the behavior. Does it make sense to print all warning after critical errors? Will those become false positive or irrelevant?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But anyway we don't really distinguish those too much, so going to merge this.

first_emitted_error = Some(err.context(cx));
}
} else {
let msg = if self.root_manifest.is_none() {
warning.message.to_string()
Expand All @@ -1233,7 +1240,12 @@ impl<'gctx> Workspace<'gctx> {
}
}
}
Ok(())

if let Some(error) = first_emitted_error {
Err(error)
} else {
Ok(())
}
}

pub fn emit_lints(&self, pkg: &Package, path: &Path) -> CargoResult<()> {
Expand Down
Loading