Skip to content

Commit

Permalink
introducing lint reason annotations (RFC 2383)
Browse files Browse the repository at this point in the history
This is just for the `reason =` name-value meta-item; the
`#[expect(lint)]` attribute also described in the RFC is a problem for
another day.

The place where we were directly calling `emit()` on a match block
(whose arms returned a mutable reference to a diagnostic-builder) was
admittedly cute, but no longer plausibly natural after adding the
if-let to the end of the `LintSource::Node` arm.

This regards rust-lang#54503.
  • Loading branch information
zackmdavis committed Oct 14, 2018
1 parent 14f42a7 commit 16e3ea2
Show file tree
Hide file tree
Showing 8 changed files with 211 additions and 20 deletions.
69 changes: 52 additions & 17 deletions src/librustc/lint/levels.rs
Original file line number Diff line number Diff line change
Expand Up @@ -199,8 +199,7 @@ impl<'a> LintLevelsBuilder<'a> {
let store = self.sess.lint_store.borrow();
let sess = self.sess;
let bad_attr = |span| {
span_err!(sess, span, E0452,
"malformed lint attribute");
struct_span_err!(sess, span, E0452, "malformed lint attribute")
};
for attr in attrs {
let level = match Level::from_str(&attr.name().as_str()) {
Expand All @@ -214,17 +213,45 @@ impl<'a> LintLevelsBuilder<'a> {
let metas = if let Some(metas) = meta.meta_item_list() {
metas
} else {
bad_attr(meta.span);
let mut err = bad_attr(meta.span);
err.emit();
continue
};

// Before processing the lint names, look for a reason (RFC 2383).
let mut reason = None;
for li in metas {
if let Some(item) = li.meta_item() {
match item.node {
ast::MetaItemKind::Word => {} // actual lint names handled later
ast::MetaItemKind::NameValue(ref name_value) => {
let name_ident = item.ident.segments[0].ident;
let name = name_ident.name.as_str();
if name == "reason" {
if let ast::LitKind::Str(rationale, _) = name_value.node {
reason = Some(rationale);
} else {
let mut err = bad_attr(name_value.span);
err.help("reason must be a string literal");
err.emit();
}
} else {
let mut err = bad_attr(item.span);
err.emit();
}
},
ast::MetaItemKind::List(_) => {
let mut err = bad_attr(item.span);
err.emit();
}
}
}
}

for li in metas {
let word = match li.word() {
Some(word) => word,
None => {
bad_attr(li.span);
continue
}
None => { continue; }
};
let tool_name = if let Some(lint_tool) = word.is_scoped() {
if !attr::is_known_lint_tool(lint_tool) {
Expand All @@ -245,7 +272,7 @@ impl<'a> LintLevelsBuilder<'a> {
let name = word.name();
match store.check_lint_name(&name.as_str(), tool_name) {
CheckLintNameResult::Ok(ids) => {
let src = LintSource::Node(name, li.span);
let src = LintSource::Node(name, li.span, reason);
for id in ids {
specs.insert(*id, (level, src));
}
Expand All @@ -255,7 +282,9 @@ impl<'a> LintLevelsBuilder<'a> {
match result {
Ok(ids) => {
let complete_name = &format!("{}::{}", tool_name.unwrap(), name);
let src = LintSource::Node(Symbol::intern(complete_name), li.span);
let src = LintSource::Node(
Symbol::intern(complete_name), li.span, reason
);
for id in ids {
specs.insert(*id, (level, src));
}
Expand Down Expand Up @@ -286,7 +315,9 @@ impl<'a> LintLevelsBuilder<'a> {
Applicability::MachineApplicable,
).emit();

let src = LintSource::Node(Symbol::intern(&new_lint_name), li.span);
let src = LintSource::Node(
Symbol::intern(&new_lint_name), li.span, reason
);
for id in ids {
specs.insert(*id, (level, src));
}
Expand Down Expand Up @@ -368,11 +399,11 @@ impl<'a> LintLevelsBuilder<'a> {
};
let forbidden_lint_name = match forbid_src {
LintSource::Default => id.to_string(),
LintSource::Node(name, _) => name.to_string(),
LintSource::Node(name, _, _) => name.to_string(),
LintSource::CommandLine(name) => name.to_string(),
};
let (lint_attr_name, lint_attr_span) = match *src {
LintSource::Node(name, span) => (name, span),
LintSource::Node(name, span, _) => (name, span),
_ => continue,
};
let mut diag_builder = struct_span_err!(self.sess,
Expand All @@ -384,15 +415,19 @@ impl<'a> LintLevelsBuilder<'a> {
forbidden_lint_name);
diag_builder.span_label(lint_attr_span, "overruled by previous forbid");
match forbid_src {
LintSource::Default => &mut diag_builder,
LintSource::Node(_, forbid_source_span) => {
LintSource::Default => {},
LintSource::Node(_, forbid_source_span, reason) => {
diag_builder.span_label(forbid_source_span,
"`forbid` level set here")
"`forbid` level set here");
if let Some(rationale) = reason {
diag_builder.note(&rationale.as_str());
}
},
LintSource::CommandLine(_) => {
diag_builder.note("`forbid` lint level was set on command line")
diag_builder.note("`forbid` lint level was set on command line");
}
}.emit();
}
diag_builder.emit();
// don't set a separate error for every lint in the group
break
}
Expand Down
9 changes: 6 additions & 3 deletions src/librustc/lint/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -470,15 +470,15 @@ pub enum LintSource {
Default,

/// Lint level was set by an attribute.
Node(ast::Name, Span),
Node(ast::Name, Span, Option<Symbol> /* RFC 2383 reason */),

/// Lint level was set by a command-line flag.
CommandLine(Symbol),
}

impl_stable_hash_for!(enum self::LintSource {
Default,
Node(name, span),
Node(name, span, reason),
CommandLine(text)
});

Expand Down Expand Up @@ -578,7 +578,10 @@ pub fn struct_lint_level<'a>(sess: &'a Session,
hyphen_case_flag_val));
}
}
LintSource::Node(lint_attr_name, src) => {
LintSource::Node(lint_attr_name, src, reason) => {
if let Some(rationale) = reason {
err.note(&rationale.as_str());
}
sess.diag_span_note_once(&mut err, DiagnosticMessageId::from(lint),
src, "lint level defined here");
if lint_attr_name.as_str() != name {
Expand Down
16 changes: 16 additions & 0 deletions src/test/ui/lint/reasons-erroneous.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
#![warn(absolute_paths_not_starting_with_crate, reason = 0)]
//~^ ERROR malformed lint attribute
//~| HELP reason must be a string literal
#![warn(anonymous_parameters, reason = b"consider these, for we have condemned them")]
//~^ ERROR malformed lint attribute
//~| HELP reason must be a string literal
#![warn(bare_trait_objects, reasons = "leaders to no sure land, guides their bearings lost")]
//~^ ERROR malformed lint attribute
#![warn(box_pointers, blerp = "or in league with robbers have reversed the signposts")]
//~^ ERROR malformed lint attribute
#![warn(elided_lifetimes_in_paths, reason("disrespectful to ancestors", "irresponsible to heirs"))]
//~^ ERROR malformed lint attribute
#![warn(ellipsis_inclusive_range_patterns, reason)]
//~^ WARN unknown lint

fn main() {}
45 changes: 45 additions & 0 deletions src/test/ui/lint/reasons-erroneous.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
error[E0452]: malformed lint attribute
--> $DIR/reasons-erroneous.rs:1:58
|
LL | #![warn(absolute_paths_not_starting_with_crate, reason = 0)]
| ^
|
= help: reason must be a string literal

error[E0452]: malformed lint attribute
--> $DIR/reasons-erroneous.rs:4:40
|
LL | #![warn(anonymous_parameters, reason = b"consider these, for we have condemned them")]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: reason must be a string literal

error[E0452]: malformed lint attribute
--> $DIR/reasons-erroneous.rs:7:29
|
LL | #![warn(bare_trait_objects, reasons = "leaders to no sure land, guides their bearings lost")]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error[E0452]: malformed lint attribute
--> $DIR/reasons-erroneous.rs:9:23
|
LL | #![warn(box_pointers, blerp = "or in league with robbers have reversed the signposts")]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error[E0452]: malformed lint attribute
--> $DIR/reasons-erroneous.rs:11:36
|
LL | #![warn(elided_lifetimes_in_paths, reason("disrespectful to ancestors", "irresponsible to heirs"))]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

warning: unknown lint: `reason`
--> $DIR/reasons-erroneous.rs:13:44
|
LL | #![warn(ellipsis_inclusive_range_patterns, reason)]
| ^^^^^^
|
= note: #[warn(unknown_lints)] on by default

error: aborting due to 5 previous errors

For more information about this error, try `rustc --explain E0452`.
19 changes: 19 additions & 0 deletions src/test/ui/lint/reasons-forbidden.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
#![forbid(
unsafe_code,
//~^ NOTE `forbid` level set here
reason = "our errors & omissions insurance policy doesn't cover unsafe Rust"
)]

use std::ptr;

fn main() {
let a_billion_dollar_mistake = ptr::null();

#[allow(unsafe_code)]
//~^ ERROR allow(unsafe_code) overruled by outer forbid(unsafe_code)
//~| NOTE overruled by previous forbid
//~| NOTE our errors & omissions insurance policy doesn't cover unsafe Rust
unsafe {
*a_billion_dollar_mistake
}
}
14 changes: 14 additions & 0 deletions src/test/ui/lint/reasons-forbidden.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
error[E0453]: allow(unsafe_code) overruled by outer forbid(unsafe_code)
--> $DIR/reasons-forbidden.rs:12:13
|
LL | unsafe_code,
| ----------- `forbid` level set here
...
LL | #[allow(unsafe_code)]
| ^^^^^^^^^^^ overruled by previous forbid
|
= note: our errors & omissions insurance policy doesn't cover unsafe Rust

error: aborting due to previous error

For more information about this error, try `rustc --explain E0453`.
31 changes: 31 additions & 0 deletions src/test/ui/lint/reasons.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
// compile-pass

#![warn(elided_lifetimes_in_paths,
//~^ NOTE lint level defined here
reason = "explicit anonymous lifetimes aid reasoning about ownership")]
#![warn(
nonstandard_style,
//~^ NOTE lint level defined here
reason = r#"people shouldn't have to change their usual style habits
to contribute to our project"#
)]
#![allow(unused, reason = "unused code has never killed anypony")]

use std::fmt;

pub struct CheaterDetectionMechanism {}

impl fmt::Debug for CheaterDetectionMechanism {
fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
//~^ WARN hidden lifetime parameters in types are deprecated
//~| NOTE explicit anonymous lifetimes aid
//~| HELP indicate the anonymous lifetime
fmt.debug_struct("CheaterDetectionMechanism").finish()
}
}

fn main() {
let Social_exchange_psychology = CheaterDetectionMechanism {};
//~^ WARN should have a snake case name such as
//~| NOTE people shouldn't have to change their usual style habits
}
28 changes: 28 additions & 0 deletions src/test/ui/lint/reasons.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
warning: hidden lifetime parameters in types are deprecated
--> $DIR/reasons.rs:19:29
|
LL | fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
| ^^^^^^^^^^^^^^- help: indicate the anonymous lifetime: `<'_>`
|
= note: explicit anonymous lifetimes aid reasoning about ownership
note: lint level defined here
--> $DIR/reasons.rs:3:9
|
LL | #![warn(elided_lifetimes_in_paths,
| ^^^^^^^^^^^^^^^^^^^^^^^^^

warning: variable `Social_exchange_psychology` should have a snake case name such as `social_exchange_psychology`
--> $DIR/reasons.rs:28:9
|
LL | let Social_exchange_psychology = CheaterDetectionMechanism {};
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: people shouldn't have to change their usual style habits
to contribute to our project
note: lint level defined here
--> $DIR/reasons.rs:7:5
|
LL | nonstandard_style,
| ^^^^^^^^^^^^^^^^^
= note: #[warn(non_snake_case)] implied by #[warn(nonstandard_style)]

0 comments on commit 16e3ea2

Please sign in to comment.