Auto merge of #3830 - flip1995:unknown_attribute, r=oli-obk

Error on unknown or deprecated attributes

The reason I decided to not make this a lint is, that unknown attributes are always a hard error in rustc and also in rustfmt, which cannot be turned off by an `allow`. This could have been done by introducing a category which would be forbid-by-default. But that would have introduced a new lint category+group for just one lint and also wouldn't be consistent with rustfmt and rustc.

6 files changed, 151 insertions, 84 deletions

diff --git a/clippy_lints/src/utils/attrs.rs b/clippy_lints/src/utils/attrs.rs
new file mode 100644
index 00000000000..db086a58e5c
--- /dev/null
+++ b/clippy_lints/src/utils/attrs.rs
@@ -0,0 +1,105 @@
+use rustc::session::Session;
+use rustc_errors::Applicability;
+use std::str::FromStr;
+use syntax::ast;
+
+/// Deprecation status of attributes known by Clippy.
+#[allow(dead_code)]
+pub enum DeprecationStatus {
+    /// Attribute is deprecated
+    Deprecated,
+    /// Attribute is deprecated and was replaced by the named attribute
+    Replaced(&'static str),
+    None,
+}
+
+pub const BUILTIN_ATTRIBUTES: &[(&str, DeprecationStatus)] = &[
+    ("author", DeprecationStatus::None),
+    ("cyclomatic_complexity", DeprecationStatus::None),
+    ("dump", DeprecationStatus::None),
+];
+
+pub struct LimitStack {
+    stack: Vec<u64>,
+}
+
+impl Drop for LimitStack {
+    fn drop(&mut self) {
+        assert_eq!(self.stack.len(), 1);
+    }
+}
+
+impl LimitStack {
+    pub fn new(limit: u64) -> Self {
+        Self { stack: vec![limit] }
+    }
+    pub fn limit(&self) -> u64 {
+        *self.stack.last().expect("there should always be a value in the stack")
+    }
+    pub fn push_attrs(&mut self, sess: &Session, attrs: &[ast::Attribute], name: &'static str) {
+        let stack = &mut self.stack;
+        parse_attrs(sess, attrs, name, |val| stack.push(val));
+    }
+    pub fn pop_attrs(&mut self, sess: &Session, attrs: &[ast::Attribute], name: &'static str) {
+        let stack = &mut self.stack;
+        parse_attrs(sess, attrs, name, |val| assert_eq!(stack.pop(), Some(val)));
+    }
+}
+
+pub fn get_attr<'a>(
+    sess: &'a Session,
+    attrs: &'a [ast::Attribute],
+    name: &'static str,
+) -> impl Iterator<Item = &'a ast::Attribute> {
+    attrs.iter().filter(move |attr| {
+        let attr_segments = &attr.path.segments;
+        if attr_segments.len() == 2 && attr_segments[0].ident.to_string() == "clippy" {
+            if let Some(deprecation_status) = BUILTIN_ATTRIBUTES
+                .iter()
+                .find(|(builtin_name, _)| *builtin_name == attr_segments[1].ident.to_string())
+                .map(|(_, deprecation_status)| deprecation_status)
+            {
+                let mut db = sess.struct_span_err(attr_segments[1].ident.span, "Usage of deprecated attribute");
+                match deprecation_status {
+                    DeprecationStatus::Deprecated => {
+                        db.emit();
+                        false
+                    },
+                    DeprecationStatus::Replaced(new_name) => {
+                        db.span_suggestion(
+                            attr_segments[1].ident.span,
+                            "consider using",
+                            new_name.to_string(),
+                            Applicability::MachineApplicable,
+                        );
+                        db.emit();
+                        false
+                    },
+                    DeprecationStatus::None => {
+                        db.cancel();
+                        attr_segments[1].ident.to_string() == name
+                    },
+                }
+            } else {
+                sess.span_err(attr_segments[1].ident.span, "Usage of unknown attribute");
+                false
+            }
+        } else {
+            false
+        }
+    })
+}
+
+fn parse_attrs<F: FnMut(u64)>(sess: &Session, attrs: &[ast::Attribute], name: &'static str, mut f: F) {
+    for attr in get_attr(sess, attrs, name) {
+        if let Some(ref value) = attr.value_str() {
+            if let Ok(value) = FromStr::from_str(&value.as_str()) {
+                f(value)
+            } else {
+                sess.span_err(attr.span, "not a number");
+            }
+        } else {
+            sess.span_err(attr.span, "bad clippy attribute");
+        }
+    }
+}
diff --git a/clippy_lints/src/utils/author.rs b/clippy_lints/src/utils/author.rs
index 6cb57a4bcf1..e61fb2bde4b 100644
--- a/clippy_lints/src/utils/author.rs
+++ b/clippy_lints/src/utils/author.rs
@@ -5,7 +5,8 @@ use crate::utils::get_attr;
 use rustc::hir;
 use rustc::hir::intravisit::{NestedVisitorMap, Visitor};
 use rustc::hir::{BindingAnnotation, Expr, ExprKind, Pat, PatKind, QPath, Stmt, StmtKind, TyKind};
-use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
+use rustc::lint::{LateContext, LateLintPass, LintArray, LintContext, LintPass};
+use rustc::session::Session;
 use rustc::{declare_tool_lint, lint_array};
 use rustc_data_structures::fx::FxHashMap;
 use syntax::ast::{Attribute, LitKind};
@@ -71,8 +72,8 @@ fn done() {
 }
 
 impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
-    fn check_item(&mut self, _cx: &LateContext<'a, 'tcx>, item: &'tcx hir::Item) {
-        if !has_attr(&item.attrs) {
+    fn check_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &'tcx hir::Item) {
+        if !has_attr(cx.sess(), &item.attrs) {
             return;
         }
         prelude();
@@ -80,8 +81,8 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
         done();
     }
 
-    fn check_impl_item(&mut self, _cx: &LateContext<'a, 'tcx>, item: &'tcx hir::ImplItem) {
-        if !has_attr(&item.attrs) {
+    fn check_impl_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &'tcx hir::ImplItem) {
+        if !has_attr(cx.sess(), &item.attrs) {
             return;
         }
         prelude();
@@ -89,8 +90,8 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
         done();
     }
 
-    fn check_trait_item(&mut self, _cx: &LateContext<'a, 'tcx>, item: &'tcx hir::TraitItem) {
-        if !has_attr(&item.attrs) {
+    fn check_trait_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &'tcx hir::TraitItem) {
+        if !has_attr(cx.sess(), &item.attrs) {
             return;
         }
         prelude();
@@ -98,8 +99,8 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
         done();
     }
 
-    fn check_variant(&mut self, _cx: &LateContext<'a, 'tcx>, var: &'tcx hir::Variant, generics: &hir::Generics) {
-        if !has_attr(&var.node.attrs) {
+    fn check_variant(&mut self, cx: &LateContext<'a, 'tcx>, var: &'tcx hir::Variant, generics: &hir::Generics) {
+        if !has_attr(cx.sess(), &var.node.attrs) {
             return;
         }
         prelude();
@@ -107,8 +108,8 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
         done();
     }
 
-    fn check_struct_field(&mut self, _cx: &LateContext<'a, 'tcx>, field: &'tcx hir::StructField) {
-        if !has_attr(&field.attrs) {
+    fn check_struct_field(&mut self, cx: &LateContext<'a, 'tcx>, field: &'tcx hir::StructField) {
+        if !has_attr(cx.sess(), &field.attrs) {
             return;
         }
         prelude();
@@ -116,8 +117,8 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
         done();
     }
 
-    fn check_expr(&mut self, _cx: &LateContext<'a, 'tcx>, expr: &'tcx hir::Expr) {
-        if !has_attr(&expr.attrs) {
+    fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx hir::Expr) {
+        if !has_attr(cx.sess(), &expr.attrs) {
             return;
         }
         prelude();
@@ -125,8 +126,8 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
         done();
     }
 
-    fn check_arm(&mut self, _cx: &LateContext<'a, 'tcx>, arm: &'tcx hir::Arm) {
-        if !has_attr(&arm.attrs) {
+    fn check_arm(&mut self, cx: &LateContext<'a, 'tcx>, arm: &'tcx hir::Arm) {
+        if !has_attr(cx.sess(), &arm.attrs) {
             return;
         }
         prelude();
@@ -134,8 +135,8 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
         done();
     }
 
-    fn check_stmt(&mut self, _cx: &LateContext<'a, 'tcx>, stmt: &'tcx hir::Stmt) {
-        if !has_attr(stmt.node.attrs()) {
+    fn check_stmt(&mut self, cx: &LateContext<'a, 'tcx>, stmt: &'tcx hir::Stmt) {
+        if !has_attr(cx.sess(), stmt.node.attrs()) {
             return;
         }
         prelude();
@@ -143,8 +144,8 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
         done();
     }
 
-    fn check_foreign_item(&mut self, _cx: &LateContext<'a, 'tcx>, item: &'tcx hir::ForeignItem) {
-        if !has_attr(&item.attrs) {
+    fn check_foreign_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &'tcx hir::ForeignItem) {
+        if !has_attr(cx.sess(), &item.attrs) {
             return;
         }
         prelude();
@@ -673,8 +674,8 @@ impl<'tcx> Visitor<'tcx> for PrintVisitor {
     }
 }
 
-fn has_attr(attrs: &[Attribute]) -> bool {
-    get_attr(attrs, "author").count() > 0
+fn has_attr(sess: &Session, attrs: &[Attribute]) -> bool {
+    get_attr(sess, attrs, "author").count() > 0
 }
 
 fn desugaring_name(des: hir::MatchSource) -> String {
diff --git a/clippy_lints/src/utils/inspector.rs b/clippy_lints/src/utils/inspector.rs
index 7df1b0eb0e3..c83e1c396bc 100644
--- a/clippy_lints/src/utils/inspector.rs
+++ b/clippy_lints/src/utils/inspector.rs
@@ -3,7 +3,8 @@
 use crate::utils::get_attr;
 use rustc::hir;
 use rustc::hir::print;
-use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
+use rustc::lint::{LateContext, LateLintPass, LintArray, LintContext, LintPass};
+use rustc::session::Session;
 use rustc::{declare_tool_lint, lint_array};
 use syntax::ast::Attribute;
 
@@ -43,14 +44,14 @@ impl LintPass for Pass {
 
 impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
     fn check_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &'tcx hir::Item) {
-        if !has_attr(&item.attrs) {
+        if !has_attr(cx.sess(), &item.attrs) {
             return;
         }
         print_item(cx, item);
     }
 
     fn check_impl_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &'tcx hir::ImplItem) {
-        if !has_attr(&item.attrs) {
+        if !has_attr(cx.sess(), &item.attrs) {
             return;
         }
         println!("impl item `{}`", item.ident.name);
@@ -100,14 +101,14 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
     //
 
     fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx hir::Expr) {
-        if !has_attr(&expr.attrs) {
+        if !has_attr(cx.sess(), &expr.attrs) {
             return;
         }
         print_expr(cx, expr, 0);
     }
 
     fn check_arm(&mut self, cx: &LateContext<'a, 'tcx>, arm: &'tcx hir::Arm) {
-        if !has_attr(&arm.attrs) {
+        if !has_attr(cx.sess(), &arm.attrs) {
             return;
         }
         for pat in &arm.pats {
@@ -122,7 +123,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
     }
 
     fn check_stmt(&mut self, cx: &LateContext<'a, 'tcx>, stmt: &'tcx hir::Stmt) {
-        if !has_attr(stmt.node.attrs()) {
+        if !has_attr(cx.sess(), stmt.node.attrs()) {
             return;
         }
         match stmt.node {
@@ -148,8 +149,8 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
     //
 }
 
-fn has_attr(attrs: &[Attribute]) -> bool {
-    get_attr(attrs, "dump").count() > 0
+fn has_attr(sess: &Session, attrs: &[Attribute]) -> bool {
+    get_attr(sess, attrs, "dump").count() > 0
 }
 
 #[allow(clippy::similar_names)]
diff --git a/clippy_lints/src/utils/mod.rs b/clippy_lints/src/utils/mod.rs
index ee148ce3558..d7ed3191ddb 100644
--- a/clippy_lints/src/utils/mod.rs
+++ b/clippy_lints/src/utils/mod.rs
@@ -8,7 +8,6 @@ use rustc::hir::intravisit::{NestedVisitorMap, Visitor};
 use rustc::hir::Node;
 use rustc::hir::*;
 use rustc::lint::{LateContext, Level, Lint, LintContext};
-use rustc::session::Session;
 use rustc::traits;
 use rustc::ty::{
     self,
@@ -20,20 +19,20 @@ use rustc_data_structures::sync::Lrc;
 use rustc_errors::Applicability;
 use std::borrow::Cow;
 use std::mem;
-use std::str::FromStr;
 use syntax::ast::{self, LitKind};
 use syntax::attr;
 use syntax::source_map::{Span, DUMMY_SP};
 use syntax::symbol;
 use syntax::symbol::{keywords, Symbol};
 
-pub mod camel_case;
-
+pub mod attrs;
 pub mod author;
+pub mod camel_case;
 pub mod comparisons;
 pub mod conf;
 pub mod constants;
 mod diagnostics;
+pub mod higher;
 mod hir_utils;
 pub mod inspector;
 pub mod internal_lints;
@@ -41,11 +40,10 @@ pub mod paths;
 pub mod ptr;
 pub mod sugg;
 pub mod usage;
+pub use self::attrs::*;
 pub use self::diagnostics::*;
 pub use self::hir_utils::{SpanlessEq, SpanlessHash};
 
-pub mod higher;
-
 /// Returns true if the two spans come from differing expansions (i.e. one is
 /// from a macro and one
 /// isn't).
@@ -662,55 +660,6 @@ pub fn is_adjusted(cx: &LateContext<'_, '_>, e: &Expr) -> bool {
     cx.tables.adjustments().get(e.hir_id).is_some()
 }
 
-pub struct LimitStack {
-    stack: Vec<u64>,
-}
-
-impl Drop for LimitStack {
-    fn drop(&mut self) {
-        assert_eq!(self.stack.len(), 1);
-    }
-}
-
-impl LimitStack {
-    pub fn new(limit: u64) -> Self {
-        Self { stack: vec![limit] }
-    }
-    pub fn limit(&self) -> u64 {
-        *self.stack.last().expect("there should always be a value in the stack")
-    }
-    pub fn push_attrs(&mut self, sess: &Session, attrs: &[ast::Attribute], name: &'static str) {
-        let stack = &mut self.stack;
-        parse_attrs(sess, attrs, name, |val| stack.push(val));
-    }
-    pub fn pop_attrs(&mut self, sess: &Session, attrs: &[ast::Attribute], name: &'static str) {
-        let stack = &mut self.stack;
-        parse_attrs(sess, attrs, name, |val| assert_eq!(stack.pop(), Some(val)));
-    }
-}
-
-pub fn get_attr<'a>(attrs: &'a [ast::Attribute], name: &'static str) -> impl Iterator<Item = &'a ast::Attribute> {
-    attrs.iter().filter(move |attr| {
-        attr.path.segments.len() == 2
-            && attr.path.segments[0].ident.to_string() == "clippy"
-            && attr.path.segments[1].ident.to_string() == name
-    })
-}
-
-fn parse_attrs<F: FnMut(u64)>(sess: &Session, attrs: &[ast::Attribute], name: &'static str, mut f: F) {
-    for attr in get_attr(attrs, name) {
-        if let Some(ref value) = attr.value_str() {
-            if let Ok(value) = FromStr::from_str(&value.as_str()) {
-                f(value)
-            } else {
-                sess.span_err(attr.span, "not a number");
-            }
-        } else {
-            sess.span_err(attr.span, "bad clippy attribute");
-        }
-    }
-}
-
 /// Return the pre-expansion span if is this comes from an expansion of the
 /// macro `name`.
 /// See also `is_direct_expn_of`.
diff --git a/tests/ui/unknown_attribute.rs b/tests/ui/unknown_attribute.rs
new file mode 100644
index 00000000000..8d695692863
--- /dev/null
+++ b/tests/ui/unknown_attribute.rs
@@ -0,0 +1,3 @@
+#[clippy::unknown]
+#[clippy::cyclomatic_complexity = "1"]
+fn main() {}
diff --git a/tests/ui/unknown_attribute.stderr b/tests/ui/unknown_attribute.stderr
new file mode 100644
index 00000000000..47e37aed246
--- /dev/null
+++ b/tests/ui/unknown_attribute.stderr
@@ -0,0 +1,8 @@
+error: Usage of unknown attribute
+  --> $DIR/unknown_attribute.rs:1:11
+   |
+LL | #[clippy::unknown]
+   |           ^^^^^^^
+
+error: aborting due to previous error
+