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

Lint errors resulting from lint groups or "warnings" meta-lint obscure the original lint name #36846

Closed
brson opened this issue Sep 30, 2016 · 3 comments
Labels
A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@brson
Copy link
Contributor

brson commented Sep 30, 2016

This has frustrated me several times. Lint errors are at their best when they tell you which lint triggered them. Example

fn main() { }
fn foo() { }
warning: function is never used: `foo`, #[warn(dead_code)] on by default
 --> main.rs:2:1
  |
2 | fn foo() { }
  | ^^^^^^^^^^^^

So now I know that if the lint is wrong I can write allow(dead_code). On the other hand:

#![deny(warnings)]
fn main() { }
fn foo() { }
error: function is never used: `foo`
 --> main.rs:3:1
  |
3 | fn foo() { }
  | ^^^^^^^^^^^^
  |
note: lint level defined here
 --> main.rs:1:9
  |
1 | #![deny(warnings)]
  |         ^^^^^^^^

error: aborting due to previous error

Now I have no idea what to write to shut the compiler up. Likewise:

#![deny(unused)]
fn main() { }
fn foo() { }
@brson brson added I-papercut A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Sep 30, 2016
@brson
Copy link
Contributor Author

brson commented Sep 30, 2016

cc @jonathandturner

@zackmdavis
Copy link
Member

Adding the lint name to the message when the source of the lint level was an attribute should be easy enough. I think we can do better, though, if we also add the attribute to the enum data recording where the lint level was set; that way, we can only add the lint name to the message when it doesn't match the attribute (as for #[deny(warnings)] and the like) and not when it's obvious from the "lint level defined here" note.

I'm happy to work on this.

@sophiajt
Copy link
Contributor

sophiajt commented Dec 1, 2016

@zackmdavis - sounds good. Would be curious to see what the mockups of your changes would look like

frewsxcv pushed a commit to frewsxcv/rust that referenced this issue Jan 9, 2017
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 rust-lang#36846.
frewsxcv pushed a commit to frewsxcv/rust that referenced this issue Jan 9, 2017
Previously, the note/message for the source of a lint being the command
line unconditionally named the individual lint, even if the actual
command specified a lint group (e.g., `-D warnings`); here, we take note
of the actual command options so we can be more specific.

This remains in the matter of rust-lang#36846.
frewsxcv pushed a commit to frewsxcv/rust that referenced this issue Jan 9, 2017
As suggested by Niko Matsakis in review
(rust-lang#38103 (comment)) regarding
the endeavor prompted by rust-lang#36846.
zackmdavis added a commit to zackmdavis/rust that referenced this issue Feb 4, 2017
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 rust-lang#36846.
zackmdavis added a commit to zackmdavis/rust that referenced this issue Feb 4, 2017
Previously, the note/message for the source of a lint being the command
line unconditionally named the individual lint, even if the actual
command specified a lint group (e.g., `-D warnings`); here, we take note
of the actual command options so we can be more specific.

This remains in the matter of rust-lang#36846.
zackmdavis added a commit to zackmdavis/rust that referenced this issue Feb 4, 2017
As suggested by Niko Matsakis in review
(rust-lang#38103 (comment)) regarding
the endeavor prompted by rust-lang#36846.
bors added a commit that referenced this issue Feb 5, 2017
…ups_or_warnings_meta-lint_obscure_the_original_lint_name, r=nikomatsakis

note individual lint name in messages set via lint group attribute

![lint_errors_resulting_from_lint_groups_obscure](https://cloud.githubusercontent.com/assets/1076988/20783614/c107d5c8-b749-11e6-85de-eada7f67c986.png)

Resolves #36846.

r? @jonathandturner

-----

***Update*** 16 December (new commits):
![lint_group_makeover_party](https://cloud.githubusercontent.com/assets/1076988/21284540/ff1ae2fc-c3d2-11e6-93be-d0689f5fa7a8.png)
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 T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

3 participants