fix(compile): build.warnings=allow should not hide denied diagnostics (#16824)

### What does this PR try to resolve?

They shouldn't be hidden and this was causing cargo to get confused:
```
[CHECKING] foo v0.0.1 ([ROOT]/foo)
[ERROR] could not compile `foo` (bin "foo")

Caused by:
  process didn't exit successfully: `rustc --crate-name foo --edition=2021 src/main.rs --error-format=json --json=diagnostic-rendered-ansi,artifacts,future-incompat --diagnostic-width=400 --crate-type bin --emit=dep-info,metadata -C embed-bitcode=no -C debuginfo=2 --check-cfg 'cfg(docsrs,test)' --check-cfg 'cfg(feature, values())' -C metadata=cbc5c944f9a4e5cb -C extra-filename=-b1a006a0c806d1bb --out-dir [ROOT]/foo/target/debug/deps -L dependency=[ROOT]/foo/target/debug/deps -Dunused_variables` ([EXIT_STATUS]: 1)
```

### How to test and review this PR?

Instead of hiding all diagnostics, regardless of what level, we are
post-processing the diagnostics and deciding when to hide them.

One day it might be nice to clean up the treating of `level` as a string
and having string comparison checks spread around this code.

Author: arlosi

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

@@ -8,6 +8,7 @@ use crate::core::compiler::future_incompat::FutureBreakageItem;
 use crate::core::compiler::locking::LockKey;
 use crate::core::compiler::timings::SectionTiming;
 use crate::util::Queue;
+use crate::util::context::WarningHandling;
 use crate::{CargoResult, core::compiler::locking::LockManager};
 
 use super::{Artifact, DiagDedupe, Job, JobId, Message};
@@ -51,6 +52,8 @@ pub struct JobState<'a, 'gctx> {
     /// Manages locks for build units when fine grain locking is enabled.
     lock_manager: Arc<LockManager>,
 
+    warning_handling: WarningHandling,
+
     // Historical versions of Cargo made use of the `'a` argument here, so to
     // leave the door open to future refactorings keep it here.
     _marker: marker::PhantomData<&'a ()>,
@@ -63,13 +66,15 @@ impl<'a, 'gctx> JobState<'a, 'gctx> {
         output: Option<&'a DiagDedupe<'gctx>>,
         rmeta_required: bool,
         lock_manager: Arc<LockManager>,
+        warning_handling: WarningHandling,
     ) -> Self {
         Self {
             id,
             messages,
             output,
             rmeta_required: Cell::new(rmeta_required),
             lock_manager,
+            warning_handling,
             _marker: marker::PhantomData,
         }
     }
@@ -106,7 +111,9 @@ impl<'a, 'gctx> JobState<'a, 'gctx> {
         lint: bool,
         fixable: bool,
     ) -> CargoResult<()> {
-        if let Some(dedupe) = self.output {
+        if level == "warning" && self.warning_handling == WarningHandling::Allow {
+            tracing::warn!("{diag}");
+        } else if let Some(dedupe) = self.output {
             let emitted = dedupe.emit_diag(&diag)?;
             if level == "warning" {
                 self.messages.push(Message::WarningCount {

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

@@ -988,9 +988,17 @@ impl<'gctx> DrainState<'gctx> {
         let is_fresh = job.freshness().is_fresh();
         let rmeta_required = build_runner.rmeta_required(unit);
         let lock_manager = build_runner.lock_manager.clone();
+        let warning_handling = build_runner.bcx.gctx.warning_handling().unwrap_or_default();
 
         let doit = move |diag_dedupe| {
-            let state = JobState::new(id, messages, diag_dedupe, rmeta_required, lock_manager);
+            let state = JobState::new(
+                id,
+                messages,
+                diag_dedupe,
+                rmeta_required,
+                lock_manager,
+                warning_handling,
+            );
             state.run_to_finish(job);
         };
 

src/cargo/core/compiler/mod.rs

@@ -105,7 +105,6 @@ use crate::core::profiles::{PanicStrategy, Profile, StripInner};
 use crate::core::{Feature, PackageId, Target};
 use crate::lints::get_key_value;
 use crate::util::OnceExt;
-use crate::util::context::WarningHandling;
 use crate::util::errors::{CargoResult, VerboseError};
 use crate::util::interning::InternedString;
 use crate::util::machine_message::{self, Message};
@@ -2026,8 +2025,7 @@ impl OutputOptions {
         drop(fs::remove_file(&path));
         let cache_cell = Some((path, OnceCell::new()));
 
-        let show_diagnostics =
-            build_runner.bcx.gctx.warning_handling().unwrap_or_default() != WarningHandling::Allow;
+        let show_diagnostics = true;
 
         let format = build_runner.bcx.build_config.message_format;
 
@@ -2045,9 +2043,7 @@ impl OutputOptions {
 
         // We always replay the output cache,
         // since it might contain future-incompat-report messages
-        let show_diagnostics = unit.show_warnings(build_runner.bcx.gctx)
-            && build_runner.bcx.gctx.warning_handling().unwrap_or_default()
-                != WarningHandling::Allow;
+        let show_diagnostics = unit.show_warnings(build_runner.bcx.gctx);
 
         let format = build_runner.bcx.build_config.message_format;
 

tests/testsuite/warning_override.rs

@@ -40,6 +40,26 @@ fn requires_nightly() {
         .run();
 }
 
+#[cargo_test]
+fn always_show_error_diags() {
+    let p = make_project_with_rustc_warning();
+    p.cargo("check")
+        .masquerade_as_nightly_cargo(&["warnings"])
+        .env("RUSTFLAGS", "-Dunused_variables")
+        .arg("-Zwarnings")
+        .arg("--config")
+        .arg("build.warnings='allow'")
+        .with_stderr_data(str![[r#"
+[CHECKING] foo v0.0.1 ([ROOT]/foo)
+[ERROR] unused variable: `x`
+...
+[ERROR] could not compile `foo` (bin "foo") due to 1 previous error
+
+"#]])
+        .with_status(101)
+        .run();
+}
+
 #[cargo_test]
 fn clippy() {
     let p = project()