Skip to content

Commit

Permalink
make lint on-by-default/implied-by messages appear only once
Browse files Browse the repository at this point in the history
From review discussion on rust-lang#38103
(rust-lang#38103 (comment)).
  • Loading branch information
zackmdavis committed Jun 26, 2017
1 parent 859c323 commit 32b8579
Show file tree
Hide file tree
Showing 5 changed files with 63 additions and 41 deletions.
26 changes: 13 additions & 13 deletions src/librustc/lint/context.rs
Expand Up @@ -513,7 +513,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 };
Expand All @@ -528,7 +527,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 {
Expand All @@ -537,20 +537,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));
}
}
}
Expand All @@ -566,10 +570,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
}

Expand Down
60 changes: 43 additions & 17 deletions src/librustc/session/mod.rs
Expand Up @@ -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>>,
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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()
}
Expand Down
10 changes: 5 additions & 5 deletions src/test/ui/lint/lint-group-style.stderr
Expand Up @@ -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)

2 changes: 0 additions & 2 deletions src/test/ui/path-lookahead.stderr
Expand Up @@ -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

6 changes: 2 additions & 4 deletions src/test/ui/span/issue-24690.stderr
Expand Up @@ -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)

0 comments on commit 32b8579

Please sign in to comment.