about summary refs log tree commit diff
diff options
context:
space:
mode:
authorAlex Crichton <alex@alexcrichton.com>2018-07-17 10:23:03 -0700
committerAlex Crichton <alex@alexcrichton.com>2018-07-17 12:20:38 -0700
commit8adf08c4373f5bdd5bbef9aa4dfd0ca5c4a2eefc (patch)
tree8dbf258f67a4c79bd1af2c86d6f907079c676659
parentdd0808dd24bbb351d19b805a9318eb9e105010b2 (diff)
downloadrust-8adf08c4373f5bdd5bbef9aa4dfd0ca5c4a2eefc.tar.gz
rust-8adf08c4373f5bdd5bbef9aa4dfd0ca5c4a2eefc.zip
rustc: Polish off `in_external_macro`
This commit polishes off this new function to compile on newer rustc as well as
update and add a suite of test cases to work with this new check for lints.
-rw-r--r--src/librustc/lint/context.rs11
-rw-r--r--src/librustc/lint/mod.rs68
-rw-r--r--src/test/ui/lint/auxiliary/lints-in-foreign-macros.rs (renamed from src/test/ui/in-band-lifetimes/ellided-lifetimes-macro-checks.rs)17
-rw-r--r--src/test/ui/lint/lints-in-foreign-macros.rs28
-rw-r--r--src/test/ui/lint/lints-in-foreign-macros.stderr27
5 files changed, 112 insertions, 39 deletions
diff --git a/src/librustc/lint/context.rs b/src/librustc/lint/context.rs
index a5517e85d44..2a44845b980 100644
--- a/src/librustc/lint/context.rs
+++ b/src/librustc/lint/context.rs
@@ -29,7 +29,7 @@ use self::TargetLint::*;
 use std::slice;
 use rustc_data_structures::sync::{RwLock, ReadGuard};
 use lint::{EarlyLintPassObject, LateLintPassObject};
-use lint::{self, Level, Lint, LintId, LintPass, LintBuffer};
+use lint::{Level, Lint, LintId, LintPass, LintBuffer};
 use lint::builtin::BuiltinLintDiagnostics;
 use lint::levels::{LintLevelSets, LintLevelsBuilder};
 use middle::privacy::AccessLevels;
@@ -468,14 +468,7 @@ pub trait LintContext<'tcx>: Sized {
 
     /// Emit a lint at the appropriate level, for a particular span.
     fn span_lint<S: Into<MultiSpan>>(&self, lint: &'static Lint, span: S, msg: &str) {
-        match self.lints().future_incompatible(LintId::of(lint)) {
-            Some(_) => self.lookup_and_emit(lint, Some(span), msg),
-            None => {
-                if !lint::in_external_macro(lint, span) {
-                    self.lookup_and_emit(lint, Some(span), msg);
-                }
-            }
-        }
+        self.lookup_and_emit(lint, Some(span), msg);
     }
 
     fn struct_span_lint<S: Into<MultiSpan>>(&self,
diff --git a/src/librustc/lint/mod.rs b/src/librustc/lint/mod.rs
index 62a39873e76..b13423552c0 100644
--- a/src/librustc/lint/mod.rs
+++ b/src/librustc/lint/mod.rs
@@ -41,7 +41,7 @@ use lint::builtin::BuiltinLintDiagnostics;
 use session::{Session, DiagnosticMessageId};
 use std::hash;
 use syntax::ast;
-use syntax::codemap::MultiSpan;
+use syntax::codemap::{MultiSpan, ExpnFormat};
 use syntax::edition::Edition;
 use syntax::symbol::Symbol;
 use syntax::visit as ast_visit;
@@ -54,8 +54,6 @@ pub use lint::context::{LateContext, EarlyContext, LintContext, LintStore,
                         check_crate, check_ast_crate,
                         FutureIncompatibleInfo, BufferedEarlyLint};
 
-use codemap::{ExpnFormat, ExpnInfo, Span };
-
 /// Specification of a single lint.
 #[derive(Copy, Clone, Debug)]
 pub struct Lint {
@@ -570,6 +568,22 @@ 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_id != LintId::of(builtin::UNREACHABLE_CODE) &&
+        lint_id != LintId::of(builtin::DEPRECATED) {
+        if err.span.primary_spans().iter().any(|s| in_external_macro(sess, *s)) {
+            err.cancel();
+        }
     }
 
     return err
@@ -672,29 +686,31 @@ pub fn provide(providers: &mut Providers) {
     providers.lint_levels = lint_levels;
 }
 
-pub fn in_external_macro<'a, T: LintContext<'a>>(cx: &T, span: Span) -> bool {
-    /// Invokes `in_macro` with the expansion info of the given span slightly
-    /// heavy, try to use
-    /// this after other checks have already happened.
-    fn in_macro_ext<'a, T: LintContext<'a>>(cx: &T, info: &ExpnInfo) -> bool {
-        // no ExpnInfo = no macro
-        if let ExpnFormat::MacroAttribute(..) = info.callee.format {
-            // these are all plugins
-            return true;
-        }
-        // no span for the callee = external macro
-        info.callee.span.map_or(true, |span| {
-            // no snippet = external macro or compiler-builtin expansion
-            cx.sess()
-                .codemap()
-                .span_to_snippet(span)
-                .ok()
-                .map_or(true, |code| !code.starts_with("macro_rules"))
-        })
+/// Returns whether `span` originates in a foreign crate's external macro.
+///
+/// This is used to test whether a lint should be entirely aborted above.
+pub fn in_external_macro(sess: &Session, span: Span) -> bool {
+    let info = match span.ctxt().outer().expn_info() {
+        Some(info) => info,
+        // no ExpnInfo means this span doesn't come from a macro
+        None => return false,
+    };
+
+    match info.format {
+        ExpnFormat::MacroAttribute(..) => return true, // definitely a plugin
+        ExpnFormat::CompilerDesugaring(_) => return true, // well, it's "external"
+        ExpnFormat::MacroBang(..) => {} // check below
     }
 
-    span.ctxt()
-        .outer()
-        .expn_info()
-        .map_or(false, |info| in_macro_ext(cx, &info))
+    let def_site = match info.def_site {
+        Some(span) => span,
+        // no span for the def_site means it's an external macro
+        None => return true,
+    };
+
+    match sess.codemap().span_to_snippet(def_site) {
+        Ok(code) => !code.starts_with("macro_rules"),
+        // no snippet = external macro or compiler-builtin expansion
+        Err(_) => true,
+    }
 }
diff --git a/src/test/ui/in-band-lifetimes/ellided-lifetimes-macro-checks.rs b/src/test/ui/lint/auxiliary/lints-in-foreign-macros.rs
index 9e53cbfb3ad..cf8e9c18de3 100644
--- a/src/test/ui/in-band-lifetimes/ellided-lifetimes-macro-checks.rs
+++ b/src/test/ui/lint/auxiliary/lints-in-foreign-macros.rs
@@ -7,9 +7,18 @@
 // <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.
-#![feature(nll)]
-#![deny(elided_lifetime_in_path)]
 
-fn main() {
-    format!("foo {}", 22)
+#[macro_export]
+macro_rules! bar {
+    () => {use std::string::ToString;}
+}
+
+#[macro_export]
+macro_rules! baz {
+    ($i:item) => ($i)
+}
+
+#[macro_export]
+macro_rules! baz2 {
+    ($($i:tt)*) => ($($i)*)
 }
diff --git a/src/test/ui/lint/lints-in-foreign-macros.rs b/src/test/ui/lint/lints-in-foreign-macros.rs
new file mode 100644
index 00000000000..0f9003877cc
--- /dev/null
+++ b/src/test/ui/lint/lints-in-foreign-macros.rs
@@ -0,0 +1,28 @@
+// 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:lints-in-foreign-macros.rs
+// compile-pass
+
+#![warn(unused_imports)]
+
+#[macro_use]
+extern crate lints_in_foreign_macros;
+
+macro_rules! foo {
+    () => {use std::string::ToString;} //~ WARN: unused import
+}
+
+mod a { foo!(); }
+mod b { bar!(); }
+mod c { baz!(use std::string::ToString;); } //~ WARN: unused import
+mod d { baz2!(use std::string::ToString;); } //~ WARN: unused import
+
+fn main() {}
diff --git a/src/test/ui/lint/lints-in-foreign-macros.stderr b/src/test/ui/lint/lints-in-foreign-macros.stderr
new file mode 100644
index 00000000000..e9f6d3d3815
--- /dev/null
+++ b/src/test/ui/lint/lints-in-foreign-macros.stderr
@@ -0,0 +1,27 @@
+warning: unused import: `std::string::ToString`
+  --> $DIR/lints-in-foreign-macros.rs:20:16
+   |
+LL |     () => {use std::string::ToString;} //~ WARN: unused import
+   |                ^^^^^^^^^^^^^^^^^^^^^
+...
+LL | mod a { foo!(); }
+   |         ------- in this macro invocation
+   |
+note: lint level defined here
+  --> $DIR/lints-in-foreign-macros.rs:14:9
+   |
+LL | #![warn(unused_imports)]
+   |         ^^^^^^^^^^^^^^
+
+warning: unused import: `std::string::ToString`
+  --> $DIR/lints-in-foreign-macros.rs:25:18
+   |
+LL | mod c { baz!(use std::string::ToString;); } //~ WARN: unused import
+   |                  ^^^^^^^^^^^^^^^^^^^^^
+
+warning: unused import: `std::string::ToString`
+  --> $DIR/lints-in-foreign-macros.rs:26:19
+   |
+LL | mod d { baz2!(use std::string::ToString;); } //~ WARN: unused import
+   |                   ^^^^^^^^^^^^^^^^^^^^^
+