note individual lint name set via lint group attribute in notes

Warning or error messages set via a lint group attribute
(e.g. `#[deny(warnings)]`) should still make it clear which individual
lint (by name) was triggered, similarly to how we include "on by
default" language for default lints. This—and, while we're here, the
existing "on by default" language—can be tucked into a note rather than
cluttering the main error message. This occasions the slightest of
refactorings (we now have to get the diagnostic-builder with the main
message first, before matching on the lint source).

This is in the matter of #36846.
This commit is contained in:
Zack M. Davis 2017-01-05 18:55:36 -08:00
parent 8967085617
commit 65b0554143
11 changed files with 72 additions and 39 deletions

View File

@ -40,6 +40,7 @@ use std::cmp;
use std::default::Default as StdDefault;
use std::mem;
use std::fmt;
use std::ops::Deref;
use syntax::attr;
use syntax::ast;
use syntax_pos::{MultiSpan, Span};
@ -446,35 +447,18 @@ pub fn raw_struct_lint<'a, S>(sess: &'a Session,
-> DiagnosticBuilder<'a>
where S: Into<MultiSpan>
{
let (mut level, source) = lvlsrc;
let (level, source) = lvlsrc;
if level == Allow {
return sess.diagnostic().struct_dummy();
}
let name = lint.name_lower();
let mut def = None;
let msg = match source {
Default => {
format!("{}, #[{}({})] on by default", msg,
level.as_str(), name)
},
CommandLine => {
format!("{} [-{} {}]", msg,
match level {
Warn => 'W', Deny => 'D', Forbid => 'F',
Allow => bug!()
}, name.replace("_", "-"))
},
Node(src) => {
def = Some(src);
msg.to_string()
}
};
// For purposes of printing, we can treat forbid as deny.
if level == Forbid { level = Deny; }
// Except for possible note details, forbid behaves like deny.
let effective_level = if level == Forbid { Deny } else { level };
let mut err = match (level, span) {
let mut err = match (effective_level, span) {
(Warn, Some(sp)) => sess.struct_span_warn(sp, &msg[..]),
(Warn, None) => sess.struct_warn(&msg[..]),
(Deny, Some(sp)) => sess.struct_span_err(sp, &msg[..]),
@ -482,6 +466,27 @@ pub fn raw_struct_lint<'a, S>(sess: &'a Session,
_ => bug!("impossible level in raw_emit_lint"),
};
match source {
Default => {
err.note(&format!("#[{}({})] on by default", level.as_str(), name));
},
CommandLine => {
err.note(&format!("[-{} {}]",
match level {
Warn => 'W', Deny => 'D', Forbid => 'F',
Allow => bug!()
}, name.replace("_", "-")));
},
Node(lint_attr_name, src) => {
def = Some(src);
if lint_attr_name.as_str().deref() != name {
let level_str = level.as_str();
err.note(&format!("#[{}({})] implies #[{}({})]",
level_str, lint_attr_name, level_str, name));
}
}
}
// Check for future incompatibility lints and issue a stronger warning.
if let Some(future_incompatible) = lints.future_incompatible(LintId::of(lint)) {
let explanation = format!("this was previously accepted by the compiler \
@ -649,6 +654,8 @@ pub trait LintContext<'tcx>: Sized {
}
};
let lint_attr_name = result.expect("lint attribute should be well-formed").0;
for (lint_id, level, span) in v {
let (now, now_source) = self.lints().get_level_source(lint_id);
if now == Forbid && level != Forbid {
@ -660,7 +667,7 @@ pub trait LintContext<'tcx>: Sized {
diag_builder.span_label(span, &format!("overruled by previous forbid"));
match now_source {
LintSource::Default => &mut diag_builder,
LintSource::Node(forbid_source_span) => {
LintSource::Node(_, forbid_source_span) => {
diag_builder.span_label(forbid_source_span,
&format!("`forbid` level set here"))
},
@ -672,7 +679,7 @@ pub trait LintContext<'tcx>: Sized {
let src = self.lints().get_level_source(lint_id).1;
self.level_stack().push((lint_id, (now, src)));
pushed += 1;
self.mut_lints().set_level(lint_id, (level, Node(span)));
self.mut_lints().set_level(lint_id, (level, Node(lint_attr_name, span)));
}
}
}

View File

@ -338,7 +338,7 @@ pub enum LintSource {
Default,
/// Lint level was set by an attribute.
Node(Span),
Node(ast::Name, Span),
/// Lint level was set by a command-line flag.
CommandLine,

View File

@ -23,6 +23,7 @@ mod bar {
//~^ WARN `Foo` is ambiguous
//~| WARN hard error in a future release
//~| NOTE see issue #38260
//~| NOTE #[warn(legacy_imports)] on by default
}
}

View File

@ -9,5 +9,7 @@
// except according to those terms.
#![deny(warnings)] //~ NOTE: lint level defined here
use std::thread; //~ ERROR: unused import
use std::thread;
//~^ ERROR: unused import
//~| NOTE: #[deny(warnings)] implies #[deny(unused_imports)]
fn main() {}

View File

@ -12,7 +12,9 @@
//~^ NOTE lint level defined here
#![allow(dead_code)]
fn CamelCase() {} //~ ERROR function `CamelCase` should have a snake case name
fn CamelCase() {}
//~^ ERROR function `CamelCase` should have a snake case name
//~| NOTE #[deny(bad_style)] implies #[deny(non_snake_case)]
#[allow(bad_style)]
mod test {
@ -22,9 +24,13 @@ mod test {
//~^ NOTE lint level defined here
//~^^ NOTE lint level defined here
mod bad {
fn CamelCase() {} //~ ERROR function `CamelCase` should have a snake case name
fn CamelCase() {}
//~^ ERROR function `CamelCase` should have a snake case name
//~| NOTE #[forbid(bad_style)] implies #[forbid(non_snake_case)]
static bad: isize = 1; //~ ERROR static variable `bad` should have an upper case name
static bad: isize = 1;
//~^ ERROR static variable `bad` should have an upper case name
//~| NOTE #[forbid(bad_style)] implies #[forbid(non_upper_case_globals)]
}
mod warn {
@ -32,9 +38,13 @@ mod test {
//~^ NOTE lint level defined here
//~| NOTE lint level defined here
fn CamelCase() {} //~ WARN function `CamelCase` should have a snake case name
fn CamelCase() {}
//~^ WARN function `CamelCase` should have a snake case name
//~| NOTE #[warn(bad_style)] implies #[warn(non_snake_case)]
struct snake_case; //~ WARN type `snake_case` should have a camel case name
struct snake_case;
//~^ WARN type `snake_case` should have a camel case name
//~| NOTE #[warn(bad_style)] implies #[warn(non_camel_case_types)]
}
}

View File

@ -11,15 +11,20 @@
// compile-flags: -F unused_features
// aux-build:lint_output_format.rs
#![feature(foo)] //~ ERROR unused or unknown feature
#![feature(foo)]
//~^ ERROR unused or unknown feature
//~| NOTE [-F unused-features]
#![feature(test_feature)]
extern crate lint_output_format;
use lint_output_format::{foo, bar};
//~^ WARNING use of deprecated item: text,
//~^ WARNING use of deprecated item: text
//~| NOTE #[warn(deprecated)] on by default
fn main() {
let _x = foo(); //~ WARNING #[warn(deprecated)] on by default
let _x = foo();
//~^ WARNING use of deprecated item: text
//~| NOTE #[warn(deprecated)] on by default
let _y = bar();
}

View File

@ -10,11 +10,11 @@
// Parser test for #37765
fn with_parens<T: ToString>(arg: T) -> String { //~WARN dead_code
return (<T as ToString>::to_string(&arg)); //~WARN unused_parens
fn with_parens<T: ToString>(arg: T) -> String { //~WARN function is never used: `with_parens`
return (<T as ToString>::to_string(&arg)); //~WARN unnecessary parentheses around `return` value
}
fn no_parens<T: ToString>(arg: T) -> String { //~WARN dead_code
fn no_parens<T: ToString>(arg: T) -> String { //~WARN function is never used: `no_parens`
return <T as ToString>::to_string(&arg);
}

View File

@ -1,4 +1,4 @@
error[E0276]: impl has stricter requirements than trait, #[deny(extra_requirement_in_impl)] on by default
error[E0276]: impl has stricter requirements than trait
--> $DIR/proj-outlives-region.rs:22:5
|
17 | fn foo() where T: 'a;
@ -7,6 +7,7 @@ error[E0276]: impl has stricter requirements than trait, #[deny(extra_requiremen
22 | fn foo() where U: 'a { } //~ ERROR E0276
| ^^^^^^^^^^^^^^^^^^^^^^^^ impl has extra requirement `U: 'a`
|
= note: #[deny(extra_requirement_in_impl)] on by default
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
= note: for more information, see issue #37166 <https://github.com/rust-lang/rust/issues/37166>

View File

@ -1,4 +1,4 @@
error[E0276]: impl has stricter requirements than trait, #[deny(extra_requirement_in_impl)] on by default
error[E0276]: impl has stricter requirements than trait
--> $DIR/region-unrelated.rs:22:5
|
17 | fn foo() where T: 'a;
@ -7,6 +7,7 @@ error[E0276]: impl has stricter requirements than trait, #[deny(extra_requiremen
22 | fn foo() where V: 'a { }
| ^^^^^^^^^^^^^^^^^^^^^^^^ impl has extra requirement `V: 'a`
|
= note: #[deny(extra_requirement_in_impl)] on by default
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
= note: for more information, see issue #37166 <https://github.com/rust-lang/rust/issues/37166>

View File

@ -4,6 +4,7 @@ error: variable `theTwo` should have a snake case name such as `the_two`
19 | let theTwo = 2;
| ^^^^^^
|
= note: #[deny(warnings)] implies #[deny(non_snake_case)]
note: lint level defined here
--> $DIR/issue-24690.rs:16:9
|
@ -15,6 +16,8 @@ error: variable `theOtherTwo` should have a snake case name such as `the_other_t
|
20 | let theOtherTwo = 2;
| ^^^^^^^^^^^
|
= note: #[deny(warnings)] implies #[deny(non_snake_case)]
error: unused variable: `theOtherTwo`
--> $DIR/issue-24690.rs:20:9
@ -22,6 +25,7 @@ error: unused variable: `theOtherTwo`
20 | let theOtherTwo = 2;
| ^^^^^^^^^^^
|
= note: #[deny(warnings)] implies #[deny(unused_variables)]
note: lint level defined here
--> $DIR/issue-24690.rs:16:9
|

View File

@ -1,6 +1,8 @@
warning: unused imports: `Eq`, `Ord`, `PartialEq`, `PartialOrd`, #[warn(unused_imports)] on by default
warning: unused imports: `Eq`, `Ord`, `PartialEq`, `PartialOrd`
--> $DIR/multispan-import-lint.rs:11:16
|
11 | use std::cmp::{Eq, Ord, min, PartialEq, PartialOrd};
| ^^ ^^^ ^^^^^^^^^ ^^^^^^^^^^
|
= note: #[warn(unused_imports)] on by default