From ca762ba9547649f57e2d8a3e56b83d0a6298fbb2 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Thu, 26 Jul 2018 14:53:15 -0700 Subject: [PATCH] rustc: Disallow machine applicability in foreign macros Recent changes to lints disallowed lints from being emitted against code located in foreign macros, except for future-incompatible lints. For a future incompatible lint, however, the automatic suggestions may not be applicable! This commit updates this code path to force all applicability suggestions made to foreign macros to never be `MachineApplicable`. This should avoid rustfix actually attempting fixing these suggestions, causing non-compiling code to be produced. Closes rust-lang/cargo#5799 --- src/librustc/lint/mod.rs | 30 +++---- src/librustc_errors/diagnostic_builder.rs | 83 ++++++++++++++----- .../suggestions-not-always-applicable.rs | 22 +++++ .../suggestions-not-always-applicable.fixed | 37 +++++++++ .../suggestions-not-always-applicable.rs | 37 +++++++++ .../suggestions-not-always-applicable.stderr | 24 ++++++ src/tools/compiletest/src/header.rs | 11 +++ src/tools/compiletest/src/runtest.rs | 8 +- 8 files changed, 217 insertions(+), 35 deletions(-) create mode 100644 src/test/ui-fulldeps/rust-2018/auxiliary/suggestions-not-always-applicable.rs create mode 100644 src/test/ui-fulldeps/rust-2018/suggestions-not-always-applicable.fixed create mode 100644 src/test/ui-fulldeps/rust-2018/suggestions-not-always-applicable.rs create mode 100644 src/test/ui-fulldeps/rust-2018/suggestions-not-always-applicable.stderr diff --git a/src/librustc/lint/mod.rs b/src/librustc/lint/mod.rs index 3c1b205620892..96b7d865dc8ba 100644 --- a/src/librustc/lint/mod.rs +++ b/src/librustc/lint/mod.rs @@ -575,7 +575,8 @@ pub fn struct_lint_level<'a>(sess: &'a Session, // Check for future incompatibility lints and issue a stronger warning. let lints = sess.lint_store.borrow(); let lint_id = LintId::of(lint); - if let Some(future_incompatible) = lints.future_incompatible(lint_id) { + let future_incompatible = lints.future_incompatible(lint_id); + if let Some(future_incompatible) = future_incompatible { const STANDARD_MESSAGE: &str = "this was previously accepted by the compiler but is being phased out; \ it will become a hard error"; @@ -593,20 +594,21 @@ pub fn struct_lint_level<'a>(sess: &'a Session, future_incompatible.reference); err.warn(&explanation); err.note(&citation); + } - // If this lint is *not* a future incompatibility warning then we want to be - // sure to not be too noisy in some situations. If this code originates in a - // foreign macro, aka something that this crate did not itself author, then - // it's likely that there's nothing this crate can do about it. We probably - // want to skip the lint entirely. - // - // For some lints though (like unreachable code) there's clear actionable - // items to take care of (delete the macro invocation). As a result we have - // a few lints we whitelist here for allowing a lint even though it's in a - // foreign macro invocation. - } else if !lint.report_in_external_macro { - if err.span.primary_spans().iter().any(|s| in_external_macro(sess, *s)) { - err.cancel(); + // If this code originates in a foreign macro, aka something that this crate + // did not itself author, then it's likely that there's nothing this crate + // can do about it. We probably want to skip the lint entirely. + if err.span.primary_spans().iter().any(|s| in_external_macro(sess, *s)) { + // Any suggestions made here are likely to be incorrect, so anything we + // emit shouldn't be automatically fixed by rustfix. + err.allow_suggestions(false); + + // If this is a future incompatible lint it'll become a hard error, so + // we have to emit *something*. Also allow lints to whitelist themselves + // on a case-by-case basis for emission in a foreign macro. + if future_incompatible.is_none() && !lint.report_in_external_macro { + err.cancel() } } diff --git a/src/librustc_errors/diagnostic_builder.rs b/src/librustc_errors/diagnostic_builder.rs index a0f3abda077f9..1b34898b99084 100644 --- a/src/librustc_errors/diagnostic_builder.rs +++ b/src/librustc_errors/diagnostic_builder.rs @@ -26,6 +26,7 @@ use syntax_pos::{MultiSpan, Span}; pub struct DiagnosticBuilder<'a> { pub handler: &'a Handler, diagnostic: Diagnostic, + allow_suggestions: bool, } /// In general, the `DiagnosticBuilder` uses deref to allow access to @@ -186,27 +187,67 @@ impl<'a> DiagnosticBuilder<'a> { msg: &str, suggestions: Vec) -> &mut Self); - forward!(pub fn span_suggestion_with_applicability(&mut self, - sp: Span, - msg: &str, - suggestion: String, - applicability: Applicability) - -> &mut Self); - forward!(pub fn span_suggestions_with_applicability(&mut self, - sp: Span, - msg: &str, - suggestions: Vec, - applicability: Applicability) - -> &mut Self); - forward!(pub fn span_suggestion_short_with_applicability(&mut self, - sp: Span, - msg: &str, - suggestion: String, - applicability: Applicability) - -> &mut Self); + pub fn span_suggestion_with_applicability(&mut self, + sp: Span, + msg: &str, + suggestion: String, + applicability: Applicability) + -> &mut Self { + if !self.allow_suggestions { + return self + } + self.diagnostic.span_suggestion_with_applicability( + sp, + msg, + suggestion, + applicability, + ); + self + } + + pub fn span_suggestions_with_applicability(&mut self, + sp: Span, + msg: &str, + suggestions: Vec, + applicability: Applicability) + -> &mut Self { + if !self.allow_suggestions { + return self + } + self.diagnostic.span_suggestions_with_applicability( + sp, + msg, + suggestions, + applicability, + ); + self + } + + pub fn span_suggestion_short_with_applicability(&mut self, + sp: Span, + msg: &str, + suggestion: String, + applicability: Applicability) + -> &mut Self { + if !self.allow_suggestions { + return self + } + self.diagnostic.span_suggestion_short_with_applicability( + sp, + msg, + suggestion, + applicability, + ); + self + } forward!(pub fn set_span>(&mut self, sp: S) -> &mut Self); forward!(pub fn code(&mut self, s: DiagnosticId) -> &mut Self); + pub fn allow_suggestions(&mut self, allow: bool) -> &mut Self { + self.allow_suggestions = allow; + self + } + /// Convenience function for internal use, clients should use one of the /// struct_* methods on Handler. pub fn new(handler: &'a Handler, level: Level, message: &str) -> DiagnosticBuilder<'a> { @@ -228,7 +269,11 @@ impl<'a> DiagnosticBuilder<'a> { /// diagnostic. pub fn new_diagnostic(handler: &'a Handler, diagnostic: Diagnostic) -> DiagnosticBuilder<'a> { - DiagnosticBuilder { handler, diagnostic } + DiagnosticBuilder { + handler, + diagnostic, + allow_suggestions: true, + } } } diff --git a/src/test/ui-fulldeps/rust-2018/auxiliary/suggestions-not-always-applicable.rs b/src/test/ui-fulldeps/rust-2018/auxiliary/suggestions-not-always-applicable.rs new file mode 100644 index 0000000000000..7ae4731fde196 --- /dev/null +++ b/src/test/ui-fulldeps/rust-2018/auxiliary/suggestions-not-always-applicable.rs @@ -0,0 +1,22 @@ +// Copyright 2018 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// no-prefer-dynamic + +#![crate_type = "proc-macro"] + +extern crate proc_macro; + +use proc_macro::*; + +#[proc_macro_attribute] +pub fn foo(_attr: TokenStream, _f: TokenStream) -> TokenStream { + "pub fn foo() -> ::Foo { ::Foo }".parse().unwrap() +} diff --git a/src/test/ui-fulldeps/rust-2018/suggestions-not-always-applicable.fixed b/src/test/ui-fulldeps/rust-2018/suggestions-not-always-applicable.fixed new file mode 100644 index 0000000000000..e5b47c70863e5 --- /dev/null +++ b/src/test/ui-fulldeps/rust-2018/suggestions-not-always-applicable.fixed @@ -0,0 +1,37 @@ +// Copyright 2018 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// aux-build:suggestions-not-always-applicable.rs +// compile-flags: --edition 2015 +// run-rustfix +// rustfix-only-machine-applicable +// compile-pass + +#![feature(rust_2018_preview)] +#![warn(rust_2018_compatibility)] + +extern crate suggestions_not_always_applicable as foo; + +pub struct Foo; + +mod test { + use crate::foo::foo; + + #[foo] //~ WARN: absolute paths must start with + //~| WARN: previously accepted + //~| WARN: absolute paths + //~| WARN: previously accepted + fn main() { + } +} + +fn main() { + test::foo(); +} diff --git a/src/test/ui-fulldeps/rust-2018/suggestions-not-always-applicable.rs b/src/test/ui-fulldeps/rust-2018/suggestions-not-always-applicable.rs new file mode 100644 index 0000000000000..e5b47c70863e5 --- /dev/null +++ b/src/test/ui-fulldeps/rust-2018/suggestions-not-always-applicable.rs @@ -0,0 +1,37 @@ +// Copyright 2018 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// aux-build:suggestions-not-always-applicable.rs +// compile-flags: --edition 2015 +// run-rustfix +// rustfix-only-machine-applicable +// compile-pass + +#![feature(rust_2018_preview)] +#![warn(rust_2018_compatibility)] + +extern crate suggestions_not_always_applicable as foo; + +pub struct Foo; + +mod test { + use crate::foo::foo; + + #[foo] //~ WARN: absolute paths must start with + //~| WARN: previously accepted + //~| WARN: absolute paths + //~| WARN: previously accepted + fn main() { + } +} + +fn main() { + test::foo(); +} diff --git a/src/test/ui-fulldeps/rust-2018/suggestions-not-always-applicable.stderr b/src/test/ui-fulldeps/rust-2018/suggestions-not-always-applicable.stderr new file mode 100644 index 0000000000000..76dc139b58f07 --- /dev/null +++ b/src/test/ui-fulldeps/rust-2018/suggestions-not-always-applicable.stderr @@ -0,0 +1,24 @@ +warning: absolute paths must start with `self`, `super`, `crate`, or an external crate name in the 2018 edition + --> $DIR/suggestions-not-always-applicable.rs:27:5 + | +LL | #[foo] //~ WARN: absolute paths must start with + | ^^^^^^ + | +note: lint level defined here + --> $DIR/suggestions-not-always-applicable.rs:18:9 + | +LL | #![warn(rust_2018_compatibility)] + | ^^^^^^^^^^^^^^^^^^^^^^^ + = note: #[warn(absolute_paths_not_starting_with_crate)] implied by #[warn(rust_2018_compatibility)] + = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in the 2018 edition! + = note: for more information, see issue TBD + +warning: absolute paths must start with `self`, `super`, `crate`, or an external crate name in the 2018 edition + --> $DIR/suggestions-not-always-applicable.rs:27:5 + | +LL | #[foo] //~ WARN: absolute paths must start with + | ^^^^^^ + | + = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in the 2018 edition! + = note: for more information, see issue TBD + diff --git a/src/tools/compiletest/src/header.rs b/src/tools/compiletest/src/header.rs index d77261f495972..5f68d00eab1d4 100644 --- a/src/tools/compiletest/src/header.rs +++ b/src/tools/compiletest/src/header.rs @@ -231,6 +231,7 @@ pub struct TestProps { pub normalize_stderr: Vec<(String, String)>, pub failure_status: i32, pub run_rustfix: bool, + pub rustfix_only_machine_applicable: bool, } impl TestProps { @@ -263,6 +264,7 @@ impl TestProps { normalize_stderr: vec![], failure_status: -1, run_rustfix: false, + rustfix_only_machine_applicable: false, } } @@ -397,6 +399,11 @@ impl TestProps { if !self.run_rustfix { self.run_rustfix = config.parse_run_rustfix(ln); } + + if !self.rustfix_only_machine_applicable { + self.rustfix_only_machine_applicable = + config.parse_rustfix_only_machine_applicable(ln); + } }); if self.failure_status == -1 { @@ -663,6 +670,10 @@ impl Config { self.parse_name_directive(line, "run-rustfix") } + fn parse_rustfix_only_machine_applicable(&self, line: &str) -> bool { + self.parse_name_directive(line, "rustfix-only-machine-applicable") + } + fn parse_edition(&self, line: &str) -> Option { self.parse_name_value_directive(line, "edition") } diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index ce7828144cd0c..c8f3956415d53 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -2624,7 +2624,11 @@ impl<'test> TestCx<'test> { let suggestions = get_suggestions_from_json( &proc_res.stderr, &HashSet::new(), - Filter::Everything, + if self.props.rustfix_only_machine_applicable { + Filter::MachineApplicableOnly + } else { + Filter::Everything + }, ).unwrap(); let fixed_code = apply_suggestions(&unfixed_code, &suggestions).expect(&format!( "failed to apply suggestions for {:?} with rustfix", @@ -2686,7 +2690,7 @@ impl<'test> TestCx<'test> { if !res.status.success() { self.fatal_proc_rec("failed to compile fixed code", &res); } - if !res.stderr.is_empty() { + if !res.stderr.is_empty() && !self.props.rustfix_only_machine_applicable { self.fatal_proc_rec("fixed code is still producing diagnostics", &res); } }