Rollup merge of rust-lang#42919 - zackmdavis:once_again_we_heard_you_the_first_time, r=eddyb

make lint on-by-default/implied-by messages appear only once

From review discussion on rust-lang#38103 (rust-lang#38103 (comment)).

![we_heard](https://user-images.githubusercontent.com/1076988/27564103-6284b78e-5a8a-11e7-9d35-f7f297ca9573.png)

r? @nikomatsakis

Author: Ariel Ben-Yehuda

src/librustc/lint/context.rs

@@ -510,7 +510,6 @@ pub fn raw_struct_lint<'a, S>(sess: &'a Session,
     }
 
     let name = lint.name_lower();
-    let mut def = None;
 
     // Except for possible note details, forbid behaves like deny.
     let effective_level = if level == Forbid { Deny } else { level };
@@ -525,7 +524,8 @@ pub fn raw_struct_lint<'a, S>(sess: &'a Session,
 
     match source {
         Default => {
-            err.note(&format!("#[{}({})] on by default", level.as_str(), name));
+            sess.diag_note_once(&mut err, lint,
+                                &format!("#[{}({})] on by default", level.as_str(), name));
         },
         CommandLine(lint_flag_val) => {
             let flag = match level {
@@ -534,20 +534,24 @@ pub fn raw_struct_lint<'a, S>(sess: &'a Session,
             };
             let hyphen_case_lint_name = name.replace("_", "-");
             if lint_flag_val.as_str() == name {
-                err.note(&format!("requested on the command line with `{} {}`",
-                                  flag, hyphen_case_lint_name));
+                sess.diag_note_once(&mut err, lint,
+                                    &format!("requested on the command line with `{} {}`",
+                                             flag, hyphen_case_lint_name));
             } else {
                 let hyphen_case_flag_val = lint_flag_val.as_str().replace("_", "-");
-                err.note(&format!("`{} {}` implied by `{} {}`",
-                                  flag, hyphen_case_lint_name, flag, hyphen_case_flag_val));
+                sess.diag_note_once(&mut err, lint,
+                                    &format!("`{} {}` implied by `{} {}`",
+                                             flag, hyphen_case_lint_name, flag,
+                                             hyphen_case_flag_val));
             }
         },
         Node(lint_attr_name, src) => {
-            def = Some(src);
+            sess.diag_span_note_once(&mut err, lint, src, "lint level defined here");
             if lint_attr_name.as_str() != name {
                 let level_str = level.as_str();
-                err.note(&format!("#[{}({})] implied by #[{}({})]",
-                                  level_str, name, level_str, lint_attr_name));
+                sess.diag_note_once(&mut err, lint,
+                                    &format!("#[{}({})] implied by #[{}({})]",
+                                             level_str, name, level_str, lint_attr_name));
             }
         }
     }
@@ -563,10 +567,6 @@ pub fn raw_struct_lint<'a, S>(sess: &'a Session,
         err.note(&citation);
     }
 
-    if let Some(span) = def {
-        sess.diag_span_note_once(&mut err, lint, span, "lint level defined here");
-    }
-
     err
 }
 

src/librustc/session/mod.rs

@@ -79,10 +79,10 @@ pub struct Session {
     pub working_dir: (String, bool),
     pub lint_store: RefCell<lint::LintStore>,
     pub lints: RefCell<lint::LintTable>,
-    /// Set of (LintId, span, message) tuples tracking lint (sub)diagnostics
-    /// that have been set once, but should not be set again, in order to avoid
-    /// redundantly verbose output (Issue #24690).
-    pub one_time_diagnostics: RefCell<FxHashSet<(lint::LintId, Span, String)>>,
+    /// Set of (LintId, Option<Span>, message) tuples tracking lint
+    /// (sub)diagnostics that have been set once, but should not be set again,
+    /// in order to avoid redundantly verbose output (Issue #24690).
+    pub one_time_diagnostics: RefCell<FxHashSet<(lint::LintId, Option<Span>, String)>>,
     pub plugin_llvm_passes: RefCell<Vec<String>>,
     pub plugin_attributes: RefCell<Vec<(String, AttributeType)>>,
     pub crate_types: RefCell<Vec<config::CrateType>>,
@@ -157,6 +157,13 @@ pub struct PerfStats {
     pub decode_def_path_tables_time: Cell<Duration>,
 }
 
+/// Enum to support dispatch of one-time diagnostics (in Session.diag_once)
+enum DiagnosticBuilderMethod {
+    Note,
+    SpanNote,
+    // add more variants as needed to support one-time diagnostics
+}
+
 impl Session {
     pub fn local_crate_disambiguator(&self) -> Symbol {
         *self.crate_disambiguator.borrow()
@@ -329,34 +336,53 @@ impl Session {
         &self.parse_sess.span_diagnostic
     }
 
-    /// Analogous to calling `.span_note` on the given DiagnosticBuilder, but
-    /// deduplicates on lint ID, span, and message for this `Session` if we're
-    /// not outputting in JSON mode.
-    //
-    // FIXME: if the need arises for one-time diagnostics other than
-    // `span_note`, we almost certainly want to generalize this
-    // "check/insert-into the one-time diagnostics map, then set message if
-    // it's not already there" code to accomodate all of them
-    pub fn diag_span_note_once<'a, 'b>(&'a self,
-                                       diag_builder: &'b mut DiagnosticBuilder<'a>,
-                                       lint: &'static lint::Lint, span: Span, message: &str) {
+    /// Analogous to calling methods on the given `DiagnosticBuilder`, but
+    /// deduplicates on lint ID, span (if any), and message for this `Session`
+    /// if we're not outputting in JSON mode.
+    fn diag_once<'a, 'b>(&'a self,
+                         diag_builder: &'b mut DiagnosticBuilder<'a>,
+                         method: DiagnosticBuilderMethod,
+                         lint: &'static lint::Lint, message: &str, span: Option<Span>) {
+        let mut do_method = || {
+            match method {
+                DiagnosticBuilderMethod::Note => {
+                    diag_builder.note(message);
+                },
+                DiagnosticBuilderMethod::SpanNote => {
+                    diag_builder.span_note(span.expect("span_note expects a span"), message);
+                }
+            }
+        };
+
         match self.opts.error_format {
             // when outputting JSON for tool consumption, the tool might want
             // the duplicates
             config::ErrorOutputType::Json => {
-                diag_builder.span_note(span, &message);
+                do_method()
             },
             _ => {
                 let lint_id = lint::LintId::of(lint);
                 let id_span_message = (lint_id, span, message.to_owned());
                 let fresh = self.one_time_diagnostics.borrow_mut().insert(id_span_message);
                 if fresh {
-                    diag_builder.span_note(span, &message);
+                    do_method()
                 }
             }
         }
     }
 
+    pub fn diag_span_note_once<'a, 'b>(&'a self,
+                                       diag_builder: &'b mut DiagnosticBuilder<'a>,
+                                       lint: &'static lint::Lint, span: Span, message: &str) {
+        self.diag_once(diag_builder, DiagnosticBuilderMethod::SpanNote, lint, message, Some(span));
+    }
+
+    pub fn diag_note_once<'a, 'b>(&'a self,
+                                  diag_builder: &'b mut DiagnosticBuilder<'a>,
+                                  lint: &'static lint::Lint, message: &str) {
+        self.diag_once(diag_builder, DiagnosticBuilderMethod::Note, lint, message, None);
+    }
+
     pub fn codemap<'a>(&'a self) -> &'a codemap::CodeMap {
         self.parse_sess.codemap()
     }

src/test/ui/lint/lint-group-style.stderr

@@ -4,64 +4,64 @@ error: function `CamelCase` should have a snake case name such as `camel_case`
 14 | fn CamelCase() {}
    | ^^^^^^^^^^^^^^^^^
    |
-   = note: #[deny(non_snake_case)] implied by #[deny(bad_style)]
 note: lint level defined here
   --> $DIR/lint-group-style.rs:11:9
    |
 11 | #![deny(bad_style)]
    |         ^^^^^^^^^
+   = note: #[deny(non_snake_case)] implied by #[deny(bad_style)]
 
 error: function `CamelCase` should have a snake case name such as `camel_case`
   --> $DIR/lint-group-style.rs:22:9
    |
 22 |         fn CamelCase() {}
    |         ^^^^^^^^^^^^^^^^^
    |
-   = note: #[forbid(non_snake_case)] implied by #[forbid(bad_style)]
 note: lint level defined here
   --> $DIR/lint-group-style.rs:20:14
    |
 20 |     #[forbid(bad_style)]
    |              ^^^^^^^^^
+   = note: #[forbid(non_snake_case)] implied by #[forbid(bad_style)]
 
 error: static variable `bad` should have an upper case name such as `BAD`
   --> $DIR/lint-group-style.rs:24:9
    |
 24 |         static bad: isize = 1;
    |         ^^^^^^^^^^^^^^^^^^^^^^
    |
-   = note: #[forbid(non_upper_case_globals)] implied by #[forbid(bad_style)]
 note: lint level defined here
   --> $DIR/lint-group-style.rs:20:14
    |
 20 |     #[forbid(bad_style)]
    |              ^^^^^^^^^
+   = note: #[forbid(non_upper_case_globals)] implied by #[forbid(bad_style)]
 
 warning: function `CamelCase` should have a snake case name such as `camel_case`
   --> $DIR/lint-group-style.rs:30:9
    |
 30 |         fn CamelCase() {}
    |         ^^^^^^^^^^^^^^^^^
    |
-   = note: #[warn(non_snake_case)] implied by #[warn(bad_style)]
 note: lint level defined here
   --> $DIR/lint-group-style.rs:28:17
    |
 28 |         #![warn(bad_style)]
    |                 ^^^^^^^^^
+   = note: #[warn(non_snake_case)] implied by #[warn(bad_style)]
 
 warning: type `snake_case` should have a camel case name such as `SnakeCase`
   --> $DIR/lint-group-style.rs:32:9
    |
 32 |         struct snake_case;
    |         ^^^^^^^^^^^^^^^^^^
    |
-   = note: #[warn(non_camel_case_types)] implied by #[warn(bad_style)]
 note: lint level defined here
   --> $DIR/lint-group-style.rs:28:17
    |
 28 |         #![warn(bad_style)]
    |                 ^^^^^^^^^
+   = note: #[warn(non_camel_case_types)] implied by #[warn(bad_style)]
 
 error: aborting due to previous error(s)
 

src/test/ui/path-lookahead.stderr

@@ -23,6 +23,4 @@ warning: function is never used: `no_parens`
 20 | |   return <T as ToString>::to_string(&arg);
 21 | | }
    | |_^
-   |
-   = note: #[warn(dead_code)] on by default
 

src/test/ui/span/issue-24690.stderr

@@ -4,33 +4,31 @@ error: variable `theTwo` should have a snake case name such as `the_two`
 19 |     let theTwo = 2;
    |         ^^^^^^
    |
-   = note: #[deny(non_snake_case)] implied by #[deny(warnings)]
 note: lint level defined here
   --> $DIR/issue-24690.rs:16:9
    |
 16 | #![deny(warnings)]
    |         ^^^^^^^^
+   = note: #[deny(non_snake_case)] implied by #[deny(warnings)]
 
 error: variable `theOtherTwo` should have a snake case name such as `the_other_two`
   --> $DIR/issue-24690.rs:20:9
    |
 20 |     let theOtherTwo = 2;
    |         ^^^^^^^^^^^
-   |
-   = note: #[deny(non_snake_case)] implied by #[deny(warnings)]
 
 error: unused variable: `theOtherTwo`
   --> $DIR/issue-24690.rs:20:9
    |
 20 |     let theOtherTwo = 2;
    |         ^^^^^^^^^^^
    |
-   = note: #[deny(unused_variables)] implied by #[deny(warnings)]
 note: lint level defined here
   --> $DIR/issue-24690.rs:16:9
    |
 16 | #![deny(warnings)]
    |         ^^^^^^^^
+   = note: #[deny(unused_variables)] implied by #[deny(warnings)]
 
 error: aborting due to previous error(s)