Skip to content

Commit

Permalink
Auto merge of rust-lang#72603 - jsgf:extern-loc, r=nikomatsakis
Browse files Browse the repository at this point in the history
Implement `--extern-location`

This PR implements `--extern-location` as a followup to rust-lang#72342 as part of the implementation of rust-lang#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 rust-lang#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`
  • Loading branch information
bors committed Feb 8, 2021
2 parents bb587b1 + 91d8c3b commit 0b7a598
Show file tree
Hide file tree
Showing 37 changed files with 534 additions and 10 deletions.
5 changes: 3 additions & 2 deletions Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -3914,6 +3914,7 @@ dependencies = [
"rustc_index",
"rustc_middle",
"rustc_parse_format",
"rustc_serialize",
"rustc_session",
"rustc_span",
"rustc_target",
Expand Down
24 changes: 24 additions & 0 deletions compiler/rustc_errors/src/diagnostic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -303,6 +305,7 @@ impl Diagnostic {
msg: msg.to_owned(),
style: SuggestionStyle::ShowCode,
applicability,
tool_metadata: Default::default(),
});
self
}
Expand All @@ -328,6 +331,7 @@ impl Diagnostic {
msg: msg.to_owned(),
style: SuggestionStyle::ShowCode,
applicability,
tool_metadata: Default::default(),
});
self
}
Expand All @@ -354,6 +358,7 @@ impl Diagnostic {
msg: msg.to_owned(),
style: SuggestionStyle::CompletelyHidden,
applicability,
tool_metadata: Default::default(),
});
self
}
Expand Down Expand Up @@ -408,6 +413,7 @@ impl Diagnostic {
msg: msg.to_owned(),
style,
applicability,
tool_metadata: Default::default(),
});
self
}
Expand Down Expand Up @@ -446,6 +452,7 @@ impl Diagnostic {
msg: msg.to_owned(),
style: SuggestionStyle::ShowCode,
applicability,
tool_metadata: Default::default(),
});
self
}
Expand Down Expand Up @@ -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() {
Expand Down
67 changes: 66 additions & 1 deletion compiler/rustc_errors/src/json.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand All @@ -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;
Expand Down Expand Up @@ -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,
Expand All @@ -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)]
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -312,6 +375,7 @@ impl Diagnostic {
.chain(sugg)
.collect(),
rendered: Some(output),
tool_metadata: ToolMetadata::default(),
}
}

Expand All @@ -327,6 +391,7 @@ impl Diagnostic {
.unwrap_or_else(|| DiagnosticSpan::from_multispan(&diag.span, je)),
children: vec![],
rendered: None,
tool_metadata: ToolMetadata::default(),
}
}
}
Expand Down
39 changes: 38 additions & 1 deletion compiler/rustc_errors/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)]
Expand Down Expand Up @@ -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();
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_lint/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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" }
31 changes: 29 additions & 2 deletions compiler/rustc_lint/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand All @@ -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;
Expand Down Expand Up @@ -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));
Expand Down
9 changes: 9 additions & 0 deletions compiler/rustc_lint_defs/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)]
Expand All @@ -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
Expand Down
Loading

0 comments on commit 0b7a598

Please sign in to comment.