Skip to content

Commit

Permalink
Auto merge of #52467 - alexcrichton:lints-and-macros, r=Manishearth
Browse files Browse the repository at this point in the history
Squash all lints tied to foreign macros by default

This PR is a continuation of #49755 (thanks for the initial jump-start @Dylan-DPC!) and is targeted at solving #48855. This change updates the lint infrastructure to, by default, ignore all lints emitted for code that originates in a foreign macro. For example if `println!("...")` injects some idiomatic warnings these are all ignored by default. The rationale here is that for almost all lints there's no action that can be taken if the code originates from a foreign lint.

Closes #48855
Closes #52483
Closes #52479
  • Loading branch information
bors committed Jul 20, 2018
2 parents a5097f3 + 8adf08c commit 4260c8b
Show file tree
Hide file tree
Showing 4 changed files with 125 additions and 1 deletion.
47 changes: 46 additions & 1 deletion src/librustc/lint/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -572,6 +572,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
Expand Down Expand Up @@ -673,3 +689,32 @@ impl<'a, 'tcx> intravisit::Visitor<'tcx> for LintLevelMapBuilder<'a, 'tcx> {
pub fn provide(providers: &mut Providers) {
providers.lint_levels = lint_levels;
}

/// 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
}

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,
}
}
24 changes: 24 additions & 0 deletions src/test/ui/lint/auxiliary/lints-in-foreign-macros.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// 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.

#[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)*)
}
28 changes: 28 additions & 0 deletions src/test/ui/lint/lints-in-foreign-macros.rs
Original file line number Diff line number Diff line change
@@ -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() {}
27 changes: 27 additions & 0 deletions src/test/ui/lint/lints-in-foreign-macros.stderr
Original file line number Diff line number Diff line change
@@ -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
| ^^^^^^^^^^^^^^^^^^^^^

0 comments on commit 4260c8b

Please sign in to comment.