Auto merge of #72603 - jsgf:extern-loc, r=nikomatsakis

Implement `--extern-location`

This PR implements `--extern-location` as a followup to #72342 as part of the implementation of #57274. The goal of this PR is to allow rustc, in coordination with the build system, to present a useful diagnostic about how to remove an unnecessary dependency from a dependency specification file (eg Cargo.toml).

EDIT: Updated to current PR state.

The location is specified for each named crate - that is, for a given `--extern foo[=path]` there can also be `--extern-location foo=<location>`. It supports ~~three~~ two styles of location:
~~1. `--extern-location foo=file:<path>:<line>` - a file path and line specification
1. `--extern-location foo=span:<path>:<start>:<end>` - a span specified as a file and start and end byte offsets~~
1. `--extern-location foo=raw:<anything>` - a raw string which is included in the output
1. `--extern-location foo=json:<anything>` - an arbitrary Json structure which is emitted via Json diagnostics in a `tool_metadata` field.

~~1 & 2 are turned into an internal `Span`, so long as the path exists and is readable, and the location is meaningful (within the file, etc). This is used as the `Span` for a fix suggestion which is reported like other fix suggestions.~~

`raw` and `json` are for the case where the location isn't best expressed as a file and location within that file. For example, it could be a rule name and the name of a dependency within that rule. `rustc` makes no attempt to parse the raw string, and simply includes it in the output diagnostic text. `json` is only included in json diagnostics. `raw` is emitted as text and also as a json string in `tool_metadata`.

If no `--extern-location` option is specified then it will emit a default json structure consisting of `{"name": name, "path": path}` corresponding to the name and path in `--extern name=path`.

This is a prototype/RFC to make some of the earlier conversations more concrete. It doesn't stand on its own - it's only useful if implemented by Cargo and other build systems. There's also a ton of implementation details which I'd appreciate a second eye on as well.

~~**NOTE** The first commit in this PR is #72342 and should be ignored for the purposes of review. The first commit is a very simplistic implementation which is basically raw-only, presented as a MVP. The second implements the full thing, and subsequent commits are incremental fixes.~~

cc `@ehuss` `@est31` `@petrochenkov` `@estebank`

Author: bors

Diff for: Cargo.lock

@@ -726,9 +726,9 @@ dependencies = [
 
 [[package]]
 name = "const_fn"
-version = "0.4.2"
+version = "0.4.3"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "ce90df4c658c62f12d78f7508cf92f9173e5184a539c10bfe54a3107b3ffd0f2"
+checksum = "c478836e029dcef17fb47c89023448c64f781a046e0300e257ad8225ae59afab"
 
 [[package]]
 name = "constant_time_eq"
@@ -3914,6 +3914,7 @@ dependencies = [
  "rustc_index",
  "rustc_middle",
  "rustc_parse_format",
+ "rustc_serialize",
  "rustc_session",
  "rustc_span",
  "rustc_target",

Diff for: compiler/rustc_errors/src/diagnostic.rs

@@ -4,7 +4,9 @@ use crate::Level;
 use crate::Substitution;
 use crate::SubstitutionPart;
 use crate::SuggestionStyle;
+use crate::ToolMetadata;
 use rustc_lint_defs::Applicability;
+use rustc_serialize::json::Json;
 use rustc_span::{MultiSpan, Span, DUMMY_SP};
 use std::fmt;
 
@@ -303,6 +305,7 @@ impl Diagnostic {
             msg: msg.to_owned(),
             style: SuggestionStyle::ShowCode,
             applicability,
+            tool_metadata: Default::default(),
         });
         self
     }
@@ -328,6 +331,7 @@ impl Diagnostic {
             msg: msg.to_owned(),
             style: SuggestionStyle::ShowCode,
             applicability,
+            tool_metadata: Default::default(),
         });
         self
     }
@@ -354,6 +358,7 @@ impl Diagnostic {
             msg: msg.to_owned(),
             style: SuggestionStyle::CompletelyHidden,
             applicability,
+            tool_metadata: Default::default(),
         });
         self
     }
@@ -408,6 +413,7 @@ impl Diagnostic {
             msg: msg.to_owned(),
             style,
             applicability,
+            tool_metadata: Default::default(),
         });
         self
     }
@@ -446,6 +452,7 @@ impl Diagnostic {
             msg: msg.to_owned(),
             style: SuggestionStyle::ShowCode,
             applicability,
+            tool_metadata: Default::default(),
         });
         self
     }
@@ -515,6 +522,23 @@ impl Diagnostic {
         self
     }
 
+    /// Adds a suggestion intended only for a tool. The intent is that the metadata encodes
+    /// the suggestion in a tool-specific way, as it may not even directly involve Rust code.
+    pub fn tool_only_suggestion_with_metadata(
+        &mut self,
+        msg: &str,
+        applicability: Applicability,
+        tool_metadata: Json,
+    ) {
+        self.suggestions.push(CodeSuggestion {
+            substitutions: vec![],
+            msg: msg.to_owned(),
+            style: SuggestionStyle::CompletelyHidden,
+            applicability,
+            tool_metadata: ToolMetadata::new(tool_metadata),
+        })
+    }
+
     pub fn set_span<S: Into<MultiSpan>>(&mut self, sp: S) -> &mut Self {
         self.span = sp.into();
         if let Some(span) = self.span.primary_span() {

Diff for: compiler/rustc_errors/src/json.rs

@@ -14,6 +14,7 @@ use rustc_span::source_map::{FilePathMapping, SourceMap};
 use crate::emitter::{Emitter, HumanReadableErrorType};
 use crate::registry::Registry;
 use crate::DiagnosticId;
+use crate::ToolMetadata;
 use crate::{CodeSuggestion, SubDiagnostic};
 use rustc_lint_defs::{Applicability, FutureBreakage};
 
@@ -26,6 +27,7 @@ use std::sync::{Arc, Mutex};
 use std::vec;
 
 use rustc_serialize::json::{as_json, as_pretty_json};
+use rustc_serialize::{Encodable, Encoder};
 
 #[cfg(test)]
 mod tests;
@@ -168,7 +170,8 @@ impl Emitter for JsonEmitter {
 
 // The following data types are provided just for serialisation.
 
-#[derive(Encodable)]
+// NOTE: this has a manual implementation of Encodable which needs to be updated in
+// parallel.
 struct Diagnostic {
     /// The primary error message.
     message: String,
@@ -180,6 +183,65 @@ struct Diagnostic {
     children: Vec<Diagnostic>,
     /// The message as rustc would render it.
     rendered: Option<String>,
+    /// Extra tool metadata
+    tool_metadata: ToolMetadata,
+}
+
+macro_rules! encode_fields {
+    (
+        $enc:expr,                  // encoder
+        $idx:expr,                  // starting field index
+        $struct:expr,               // struct we're serializing
+        $struct_name:ident,         // struct name
+        [ $($name:ident),+$(,)? ],  // fields to encode
+        [ $($ignore:ident),+$(,)? ] // fields we're skipping
+    ) => {
+        {
+            // Pattern match to make sure all fields are accounted for
+            let $struct_name { $($name,)+ $($ignore: _,)+ } = $struct;
+            let mut idx = $idx;
+            $(
+                $enc.emit_struct_field(
+                    stringify!($name),
+                    idx,
+                    |enc| $name.encode(enc),
+                )?;
+                idx += 1;
+            )+
+            idx
+        }
+    };
+}
+
+// Special-case encoder to skip tool_metadata if not set
+impl<E: Encoder> Encodable<E> for Diagnostic {
+    fn encode(&self, s: &mut E) -> Result<(), E::Error> {
+        s.emit_struct("diagnostic", 7, |s| {
+            let mut idx = 0;
+
+            idx = encode_fields!(
+                s,
+                idx,
+                self,
+                Self,
+                [message, code, level, spans, children, rendered],
+                [tool_metadata]
+            );
+            if self.tool_metadata.is_set() {
+                idx = encode_fields!(
+                    s,
+                    idx,
+                    self,
+                    Self,
+                    [tool_metadata],
+                    [message, code, level, spans, children, rendered]
+                );
+            }
+
+            let _ = idx;
+            Ok(())
+        })
+    }
 }
 
 #[derive(Encodable)]
@@ -269,6 +331,7 @@ impl Diagnostic {
             spans: DiagnosticSpan::from_suggestion(sugg, je),
             children: vec![],
             rendered: None,
+            tool_metadata: sugg.tool_metadata.clone(),
         });
 
         // generate regular command line output and store it in the json
@@ -312,6 +375,7 @@ impl Diagnostic {
                 .chain(sugg)
                 .collect(),
             rendered: Some(output),
+            tool_metadata: ToolMetadata::default(),
         }
     }
 
@@ -327,6 +391,7 @@ impl Diagnostic {
                 .unwrap_or_else(|| DiagnosticSpan::from_multispan(&diag.span, je)),
             children: vec![],
             rendered: None,
+            tool_metadata: ToolMetadata::default(),
         }
     }
 }

Diff for: compiler/rustc_errors/src/lib.rs

@@ -23,10 +23,13 @@ use rustc_data_structures::sync::{self, Lock, Lrc};
 use rustc_data_structures::AtomicRef;
 use rustc_lint_defs::FutureBreakage;
 pub use rustc_lint_defs::{pluralize, Applicability};
+use rustc_serialize::json::Json;
+use rustc_serialize::{Decodable, Decoder, Encodable, Encoder};
 use rustc_span::source_map::SourceMap;
 use rustc_span::{Loc, MultiSpan, Span};
 
 use std::borrow::Cow;
+use std::hash::{Hash, Hasher};
 use std::panic;
 use std::path::Path;
 use std::{error, fmt};
@@ -73,6 +76,39 @@ impl SuggestionStyle {
     }
 }
 
+#[derive(Clone, Debug, PartialEq, Default)]
+pub struct ToolMetadata(pub Option<Json>);
+
+impl ToolMetadata {
+    fn new(json: Json) -> Self {
+        ToolMetadata(Some(json))
+    }
+
+    fn is_set(&self) -> bool {
+        self.0.is_some()
+    }
+}
+
+impl Hash for ToolMetadata {
+    fn hash<H: Hasher>(&self, _state: &mut H) {}
+}
+
+// Doesn't really need to round-trip
+impl<D: Decoder> Decodable<D> for ToolMetadata {
+    fn decode(_d: &mut D) -> Result<Self, D::Error> {
+        Ok(ToolMetadata(None))
+    }
+}
+
+impl<S: Encoder> Encodable<S> for ToolMetadata {
+    fn encode(&self, e: &mut S) -> Result<(), S::Error> {
+        match &self.0 {
+            None => e.emit_unit(),
+            Some(json) => json.encode(e),
+        }
+    }
+}
+
 #[derive(Clone, Debug, PartialEq, Hash, Encodable, Decodable)]
 pub struct CodeSuggestion {
     /// Each substitute can have multiple variants due to multiple
@@ -106,6 +142,8 @@ pub struct CodeSuggestion {
     /// which are useful for users but not useful for
     /// tools like rustfix
     pub applicability: Applicability,
+    /// Tool-specific metadata
+    pub tool_metadata: ToolMetadata,
 }
 
 #[derive(Clone, Debug, PartialEq, Hash, Encodable, Decodable)]
@@ -775,7 +813,6 @@ impl HandlerInner {
         }
 
         let already_emitted = |this: &mut Self| {
-            use std::hash::Hash;
             let mut hasher = StableHasher::new();
             diagnostic.hash(&mut hasher);
             let diagnostic_hash = hasher.finish();

Diff for: compiler/rustc_lint/Cargo.toml

@@ -19,5 +19,6 @@ rustc_data_structures = { path = "../rustc_data_structures" }
 rustc_feature = { path = "../rustc_feature" }
 rustc_index = { path = "../rustc_index" }
 rustc_session = { path = "../rustc_session" }
+rustc_serialize = { path = "../rustc_serialize" }
 rustc_trait_selection = { path = "../rustc_trait_selection" }
 rustc_parse_format = { path = "../rustc_parse_format" }

Diff for: compiler/rustc_lint/src/context.rs

@@ -21,7 +21,9 @@ use crate::passes::{EarlyLintPassObject, LateLintPassObject};
 use rustc_ast as ast;
 use rustc_data_structures::fx::FxHashMap;
 use rustc_data_structures::sync;
-use rustc_errors::{add_elided_lifetime_in_path_suggestion, struct_span_err, Applicability};
+use rustc_errors::{
+    add_elided_lifetime_in_path_suggestion, struct_span_err, Applicability, SuggestionStyle,
+};
 use rustc_hir as hir;
 use rustc_hir::def::Res;
 use rustc_hir::def_id::{CrateNum, DefId};
@@ -32,7 +34,8 @@ use rustc_middle::middle::stability;
 use rustc_middle::ty::layout::{LayoutError, TyAndLayout};
 use rustc_middle::ty::print::with_no_trimmed_paths;
 use rustc_middle::ty::{self, print::Printer, subst::GenericArg, Ty, TyCtxt};
-use rustc_session::lint::BuiltinLintDiagnostics;
+use rustc_serialize::json::Json;
+use rustc_session::lint::{BuiltinLintDiagnostics, ExternDepSpec};
 use rustc_session::lint::{FutureIncompatibleInfo, Level, Lint, LintBuffer, LintId};
 use rustc_session::Session;
 use rustc_session::SessionLintStore;
@@ -639,6 +642,30 @@ pub trait LintContext: Sized {
                 BuiltinLintDiagnostics::LegacyDeriveHelpers(span) => {
                     db.span_label(span, "the attribute is introduced here");
                 }
+                BuiltinLintDiagnostics::ExternDepSpec(krate, loc) => {
+                    let json = match loc {
+                        ExternDepSpec::Json(json) => {
+                            db.help(&format!("remove unnecessary dependency `{}`", krate));
+                            json
+                        }
+                        ExternDepSpec::Raw(raw) => {
+                            db.help(&format!("remove unnecessary dependency `{}` at `{}`", krate, raw));
+                            db.span_suggestion_with_style(
+                                DUMMY_SP,
+                                "raw extern location",
+                                raw.clone(),
+                                Applicability::Unspecified,
+                                SuggestionStyle::CompletelyHidden,
+                            );
+                            Json::String(raw)
+                        }
+                    };
+                    db.tool_only_suggestion_with_metadata(
+                        "json extern location",
+                        Applicability::Unspecified,
+                        json
+                    );
+                }
             }
             // Rewrap `db`, and pass control to the user.
             decorate(LintDiagnosticBuilder::new(db));

Diff for: compiler/rustc_lint_defs/src/lib.rs

@@ -4,6 +4,7 @@ extern crate rustc_macros;
 pub use self::Level::*;
 use rustc_ast::node_id::{NodeId, NodeMap};
 use rustc_data_structures::stable_hasher::{HashStable, StableHasher, ToStableHashKey};
+use rustc_serialize::json::Json;
 use rustc_span::edition::Edition;
 use rustc_span::{sym, symbol::Ident, MultiSpan, Span, Symbol};
 use rustc_target::spec::abi::Abi;
@@ -239,6 +240,13 @@ impl<HCX> ToStableHashKey<HCX> for LintId {
     }
 }
 
+// Duplicated from rustc_session::config::ExternDepSpec to avoid cyclic dependency
+#[derive(PartialEq)]
+pub enum ExternDepSpec {
+    Json(Json),
+    Raw(String),
+}
+
 // This could be a closure, but then implementing derive trait
 // becomes hacky (and it gets allocated).
 #[derive(PartialEq)]
@@ -257,6 +265,7 @@ pub enum BuiltinLintDiagnostics {
     UnusedDocComment(Span),
     PatternsInFnsWithoutBody(Span, Ident),
     LegacyDeriveHelpers(Span),
+    ExternDepSpec(String, ExternDepSpec),
 }
 
 /// Lints that are buffered up early on in the `Session` before the