Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Emit helpful note messages only once #24690

Closed
bluss opened this issue Apr 22, 2015 · 4 comments
Closed

Emit helpful note messages only once #24690

bluss opened this issue Apr 22, 2015 · 4 comments
Labels
A-diagnostics Area: Messages for errors, warnings, and lints

Comments

@bluss
Copy link
Member

bluss commented Apr 22, 2015

Example: Adding #![deny(lint_name)] to a crate will point out the location of this line for every time the lint fires (note: lint level defined here). This should be listed only once. It should be reasonably easy to deduplicate these based upon equal span and message.

src/adaptors.rs:84:5: 87:6 warning: missing documentation for a method
src/adaptors.rs:84     pub fn new(iter: I, map: fn(I::Item) -> B) -> Self
src/adaptors.rs:85     {
src/adaptors.rs:86         FnMap{iter: iter, map: map}
src/adaptors.rs:87     }
src/lib.rs:1:9: 1:21 note: lint level defined here
src/lib.rs:1 #![warn(missing_docs)]
                     ^~~~~~~~~~~~
src/islice.rs:30:5: 37:6 warning: missing documentation for a method
src/islice.rs:30     pub fn new<R: GenericRange>(iter: I, range: R) -> Self
src/islice.rs:31     {
src/islice.rs:32         ISlice {
src/islice.rs:33             start: range.start().unwrap_or(0),
src/islice.rs:34             end: range.end().unwrap_or(::std::usize::MAX),
src/islice.rs:35             iter: iter,
                 ...
src/lib.rs:1:9: 1:21 note: lint level defined here
src/lib.rs:1 #![warn(missing_docs)]
                     ^~~~~~~~~~~~
src/misc.rs:15:5: 15:17 warning: missing documentation for an associated type
src/misc.rs:15     type Result;
                   ^~~~~~~~~~~~
src/lib.rs:1:9: 1:21 note: lint level defined here
src/lib.rs:1 #![warn(missing_docs)]
@bluss bluss changed the title Emit helpful note messages only once per occurrence Emit helpful note messages only once Apr 22, 2015
@steveklabnik steveklabnik added I-papercut A-diagnostics Area: Messages for errors, warnings, and lints labels Apr 22, 2015
@mitaa
Copy link
Contributor

mitaa commented Jan 19, 2016

I think this is a good "E-easy" candidate.

Relevant code is in src/librustc/lint/context.rs.
Using a HashSet<(Span, &str)> as a field on LintStore should work, so that we only emit a note the first time we encounter it.

zackmdavis added a commit to zackmdavis/rust that referenced this issue Jun 5, 2016
We introduce a new `one_time_diagnostics` field on
`rustc::session::Session` to hold a hashset of diagnostic messages we've
set once but don't want to see again (as uniquified by span and message
text), "lint level defined here" being the motivating example dealt with
here. It is the responsibility of the caller setting a diagnostic to
decide whether to add to `one_time_diagnostics`; there are other
situations where we likely do want it to be possible for a note to
appear twice on the same span with the same message (e.g., "prior
assignment occurs here").

This is in the matter of rust-lang#24690.
zackmdavis added a commit to zackmdavis/rust that referenced this issue Oct 15, 2016
We introduce a new `one_time_diagnostics` field on
`rustc::session::Session` to hold a hashset of diagnostic messages we've
set once but don't want to see again (as uniquified by span and message
text), "lint level defined here" being the motivating example dealt with
here.

This is in the matter of rust-lang#24690.
@nikomatsakis
Copy link
Contributor

Question -- in #37191, we get only "lint level defined here" per #[allow], even if it applies to multiple lints (e.g., #[allow(warnings)]). Do people think it'd be better to have it like this, or would you rather see different messages for (say) dead_code and unused_variable, even if they both apply to the same #[allow]?

zackmdavis added a commit to zackmdavis/rust that referenced this issue Oct 27, 2016
Jonathan D. Turner pointed out that we don't want to dedup in JSON
mode. Since the compile-test runner uses JSON output, we regrettably
need to revert the edits to existing tests; one imagines that testing
for one-time diagnosticity for humans will have to be added as a UI
test.

This remains in the matter of rust-lang#24690.
zackmdavis added a commit to zackmdavis/rust that referenced this issue Oct 27, 2016
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 rust-lang#24690.
bors added a commit that referenced this issue Oct 31, 2016
… r=nikomatsakis

introing one-time diagnostics: only emit "lint level defined here" once

This is a revised resubmission of PR #34084 (which was closed due to inactivity on account of time constraints on the author's part).
---

We introduce a new `one_time_diagnostics` field on
`rustc::session::Session` to hold a hashset of diagnostic messages we've
set once but don't want to see again (as uniquified by span and message
text), "lint level defined here" being the motivating example dealt with
here.

This is in the matter of #24690.
---

r? @nikomatsakis
@zackmdavis
Copy link
Member

Can someone with perms close this? I forgot to use the magic words in #37191.

@bluss
Copy link
Member Author

bluss commented Feb 5, 2017

Great! Fixed in #37191

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints
Projects
None yet
Development

No branches or pull requests

5 participants