about summary refs log tree commit diff
diff options
context:
space:
mode:
authorZack M. Davis <code@zackmdavis.net>2018-09-29 17:25:26 -0700
committerZack M. Davis <code@zackmdavis.net>2018-10-27 12:28:48 -0700
commit630c6a544f91a29f2a1749dd438e7082400295da (patch)
tree1d6100acf111dbce3aaf8c81272b18403a57f454
parentf32f1113c93fe84f252293ae4af86be1ceae4a84 (diff)
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.
-rw-r--r--src/librustc/lint/levels.rs69
-rw-r--r--src/librustc/lint/mod.rs9
-rw-r--r--src/test/ui/lint/reasons-erroneous.rs16
-rw-r--r--src/test/ui/lint/reasons-erroneous.stderr45
-rw-r--r--src/test/ui/lint/reasons-forbidden.rs19
-rw-r--r--src/test/ui/lint/reasons-forbidden.stderr14
-rw-r--r--src/test/ui/lint/reasons.rs31
-rw-r--r--src/test/ui/lint/reasons.stderr28
8 files changed, 211 insertions, 20 deletions
diff --git a/src/librustc/lint/levels.rs b/src/librustc/lint/levels.rs
index 6a4f7346745..86d0a5a4790 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 5ac0c0d32dc..afd78008109 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<Symbol> /* 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 00000000000..68f2ce42847
--- /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 00000000000..25bdda178ee
--- /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 00000000000..cad62b3a3c1
--- /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 00000000000..cc9e787b2d4
--- /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 00000000000..9166caf9ebe
--- /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 00000000000..bdaf848c340
--- /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)]
+