Skip to content

Conversation

Muscraft
Copy link
Member

While emitting warnings, if cargo encounters a critical warning or an error while linting, it returns early, potentially before all packages could emit their warnings or be linted. This could make it so users have to run a cargo command more than once before seeing all of the relevant output. Beyond this, having an error caused by a lint be a "stop the world" event seems wrong, as it (currently) doesn't inhibit outputting other warnings or linting other packages.

To address this, I made it so that cargo waits until the end of emit_warnings to return an error if one was encountered. To keep the cause of the error the same as before, cargo saves off the first error it encounters, and returns it at the end.

@rustbot
Copy link
Collaborator

rustbot commented Aug 27, 2025

r? @weihanglo

rustbot has assigned @weihanglo.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added A-workspaces Area: workspaces S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 27, 2025
@weihanglo
Copy link
Member

Would you mind comparing with clippy and rustc whether they have the same behavior? It is not necessary Cargo needs to follow but good to know where we are

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?

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.

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.

@weihanglo
Copy link
Member

Ah. Forgot to merge this. Do it now.

@weihanglo weihanglo added this pull request to the merge queue Sep 1, 2025
Merged via the queue into rust-lang:master with commit d2a66d6 Sep 1, 2025
25 checks passed
@weihanglo weihanglo deleted the lints-improvements branch September 1, 2025 00:33
@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 1, 2025
bors added a commit to rust-lang/rust that referenced this pull request Sep 3, 2025
Update cargo submodule

9 commits in a6c58d43051d01d83f55a3e61ef5f5b2b0dd6bd9..78a5531ed6608192b5f97eda64ba6e1f6a633eff
2025-08-26 23:05:12 +0000 to 2025-09-03 17:46:38 +0000
- fix(publish): Don't generate final artifacts (rust-lang/cargo#15910)
- chore: Address most typos (rust-lang/cargo#15911)
- chore(deps): update rust crate annotate-snippets to 0.12.1 (rust-lang/cargo#15909)
- test(script): Switch frontmatter tests to end-to-end (rust-lang/cargo#15899)
- chore: fix some typos and grammar (rust-lang/cargo#15905)
- Update dependencies (rust-lang/cargo#15904)
- feat: Don't stop at first error when emitting lints and warnings (rust-lang/cargo#15889)
- fix(cli): Show the bad manifest path (rust-lang/cargo#15896)
- chore(deps): update rust crate tracing-subscriber to v0.3.20 [security] (rust-lang/cargo#15898)

r? ghost
bors added a commit to rust-lang/rust that referenced this pull request Sep 4, 2025
Update cargo submodule

12 commits in a6c58d43051d01d83f55a3e61ef5f5b2b0dd6bd9..761c4658d0079d607e6d33cf0c060e61a617cad3
2025-08-26 23:05:12 +0000 to 2025-09-04 01:25:01 +0000
- refactor(shell): Prepare for `Report`s being generated in more places (rust-lang/cargo#15920)
- refactor(frontmatter): Pull out as a dedicated mod (rust-lang/cargo#15914)
- chore: downgrade to libc@0.2.174 (rust-lang/cargo#15918)
- fix(publish): Don't generate final artifacts (rust-lang/cargo#15910)
- chore: Address most typos (rust-lang/cargo#15911)
- chore(deps): update rust crate annotate-snippets to 0.12.1 (rust-lang/cargo#15909)
- test(script): Switch frontmatter tests to end-to-end (rust-lang/cargo#15899)
- chore: fix some typos and grammar (rust-lang/cargo#15905)
- Update dependencies (rust-lang/cargo#15904)
- feat: Don't stop at first error when emitting lints and warnings (rust-lang/cargo#15889)
- fix(cli): Show the bad manifest path (rust-lang/cargo#15896)
- chore(deps): update rust crate tracing-subscriber to v0.3.20 [security] (rust-lang/cargo#15898)
@rustbot rustbot added this to the 1.91.0 milestone Sep 4, 2025
github-actions bot pushed a commit to rust-lang/miri that referenced this pull request Sep 5, 2025
Update cargo submodule

12 commits in a6c58d43051d01d83f55a3e61ef5f5b2b0dd6bd9..761c4658d0079d607e6d33cf0c060e61a617cad3
2025-08-26 23:05:12 +0000 to 2025-09-04 01:25:01 +0000
- refactor(shell): Prepare for `Report`s being generated in more places (rust-lang/cargo#15920)
- refactor(frontmatter): Pull out as a dedicated mod (rust-lang/cargo#15914)
- chore: downgrade to libc@0.2.174 (rust-lang/cargo#15918)
- fix(publish): Don't generate final artifacts (rust-lang/cargo#15910)
- chore: Address most typos (rust-lang/cargo#15911)
- chore(deps): update rust crate annotate-snippets to 0.12.1 (rust-lang/cargo#15909)
- test(script): Switch frontmatter tests to end-to-end (rust-lang/cargo#15899)
- chore: fix some typos and grammar (rust-lang/cargo#15905)
- Update dependencies (rust-lang/cargo#15904)
- feat: Don't stop at first error when emitting lints and warnings (rust-lang/cargo#15889)
- fix(cli): Show the bad manifest path (rust-lang/cargo#15896)
- chore(deps): update rust crate tracing-subscriber to v0.3.20 [security] (rust-lang/cargo#15898)
github-actions bot pushed a commit to rust-lang/rustc-dev-guide that referenced this pull request Sep 8, 2025
Update cargo submodule

12 commits in a6c58d43051d01d83f55a3e61ef5f5b2b0dd6bd9..761c4658d0079d607e6d33cf0c060e61a617cad3
2025-08-26 23:05:12 +0000 to 2025-09-04 01:25:01 +0000
- refactor(shell): Prepare for `Report`s being generated in more places (rust-lang/cargo#15920)
- refactor(frontmatter): Pull out as a dedicated mod (rust-lang/cargo#15914)
- chore: downgrade to libc@0.2.174 (rust-lang/cargo#15918)
- fix(publish): Don't generate final artifacts (rust-lang/cargo#15910)
- chore: Address most typos (rust-lang/cargo#15911)
- chore(deps): update rust crate annotate-snippets to 0.12.1 (rust-lang/cargo#15909)
- test(script): Switch frontmatter tests to end-to-end (rust-lang/cargo#15899)
- chore: fix some typos and grammar (rust-lang/cargo#15905)
- Update dependencies (rust-lang/cargo#15904)
- feat: Don't stop at first error when emitting lints and warnings (rust-lang/cargo#15889)
- fix(cli): Show the bad manifest path (rust-lang/cargo#15896)
- chore(deps): update rust crate tracing-subscriber to v0.3.20 [security] (rust-lang/cargo#15898)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-workspaces Area: workspaces

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants