]> git.lizzy.rs Git - rust.git/blobdiff - clippy_lints/src/attrs.rs
Auto merge of #6278 - ThibsG:DerefAddrOf, r=llogiq
[rust.git] / clippy_lints / src / attrs.rs
index 2505ff32fe52333d919ceee9b2947c96b11b2e62..a004abb58b83ed0097f64e265a7a4b5126347e8d 100644 (file)
@@ -1,13 +1,12 @@
 //! checks for attributes
 
-use crate::reexport::Name;
 use crate::utils::{
-    first_line_of_span, is_present_in_source, match_def_path, paths, snippet_opt, span_lint, span_lint_and_sugg,
-    span_lint_and_then, without_block_comments,
+    first_line_of_span, is_present_in_source, match_def_path, paths, snippet_opt, span_lint, span_lint_and_help,
+    span_lint_and_sugg, span_lint_and_then, without_block_comments,
 };
 use if_chain::if_chain;
-use rustc_ast::ast::{AttrKind, AttrStyle, Attribute, Lit, LitKind, MetaItemKind, NestedMetaItem};
 use rustc_ast::util::lev_distance::find_best_match_for_name;
+use rustc_ast::{AttrKind, AttrStyle, Attribute, Lit, LitKind, MetaItemKind, NestedMetaItem};
 use rustc_errors::Applicability;
 use rustc_hir::{
     Block, Expr, ExprKind, ImplItem, ImplItemKind, Item, ItemKind, StmtKind, TraitFn, TraitItem, TraitItemKind,
@@ -17,7 +16,8 @@
 use rustc_middle::ty;
 use rustc_session::{declare_lint_pass, declare_tool_lint};
 use rustc_span::source_map::Span;
-use rustc_span::symbol::Symbol;
+use rustc_span::sym;
+use rustc_span::symbol::{Symbol, SymbolStr};
 use semver::Version;
 
 static UNIX_SYSTEMS: &[&str] = &[
@@ -72,8 +72,9 @@
     /// **What it does:** Checks for `extern crate` and `use` items annotated with
     /// lint attributes.
     ///
-    /// This lint whitelists `#[allow(unused_imports)]`, `#[allow(deprecated)]` and
-    /// `#[allow(unreachable_pub)]` on `use` items and `#[allow(unused_imports)]` on
+    /// This lint permits `#[allow(unused_imports)]`, `#[allow(deprecated)]`,
+    /// `#[allow(unreachable_pub)]`, `#[allow(clippy::wildcard_imports)]` and
+    /// `#[allow(clippy::enum_glob_use)]` on `use` items and `#[allow(unused_imports)]` on
     /// `extern crate` items with a `#[macro_use]` attribute.
     ///
     /// **Why is this bad?** Lint attributes have no effect on crate imports. Most
     /// **Example:**
     /// ```rust
     /// // Good (as inner attribute)
-    /// #![inline(always)]
+    /// #![allow(dead_code)]
     ///
     /// fn this_is_fine() { }
     ///
     /// // Bad
-    /// #[inline(always)]
+    /// #[allow(dead_code)]
     ///
     /// fn not_quite_good_code() { }
     ///
     /// // Good (as outer attribute)
-    /// #[inline(always)]
+    /// #[allow(dead_code)]
     /// fn this_is_fine_too() { }
     /// ```
     pub EMPTY_LINE_AFTER_OUTER_ATTR,
     "unknown_lints for scoped Clippy lints"
 }
 
+declare_clippy_lint! {
+    /// **What it does:** Checks for `warn`/`deny`/`forbid` attributes targeting the whole clippy::restriction category.
+    ///
+    /// **Why is this bad?** Restriction lints sometimes are in contrast with other lints or even go against idiomatic rust.
+    /// These lints should only be enabled on a lint-by-lint basis and with careful consideration.
+    ///
+    /// **Known problems:** None.
+    ///
+    /// **Example:**
+    /// Bad:
+    /// ```rust
+    /// #![deny(clippy::restriction)]
+    /// ```
+    ///
+    /// Good:
+    /// ```rust
+    /// #![deny(clippy::as_conversions)]
+    /// ```
+    pub BLANKET_CLIPPY_RESTRICTION_LINTS,
+    style,
+    "enabling the complete restriction group"
+}
+
 declare_clippy_lint! {
     /// **What it does:** Checks for `#[cfg_attr(rustfmt, rustfmt_skip)]` and suggests to replace it
     /// with `#[rustfmt::skip]`.
     DEPRECATED_SEMVER,
     USELESS_ATTRIBUTE,
     UNKNOWN_CLIPPY_LINTS,
+    BLANKET_CLIPPY_RESTRICTION_LINTS,
 ]);
 
 impl<'tcx> LateLintPass<'tcx> for Attributes {
     fn check_attribute(&mut self, cx: &LateContext<'tcx>, attr: &'tcx Attribute) {
         if let Some(items) = &attr.meta_item_list() {
             if let Some(ident) = attr.ident() {
-                match &*ident.as_str() {
+                let ident = &*ident.as_str();
+                match ident {
                     "allow" | "warn" | "deny" | "forbid" => {
-                        check_clippy_lint_names(cx, items);
+                        check_clippy_lint_names(cx, ident, items);
                     },
                     _ => {},
                 }
-                if items.is_empty() || !attr.check_name(sym!(deprecated)) {
+                if items.is_empty() || !attr.has_name(sym::deprecated) {
                     return;
                 }
                 for item in items {
                     if_chain! {
                         if let NestedMetaItem::MetaItem(mi) = &item;
                         if let MetaItemKind::NameValue(lit) = &mi.kind;
-                        if mi.check_name(sym!(since));
+                        if mi.has_name(sym::since);
                         then {
                             check_semver(cx, item.span(), lit);
                         }
@@ -284,7 +310,7 @@ fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'_>) {
         }
         match item.kind {
             ItemKind::ExternCrate(..) | ItemKind::Use(..) => {
-                let skip_unused_imports = item.attrs.iter().any(|attr| attr.check_name(sym!(macro_use)));
+                let skip_unused_imports = item.attrs.iter().any(|attr| attr.has_name(sym::macro_use));
 
                 for attr in item.attrs {
                     if in_external_macro(cx.sess(), attr.span) {
@@ -294,15 +320,19 @@ fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'_>) {
                         if let Some(ident) = attr.ident() {
                             match &*ident.as_str() {
                                 "allow" | "warn" | "deny" | "forbid" => {
-                                    // whitelist `unused_imports`, `deprecated` and `unreachable_pub` for `use` items
+                                    // permit `unused_imports`, `deprecated`, `unreachable_pub`,
+                                    // `clippy::wildcard_imports`, and `clippy::enum_glob_use` for `use` items
                                     // and `unused_imports` for `extern crate` items with `macro_use`
                                     for lint in lint_list {
                                         match item.kind {
                                             ItemKind::Use(..) => {
                                                 if is_word(lint, sym!(unused_imports))
-                                                    || is_word(lint, sym!(deprecated))
+                                                    || is_word(lint, sym::deprecated)
                                                     || is_word(lint, sym!(unreachable_pub))
                                                     || is_word(lint, sym!(unused))
+                                                    || extract_clippy_lint(lint)
+                                                        .map_or(false, |s| s == "wildcard_imports")
+                                                    || extract_clippy_lint(lint).map_or(false, |s| s == "enum_glob_use")
                                                 {
                                                     return;
                                                 }
@@ -363,38 +393,47 @@ fn check_trait_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx TraitItem<'_>
     }
 }
 
-#[allow(clippy::single_match_else)]
-fn check_clippy_lint_names(cx: &LateContext<'_>, items: &[NestedMetaItem]) {
+/// Returns the lint name if it is clippy lint.
+fn extract_clippy_lint(lint: &NestedMetaItem) -> Option<SymbolStr> {
+    if_chain! {
+        if let Some(meta_item) = lint.meta_item();
+        if meta_item.path.segments.len() > 1;
+        if let tool_name = meta_item.path.segments[0].ident;
+        if tool_name.as_str() == "clippy";
+        let lint_name = meta_item.path.segments.last().unwrap().ident.name;
+        then {
+            return Some(lint_name.as_str());
+        }
+    }
+    None
+}
+
+fn check_clippy_lint_names(cx: &LateContext<'_>, ident: &str, items: &[NestedMetaItem]) {
     let lint_store = cx.lints();
     for lint in items {
-        if_chain! {
-            if let Some(meta_item) = lint.meta_item();
-            if meta_item.path.segments.len() > 1;
-            if let tool_name = meta_item.path.segments[0].ident;
-            if tool_name.as_str() == "clippy";
-            let name = meta_item.path.segments.last().unwrap().ident.name;
-            if let CheckLintNameResult::Tool(Err((None, _))) = lint_store.check_lint_name(
-                &name.as_str(),
-                Some(tool_name.name),
-            );
-            then {
+        if let Some(lint_name) = extract_clippy_lint(lint) {
+            if let CheckLintNameResult::Tool(Err((None, _))) = lint_store.check_lint_name(&lint_name, Some(sym::clippy))
+            {
                 span_lint_and_then(
                     cx,
                     UNKNOWN_CLIPPY_LINTS,
                     lint.span(),
-                    &format!("unknown clippy lint: clippy::{}", name),
+                    &format!("unknown clippy lint: clippy::{}", lint_name),
                     |diag| {
-                        let name_lower = name.as_str().to_lowercase();
-                        let symbols = lint_store.get_lints().iter().map(
-                            |l| Symbol::intern(&l.name_lower())
-                        ).collect::<Vec<_>>();
+                        let name_lower = lint_name.to_lowercase();
+                        let symbols = lint_store
+                            .get_lints()
+                            .iter()
+                            .map(|l| Symbol::intern(&l.name_lower()))
+                            .collect::<Vec<_>>();
                         let sugg = find_best_match_for_name(
                             symbols.iter(),
-                            &format!("clippy::{}", name_lower),
+                            Symbol::intern(&format!("clippy::{}", name_lower)),
                             None,
                         );
-                        if name.as_str().chars().any(char::is_uppercase)
-                            && lint_store.find_lints(&format!("clippy::{}", name_lower)).is_ok() {
+                        if lint_name.chars().any(char::is_uppercase)
+                            && lint_store.find_lints(&format!("clippy::{}", name_lower)).is_ok()
+                        {
                             diag.span_suggestion(
                                 lint.span(),
                                 "lowercase the lint name",
@@ -409,16 +448,25 @@ fn check_clippy_lint_names(cx: &LateContext<'_>, items: &[NestedMetaItem]) {
                                 Applicability::MachineApplicable,
                             );
                         }
-                    }
+                    },
+                );
+            } else if lint_name == "restriction" && ident != "allow" {
+                span_lint_and_help(
+                    cx,
+                    BLANKET_CLIPPY_RESTRICTION_LINTS,
+                    lint.span(),
+                    "restriction lints are not meant to be all enabled",
+                    None,
+                    "try enabling only the lints you really need",
                 );
             }
-        };
+        }
     }
 }
 
 fn is_relevant_item(cx: &LateContext<'_>, item: &Item<'_>) -> bool {
     if let ItemKind::Fn(_, _, eid) = item.kind {
-        is_relevant_expr(cx, cx.tcx.body_tables(eid), &cx.tcx.hir().body(eid).value)
+        is_relevant_expr(cx, cx.tcx.typeck_body(eid), &cx.tcx.hir().body(eid).value)
     } else {
         true
     }
@@ -426,7 +474,7 @@ fn is_relevant_item(cx: &LateContext<'_>, item: &Item<'_>) -> bool {
 
 fn is_relevant_impl(cx: &LateContext<'_>, item: &ImplItem<'_>) -> bool {
     match item.kind {
-        ImplItemKind::Fn(_, eid) => is_relevant_expr(cx, cx.tcx.body_tables(eid), &cx.tcx.hir().body(eid).value),
+        ImplItemKind::Fn(_, eid) => is_relevant_expr(cx, cx.tcx.typeck_body(eid), &cx.tcx.hir().body(eid).value),
         _ => false,
     }
 }
@@ -435,36 +483,37 @@ fn is_relevant_trait(cx: &LateContext<'_>, item: &TraitItem<'_>) -> bool {
     match item.kind {
         TraitItemKind::Fn(_, TraitFn::Required(_)) => true,
         TraitItemKind::Fn(_, TraitFn::Provided(eid)) => {
-            is_relevant_expr(cx, cx.tcx.body_tables(eid), &cx.tcx.hir().body(eid).value)
+            is_relevant_expr(cx, cx.tcx.typeck_body(eid), &cx.tcx.hir().body(eid).value)
         },
         _ => false,
     }
 }
 
-fn is_relevant_block(cx: &LateContext<'_>, tables: &ty::TypeckTables<'_>, block: &Block<'_>) -> bool {
-    if let Some(stmt) = block.stmts.first() {
-        match &stmt.kind {
+fn is_relevant_block(cx: &LateContext<'_>, typeck_results: &ty::TypeckResults<'_>, block: &Block<'_>) -> bool {
+    block.stmts.first().map_or(
+        block
+            .expr
+            .as_ref()
+            .map_or(false, |e| is_relevant_expr(cx, typeck_results, e)),
+        |stmt| match &stmt.kind {
             StmtKind::Local(_) => true,
-            StmtKind::Expr(expr) | StmtKind::Semi(expr) => is_relevant_expr(cx, tables, expr),
+            StmtKind::Expr(expr) | StmtKind::Semi(expr) => is_relevant_expr(cx, typeck_results, expr),
             _ => false,
-        }
-    } else {
-        block.expr.as_ref().map_or(false, |e| is_relevant_expr(cx, tables, e))
-    }
+        },
+    )
 }
 
-fn is_relevant_expr(cx: &LateContext<'_>, tables: &ty::TypeckTables<'_>, expr: &Expr<'_>) -> bool {
+fn is_relevant_expr(cx: &LateContext<'_>, typeck_results: &ty::TypeckResults<'_>, expr: &Expr<'_>) -> bool {
     match &expr.kind {
-        ExprKind::Block(block, _) => is_relevant_block(cx, tables, block),
-        ExprKind::Ret(Some(e)) => is_relevant_expr(cx, tables, e),
+        ExprKind::Block(block, _) => is_relevant_block(cx, typeck_results, block),
+        ExprKind::Ret(Some(e)) => is_relevant_expr(cx, typeck_results, e),
         ExprKind::Ret(None) | ExprKind::Break(_, None) => false,
         ExprKind::Call(path_expr, _) => {
             if let ExprKind::Path(qpath) = &path_expr.kind {
-                if let Some(fun_id) = tables.qpath_res(qpath, path_expr.hir_id).opt_def_id() {
-                    !match_def_path(cx, fun_id, &paths::BEGIN_PANIC)
-                } else {
-                    true
-                }
+                typeck_results
+                    .qpath_res(qpath, path_expr.hir_id)
+                    .opt_def_id()
+                    .map_or(true, |fun_id| !match_def_path(cx, fun_id, &paths::BEGIN_PANIC))
             } else {
                 true
             }
@@ -473,17 +522,17 @@ fn is_relevant_expr(cx: &LateContext<'_>, tables: &ty::TypeckTables<'_>, expr: &
     }
 }
 
-fn check_attrs(cx: &LateContext<'_>, span: Span, name: Name, attrs: &[Attribute]) {
+fn check_attrs(cx: &LateContext<'_>, span: Span, name: Symbol, attrs: &[Attribute]) {
     if span.from_expansion() {
         return;
     }
 
     for attr in attrs {
         if let Some(values) = attr.meta_item_list() {
-            if values.len() != 1 || !attr.check_name(sym!(inline)) {
+            if values.len() != 1 || !attr.has_name(sym::inline) {
                 continue;
             }
-            if is_word(&values[0], sym!(always)) {
+            if is_word(&values[0], sym::always) {
                 span_lint(
                     cx,
                     INLINE_ALWAYS,
@@ -514,7 +563,7 @@ fn check_semver(cx: &LateContext<'_>, span: Span, lit: &Lit) {
 
 fn is_word(nmi: &NestedMetaItem, expected: Symbol) -> bool {
     if let NestedMetaItem::MetaItem(mi) = &nmi {
-        mi.is_word() && mi.check_name(expected)
+        mi.is_word() && mi.has_name(expected)
     } else {
         false
     }
@@ -527,7 +576,7 @@ fn is_word(nmi: &NestedMetaItem, expected: Symbol) -> bool {
 ]);
 
 impl EarlyLintPass for EarlyAttributes {
-    fn check_item(&mut self, cx: &EarlyContext<'_>, item: &rustc_ast::ast::Item) {
+    fn check_item(&mut self, cx: &EarlyContext<'_>, item: &rustc_ast::Item) {
         check_empty_line_after_outer_attr(cx, item);
     }
 
@@ -537,7 +586,7 @@ fn check_attribute(&mut self, cx: &EarlyContext<'_>, attr: &Attribute) {
     }
 }
 
-fn check_empty_line_after_outer_attr(cx: &EarlyContext<'_>, item: &rustc_ast::ast::Item) {
+fn check_empty_line_after_outer_attr(cx: &EarlyContext<'_>, item: &rustc_ast::Item) {
     for attr in &item.attrs {
         let attr_item = if let AttrKind::Normal(ref attr) = attr.kind {
             attr
@@ -562,7 +611,7 @@ fn check_empty_line_after_outer_attr(cx: &EarlyContext<'_>, item: &rustc_ast::as
                         cx,
                         EMPTY_LINE_AFTER_OUTER_ATTR,
                         begin_of_attr_to_item,
-                        "Found an empty line after an outer attribute. \
+                        "found an empty line after an outer attribute. \
                         Perhaps you forgot to add a `!` to make it an inner attribute?",
                     );
                 }
@@ -574,15 +623,15 @@ fn check_empty_line_after_outer_attr(cx: &EarlyContext<'_>, item: &rustc_ast::as
 fn check_deprecated_cfg_attr(cx: &EarlyContext<'_>, attr: &Attribute) {
     if_chain! {
         // check cfg_attr
-        if attr.check_name(sym!(cfg_attr));
+        if attr.has_name(sym::cfg_attr);
         if let Some(items) = attr.meta_item_list();
         if items.len() == 2;
         // check for `rustfmt`
         if let Some(feature_item) = items[0].meta_item();
-        if feature_item.check_name(sym!(rustfmt));
+        if feature_item.has_name(sym::rustfmt);
         // check for `rustfmt_skip` and `rustfmt::skip`
         if let Some(skip_item) = &items[1].meta_item();
-        if skip_item.check_name(sym!(rustfmt_skip)) ||
+        if skip_item.has_name(sym!(rustfmt_skip)) ||
             skip_item.path.segments.last().expect("empty path in attribute").ident.name == sym!(skip);
         // Only lint outer attributes, because custom inner attributes are unstable
         // Tracking issue: https://github.com/rust-lang/rust/issues/54726
@@ -641,7 +690,7 @@ fn find_mismatched_target_os(items: &[NestedMetaItem]) -> Vec<(&str, Span)> {
     }
 
     if_chain! {
-        if attr.check_name(sym!(cfg));
+        if attr.has_name(sym::cfg);
         if let Some(list) = attr.meta_item_list();
         let mismatched = find_mismatched_target_os(&list);
         if !mismatched.is_empty();