Skip to content

Commit ef6a072

Browse files
committed
deduplicate one-time diagnostics on lint ID as well as span and message
Some lint-level attributes (like `bad-style`, or, more dramatically, `warnings`) can affect more than one lint; it seems fairer to point out the attribute once for each distinct lint affected. Also, a UI test is added. This remains in the matter of #24690.
1 parent 8d1da84 commit ef6a072

File tree

4 files changed

+64
-9
lines changed

4 files changed

+64
-9
lines changed

src/librustc/lint/context.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -452,7 +452,7 @@ pub fn raw_struct_lint<'a>(sess: &'a Session,
452452
}
453453

454454
if let Some(span) = def {
455-
sess.diag_span_note_once(&mut err, span, "lint level defined here");
455+
sess.diag_span_note_once(&mut err, lint, span, "lint level defined here");
456456
}
457457

458458
err

src/librustc/session/mod.rs

+9-8
Original file line numberDiff line numberDiff line change
@@ -75,10 +75,10 @@ pub struct Session {
7575
pub working_dir: PathBuf,
7676
pub lint_store: RefCell<lint::LintStore>,
7777
pub lints: RefCell<NodeMap<Vec<(lint::LintId, Span, String)>>>,
78-
/// Set of (span, message) tuples tracking lint (sub)diagnostics that have
79-
/// been set once, but should not be set again, in order to avoid
78+
/// Set of (LintId, span, message) tuples tracking lint (sub)diagnostics
79+
/// that have been set once, but should not be set again, in order to avoid
8080
/// redundantly verbose output (Issue #24690).
81-
pub one_time_diagnostics: RefCell<FnvHashSet<(Span, String)>>,
81+
pub one_time_diagnostics: RefCell<FnvHashSet<(lint::LintId, Span, String)>>,
8282
pub plugin_llvm_passes: RefCell<Vec<String>>,
8383
pub mir_passes: RefCell<mir_pass::Passes>,
8484
pub plugin_attributes: RefCell<Vec<(String, AttributeType)>>,
@@ -294,25 +294,26 @@ impl Session {
294294
}
295295

296296
/// Analogous to calling `.span_note` on the given DiagnosticBuilder, but
297-
/// deduplicates on span and message for this `Session` if we're not
298-
/// outputting in JSON mode.
297+
/// deduplicates on lint ID, span, and message for this `Session` if we're
298+
/// not outputting in JSON mode.
299299
//
300300
// FIXME: if the need arises for one-time diagnostics other than
301301
// `span_note`, we almost certainly want to generalize this
302302
// "check/insert-into the one-time diagnostics map, then set message if
303303
// it's not already there" code to accomodate all of them
304304
pub fn diag_span_note_once<'a, 'b>(&'a self,
305305
diag_builder: &'b mut DiagnosticBuilder<'a>,
306-
span: Span, message: &str) {
306+
lint: &'static lint::Lint, span: Span, message: &str) {
307307
match self.opts.error_format {
308308
// when outputting JSON for tool consumption, the tool might want
309309
// the duplicates
310310
config::ErrorOutputType::Json => {
311311
diag_builder.span_note(span, &message);
312312
},
313313
_ => {
314-
let span_message = (span, message.to_owned());
315-
let fresh = self.one_time_diagnostics.borrow_mut().insert(span_message);
314+
let lint_id = lint::LintId::of(lint);
315+
let id_span_message = (lint_id, span, message.to_owned());
316+
let fresh = self.one_time_diagnostics.borrow_mut().insert(id_span_message);
316317
if fresh {
317318
diag_builder.span_note(span, &message);
318319
}

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

+22
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
// Copyright 2016 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+
//! A test to ensure that helpful `note` messages aren't emitted more often
12+
//! than necessary.
13+
14+
// Although there are three errors, we should only get two "lint level defined
15+
// here" notes pointing at the `warnings` span, one for each error type.
16+
#![deny(warnings)]
17+
18+
fn main() {
19+
let theTwo = 2;
20+
let theOtherTwo = 2;
21+
println!("{}", theTwo);
22+
}

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

+32
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
error: unused variable: `theOtherTwo`
2+
--> $DIR/issue-24690.rs:20:9
3+
|
4+
20 | let theOtherTwo = 2;
5+
| ^^^^^^^^^^^
6+
|
7+
note: lint level defined here
8+
--> $DIR/issue-24690.rs:16:9
9+
|
10+
16 | #![deny(warnings)]
11+
| ^^^^^^^^
12+
13+
error: variable `theTwo` should have a snake case name such as `the_two`
14+
--> $DIR/issue-24690.rs:19:9
15+
|
16+
19 | let theTwo = 2;
17+
| ^^^^^^
18+
|
19+
note: lint level defined here
20+
--> $DIR/issue-24690.rs:16:9
21+
|
22+
16 | #![deny(warnings)]
23+
| ^^^^^^^^
24+
25+
error: variable `theOtherTwo` should have a snake case name such as `the_other_two`
26+
--> $DIR/issue-24690.rs:20:9
27+
|
28+
20 | let theOtherTwo = 2;
29+
| ^^^^^^^^^^^
30+
31+
error: aborting due to 3 previous errors
32+

0 commit comments

Comments
 (0)