Auto merge of #30878 - brson:raw-pointer-derive, r=brson

This adds back the raw_pointer_derive lint as a 'removed' lint, so that its removal does not cause errors (#30346) but warnings.

In the process I discovered regressions in the code for renamed and removed lints, which didn't appear to have any tests. The addition of a second lint pass (ast vs. hir) meant that attributes were being inspected twice, renamed and removed warnings printed twice. I restructured the code so these tests are only done once and added tests. Unfortunately it makes the patch more complicated for the needed beta backport.

r? @nikomatsakis

10 files changed, 272 insertions, 38 deletions

diff --git a/src/librustc/lint/builtin.rs b/src/librustc/lint/builtin.rs
index 3689312bed1..3676e230646 100644
--- a/src/librustc/lint/builtin.rs
+++ b/src/librustc/lint/builtin.rs
@@ -136,6 +136,12 @@ declare_lint! {
     "unit struct or enum variant erroneously allowed to match via path::ident(..)"
 }
 
+declare_lint! {
+    pub RAW_POINTER_DERIVE,
+    Warn,
+    "uses of #[derive] with raw pointers are rarely correct"
+}
+
 /// Does nothing as a lint pass, but registers some `Lint`s
 /// which are used by other parts of the compiler.
 #[derive(Copy, Clone)]
@@ -163,7 +169,8 @@ impl LintPass for HardwiredLints {
             PRIVATE_IN_PUBLIC,
             INVALID_TYPE_PARAM_DEFAULT,
             MATCH_OF_UNIT_VARIANT_VIA_PAREN_DOTDOT,
-            CONST_ERR
+            CONST_ERR,
+            RAW_POINTER_DERIVE
         )
     }
 }
diff --git a/src/librustc/lint/context.rs b/src/librustc/lint/context.rs
index 1abfa2bfc59..c41a361fcc3 100644
--- a/src/librustc/lint/context.rs
+++ b/src/librustc/lint/context.rs
@@ -247,21 +247,10 @@ impl LintStore {
     {
         match self.by_name.get(lint_name) {
             Some(&Id(lint_id)) => Ok(lint_id),
-            Some(&Renamed(ref new_name, lint_id)) => {
-                let warning = format!("lint {} has been renamed to {}",
-                                      lint_name, new_name);
-                match span {
-                    Some(span) => sess.span_warn(span, &warning[..]),
-                    None => sess.warn(&warning[..]),
-                };
+            Some(&Renamed(_, lint_id)) => {
                 Ok(lint_id)
             },
             Some(&Removed(ref reason)) => {
-                let warning = format!("lint {} has been removed: {}", lint_name, reason);
-                match span {
-                    Some(span) => sess.span_warn(span, &warning[..]),
-                    None => sess.warn(&warning[..])
-                }
                 Err(FindLintError::Removed)
             },
             None => Err(FindLintError::NotFound)
@@ -270,8 +259,12 @@ impl LintStore {
 
     pub fn process_command_line(&mut self, sess: &Session) {
         for &(ref lint_name, level) in &sess.opts.lint_opts {
+            check_lint_name_cmdline(sess, self,
+                                    &lint_name[..], level);
+
             match self.find_lint(&lint_name[..], sess, None) {
                 Ok(lint_id) => self.set_level(lint_id, (level, CommandLine)),
+                Err(FindLintError::Removed) => { }
                 Err(_) => {
                     match self.lint_groups.iter().map(|(&x, pair)| (x, pair.0.clone()))
                                                  .collect::<FnvHashMap<&'static str,
@@ -283,8 +276,11 @@ impl LintStore {
                                      self.set_level(*lint_id, (level, CommandLine)))
                              .collect::<Vec<()>>();
                         }
-                        None => sess.err(&format!("unknown {} flag: {}",
-                                                 level.as_str(), lint_name)),
+                        None => {
+                            // The lint or lint group doesn't exist.
+                            // This is an error, but it was handled
+                            // by check_lint_name_cmdline.
+                        }
                     }
                 }
             }
@@ -359,29 +355,39 @@ pub fn gather_attrs(attrs: &[ast::Attribute])
                     -> Vec<Result<(InternedString, Level, Span), Span>> {
     let mut out = vec!();
     for attr in attrs {
-        let level = match Level::from_str(&attr.name()) {
-            None => continue,
-            Some(lvl) => lvl,
-        };
+        let r = gather_attr(attr);
+        out.extend(r.into_iter());
+    }
+    out
+}
 
-        attr::mark_used(attr);
+pub fn gather_attr(attr: &ast::Attribute)
+                   -> Vec<Result<(InternedString, Level, Span), Span>> {
+    let mut out = vec!();
 
-        let meta = &attr.node.value;
-        let metas = match meta.node {
-            ast::MetaList(_, ref metas) => metas,
-            _ => {
-                out.push(Err(meta.span));
-                continue;
-            }
-        };
+    let level = match Level::from_str(&attr.name()) {
+        None => return out,
+        Some(lvl) => lvl,
+    };
 
-        for meta in metas {
-            out.push(match meta.node {
-                ast::MetaWord(ref lint_name) => Ok((lint_name.clone(), level, meta.span)),
-                _ => Err(meta.span),
-            });
+    attr::mark_used(attr);
+
+    let meta = &attr.node.value;
+    let metas = match meta.node {
+        ast::MetaList(_, ref metas) => metas,
+        _ => {
+            out.push(Err(meta.span));
+            return out;
         }
+    };
+
+    for meta in metas {
+        out.push(match meta.node {
+            ast::MetaWord(ref lint_name) => Ok((lint_name.clone(), level, meta.span)),
+            _ => Err(meta.span),
+        });
     }
+
     out
 }
 
@@ -587,9 +593,9 @@ pub trait LintContext: Sized {
                                                            (*lint_id, level, span))
                                                       .collect(),
                                 None => {
-                                    self.span_lint(builtin::UNKNOWN_LINTS, span,
-                                                   &format!("unknown `{}` attribute: `{}`",
-                                                            level.as_str(), lint_name));
+                                    // The lint or lint group doesn't exist.
+                                    // This is an error, but it was handled
+                                    // by check_lint_name_attribute.
                                     continue;
                                 }
                             }
@@ -901,6 +907,7 @@ impl<'a, 'tcx, 'v> hir_visit::Visitor<'v> for LateContext<'a, 'tcx> {
     }
 
     fn visit_attribute(&mut self, attr: &ast::Attribute) {
+        check_lint_name_attribute(self, attr);
         run_lints!(self, check_attribute, late_passes, attr);
     }
 }
@@ -1114,6 +1121,113 @@ impl LateLintPass for GatherNodeLevels {
     }
 }
 
+enum CheckLintNameResult<'a> {
+    Ok,
+    // Lint doesn't exist
+    NoLint,
+    // The lint is either renamed or removed and a warning was
+    // generated in the DiagnosticBuilder
+    Mentioned(DiagnosticBuilder<'a>)
+}
+
+/// Checks the name of a lint for its existence, and whether it was
+/// renamed or removed. Generates a DiagnosticBuilder containing a
+/// warning for renamed and removed lints. This is over both lint
+/// names from attributes and those passed on the command line. Since
+/// it emits non-fatal warnings and there are *two* lint passes that
+/// inspect attributes, this is only run from the late pass to avoid
+/// printing duplicate warnings.
+fn check_lint_name<'a>(sess: &'a Session,
+                       lint_cx: &LintStore,
+                       lint_name: &str,
+                       span: Option<Span>) -> CheckLintNameResult<'a> {
+    match lint_cx.by_name.get(lint_name) {
+        Some(&Renamed(ref new_name, _)) => {
+            let warning = format!("lint {} has been renamed to {}",
+                                  lint_name, new_name);
+            let db = match span {
+                Some(span) => sess.struct_span_warn(span, &warning[..]),
+                None => sess.struct_warn(&warning[..]),
+            };
+            CheckLintNameResult::Mentioned(db)
+        },
+        Some(&Removed(ref reason)) => {
+            let warning = format!("lint {} has been removed: {}", lint_name, reason);
+            let db = match span {
+                Some(span) => sess.struct_span_warn(span, &warning[..]),
+                None => sess.struct_warn(&warning[..])
+            };
+            CheckLintNameResult::Mentioned(db)
+        },
+        None => {
+            match lint_cx.lint_groups.get(lint_name) {
+                None => {
+                    CheckLintNameResult::NoLint
+                }
+                Some(_) => {
+                    /* lint group exists */
+                    CheckLintNameResult::Ok
+                }
+            }
+        }
+        Some(_) => {
+            /* lint exists */
+            CheckLintNameResult::Ok
+        }
+    }
+}
+
+// Checks the validity of lint names derived from attributes
+fn check_lint_name_attribute(cx: &LateContext, attr: &ast::Attribute) {
+    for result in gather_attr(attr) {
+        match result {
+            Err(_) => {
+                // Malformed lint attr. Reported by with_lint_attrs
+                continue;
+            }
+            Ok((lint_name, _, span)) => {
+                match check_lint_name(&cx.tcx.sess, &cx.lints, &lint_name[..], Some(span)) {
+                    CheckLintNameResult::Ok => (),
+                    CheckLintNameResult::Mentioned(mut db) => {
+                        db.emit();
+                    }
+                    CheckLintNameResult::NoLint => {
+                        cx.span_lint(builtin::UNKNOWN_LINTS, span,
+                                     &format!("unknown lint: `{}`",
+                                              lint_name));
+                    }
+                }
+            }
+        }
+    }
+}
+
+// Checks the validity of lint names derived from the command line
+fn check_lint_name_cmdline(sess: &Session, lint_cx: &LintStore,
+                           lint_name: &str, level: Level) {
+    let db = match check_lint_name(sess, lint_cx, lint_name, None) {
+        CheckLintNameResult::Ok => None,
+        CheckLintNameResult::Mentioned(db) => Some(db),
+        CheckLintNameResult::NoLint => {
+            Some(sess.struct_err(&format!("unknown lint: `{}`", lint_name)))
+        }
+    };
+
+    if let Some(mut db) = db {
+        let msg = format!("requested on the command line with `{} {}`",
+                          match level {
+                              Level::Allow => "-A",
+                              Level::Warn => "-W",
+                              Level::Deny => "-D",
+                              Level::Forbid => "-F",
+                          },
+                          lint_name);
+        db.note(&msg);
+        db.emit();
+    }
+}
+
+
 /// Perform lint checking on a crate.
 ///
 /// Consumes the `lint_store` field of the `Session`.
diff --git a/src/librustc_lint/lib.rs b/src/librustc_lint/lib.rs
index 53acbe3af67..e0ed20d31ff 100644
--- a/src/librustc_lint/lib.rs
+++ b/src/librustc_lint/lib.rs
@@ -172,9 +172,12 @@ pub fn register_builtins(store: &mut lint::LintStore, sess: Option<&Session>) {
     // We have one lint pass defined specially
     store.register_late_pass(sess, false, box lint::GatherNodeLevels);
 
-    // Insert temporary renamings for a one-time deprecation
+    // Register renamed and removed lints
     store.register_renamed("unknown_features", "unused_features");
-
     store.register_removed("unsigned_negation", "replaced by negate_unsigned feature gate");
     store.register_removed("negate_unsigned", "cast a signed value instead");
+    store.register_removed("raw_pointer_derive", "using derive with raw pointers is ok");
+    // This was renamed to raw_pointer_derive, which was then removed,
+    // so it is also considered removed
+    store.register_removed("raw_pointer_deriving", "using derive with raw pointers is ok");
 }
diff --git a/src/test/compile-fail/lint-malformed.rs b/src/test/compile-fail/lint-malformed.rs
new file mode 100644
index 00000000000..592e2b11905
--- /dev/null
+++ b/src/test/compile-fail/lint-malformed.rs
@@ -0,0 +1,14 @@
+// Copyright 2016 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+#![deny = "foo"] //~ ERR malformed lint attribute
+#![allow(bar = "baz")] //~ ERR malformed lint attribute
+
+fn main() { }
diff --git a/src/test/compile-fail/lint-removed-cmdline.rs b/src/test/compile-fail/lint-removed-cmdline.rs
new file mode 100644
index 00000000000..d6bfd1eec39
--- /dev/null
+++ b/src/test/compile-fail/lint-removed-cmdline.rs
@@ -0,0 +1,20 @@
+// Copyright 2016 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+// The raw_pointer_derived lint warns about its removal
+// cc #30346
+
+// compile-flags:-D raw_pointer_derive
+
+// error-pattern:lint raw_pointer_derive has been removed
+// error-pattern:requested on the command line with `-D raw_pointer_derive`
+
+#[deny(warnings)]
+fn main() { let unused = (); }
diff --git a/src/test/compile-fail/lint-removed.rs b/src/test/compile-fail/lint-removed.rs
new file mode 100644
index 00000000000..e196e128b17
--- /dev/null
+++ b/src/test/compile-fail/lint-removed.rs
@@ -0,0 +1,16 @@
+// Copyright 2016 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+// The raw_pointer_derived lint only warns about its own removal
+// cc #30346
+
+#[deny(raw_pointer_derive)] //~ WARN raw_pointer_derive has been removed
+#[deny(warnings)]
+fn main() { let unused = (); } //~ ERR unused
diff --git a/src/test/compile-fail/lint-renamed-cmdline.rs b/src/test/compile-fail/lint-renamed-cmdline.rs
new file mode 100644
index 00000000000..3b352b384ff
--- /dev/null
+++ b/src/test/compile-fail/lint-renamed-cmdline.rs
@@ -0,0 +1,18 @@
+// Copyright 2016 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+// compile-flags:-D unknown_features
+
+// error-pattern:lint unknown_features has been renamed to unused_features
+// error-pattern:requested on the command line with `-D unknown_features`
+// error-pattern:unused
+
+#[deny(unused)]
+fn main() { let unused = (); }
diff --git a/src/test/compile-fail/lint-renamed.rs b/src/test/compile-fail/lint-renamed.rs
new file mode 100644
index 00000000000..2e85a323a1c
--- /dev/null
+++ b/src/test/compile-fail/lint-renamed.rs
@@ -0,0 +1,13 @@
+// Copyright 2016 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+#[deny(unknown_features)] //~ WARN lint unknown_features has been renamed to unused_features
+#[deny(unused)]
+fn main() { let unused = (); } //~ ERR unused
diff --git a/src/test/compile-fail/lint-unknown-lint-cmdline.rs b/src/test/compile-fail/lint-unknown-lint-cmdline.rs
new file mode 100644
index 00000000000..0c41959f8a7
--- /dev/null
+++ b/src/test/compile-fail/lint-unknown-lint-cmdline.rs
@@ -0,0 +1,16 @@
+// Copyright 2016 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+// compile-flags:-D bogus
+
+// error-pattern:unknown lint
+// error-pattern:requested on the command line with `-D bogus`
+
+fn main() { }
diff --git a/src/test/compile-fail/lint-unknown-lint.rs b/src/test/compile-fail/lint-unknown-lint.rs
new file mode 100644
index 00000000000..8f20a2c8ab7
--- /dev/null
+++ b/src/test/compile-fail/lint-unknown-lint.rs
@@ -0,0 +1,13 @@
+// Copyright 2016 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+#![allow(not_a_real_lint)] //~ WARN unknown lint
+#![deny(unused)]
+fn main() { let unused = (); } //~ ERR unused variable