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 2e654fe9929a..40e4efb397d3 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 61674ada6fa6..2536fc648c70 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 3d50c95d3f4f..7df467ab0d4f 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 54c726d84621..0dec26e4f741 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 b9546143a054..9543d01597d0 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 2765239d5e64..d311076c39e8 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 0dec26e4f741..7a8717ada4c3 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 9a2560b04583..3e523fca92a0 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 7a8717ada4c3..681e14a61374 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 3b4c5da10f20..9181cca215c8 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 0e6e96438d81..743f22b6b314 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()