From 937bc2e04aaad37f97d367f7c5073c352e357bb5 Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Thu, 18 Jan 2018 17:17:46 +0530 Subject: [PATCH 1/3] Add approximate suggestions for rustfix This adds `span_approximate_suggestion()` that lets you emit a suggestion marked as "approximate" in the JSON output. UI users see no difference. This is for when rustc and clippy wish to emit suggestions which will make sense to the reader (e.g. they may have placeholders like ``) but are not source-applicable, so that rustfix/etc can ignore these. fixes #39254 --- src/librustc_errors/diagnostic.rs | 37 +++++++++++++++++++++++ src/librustc_errors/diagnostic_builder.rs | 10 ++++++ src/librustc_errors/lib.rs | 6 ++++ src/libsyntax/json.rs | 18 ++++++----- 4 files changed, 64 insertions(+), 7 deletions(-) diff --git a/src/librustc_errors/diagnostic.rs b/src/librustc_errors/diagnostic.rs index 2e654fe9929a6..40e4efb397d30 100644 --- a/src/librustc_errors/diagnostic.rs +++ b/src/librustc_errors/diagnostic.rs @@ -222,6 +222,7 @@ impl Diagnostic { }], msg: msg.to_owned(), show_code_when_inline: false, + approximate: false, }); self } @@ -252,6 +253,7 @@ impl Diagnostic { }], msg: msg.to_owned(), show_code_when_inline: true, + approximate: false, }); self } @@ -267,6 +269,41 @@ impl Diagnostic { }).collect(), msg: msg.to_owned(), show_code_when_inline: true, + approximate: false, + }); + self + } + + /// This is a suggestion that may contain mistakes or fillers and should + /// be read and understood by a human. + pub fn span_approximate_suggestion(&mut self, sp: Span, msg: &str, + suggestion: String) -> &mut Self { + self.suggestions.push(CodeSuggestion { + substitutions: vec![Substitution { + parts: vec![SubstitutionPart { + snippet: suggestion, + span: sp, + }], + }], + msg: msg.to_owned(), + show_code_when_inline: true, + approximate: true, + }); + self + } + + pub fn span_approximate_suggestions(&mut self, sp: Span, msg: &str, + suggestions: Vec) -> &mut Self { + self.suggestions.push(CodeSuggestion { + substitutions: suggestions.into_iter().map(|snippet| Substitution { + parts: vec![SubstitutionPart { + snippet, + span: sp, + }], + }).collect(), + msg: msg.to_owned(), + show_code_when_inline: true, + approximate: true, }); self } diff --git a/src/librustc_errors/diagnostic_builder.rs b/src/librustc_errors/diagnostic_builder.rs index 61674ada6fa63..2536fc648c70a 100644 --- a/src/librustc_errors/diagnostic_builder.rs +++ b/src/librustc_errors/diagnostic_builder.rs @@ -186,6 +186,16 @@ impl<'a> DiagnosticBuilder<'a> { msg: &str, suggestions: Vec) -> &mut Self); + forward!(pub fn span_approximate_suggestion(&mut self, + sp: Span, + msg: &str, + suggestion: String) + -> &mut Self); + forward!(pub fn span_approximate_suggestions(&mut self, + sp: Span, + msg: &str, + suggestions: Vec) + -> &mut Self); forward!(pub fn set_span>(&mut self, sp: S) -> &mut Self); forward!(pub fn code(&mut self, s: DiagnosticId) -> &mut Self); diff --git a/src/librustc_errors/lib.rs b/src/librustc_errors/lib.rs index 3d50c95d3f4f9..7df467ab0d4f4 100644 --- a/src/librustc_errors/lib.rs +++ b/src/librustc_errors/lib.rs @@ -83,6 +83,12 @@ pub struct CodeSuggestion { pub substitutions: Vec, pub msg: String, pub show_code_when_inline: bool, + /// Whether or not the suggestion is approximate + /// + /// Sometimes we may show suggestions with placeholders, + /// which are useful for users but not useful for + /// tools like rustfix + pub approximate: bool, } #[derive(Clone, Debug, PartialEq, Hash, RustcEncodable, RustcDecodable)] diff --git a/src/libsyntax/json.rs b/src/libsyntax/json.rs index 54c726d84621f..0dec26e4f741b 100644 --- a/src/libsyntax/json.rs +++ b/src/libsyntax/json.rs @@ -121,6 +121,8 @@ struct DiagnosticSpan { /// If we are suggesting a replacement, this will contain text /// that should be sliced in atop this span. suggested_replacement: Option, + /// If the suggestion is approximate + suggestion_approximate: Option, /// Macro invocations that created the code at this span, if any. expansion: Option>, } @@ -220,7 +222,7 @@ impl Diagnostic { impl DiagnosticSpan { fn from_span_label(span: SpanLabel, - suggestion: Option<&String>, + suggestion: Option<(&String, bool)>, je: &JsonEmitter) -> DiagnosticSpan { Self::from_span_etc(span.span, @@ -233,7 +235,7 @@ impl DiagnosticSpan { fn from_span_etc(span: Span, is_primary: bool, label: Option, - suggestion: Option<&String>, + suggestion: Option<(&String, bool)>, je: &JsonEmitter) -> DiagnosticSpan { // obtain the full backtrace from the `macro_backtrace` @@ -253,7 +255,7 @@ impl DiagnosticSpan { fn from_span_full(span: Span, is_primary: bool, label: Option, - suggestion: Option<&String>, + suggestion: Option<(&String, bool)>, mut backtrace: vec::IntoIter, je: &JsonEmitter) -> DiagnosticSpan { @@ -291,7 +293,8 @@ impl DiagnosticSpan { column_end: end.col.0 + 1, is_primary, text: DiagnosticSpanLine::from_span(span, je), - suggested_replacement: suggestion.cloned(), + suggested_replacement: suggestion.map(|x| x.0.clone()), + suggestion_approximate: suggestion.map(|x| x.1), expansion: backtrace_step, label, } @@ -309,14 +312,15 @@ impl DiagnosticSpan { suggestion.substitutions .iter() .flat_map(|substitution| { - substitution.parts.iter().map(move |suggestion| { + substitution.parts.iter().map(move |suggestion_inner| { let span_label = SpanLabel { - span: suggestion.span, + span: suggestion_inner.span, is_primary: true, label: None, }; DiagnosticSpan::from_span_label(span_label, - Some(&suggestion.snippet), + Some((&suggestion_inner.snippet, + suggestion.approximate)), je) }) }) From a53bdc6212b9abf8538a877ebfde214120bf061f Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Fri, 19 Jan 2018 13:04:14 +0530 Subject: [PATCH 2/3] Add -Zapproximate-suggestions --- src/librustc/session/config.rs | 2 ++ src/librustc/session/mod.rs | 6 ++++-- src/libsyntax/json.rs | 22 ++++++++++++++++++---- 3 files changed, 24 insertions(+), 6 deletions(-) diff --git a/src/librustc/session/config.rs b/src/librustc/session/config.rs index b9546143a054b..9543d01597d04 100644 --- a/src/librustc/session/config.rs +++ b/src/librustc/session/config.rs @@ -1288,6 +1288,8 @@ options! {DebuggingOptions, DebuggingSetter, basic_debugging_options, dep_info_omit_d_target: bool = (false, parse_bool, [TRACKED], "in dep-info output, omit targets for tracking dependencies of the dep-info files \ themselves"), + approximate_suggestions: bool = (false, parse_bool, [UNTRACKED], + "include machine-applicability of suggestions in JSON output"), unpretty: Option = (None, parse_unpretty, [UNTRACKED], "Present the input source, unstable (and less-pretty) variants; valid types are any of the types for `--pretty`, as well as: diff --git a/src/librustc/session/mod.rs b/src/librustc/session/mod.rs index 2765239d5e649..d311076c39e8d 100644 --- a/src/librustc/session/mod.rs +++ b/src/librustc/session/mod.rs @@ -910,10 +910,12 @@ pub fn build_session_with_codemap(sopts: config::Options, Box::new(EmitterWriter::new(dst, Some(codemap.clone()), false)) } (config::ErrorOutputType::Json(pretty), None) => { - Box::new(JsonEmitter::stderr(Some(registry), codemap.clone(), pretty)) + Box::new(JsonEmitter::stderr(Some(registry), codemap.clone(), + pretty, sopts.debugging_opts.approximate_suggestions)) } (config::ErrorOutputType::Json(pretty), Some(dst)) => { - Box::new(JsonEmitter::new(dst, Some(registry), codemap.clone(), pretty)) + Box::new(JsonEmitter::new(dst, Some(registry), codemap.clone(), + pretty, sopts.debugging_opts.approximate_suggestions)) } (config::ErrorOutputType::Short(color_config), None) => { Box::new(EmitterWriter::stderr(color_config, Some(codemap.clone()), true)) diff --git a/src/libsyntax/json.rs b/src/libsyntax/json.rs index 0dec26e4f741b..7a8717ada4c3f 100644 --- a/src/libsyntax/json.rs +++ b/src/libsyntax/json.rs @@ -38,34 +38,41 @@ pub struct JsonEmitter { registry: Option, cm: Rc, pretty: bool, + /// Whether "approximate suggestions" are enabled in the config + approximate_suggestions: bool, } impl JsonEmitter { pub fn stderr(registry: Option, code_map: Rc, - pretty: bool) -> JsonEmitter { + pretty: bool, + approximate_suggestions: bool) -> JsonEmitter { JsonEmitter { dst: Box::new(io::stderr()), registry, cm: code_map, pretty, + approximate_suggestions, } } pub fn basic(pretty: bool) -> JsonEmitter { let file_path_mapping = FilePathMapping::empty(); - JsonEmitter::stderr(None, Rc::new(CodeMap::new(file_path_mapping)), pretty) + JsonEmitter::stderr(None, Rc::new(CodeMap::new(file_path_mapping)), + pretty, false) } pub fn new(dst: Box, registry: Option, code_map: Rc, - pretty: bool) -> JsonEmitter { + pretty: bool, + approximate_suggestions: bool) -> JsonEmitter { JsonEmitter { dst, registry, cm: code_map, pretty, + approximate_suggestions, } } } @@ -283,6 +290,13 @@ impl DiagnosticSpan { def_site_span, }) }); + + let suggestion_approximate = if je.approximate_suggestions { + suggestion.map(|x| x.1) + } else { + None + }; + DiagnosticSpan { file_name: start.file.name.to_string(), byte_start: span.lo().0 - start.file.start_pos.0, @@ -294,7 +308,7 @@ impl DiagnosticSpan { is_primary, text: DiagnosticSpanLine::from_span(span, je), suggested_replacement: suggestion.map(|x| x.0.clone()), - suggestion_approximate: suggestion.map(|x| x.1), + suggestion_approximate, expansion: backtrace_step, label, } From 540f95d9fad41a605e4c8b898d77f47374a76cbd Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Wed, 24 Jan 2018 11:50:30 +0530 Subject: [PATCH 3/3] Add internal-only rustc_serialize_exclude_null attribute for making the field only exist in the json if the flag is passed --- src/libsyntax/feature_gate.rs | 5 +++++ src/libsyntax/json.rs | 2 ++ src/libsyntax/lib.rs | 1 + src/libsyntax_ext/deriving/encodable.rs | 16 ++++++++++++++-- 4 files changed, 22 insertions(+), 2 deletions(-) diff --git a/src/libsyntax/feature_gate.rs b/src/libsyntax/feature_gate.rs index 9a2560b04583d..3e523fca92a03 100644 --- a/src/libsyntax/feature_gate.rs +++ b/src/libsyntax/feature_gate.rs @@ -788,6 +788,11 @@ pub const BUILTIN_ATTRIBUTES: &'static [(&'static str, AttributeType, AttributeG is just used for rustc unit tests \ and will never be stable", cfg_fn!(rustc_attrs))), + ("rustc_serialize_exclude_null", Normal, Gated(Stability::Unstable, + "rustc_attrs", + "the `#[rustc_serialize_exclude_null]` attribute \ + is an internal-only feature", + cfg_fn!(rustc_attrs))), ("rustc_synthetic", Whitelisted, Gated(Stability::Unstable, "rustc_attrs", "this attribute \ diff --git a/src/libsyntax/json.rs b/src/libsyntax/json.rs index 7a8717ada4c3f..681e14a613740 100644 --- a/src/libsyntax/json.rs +++ b/src/libsyntax/json.rs @@ -108,6 +108,7 @@ struct Diagnostic { } #[derive(RustcEncodable)] +#[allow(unused_attributes)] struct DiagnosticSpan { file_name: String, byte_start: u32, @@ -129,6 +130,7 @@ struct DiagnosticSpan { /// that should be sliced in atop this span. suggested_replacement: Option, /// If the suggestion is approximate + #[rustc_serialize_exclude_null] suggestion_approximate: Option, /// Macro invocations that created the code at this span, if any. expansion: Option>, diff --git a/src/libsyntax/lib.rs b/src/libsyntax/lib.rs index 3b4c5da10f20b..9181cca215c84 100644 --- a/src/libsyntax/lib.rs +++ b/src/libsyntax/lib.rs @@ -25,6 +25,7 @@ #![feature(match_default_bindings)] #![feature(i128_type)] #![feature(const_atomic_usize_new)] +#![feature(rustc_attrs)] // See librustc_cratesio_shim/Cargo.toml for a comment explaining this. #[allow(unused_extern_crates)] diff --git a/src/libsyntax_ext/deriving/encodable.rs b/src/libsyntax_ext/deriving/encodable.rs index 0e6e96438d817..743f22b6b3140 100644 --- a/src/libsyntax_ext/deriving/encodable.rs +++ b/src/libsyntax_ext/deriving/encodable.rs @@ -190,7 +190,7 @@ fn encodable_substructure(cx: &mut ExtCtxt, Struct(_, ref fields) => { let emit_struct_field = cx.ident_of("emit_struct_field"); let mut stmts = Vec::new(); - for (i, &FieldInfo { name, ref self_, span, .. }) in fields.iter().enumerate() { + for (i, &FieldInfo { name, ref self_, span, attrs, .. }) in fields.iter().enumerate() { let name = match name { Some(id) => id.name, None => Symbol::intern(&format!("_field{}", i)), @@ -212,7 +212,19 @@ fn encodable_substructure(cx: &mut ExtCtxt, } else { cx.expr(span, ExprKind::Ret(Some(call))) }; - stmts.push(cx.stmt_expr(call)); + + // This exists for https://github.com/rust-lang/rust/pull/47540 + // + // If we decide to stabilize that flag this can be removed + let expr = if attrs.iter().any(|a| a.check_name("rustc_serialize_exclude_null")) { + let is_some = cx.ident_of("is_some"); + let condition = cx.expr_method_call(span, self_.clone(), is_some, vec![]); + cx.expr_if(span, condition, call, None) + } else { + call + }; + let stmt = cx.stmt_expr(expr); + stmts.push(stmt); } // unit structs have no fields and need to return Ok()