Auto merge of #52756 - alexcrichton:cap-applicable, r=oli-obk

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

@@ -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()
         }
     }
 

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<String>)
                                      -> &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<String>,
-                                                 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<String>,
+                                               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<S: Into<MultiSpan>>(&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,
+        }
     }
 }
 

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 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, 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()
+}

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 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, 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();
+}

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 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, 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();
+}

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
+

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<String> {
         self.parse_name_value_directive(line, "edition")
     }

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);
             }
         }