From 1463d6f69f4d20b94681b1a44db96fe3aa611b4c Mon Sep 17 00:00:00 2001 From: flip1995 Date: Thu, 28 Feb 2019 16:44:42 +0100 Subject: [PATCH 1/2] Error an unknown or deprecated Clippy attribute --- clippy_lints/src/utils/attrs.rs | 105 ++++++++++++++++++++++++++++ clippy_lints/src/utils/author.rs | 43 ++++++------ clippy_lints/src/utils/inspector.rs | 17 ++--- clippy_lints/src/utils/mod.rs | 59 ++-------------- 4 files changed, 140 insertions(+), 84 deletions(-) create mode 100644 clippy_lints/src/utils/attrs.rs diff --git a/clippy_lints/src/utils/attrs.rs b/clippy_lints/src/utils/attrs.rs new file mode 100644 index 000000000..db086a58e --- /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, +} + +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 { + 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(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 6cb57a4bc..e61fb2bde 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 7df1b0eb0..c83e1c396 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 ee148ce35..d7ed3191d 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, -} - -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 { - 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(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`. From c4eb7801565fe0c69f3f199f64db40dfdd084590 Mon Sep 17 00:00:00 2001 From: flip1995 Date: Thu, 28 Feb 2019 16:47:00 +0100 Subject: [PATCH 2/2] Add test for unknown Clippy attributes --- tests/ui/unknown_attribute.rs | 3 +++ tests/ui/unknown_attribute.stderr | 8 ++++++++ 2 files changed, 11 insertions(+) create mode 100644 tests/ui/unknown_attribute.rs create mode 100644 tests/ui/unknown_attribute.stderr diff --git a/tests/ui/unknown_attribute.rs b/tests/ui/unknown_attribute.rs new file mode 100644 index 000000000..8d6956928 --- /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 000000000..47e37aed2 --- /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 +