From 630c6a544f91a29f2a1749dd438e7082400295da Mon Sep 17 00:00:00 2001 From: "Zack M. Davis" Date: Sat, 29 Sep 2018 17:25:26 -0700 Subject: [PATCH] introducing lint reason annotations (RFC 2383) 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 #54503. --- src/librustc/lint/levels.rs | 69 +++++++++++++++++------ src/librustc/lint/mod.rs | 9 ++- src/test/ui/lint/reasons-erroneous.rs | 16 ++++++ src/test/ui/lint/reasons-erroneous.stderr | 45 +++++++++++++++ src/test/ui/lint/reasons-forbidden.rs | 19 +++++++ src/test/ui/lint/reasons-forbidden.stderr | 14 +++++ src/test/ui/lint/reasons.rs | 31 ++++++++++ src/test/ui/lint/reasons.stderr | 28 +++++++++ 8 files changed, 211 insertions(+), 20 deletions(-) create mode 100644 src/test/ui/lint/reasons-erroneous.rs create mode 100644 src/test/ui/lint/reasons-erroneous.stderr create mode 100644 src/test/ui/lint/reasons-forbidden.rs create mode 100644 src/test/ui/lint/reasons-forbidden.stderr create mode 100644 src/test/ui/lint/reasons.rs create mode 100644 src/test/ui/lint/reasons.stderr diff --git a/src/librustc/lint/levels.rs b/src/librustc/lint/levels.rs index 6a4f73467456..86d0a5a47900 100644 --- a/src/librustc/lint/levels.rs +++ b/src/librustc/lint/levels.rs @@ -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()) { @@ -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) { @@ -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)); } @@ -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)); } @@ -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)); } @@ -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, @@ -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 } diff --git a/src/librustc/lint/mod.rs b/src/librustc/lint/mod.rs index 5ac0c0d32dcd..afd780081098 100644 --- a/src/librustc/lint/mod.rs +++ b/src/librustc/lint/mod.rs @@ -470,7 +470,7 @@ pub enum LintSource { Default, /// Lint level was set by an attribute. - Node(ast::Name, Span), + Node(ast::Name, Span, Option /* RFC 2383 reason */), /// Lint level was set by a command-line flag. CommandLine(Symbol), @@ -478,7 +478,7 @@ pub enum LintSource { impl_stable_hash_for!(enum self::LintSource { Default, - Node(name, span), + Node(name, span, reason), CommandLine(text) }); @@ -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 { diff --git a/src/test/ui/lint/reasons-erroneous.rs b/src/test/ui/lint/reasons-erroneous.rs new file mode 100644 index 000000000000..68f2ce42847d --- /dev/null +++ b/src/test/ui/lint/reasons-erroneous.rs @@ -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() {} diff --git a/src/test/ui/lint/reasons-erroneous.stderr b/src/test/ui/lint/reasons-erroneous.stderr new file mode 100644 index 000000000000..25bdda178eee --- /dev/null +++ b/src/test/ui/lint/reasons-erroneous.stderr @@ -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`. diff --git a/src/test/ui/lint/reasons-forbidden.rs b/src/test/ui/lint/reasons-forbidden.rs new file mode 100644 index 000000000000..cad62b3a3c16 --- /dev/null +++ b/src/test/ui/lint/reasons-forbidden.rs @@ -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 + } +} diff --git a/src/test/ui/lint/reasons-forbidden.stderr b/src/test/ui/lint/reasons-forbidden.stderr new file mode 100644 index 000000000000..cc9e787b2d42 --- /dev/null +++ b/src/test/ui/lint/reasons-forbidden.stderr @@ -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`. diff --git a/src/test/ui/lint/reasons.rs b/src/test/ui/lint/reasons.rs new file mode 100644 index 000000000000..9166caf9ebec --- /dev/null +++ b/src/test/ui/lint/reasons.rs @@ -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 +} diff --git a/src/test/ui/lint/reasons.stderr b/src/test/ui/lint/reasons.stderr new file mode 100644 index 000000000000..bdaf848c340e --- /dev/null +++ b/src/test/ui/lint/reasons.stderr @@ -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)] +