about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2022-04-28 04:17:52 +0000
committerbors <bors@rust-lang.org>2022-04-28 04:17:52 +0000
commit0e7915d11f6888f005e78c2358fcdc48ff655753 (patch)
treeb5df5a22a17acee9c13ce2a0e469704539dd0850
parent81799cd8fd841e23b52876ae5e22faeb3ad04eb5 (diff)
parentc6bafa7322943643f37f8818bcb16dad28f53d26 (diff)
downloadrust-0e7915d11f6888f005e78c2358fcdc48ff655753.tar.gz
rust-0e7915d11f6888f005e78c2358fcdc48ff655753.zip
Auto merge of #96085 - jsgf:deny-unused-deps, r=compiler-errors
Make sure `-Dunused-crate-dependencies --json unused-externs` makes rustc exit with error status

This PR:
- fixes compiletest to understand unused extern notifications
- adds tests for `--json unused-externs`
- makes sure that deny-level unused externs notifications are treated as compile errors
  - refactors the `emit_unused_externs` callstack to plumb through the level as an enum as a string, and adds `Level::is_error`

Update: adds `--json unused-externs-silent` with the original behaviour since Cargo needs it. Should address `@est31's` concerns.

Fixes: https://github.com/rust-lang/rust/issues/96068
-rw-r--r--compiler/rustc_errors/src/emitter.rs7
-rw-r--r--compiler/rustc_errors/src/json.rs3
-rw-r--r--compiler/rustc_errors/src/lib.rs17
-rw-r--r--compiler/rustc_lint_defs/src/lib.rs7
-rw-r--r--compiler/rustc_metadata/src/creader.rs15
-rw-r--r--compiler/rustc_session/src/config.rs38
-rw-r--r--compiler/rustc_session/src/options.rs2
-rw-r--r--src/librustdoc/config.rs6
-rw-r--r--src/librustdoc/doctest.rs4
-rw-r--r--src/test/ui/unused-crate-deps/deny-attr.rs9
-rw-r--r--src/test/ui/unused-crate-deps/deny-attr.stderr14
-rw-r--r--src/test/ui/unused-crate-deps/deny-cmdline-json-silent.rs8
-rw-r--r--src/test/ui/unused-crate-deps/deny-cmdline-json-silent.stderr1
-rw-r--r--src/test/ui/unused-crate-deps/deny-cmdline-json.rs7
-rw-r--r--src/test/ui/unused-crate-deps/deny-cmdline-json.stderr1
-rw-r--r--src/test/ui/unused-crate-deps/deny-cmdline.rs8
-rw-r--r--src/test/ui/unused-crate-deps/deny-cmdline.stderr10
-rw-r--r--src/test/ui/unused-crate-deps/warn-cmdline-json.rs8
-rw-r--r--src/test/ui/unused-crate-deps/warn-cmdline-json.stderr1
-rw-r--r--src/tools/compiletest/src/json.rs11
20 files changed, 156 insertions, 21 deletions
diff --git a/compiler/rustc_errors/src/emitter.rs b/compiler/rustc_errors/src/emitter.rs
index 47cdf39cd52..5dd743e8d00 100644
--- a/compiler/rustc_errors/src/emitter.rs
+++ b/compiler/rustc_errors/src/emitter.rs
@@ -212,7 +212,12 @@ pub trait Emitter {
     fn emit_future_breakage_report(&mut self, _diags: Vec<Diagnostic>) {}
 
     /// Emit list of unused externs
-    fn emit_unused_externs(&mut self, _lint_level: &str, _unused_externs: &[&str]) {}
+    fn emit_unused_externs(
+        &mut self,
+        _lint_level: rustc_lint_defs::Level,
+        _unused_externs: &[&str],
+    ) {
+    }
 
     /// Checks if should show explanations about "rustc --explain"
     fn should_show_explain(&self) -> bool {
diff --git a/compiler/rustc_errors/src/json.rs b/compiler/rustc_errors/src/json.rs
index d680e7fab70..6ff52182d6b 100644
--- a/compiler/rustc_errors/src/json.rs
+++ b/compiler/rustc_errors/src/json.rs
@@ -171,7 +171,8 @@ impl Emitter for JsonEmitter {
         }
     }
 
-    fn emit_unused_externs(&mut self, lint_level: &str, unused_externs: &[&str]) {
+    fn emit_unused_externs(&mut self, lint_level: rustc_lint_defs::Level, unused_externs: &[&str]) {
+        let lint_level = lint_level.as_str();
         let data = UnusedExterns { lint_level, unused_extern_names: unused_externs };
         let result = if self.pretty {
             writeln!(&mut self.dst, "{}", as_pretty_json(&data))
diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs
index 4e6ab0edf66..a64133bb7f4 100644
--- a/compiler/rustc_errors/src/lib.rs
+++ b/compiler/rustc_errors/src/lib.rs
@@ -969,8 +969,19 @@ impl Handler {
         self.inner.borrow_mut().emitter.emit_future_breakage_report(diags)
     }
 
-    pub fn emit_unused_externs(&self, lint_level: &str, unused_externs: &[&str]) {
-        self.inner.borrow_mut().emit_unused_externs(lint_level, unused_externs)
+    pub fn emit_unused_externs(
+        &self,
+        lint_level: rustc_lint_defs::Level,
+        loud: bool,
+        unused_externs: &[&str],
+    ) {
+        let mut inner = self.inner.borrow_mut();
+
+        if loud && lint_level.is_error() {
+            inner.bump_err_count();
+        }
+
+        inner.emit_unused_externs(lint_level, unused_externs)
     }
 
     pub fn update_unstable_expectation_id(
@@ -1141,7 +1152,7 @@ impl HandlerInner {
         self.emitter.emit_artifact_notification(path, artifact_type);
     }
 
-    fn emit_unused_externs(&mut self, lint_level: &str, unused_externs: &[&str]) {
+    fn emit_unused_externs(&mut self, lint_level: rustc_lint_defs::Level, unused_externs: &[&str]) {
         self.emitter.emit_unused_externs(lint_level, unused_externs);
     }
 
diff --git a/compiler/rustc_lint_defs/src/lib.rs b/compiler/rustc_lint_defs/src/lib.rs
index f590172af4f..57b4f96dc10 100644
--- a/compiler/rustc_lint_defs/src/lib.rs
+++ b/compiler/rustc_lint_defs/src/lib.rs
@@ -214,6 +214,13 @@ impl Level {
             _ => None,
         }
     }
+
+    pub fn is_error(self) -> bool {
+        match self {
+            Level::Allow | Level::Expect(_) | Level::Warn | Level::ForceWarn => false,
+            Level::Deny | Level::Forbid => true,
+        }
+    }
 }
 
 /// Specification of a single lint.
diff --git a/compiler/rustc_metadata/src/creader.rs b/compiler/rustc_metadata/src/creader.rs
index e8cdae7fd25..3c545e6a0d2 100644
--- a/compiler/rustc_metadata/src/creader.rs
+++ b/compiler/rustc_metadata/src/creader.rs
@@ -195,10 +195,12 @@ impl CStore {
     }
 
     pub fn report_unused_deps(&self, tcx: TyCtxt<'_>) {
+        let json_unused_externs = tcx.sess.opts.json_unused_externs;
+
         // We put the check for the option before the lint_level_at_node call
         // because the call mutates internal state and introducing it
         // leads to some ui tests failing.
-        if !tcx.sess.opts.json_unused_externs {
+        if !json_unused_externs.is_enabled() {
             return;
         }
         let level = tcx
@@ -208,10 +210,11 @@ impl CStore {
             let unused_externs =
                 self.unused_externs.iter().map(|ident| ident.to_ident_string()).collect::<Vec<_>>();
             let unused_externs = unused_externs.iter().map(String::as_str).collect::<Vec<&str>>();
-            tcx.sess
-                .parse_sess
-                .span_diagnostic
-                .emit_unused_externs(level.as_str(), &unused_externs);
+            tcx.sess.parse_sess.span_diagnostic.emit_unused_externs(
+                level,
+                json_unused_externs.is_loud(),
+                &unused_externs,
+            );
         }
     }
 }
@@ -917,7 +920,7 @@ impl<'a> CrateLoader<'a> {
             }
 
             // Got a real unused --extern
-            if self.sess.opts.json_unused_externs {
+            if self.sess.opts.json_unused_externs.is_enabled() {
                 self.cstore.unused_externs.push(name_interned);
                 continue;
             }
diff --git a/compiler/rustc_session/src/config.rs b/compiler/rustc_session/src/config.rs
index adb2b79038f..530c1a06f8f 100644
--- a/compiler/rustc_session/src/config.rs
+++ b/compiler/rustc_session/src/config.rs
@@ -757,7 +757,7 @@ impl Default for Options {
             real_rust_source_base_dir: None,
             edition: DEFAULT_EDITION,
             json_artifact_notifications: false,
-            json_unused_externs: false,
+            json_unused_externs: JsonUnusedExterns::No,
             json_future_incompat: false,
             pretty: None,
             working_dir: RealFileName::LocalPath(std::env::current_dir().unwrap()),
@@ -1498,10 +1498,37 @@ pub fn parse_color(matches: &getopts::Matches) -> ColorConfig {
 pub struct JsonConfig {
     pub json_rendered: HumanReadableErrorType,
     pub json_artifact_notifications: bool,
-    pub json_unused_externs: bool,
+    pub json_unused_externs: JsonUnusedExterns,
     pub json_future_incompat: bool,
 }
 
+/// Report unused externs in event stream
+#[derive(Copy, Clone)]
+pub enum JsonUnusedExterns {
+    /// Do not
+    No,
+    /// Report, but do not exit with failure status for deny/forbid
+    Silent,
+    /// Report, and also exit with failure status for deny/forbid
+    Loud,
+}
+
+impl JsonUnusedExterns {
+    pub fn is_enabled(&self) -> bool {
+        match self {
+            JsonUnusedExterns::No => false,
+            JsonUnusedExterns::Loud | JsonUnusedExterns::Silent => true,
+        }
+    }
+
+    pub fn is_loud(&self) -> bool {
+        match self {
+            JsonUnusedExterns::No | JsonUnusedExterns::Silent => false,
+            JsonUnusedExterns::Loud => true,
+        }
+    }
+}
+
 /// Parse the `--json` flag.
 ///
 /// The first value returned is how to render JSON diagnostics, and the second
@@ -1511,7 +1538,7 @@ pub fn parse_json(matches: &getopts::Matches) -> JsonConfig {
         HumanReadableErrorType::Default;
     let mut json_color = ColorConfig::Never;
     let mut json_artifact_notifications = false;
-    let mut json_unused_externs = false;
+    let mut json_unused_externs = JsonUnusedExterns::No;
     let mut json_future_incompat = false;
     for option in matches.opt_strs("json") {
         // For now conservatively forbid `--color` with `--json` since `--json`
@@ -1529,7 +1556,8 @@ pub fn parse_json(matches: &getopts::Matches) -> JsonConfig {
                 "diagnostic-short" => json_rendered = HumanReadableErrorType::Short,
                 "diagnostic-rendered-ansi" => json_color = ColorConfig::Always,
                 "artifacts" => json_artifact_notifications = true,
-                "unused-externs" => json_unused_externs = true,
+                "unused-externs" => json_unused_externs = JsonUnusedExterns::Loud,
+                "unused-externs-silent" => json_unused_externs = JsonUnusedExterns::Silent,
                 "future-incompat" => json_future_incompat = true,
                 s => early_error(
                     ErrorOutputType::default(),
@@ -2229,7 +2257,7 @@ pub fn build_session_options(matches: &getopts::Matches) -> Options {
 
     check_debug_option_stability(&debugging_opts, error_format, json_rendered);
 
-    if !debugging_opts.unstable_options && json_unused_externs {
+    if !debugging_opts.unstable_options && json_unused_externs.is_enabled() {
         early_error(
             error_format,
             "the `-Z unstable-options` flag must also be passed to enable \
diff --git a/compiler/rustc_session/src/options.rs b/compiler/rustc_session/src/options.rs
index 96f50e57ac4..14e918660dd 100644
--- a/compiler/rustc_session/src/options.rs
+++ b/compiler/rustc_session/src/options.rs
@@ -221,7 +221,7 @@ top_level_options!(
         json_artifact_notifications: bool [TRACKED],
 
         /// `true` if we're emitting a JSON blob containing the unused externs
-        json_unused_externs: bool [UNTRACKED],
+        json_unused_externs: JsonUnusedExterns [UNTRACKED],
 
         /// `true` if we're emitting a JSON job containing a future-incompat report for lints
         json_future_incompat: bool [TRACKED],
diff --git a/src/librustdoc/config.rs b/src/librustdoc/config.rs
index cee3dcb416f..1ff2c8191e5 100644
--- a/src/librustdoc/config.rs
+++ b/src/librustdoc/config.rs
@@ -10,7 +10,9 @@ use rustc_session::config::{
     self, parse_crate_types_from_list, parse_externs, parse_target_triple, CrateType,
 };
 use rustc_session::config::{get_cmd_lint_options, nightly_options};
-use rustc_session::config::{CodegenOptions, DebuggingOptions, ErrorOutputType, Externs};
+use rustc_session::config::{
+    CodegenOptions, DebuggingOptions, ErrorOutputType, Externs, JsonUnusedExterns,
+};
 use rustc_session::getopts;
 use rustc_session::lint::Level;
 use rustc_session::search_paths::SearchPath;
@@ -147,7 +149,7 @@ crate struct Options {
     /// documentation.
     crate run_check: bool,
     /// Whether doctests should emit unused externs
-    crate json_unused_externs: bool,
+    crate json_unused_externs: JsonUnusedExterns,
     /// Whether to skip capturing stdout and stderr of tests.
     crate nocapture: bool,
 
diff --git a/src/librustdoc/doctest.rs b/src/librustdoc/doctest.rs
index 45ac16e75aa..82e367427ef 100644
--- a/src/librustdoc/doctest.rs
+++ b/src/librustdoc/doctest.rs
@@ -168,7 +168,7 @@ crate fn run(options: RustdocOptions) -> Result<(), ErrorGuaranteed> {
 
     // Collect and warn about unused externs, but only if we've gotten
     // reports for each doctest
-    if json_unused_externs {
+    if json_unused_externs.is_enabled() {
         let unused_extern_reports: Vec<_> =
             std::mem::take(&mut unused_extern_reports.lock().unwrap());
         if unused_extern_reports.len() == compiling_test_count {
@@ -337,7 +337,7 @@ fn run_test(
     if lang_string.test_harness {
         compiler.arg("--test");
     }
-    if rustdoc_options.json_unused_externs && !lang_string.compile_fail {
+    if rustdoc_options.json_unused_externs.is_enabled() && !lang_string.compile_fail {
         compiler.arg("--error-format=json");
         compiler.arg("--json").arg("unused-externs");
         compiler.arg("-Z").arg("unstable-options");
diff --git a/src/test/ui/unused-crate-deps/deny-attr.rs b/src/test/ui/unused-crate-deps/deny-attr.rs
new file mode 100644
index 00000000000..e9ab18ff63f
--- /dev/null
+++ b/src/test/ui/unused-crate-deps/deny-attr.rs
@@ -0,0 +1,9 @@
+// Check for unused crate dep, no path
+
+// edition:2018
+// aux-crate:bar=bar.rs
+
+#![deny(unused_crate_dependencies)]
+//~^ ERROR external crate `bar` unused in
+
+fn main() {}
diff --git a/src/test/ui/unused-crate-deps/deny-attr.stderr b/src/test/ui/unused-crate-deps/deny-attr.stderr
new file mode 100644
index 00000000000..93694f6827f
--- /dev/null
+++ b/src/test/ui/unused-crate-deps/deny-attr.stderr
@@ -0,0 +1,14 @@
+error: external crate `bar` unused in `deny_attr`: remove the dependency or add `use bar as _;`
+  --> $DIR/deny-attr.rs:6:1
+   |
+LL | #![deny(unused_crate_dependencies)]
+   | ^
+   |
+note: the lint level is defined here
+  --> $DIR/deny-attr.rs:6:9
+   |
+LL | #![deny(unused_crate_dependencies)]
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: aborting due to previous error
+
diff --git a/src/test/ui/unused-crate-deps/deny-cmdline-json-silent.rs b/src/test/ui/unused-crate-deps/deny-cmdline-json-silent.rs
new file mode 100644
index 00000000000..fd9a61d6caa
--- /dev/null
+++ b/src/test/ui/unused-crate-deps/deny-cmdline-json-silent.rs
@@ -0,0 +1,8 @@
+// Check for unused crate dep, json event, deny but we're not reporting that in exit status
+
+// edition:2018
+// check-pass
+// compile-flags: -Dunused-crate-dependencies -Zunstable-options --json unused-externs-silent --error-format=json
+// aux-crate:bar=bar.rs
+
+fn main() {}
diff --git a/src/test/ui/unused-crate-deps/deny-cmdline-json-silent.stderr b/src/test/ui/unused-crate-deps/deny-cmdline-json-silent.stderr
new file mode 100644
index 00000000000..595619f3a8a
--- /dev/null
+++ b/src/test/ui/unused-crate-deps/deny-cmdline-json-silent.stderr
@@ -0,0 +1 @@
+{"lint_level":"deny","unused_extern_names":["bar"]}
diff --git a/src/test/ui/unused-crate-deps/deny-cmdline-json.rs b/src/test/ui/unused-crate-deps/deny-cmdline-json.rs
new file mode 100644
index 00000000000..2b369dee5a0
--- /dev/null
+++ b/src/test/ui/unused-crate-deps/deny-cmdline-json.rs
@@ -0,0 +1,7 @@
+// Check for unused crate dep, json event, deny, expect compile failure
+
+// edition:2018
+// compile-flags: -Dunused-crate-dependencies  -Zunstable-options --json unused-externs --error-format=json
+// aux-crate:bar=bar.rs
+
+fn main() {}
diff --git a/src/test/ui/unused-crate-deps/deny-cmdline-json.stderr b/src/test/ui/unused-crate-deps/deny-cmdline-json.stderr
new file mode 100644
index 00000000000..595619f3a8a
--- /dev/null
+++ b/src/test/ui/unused-crate-deps/deny-cmdline-json.stderr
@@ -0,0 +1 @@
+{"lint_level":"deny","unused_extern_names":["bar"]}
diff --git a/src/test/ui/unused-crate-deps/deny-cmdline.rs b/src/test/ui/unused-crate-deps/deny-cmdline.rs
new file mode 100644
index 00000000000..69e28b3319a
--- /dev/null
+++ b/src/test/ui/unused-crate-deps/deny-cmdline.rs
@@ -0,0 +1,8 @@
+// Check for unused crate dep, deny, expect failure
+
+// edition:2018
+// compile-flags: -Dunused-crate-dependencies
+// aux-crate:bar=bar.rs
+
+fn main() {}
+//~^ ERROR external crate `bar` unused in
diff --git a/src/test/ui/unused-crate-deps/deny-cmdline.stderr b/src/test/ui/unused-crate-deps/deny-cmdline.stderr
new file mode 100644
index 00000000000..0951dc670fe
--- /dev/null
+++ b/src/test/ui/unused-crate-deps/deny-cmdline.stderr
@@ -0,0 +1,10 @@
+error: external crate `bar` unused in `deny_cmdline`: remove the dependency or add `use bar as _;`
+  --> $DIR/deny-cmdline.rs:7:1
+   |
+LL | fn main() {}
+   | ^
+   |
+   = note: requested on the command line with `-D unused-crate-dependencies`
+
+error: aborting due to previous error
+
diff --git a/src/test/ui/unused-crate-deps/warn-cmdline-json.rs b/src/test/ui/unused-crate-deps/warn-cmdline-json.rs
new file mode 100644
index 00000000000..4826c0062d0
--- /dev/null
+++ b/src/test/ui/unused-crate-deps/warn-cmdline-json.rs
@@ -0,0 +1,8 @@
+// Check for unused crate dep, warn, json event, expect pass
+
+// edition:2018
+// check-pass
+// compile-flags: -Wunused-crate-dependencies -Zunstable-options --json unused-externs --error-format=json
+// aux-crate:bar=bar.rs
+
+fn main() {}
diff --git a/src/test/ui/unused-crate-deps/warn-cmdline-json.stderr b/src/test/ui/unused-crate-deps/warn-cmdline-json.stderr
new file mode 100644
index 00000000000..98dbd763927
--- /dev/null
+++ b/src/test/ui/unused-crate-deps/warn-cmdline-json.stderr
@@ -0,0 +1 @@
+{"lint_level":"warn","unused_extern_names":["bar"]}
diff --git a/src/tools/compiletest/src/json.rs b/src/tools/compiletest/src/json.rs
index a5ff779a4ab..10726b98420 100644
--- a/src/tools/compiletest/src/json.rs
+++ b/src/tools/compiletest/src/json.rs
@@ -23,6 +23,14 @@ struct ArtifactNotification {
     artifact: PathBuf,
 }
 
+#[derive(Deserialize)]
+struct UnusedExternNotification {
+    #[allow(dead_code)]
+    lint_level: String,
+    #[allow(dead_code)]
+    unused_extern_names: Vec<String>,
+}
+
 #[derive(Deserialize, Clone)]
 struct DiagnosticSpan {
     file_name: String,
@@ -113,6 +121,9 @@ pub fn extract_rendered(output: &str) -> String {
                 } else if serde_json::from_str::<ArtifactNotification>(line).is_ok() {
                     // Ignore the notification.
                     None
+                } else if serde_json::from_str::<UnusedExternNotification>(line).is_ok() {
+                    // Ignore the notification.
+                    None
                 } else {
                     print!(
                         "failed to decode compiler output as json: line: {}\noutput: {}",