rustc: Fix bugs in renamed and removed lints and re-add raw_pointer_derive

brson · brson · commit a74d64713c38 · 2016-01-16T06:33:48.000Z
cc #30346 Conflicts: src/librustc_lint/lib.rs
diff --git a/src/librustc/lint/builtin.rs b/src/librustc/lint/builtin.rs
@@ -127,6 +127,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)]
@@ -152,7 +158,8 @@ impl LintPass for HardwiredLints {
             TRIVIAL_CASTS,
             TRIVIAL_NUMERIC_CASTS,
             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
@@ -218,21 +218,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)
@@ -241,8 +230,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,
@@ -254,8 +247,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.
+                        }
                     }
                 }
             }
@@ -330,29 +326,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
 }
 
@@ -510,9 +516,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;
                                 }
                             }
@@ -824,6 +830,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);
     }
 }
@@ -1037,6 +1044,110 @@ impl LateLintPass for GatherNodeLevels {
     }
 }
 
+enum CheckLintNameResult {
+    Ok,
+    // Lint doesn't exist
+    NoLint,
+    // The lint is either renamed or removed and a warning was generated
+    Mentioned
+}
+
+/// 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(sess: &Session,
+                   lint_cx: &LintStore,
+                   lint_name: &str,
+                   span: Option<Span>) -> CheckLintNameResult {
+    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);
+            match span {
+                Some(span) => sess.span_warn(span, &warning[..]),
+                None => sess.warn(&warning[..]),
+            };
+            CheckLintNameResult::Mentioned
+        },
+        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[..])
+            };
+            CheckLintNameResult::Mentioned
+        },
+        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 => (),
+                    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 explain = match check_lint_name(sess, lint_cx, lint_name, None) {
+        CheckLintNameResult::Ok => false,
+        CheckLintNameResult::Mentioned => true,
+        CheckLintNameResult::NoLint => {
+            sess.err(&format!("unknown lint: `{}`", lint_name));
+            true
+        }
+    };
+
+    if explain {
+        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);
+        sess.note(&msg);
+    }
+}
+
+
 /// 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
@@ -152,8 +152,11 @@ 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("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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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

Original file line number	Diff line number	Diff line change
`@@ -127,6 +127,12 @@ declare_lint! {`
`127`	`127`	`"unit struct or enum variant erroneously allowed to match via path::ident(..)"`
`128`	`128`	`}`
`129`	`129`
	`130`	`+declare_lint! {`
	`131`	`+ pub RAW_POINTER_DERIVE,`
	`132`	`+ Warn,`
	`133`	`+ "uses of #[derive] with raw pointers are rarely correct"`
	`134`	`+}`
	`135`	`+`
`130`	`136`	/// Does nothing as a lint pass, but registers some `Lint`s
`131`	`137`	`/// which are used by other parts of the compiler.`
`132`	`138`	`#[derive(Copy, Clone)]`
`@@ -152,7 +158,8 @@ impl LintPass for HardwiredLints {`
`152`	`158`	`TRIVIAL_CASTS,`
`153`	`159`	`TRIVIAL_NUMERIC_CASTS,`
`154`	`160`	`MATCH_OF_UNIT_VARIANT_VIA_PAREN_DOTDOT,`
`155`		`- CONST_ERR`
	`161`	`+ CONST_ERR,`
	`162`	`+ RAW_POINTER_DERIVE`
`156`	`163`	`)`
`157`	`164`	`}`
`158`	`165`	`}`