fix(compile): Stop on denying warnings without --keep-going (#16725)

### What does this PR try to resolve?

This had been discussed a little bit on
[zulip](https://rust-lang.zulipchat.com/#narrow/channel/246057-t-cargo/topic/Environment.20variable.20for.20.5Blints.5D.20table.3F/near/553969201).
This then came up in usability feedback from @Urgau with
rust-lang/rust's x.py having adopted `build.warnings`.
The output from all of the other build targets causes the warnings to
disappear off the screen which doesn't happen with errors because the
build ends immediately.

So we're making warnings act more like errors.

### How to test and review this PR?

This is a follow up to #16721

Author: weihanglo

src/cargo/core/compiler/job_queue/mod.rs

@@ -211,7 +211,7 @@ struct DrainState<'gctx> {
 }
 
 /// Count of warnings, used to print a summary after the job succeeds
-#[derive(Default)]
+#[derive(Default, Clone)]
 pub struct WarningCount {
     /// total number of warnings
     pub total: usize,
@@ -248,7 +248,7 @@ impl WarningCount {
 
 /// Used to keep track of how many fixable warnings there are
 /// and if fixable warnings are allowed
-#[derive(Default)]
+#[derive(Default, Copy, Clone)]
 pub enum FixableWarnings {
     NotAllowed,
     #[default]
@@ -668,20 +668,40 @@ impl<'gctx> DrainState<'gctx> {
             Message::FixDiagnostic(msg) => {
                 self.print.print(&msg)?;
             }
-            Message::Finish(id, artifact, result) => {
+            Message::Finish(id, artifact, mut result) => {
                 let unit = match artifact {
                     // If `id` has completely finished we remove it
                     // from the `active` map ...
                     Artifact::All => {
                         trace!("end: {:?}", id);
                         self.finished += 1;
-                        self.report_warning_count(
-                            build_runner,
-                            id,
-                            &build_runner.bcx.rustc().workspace_wrapper,
-                            warning_handling,
-                        );
-                        self.active.remove(&id).unwrap()
+                        let unit = self.active.remove(&id).unwrap();
+                        // An error could add an entry for a `Unit`
+                        // with 0 warnings but having fixable
+                        // warnings be disallowed
+                        let count = self
+                            .warning_count
+                            .get(&id)
+                            .filter(|count| 0 < count.total)
+                            .cloned();
+                        if let Some(count) = count {
+                            self.report_warning_count(
+                                build_runner,
+                                &unit,
+                                &count,
+                                &build_runner.bcx.rustc().workspace_wrapper,
+                                warning_handling,
+                            );
+                            let stop_on_warnings = warning_handling == WarningHandling::Deny
+                                && 0 < count.lints
+                                && !build_runner.bcx.build_config.keep_going;
+                            if stop_on_warnings {
+                                result = Err(anyhow::format_err!(
+                                    "warnings are denied by `build.warnings` configuration"
+                                ))
+                            }
+                        }
+                        unit
                     }
                     // ... otherwise if it hasn't finished we leave it
                     // in there as we'll get another `Finish` later on.
@@ -1052,20 +1072,13 @@ impl<'gctx> DrainState<'gctx> {
     fn report_warning_count(
         &mut self,
         runner: &mut BuildRunner<'_, '_>,
-        id: JobId,
+        unit: &Unit,
+        count: &WarningCount,
         rustc_workspace_wrapper: &Option<PathBuf>,
         warning_handling: WarningHandling,
     ) {
         let gctx = runner.bcx.gctx;
-        let count = match self.warning_count.get(&id) {
-            // An error could add an entry for a `Unit`
-            // with 0 warnings but having fixable
-            // warnings be disallowed
-            Some(count) if count.total > 0 => count,
-            None | Some(_) => return,
-        };
         runner.compilation.lint_warning_count += count.lints;
-        let unit = &self.active[&id];
         let mut message = descriptive_pkg_name(&unit.pkg.name(), &unit.target, &unit.mode);
         message.push_str(" generated ");
         match count.total {

src/doc/src/reference/unstable.md

@@ -1859,7 +1859,7 @@ in the future.
 Controls how Cargo handles warnings. Allowed values are:
 * `warn`: warnings are emitted as warnings (default).
 * `allow`: warnings are hidden.
-* `deny`: if warnings are emitted, an error will be raised at the end of the operation and the process will exit with a failure exit code. 
+* `deny`: if warnings are emitted, an error will be raised at the end of the current crate and the process. Use `--keep-going` to see all warnings.
 
 ## feature unification
 

tests/testsuite/warning_override.rs

@@ -51,7 +51,6 @@ fn rustc_caching_allow_first() {
   = [NOTE] `#[warn(unused_variables)]` [..]on by default
 
 [ERROR] `foo` (bin "foo") generated 1 warning[..]
-[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
 [ERROR] warnings are denied by `build.warnings` configuration
 
 "#]])
@@ -78,7 +77,6 @@ fn rustc_caching_deny_first() {
   = [NOTE] `#[warn(unused_variables)]` [..]on by default
 
 [ERROR] `foo` (bin "foo") generated 1 warning[..]
-[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
 [ERROR] warnings are denied by `build.warnings` configuration
 
 "#]])
@@ -115,7 +113,6 @@ fn config() {
   = [NOTE] `#[warn(unused_variables)]` [..]on by default
 
 [ERROR] `foo` (bin "foo") generated 1 warning[..]
-[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
 [ERROR] warnings are denied by `build.warnings` configuration
 
 "#]])
@@ -195,7 +192,6 @@ fn clippy() {
 [WARNING] unused import: `std::io`
 ...
 [ERROR] `foo` (lib) generated 1 warning (run `cargo clippy --fix --lib -p foo` to apply 1 suggestion)
-[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
 [ERROR] warnings are denied by `build.warnings` configuration
 
 "#]])
@@ -289,3 +285,57 @@ fn hard_warning_allow() {
         .with_status(0)
         .run();
 }
+
+#[cargo_test]
+fn keep_going() {
+    let p = project()
+        .file(
+            "Cargo.toml",
+            &format!(
+                r#"
+                [package]
+                name = "foo"
+                version = "0.0.1"
+                edition = "2021"
+            "#
+            ),
+        )
+        .file("build.rs", "fn main() { let x = 3; }")
+        .file("src/main.rs", "fn main() { let y = 4; }")
+        .build();
+
+    p.cargo("check")
+        .masquerade_as_nightly_cargo(&["warnings"])
+        .arg("-Zwarnings")
+        .arg("--config")
+        .arg("build.warnings='deny'")
+        .with_stderr_data(str![[r#"
+[COMPILING] foo v0.0.1 ([ROOT]/foo)
+[WARNING] unused variable: `x`
+...
+[ERROR] `foo` (build script) generated 1 warning
+[ERROR] warnings are denied by `build.warnings` configuration
+
+"#]])
+        .with_status(101)
+        .run();
+
+    p.cargo("check --keep-going")
+        .masquerade_as_nightly_cargo(&["warnings"])
+        .arg("-Zwarnings")
+        .arg("--config")
+        .arg("build.warnings='deny'")
+        .with_stderr_data(str![[r#"
+[WARNING] unused variable: `x`
+...
+[ERROR] `foo` (build script) generated 1 warning
+[COMPILING] foo v0.0.1 ([ROOT]/foo)
+...
+[ERROR] `foo` (bin "foo") generated 1 warning (run `cargo fix --bin "foo" -p foo` to apply 1 suggestion)
+[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
+[ERROR] warnings are denied by `build.warnings` configuration
+
+"#]])
+        .with_status(101)
+        .run();
+}