Skip to content

Commit d7777ae

Browse files
borsbors
committed
Auto merge of #38103 - zackmdavis:lint_errors_resulting_from_lint_groups_or_warnings_meta-lint_obscure_the_original_lint_name, r=nikomatsakis
note individual lint name in messages set via lint group attribute ![lint_errors_resulting_from_lint_groups_obscure](https://cloud.githubusercontent.com/assets/1076988/20783614/c107d5c8-b749-11e6-85de-eada7f67c986.png) Resolves #36846. r? @jonathandturner ----- ***Update*** 16 December (new commits): ![lint_group_makeover_party](https://cloud.githubusercontent.com/assets/1076988/21284540/ff1ae2fc-c3d2-11e6-93be-d0689f5fa7a8.png)
2 parents ea7a648 + 72af42e commit d7777ae

20 files changed

+263
-79
lines changed

Diff for: ‎src/librustc/lint/context.rs

+43-26
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,10 @@ use std::cmp;
4040
use std::default::Default as StdDefault;
4141
use std::mem;
4242
use std::fmt;
43+
use std::ops::Deref;
4344
use syntax::attr;
4445
use syntax::ast;
46+
use syntax::symbol::Symbol;
4547
use syntax_pos::{MultiSpan, Span};
4648
use errors::{self, Diagnostic, DiagnosticBuilder};
4749
use hir;
@@ -299,8 +301,9 @@ impl LintStore {
299301
check_lint_name_cmdline(sess, self,
300302
&lint_name[..], level);
301303

304+
let lint_flag_val = Symbol::intern(&lint_name);
302305
match self.find_lint(&lint_name[..], sess, None) {
303-
Ok(lint_id) => self.set_level(lint_id, (level, CommandLine)),
306+
Ok(lint_id) => self.set_level(lint_id, (level, CommandLine(lint_flag_val))),
304307
Err(FindLintError::Removed) => { }
305308
Err(_) => {
306309
match self.lint_groups.iter().map(|(&x, pair)| (x, pair.0.clone()))
@@ -310,7 +313,7 @@ impl LintStore {
310313
Some(v) => {
311314
v.iter()
312315
.map(|lint_id: &LintId|
313-
self.set_level(*lint_id, (level, CommandLine)))
316+
self.set_level(*lint_id, (level, CommandLine(lint_flag_val))))
314317
.collect::<Vec<()>>();
315318
}
316319
None => {
@@ -446,42 +449,54 @@ pub fn raw_struct_lint<'a, S>(sess: &'a Session,
446449
-> DiagnosticBuilder<'a>
447450
where S: Into<MultiSpan>
448451
{
449-
let (mut level, source) = lvlsrc;
452+
let (level, source) = lvlsrc;
450453
if level == Allow {
451454
return sess.diagnostic().struct_dummy();
452455
}
453456

454457
let name = lint.name_lower();
455458
let mut def = None;
456-
let msg = match source {
457-
Default => {
458-
format!("{}, #[{}({})] on by default", msg,
459-
level.as_str(), name)
460-
},
461-
CommandLine => {
462-
format!("{} [-{} {}]", msg,
463-
match level {
464-
Warn => 'W', Deny => 'D', Forbid => 'F',
465-
Allow => bug!()
466-
}, name.replace("_", "-"))
467-
},
468-
Node(src) => {
469-
def = Some(src);
470-
msg.to_string()
471-
}
472-
};
473459

474-
// For purposes of printing, we can treat forbid as deny.
475-
if level == Forbid { level = Deny; }
460+
// Except for possible note details, forbid behaves like deny.
461+
let effective_level = if level == Forbid { Deny } else { level };
476462

477-
let mut err = match (level, span) {
463+
let mut err = match (effective_level, span) {
478464
(Warn, Some(sp)) => sess.struct_span_warn(sp, &msg[..]),
479465
(Warn, None) => sess.struct_warn(&msg[..]),
480466
(Deny, Some(sp)) => sess.struct_span_err(sp, &msg[..]),
481467
(Deny, None) => sess.struct_err(&msg[..]),
482468
_ => bug!("impossible level in raw_emit_lint"),
483469
};
484470

471+
match source {
472+
Default => {
473+
err.note(&format!("#[{}({})] on by default", level.as_str(), name));
474+
},
475+
CommandLine(lint_flag_val) => {
476+
let flag = match level {
477+
Warn => "-W", Deny => "-D", Forbid => "-F",
478+
Allow => bug!("earlier conditional return should handle Allow case")
479+
};
480+
let hyphen_case_lint_name = name.replace("_", "-");
481+
if lint_flag_val.as_str().deref() == name {
482+
err.note(&format!("requested on the command line with `{} {}`",
483+
flag, hyphen_case_lint_name));
484+
} else {
485+
let hyphen_case_flag_val = lint_flag_val.as_str().replace("_", "-");
486+
err.note(&format!("`{} {}` implied by `{} {}`",
487+
flag, hyphen_case_lint_name, flag, hyphen_case_flag_val));
488+
}
489+
},
490+
Node(lint_attr_name, src) => {
491+
def = Some(src);
492+
if lint_attr_name.as_str().deref() != name {
493+
let level_str = level.as_str();
494+
err.note(&format!("#[{}({})] implied by #[{}({})]",
495+
level_str, name, level_str, lint_attr_name));
496+
}
497+
}
498+
}
499+
485500
// Check for future incompatibility lints and issue a stronger warning.
486501
if let Some(future_incompatible) = lints.future_incompatible(LintId::of(lint)) {
487502
let explanation = format!("this was previously accepted by the compiler \
@@ -649,6 +664,8 @@ pub trait LintContext<'tcx>: Sized {
649664
}
650665
};
651666

667+
let lint_attr_name = result.expect("lint attribute should be well-formed").0;
668+
652669
for (lint_id, level, span) in v {
653670
let (now, now_source) = self.lints().get_level_source(lint_id);
654671
if now == Forbid && level != Forbid {
@@ -660,19 +677,19 @@ pub trait LintContext<'tcx>: Sized {
660677
diag_builder.span_label(span, &format!("overruled by previous forbid"));
661678
match now_source {
662679
LintSource::Default => &mut diag_builder,
663-
LintSource::Node(forbid_source_span) => {
680+
LintSource::Node(_, forbid_source_span) => {
664681
diag_builder.span_label(forbid_source_span,
665682
&format!("`forbid` level set here"))
666683
},
667-
LintSource::CommandLine => {
684+
LintSource::CommandLine(_) => {
668685
diag_builder.note("`forbid` lint level was set on command line")
669686
}
670687
}.emit()
671688
} else if now != level {
672689
let src = self.lints().get_level_source(lint_id).1;
673690
self.level_stack().push((lint_id, (now, src)));
674691
pushed += 1;
675-
self.mut_lints().set_level(lint_id, (level, Node(span)));
692+
self.mut_lints().set_level(lint_id, (level, Node(lint_attr_name, span)));
676693
}
677694
}
678695
}

Diff for: ‎src/librustc/lint/mod.rs

+3-2
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ use std::ascii::AsciiExt;
3838
use syntax_pos::Span;
3939
use syntax::visit as ast_visit;
4040
use syntax::ast;
41+
use syntax::symbol::Symbol;
4142

4243
pub use lint::context::{LateContext, EarlyContext, LintContext, LintStore,
4344
raw_emit_lint, check_crate, check_ast_crate, gather_attrs,
@@ -338,10 +339,10 @@ pub enum LintSource {
338339
Default,
339340

340341
/// Lint level was set by an attribute.
341-
Node(Span),
342+
Node(ast::Name, Span),
342343

343344
/// Lint level was set by a command-line flag.
344-
CommandLine,
345+
CommandLine(Symbol),
345346
}
346347

347348
pub type LevelSource = (Level, LintSource);

Diff for: ‎src/test/compile-fail/imports/rfc-1560-warning-cycle.rs

+1
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ mod bar {
2323
//~^ WARN `Foo` is ambiguous
2424
//~| WARN hard error in a future release
2525
//~| NOTE see issue #38260
26+
//~| NOTE #[warn(legacy_imports)] on by default
2627
}
2728
}
2829

Diff for: ‎src/test/compile-fail/issue-30730.rs

+3-1
Original file line numberDiff line numberDiff line change
@@ -9,5 +9,7 @@
99
// except according to those terms.
1010

1111
#![deny(warnings)] //~ NOTE: lint level defined here
12-
use std::thread; //~ ERROR: unused import
12+
use std::thread;
13+
//~^ ERROR: unused import
14+
//~| NOTE: #[deny(unused_imports)] implied by #[deny(warnings)]
1315
fn main() {}

Diff for: ‎src/test/compile-fail/lint-group-style.rs

-41
This file was deleted.

Diff for: ‎src/test/compile-fail/lint-output-format-2.rs

+8-3
Original file line numberDiff line numberDiff line change
@@ -11,15 +11,20 @@
1111
// compile-flags: -F unused_features
1212
// aux-build:lint_output_format.rs
1313

14-
#![feature(foo)] //~ ERROR unused or unknown feature
14+
#![feature(foo)]
15+
//~^ ERROR unused or unknown feature
16+
//~| NOTE requested on the command line with `-F unused-features`
1517

1618
#![feature(test_feature)]
1719

1820
extern crate lint_output_format;
1921
use lint_output_format::{foo, bar};
20-
//~^ WARNING use of deprecated item: text,
22+
//~^ WARNING use of deprecated item: text
23+
//~| NOTE #[warn(deprecated)] on by default
2124

2225
fn main() {
23-
let _x = foo(); //~ WARNING #[warn(deprecated)] on by default
26+
let _x = foo();
27+
//~^ WARNING use of deprecated item: text
28+
//~| NOTE #[warn(deprecated)] on by default
2429
let _y = bar();
2530
}

Diff for: ‎src/test/run-pass/path-lookahead.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,11 @@
1010

1111
// Parser test for #37765
1212

13-
fn with_parens<T: ToString>(arg: T) -> String { //~WARN dead_code
14-
return (<T as ToString>::to_string(&arg)); //~WARN unused_parens
13+
fn with_parens<T: ToString>(arg: T) -> String { //~WARN function is never used: `with_parens`
14+
return (<T as ToString>::to_string(&arg)); //~WARN unnecessary parentheses around `return` value
1515
}
1616

17-
fn no_parens<T: ToString>(arg: T) -> String { //~WARN dead_code
17+
fn no_parens<T: ToString>(arg: T) -> String { //~WARN function is never used: `no_parens`
1818
return <T as ToString>::to_string(&arg);
1919
}
2020

Diff for: ‎src/test/ui/compare-method/proj-outlives-region.stderr

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

Diff for: ‎src/test/ui/compare-method/region-unrelated.stderr

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

Diff for: ‎src/test/ui/lint/command-line-lint-group-allow.rs

+15
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
// compile-flags: -A bad-style
12+
13+
fn main() {
14+
let _InappropriateCamelCasing = true;
15+
}

Diff for: ‎src/test/ui/lint/command-line-lint-group-deny.rs

+15
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
// compile-flags: -D bad-style
12+
13+
fn main() {
14+
let _InappropriateCamelCasing = true;
15+
}

Diff for: ‎src/test/ui/lint/command-line-lint-group-deny.stderr

+10
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
error: variable `_InappropriateCamelCasing` should have a snake case name such as `_inappropriate_camel_casing`
2+
--> $DIR/command-line-lint-group-deny.rs:14:9
3+
|
4+
14 | let _InappropriateCamelCasing = true;
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^
6+
|
7+
= note: `-D non-snake-case` implied by `-D bad-style`
8+
9+
error: aborting due to previous error
10+

Diff for: ‎src/test/ui/lint/command-line-lint-group-forbid.rs

+15
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
// compile-flags: -F bad-style
12+
13+
fn main() {
14+
let _InappropriateCamelCasing = true;
15+
}

Diff for: ‎src/test/ui/lint/command-line-lint-group-forbid.stderr

+10
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
error: variable `_InappropriateCamelCasing` should have a snake case name such as `_inappropriate_camel_casing`
2+
--> $DIR/command-line-lint-group-forbid.rs:14:9
3+
|
4+
14 | let _InappropriateCamelCasing = true;
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^
6+
|
7+
= note: `-F non-snake-case` implied by `-F bad-style`
8+
9+
error: aborting due to previous error
10+

Diff for: ‎src/test/ui/lint/command-line-lint-group-warn.rs

+15
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
// compile-flags: -W bad-style
12+
13+
fn main() {
14+
let _InappropriateCamelCasing = true;
15+
}

Diff for: ‎src/test/ui/lint/command-line-lint-group-warn.stderr

+8
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
warning: variable `_InappropriateCamelCasing` should have a snake case name such as `_inappropriate_camel_casing`
2+
--> $DIR/command-line-lint-group-warn.rs:14:9
3+
|
4+
14 | let _InappropriateCamelCasing = true;
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^
6+
|
7+
= note: `-W non-snake-case` implied by `-W bad-style`
8+

0 commit comments

Comments
 (0)