From 65b05541432c1fe7b75fb2dd3b020f933f3b5f4b Mon Sep 17 00:00:00 2001 From: "Zack M. Davis" Date: Thu, 5 Jan 2017 18:55:36 -0800 Subject: [PATCH] note individual lint name set via lint group attribute in notes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- src/librustc/lint/context.rs | 53 +++++++++++-------- src/librustc/lint/mod.rs | 2 +- .../imports/rfc-1560-warning-cycle.rs | 1 + src/test/compile-fail/issue-30730.rs | 4 +- src/test/compile-fail/lint-group-style.rs | 20 +++++-- src/test/compile-fail/lint-output-format-2.rs | 11 ++-- src/test/run-pass/path-lookahead.rs | 6 +-- .../proj-outlives-region.stderr | 3 +- .../ui/compare-method/region-unrelated.stderr | 3 +- src/test/ui/span/issue-24690.stderr | 4 ++ src/test/ui/span/multispan-import-lint.stderr | 4 +- 11 files changed, 72 insertions(+), 39 deletions(-) diff --git a/src/librustc/lint/context.rs b/src/librustc/lint/context.rs index 362117d860a..62f19412d52 100644 --- a/src/librustc/lint/context.rs +++ b/src/librustc/lint/context.rs @@ -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 { - 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))); } } } diff --git a/src/librustc/lint/mod.rs b/src/librustc/lint/mod.rs index d12065ca86e..9c2a892a5fc 100644 --- a/src/librustc/lint/mod.rs +++ b/src/librustc/lint/mod.rs @@ -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, diff --git a/src/test/compile-fail/imports/rfc-1560-warning-cycle.rs b/src/test/compile-fail/imports/rfc-1560-warning-cycle.rs index eb36129799a..1d67bf3a1cd 100644 --- a/src/test/compile-fail/imports/rfc-1560-warning-cycle.rs +++ b/src/test/compile-fail/imports/rfc-1560-warning-cycle.rs @@ -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 } } diff --git a/src/test/compile-fail/issue-30730.rs b/src/test/compile-fail/issue-30730.rs index 82804bb7474..6082740f427 100644 --- a/src/test/compile-fail/issue-30730.rs +++ b/src/test/compile-fail/issue-30730.rs @@ -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() {} diff --git a/src/test/compile-fail/lint-group-style.rs b/src/test/compile-fail/lint-group-style.rs index b2e6072c985..a8c37703475 100644 --- a/src/test/compile-fail/lint-group-style.rs +++ b/src/test/compile-fail/lint-group-style.rs @@ -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)] } } diff --git a/src/test/compile-fail/lint-output-format-2.rs b/src/test/compile-fail/lint-output-format-2.rs index 2f74325d19c..8b76bedb003 100644 --- a/src/test/compile-fail/lint-output-format-2.rs +++ b/src/test/compile-fail/lint-output-format-2.rs @@ -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(); } diff --git a/src/test/run-pass/path-lookahead.rs b/src/test/run-pass/path-lookahead.rs index 017259af190..5c195c8a4c6 100644 --- a/src/test/run-pass/path-lookahead.rs +++ b/src/test/run-pass/path-lookahead.rs @@ -10,11 +10,11 @@ // Parser test for #37765 -fn with_parens(arg: T) -> String { //~WARN dead_code - return (::to_string(&arg)); //~WARN unused_parens +fn with_parens(arg: T) -> String { //~WARN function is never used: `with_parens` + return (::to_string(&arg)); //~WARN unnecessary parentheses around `return` value } -fn no_parens(arg: T) -> String { //~WARN dead_code +fn no_parens(arg: T) -> String { //~WARN function is never used: `no_parens` return ::to_string(&arg); } diff --git a/src/test/ui/compare-method/proj-outlives-region.stderr b/src/test/ui/compare-method/proj-outlives-region.stderr index 021b571fe79..2a707c6eb8b 100644 --- a/src/test/ui/compare-method/proj-outlives-region.stderr +++ b/src/test/ui/compare-method/proj-outlives-region.stderr @@ -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 diff --git a/src/test/ui/compare-method/region-unrelated.stderr b/src/test/ui/compare-method/region-unrelated.stderr index 4df337c5257..9e822bd8b07 100644 --- a/src/test/ui/compare-method/region-unrelated.stderr +++ b/src/test/ui/compare-method/region-unrelated.stderr @@ -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 diff --git a/src/test/ui/span/issue-24690.stderr b/src/test/ui/span/issue-24690.stderr index dbe5e31287e..623bb4a0d26 100644 --- a/src/test/ui/span/issue-24690.stderr +++ b/src/test/ui/span/issue-24690.stderr @@ -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 | diff --git a/src/test/ui/span/multispan-import-lint.stderr b/src/test/ui/span/multispan-import-lint.stderr index b581584eee7..4b1ca7f98bb 100644 --- a/src/test/ui/span/multispan-import-lint.stderr +++ b/src/test/ui/span/multispan-import-lint.stderr @@ -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