]> git.lizzy.rs Git - rust.git/blobdiff - clippy_lints/src/attrs.rs
Auto merge of #4551 - mikerite:fix-ice-reporting, r=llogiq
[rust.git] / clippy_lints / src / attrs.rs
index c054a00894e75ebcafe2d77cd91747236bf2ba50..a4b411d751998ec3f897373929ee52190d40c9f5 100644 (file)
@@ -2,20 +2,22 @@
 
 use crate::reexport::*;
 use crate::utils::{
-    in_macro, last_line_of_span, match_def_path, paths, snippet_opt, span_lint, span_lint_and_sugg, span_lint_and_then,
-    without_block_comments,
+    is_present_in_source, last_line_of_span, match_def_path, paths, snippet_opt, span_lint, span_lint_and_sugg,
+    span_lint_and_then, without_block_comments,
 };
 use if_chain::if_chain;
 use rustc::hir::*;
 use rustc::lint::{
-    CheckLintNameResult, EarlyContext, EarlyLintPass, LateContext, LateLintPass, LintArray, LintContext, LintPass,
+    in_external_macro, CheckLintNameResult, EarlyContext, EarlyLintPass, LateContext, LateLintPass, LintArray,
+    LintContext, LintPass,
 };
-use rustc::ty::{self, TyCtxt};
-use rustc::{declare_tool_lint, lint_array};
+use rustc::ty;
+use rustc::{declare_lint_pass, declare_tool_lint};
 use rustc_errors::Applicability;
 use semver::Version;
 use syntax::ast::{AttrStyle, Attribute, Lit, LitKind, MetaItemKind, NestedMetaItem};
 use syntax::source_map::Span;
+use syntax_pos::symbol::Symbol;
 
 declare_clippy_lint! {
     /// **What it does:** Checks for items annotated with `#[inline(always)]`,
@@ -47,9 +49,9 @@
     /// **What it does:** Checks for `extern crate` and `use` items annotated with
     /// lint attributes.
     ///
-    /// This lint whitelists `#[allow(unused_imports)]` and `#[allow(deprecated)]` on
-    /// `use` items and `#[allow(unused_imports)]` on `extern crate` items with a
-    /// `#[macro_use]` attribute.
+    /// This lint whitelists `#[allow(unused_imports)]`, `#[allow(deprecated)]` and
+    /// `#[allow(unreachable_pub)]` 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
     /// likely a `!` was forgotten.
     ///
     /// **Example:**
     /// ```rust
-    /// // Bad
-    /// #[inline(always)]
-    ///
-    /// fn not_quite_good_code(..) { ... }
-    ///
     /// // Good (as inner attribute)
     /// #![inline(always)]
     ///
-    /// fn this_is_fine(..) { ... }
+    /// fn this_is_fine() { }
+    ///
+    /// // Bad
+    /// #[inline(always)]
+    ///
+    /// fn not_quite_good_code() { }
     ///
     /// // Good (as outer attribute)
     /// #[inline(always)]
-    /// fn this_is_fine_too(..) { ... }
+    /// fn this_is_fine_too() { }
     /// ```
     pub EMPTY_LINE_AFTER_OUTER_ATTR,
     nursery,
 
 declare_clippy_lint! {
     /// **What it does:** Checks for `allow`/`warn`/`deny`/`forbid` attributes with scoped clippy
-    /// lints and if those lints exist in clippy. If there is a uppercase letter in the lint name
+    /// lints and if those lints exist in clippy. If there is an uppercase letter in the lint name
     /// (not the tool name) and a lowercase version of this lint exists, it will suggest to lowercase
     /// the lint name.
     ///
     "usage of `cfg_attr(rustfmt)` instead of `tool_attributes`"
 }
 
-#[derive(Copy, Clone)]
-pub struct AttrPass;
-
-impl LintPass for AttrPass {
-    fn get_lints(&self) -> LintArray {
-        lint_array!(
-            INLINE_ALWAYS,
-            DEPRECATED_SEMVER,
-            USELESS_ATTRIBUTE,
-            EMPTY_LINE_AFTER_OUTER_ATTR,
-            UNKNOWN_CLIPPY_LINTS,
-        )
-    }
+declare_lint_pass!(Attributes => [
+    INLINE_ALWAYS,
+    DEPRECATED_SEMVER,
+    USELESS_ATTRIBUTE,
+    EMPTY_LINE_AFTER_OUTER_ATTR,
+    UNKNOWN_CLIPPY_LINTS,
+]);
 
-    fn name(&self) -> &'static str {
-        "Attributes"
-    }
-}
-
-impl<'a, 'tcx> LateLintPass<'a, 'tcx> for AttrPass {
+impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Attributes {
     fn check_attribute(&mut self, cx: &LateContext<'a, 'tcx>, attr: &'tcx Attribute) {
         if let Some(items) = &attr.meta_item_list() {
             if let Some(ident) = attr.ident() {
@@ -215,14 +206,14 @@ fn check_attribute(&mut self, cx: &LateContext<'a, 'tcx>, attr: &'tcx Attribute)
                     },
                     _ => {},
                 }
-                if items.is_empty() || !attr.check_name("deprecated") {
+                if items.is_empty() || !attr.check_name(sym!(deprecated)) {
                     return;
                 }
                 for item in items {
                     if_chain! {
                         if let NestedMetaItem::MetaItem(mi) = &item;
                         if let MetaItemKind::NameValue(lit) = &mi.node;
-                        if mi.check_name("since");
+                        if mi.check_name(sym!(since));
                         then {
                             check_semver(cx, item.span(), lit);
                         }
@@ -233,32 +224,38 @@ fn check_attribute(&mut self, cx: &LateContext<'a, 'tcx>, attr: &'tcx Attribute)
     }
 
     fn check_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &'tcx Item) {
-        if is_relevant_item(cx.tcx, item) {
+        if is_relevant_item(cx, item) {
             check_attrs(cx, item.span, item.ident.name, &item.attrs)
         }
         match item.node {
             ItemKind::ExternCrate(..) | ItemKind::Use(..) => {
-                let skip_unused_imports = item.attrs.iter().any(|attr| attr.check_name("macro_use"));
+                let skip_unused_imports = item.attrs.iter().any(|attr| attr.check_name(sym!(macro_use)));
 
                 for attr in &item.attrs {
+                    if in_external_macro(cx.sess(), attr.span) {
+                        return;
+                    }
                     if let Some(lint_list) = &attr.meta_item_list() {
                         if let Some(ident) = attr.ident() {
                             match &*ident.as_str() {
                                 "allow" | "warn" | "deny" | "forbid" => {
-                                    // whitelist `unused_imports` and `deprecated` for `use` items
+                                    // whitelist `unused_imports`, `deprecated` and `unreachable_pub` for `use` items
                                     // and `unused_imports` for `extern crate` items with `macro_use`
                                     for lint in lint_list {
                                         match item.node {
                                             ItemKind::Use(..) => {
-                                                if is_word(lint, "unused_imports") || is_word(lint, "deprecated") {
+                                                if is_word(lint, sym!(unused_imports))
+                                                    || is_word(lint, sym!(deprecated))
+                                                    || is_word(lint, sym!(unreachable_pub))
+                                                {
                                                     return;
                                                 }
                                             },
                                             ItemKind::ExternCrate(..) => {
-                                                if is_word(lint, "unused_imports") && skip_unused_imports {
+                                                if is_word(lint, sym!(unused_imports)) && skip_unused_imports {
                                                     return;
                                                 }
-                                                if is_word(lint, "unused_extern_crates") {
+                                                if is_word(lint, sym!(unused_extern_crates)) {
                                                     return;
                                                 }
                                             },
@@ -280,7 +277,7 @@ fn check_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &'tcx Item) {
                                                         line_span,
                                                         "if you just forgot a `!`, use",
                                                         sugg,
-                                                        Applicability::MachineApplicable,
+                                                        Applicability::MaybeIncorrect,
                                                     );
                                                 },
                                             );
@@ -298,13 +295,13 @@ fn check_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &'tcx Item) {
     }
 
     fn check_impl_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &'tcx ImplItem) {
-        if is_relevant_impl(cx.tcx, item) {
+        if is_relevant_impl(cx, item) {
             check_attrs(cx, item.span, item.ident.name, &item.attrs)
         }
     }
 
     fn check_trait_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &'tcx TraitItem) {
-        if is_relevant_trait(cx.tcx, item) {
+        if is_relevant_trait(cx, item) {
             check_attrs(cx, item.span, item.ident.name, &item.attrs)
         }
     }
@@ -322,7 +319,7 @@ fn check_clippy_lint_names(cx: &LateContext<'_, '_>, items: &[NestedMetaItem]) {
             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.as_str()),
+                Some(tool_name.name),
             );
             then {
                 span_lint_and_then(
@@ -335,7 +332,7 @@ fn check_clippy_lint_names(cx: &LateContext<'_, '_>, items: &[NestedMetaItem]) {
                             let name_lower = name.as_str().to_lowercase();
                             match lint_store.check_lint_name(
                                 &name_lower,
-                                Some(tool_name.as_str())
+                                Some(tool_name.name)
                             ) {
                                 // FIXME: can we suggest similar lint names here?
                                 // https://github.com/rust-lang/rust/pull/56992
@@ -357,52 +354,52 @@ fn check_clippy_lint_names(cx: &LateContext<'_, '_>, items: &[NestedMetaItem]) {
     }
 }
 
-fn is_relevant_item<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, item: &Item) -> bool {
+fn is_relevant_item(cx: &LateContext<'_, '_>, item: &Item) -> bool {
     if let ItemKind::Fn(_, _, _, eid) = item.node {
-        is_relevant_expr(tcx, tcx.body_tables(eid), &tcx.hir().body(eid).value)
+        is_relevant_expr(cx, cx.tcx.body_tables(eid), &cx.tcx.hir().body(eid).value)
     } else {
         true
     }
 }
 
-fn is_relevant_impl<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, item: &ImplItem) -> bool {
+fn is_relevant_impl(cx: &LateContext<'_, '_>, item: &ImplItem) -> bool {
     match item.node {
-        ImplItemKind::Method(_, eid) => is_relevant_expr(tcx, tcx.body_tables(eid), &tcx.hir().body(eid).value),
+        ImplItemKind::Method(_, eid) => is_relevant_expr(cx, cx.tcx.body_tables(eid), &cx.tcx.hir().body(eid).value),
         _ => false,
     }
 }
 
-fn is_relevant_trait<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, item: &TraitItem) -> bool {
+fn is_relevant_trait(cx: &LateContext<'_, '_>, item: &TraitItem) -> bool {
     match item.node {
         TraitItemKind::Method(_, TraitMethod::Required(_)) => true,
         TraitItemKind::Method(_, TraitMethod::Provided(eid)) => {
-            is_relevant_expr(tcx, tcx.body_tables(eid), &tcx.hir().body(eid).value)
+            is_relevant_expr(cx, cx.tcx.body_tables(eid), &cx.tcx.hir().body(eid).value)
         },
         _ => false,
     }
 }
 
-fn is_relevant_block<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, tables: &ty::TypeckTables<'_>, block: &Block) -> bool {
+fn is_relevant_block(cx: &LateContext<'_, '_>, tables: &ty::TypeckTables<'_>, block: &Block) -> bool {
     if let Some(stmt) = block.stmts.first() {
         match &stmt.node {
             StmtKind::Local(_) => true,
-            StmtKind::Expr(expr) | StmtKind::Semi(expr) => is_relevant_expr(tcx, tables, expr),
+            StmtKind::Expr(expr) | StmtKind::Semi(expr) => is_relevant_expr(cx, tables, expr),
             _ => false,
         }
     } else {
-        block.expr.as_ref().map_or(false, |e| is_relevant_expr(tcx, tables, e))
+        block.expr.as_ref().map_or(false, |e| is_relevant_expr(cx, tables, e))
     }
 }
 
-fn is_relevant_expr<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, tables: &ty::TypeckTables<'_>, expr: &Expr) -> bool {
+fn is_relevant_expr(cx: &LateContext<'_, '_>, tables: &ty::TypeckTables<'_>, expr: &Expr) -> bool {
     match &expr.node {
-        ExprKind::Block(block, _) => is_relevant_block(tcx, tables, block),
-        ExprKind::Ret(Some(e)) => is_relevant_expr(tcx, tables, e),
+        ExprKind::Block(block, _) => is_relevant_block(cx, tables, block),
+        ExprKind::Ret(Some(e)) => is_relevant_expr(cx, tables, e),
         ExprKind::Ret(None) | ExprKind::Break(_, None) => false,
         ExprKind::Call(path_expr, _) => {
             if let ExprKind::Path(qpath) = &path_expr.node {
-                if let Some(fun_id) = tables.qpath_def(qpath, path_expr.hir_id).opt_def_id() {
-                    !match_def_path(tcx, fun_id, &paths::BEGIN_PANIC)
+                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
                 }
@@ -415,7 +412,7 @@ fn is_relevant_expr<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, tables: &ty::TypeckTa
 }
 
 fn check_attrs(cx: &LateContext<'_, '_>, span: Span, name: Name, attrs: &[Attribute]) {
-    if in_macro(span) {
+    if span.from_expansion() {
         return;
     }
 
@@ -448,10 +445,10 @@ fn check_attrs(cx: &LateContext<'_, '_>, span: Span, name: Name, attrs: &[Attrib
         }
 
         if let Some(values) = attr.meta_item_list() {
-            if values.len() != 1 || !attr.check_name("inline") {
+            if values.len() != 1 || !attr.check_name(sym!(inline)) {
                 continue;
             }
-            if is_word(&values[0], "always") {
+            if is_word(&values[0], sym!(always)) {
                 span_lint(
                     cx,
                     INLINE_ALWAYS,
@@ -480,7 +477,7 @@ fn check_semver(cx: &LateContext<'_, '_>, span: Span, lit: &Lit) {
     );
 }
 
-fn is_word(nmi: &NestedMetaItem, expected: &str) -> bool {
+fn is_word(nmi: &NestedMetaItem, expected: Symbol) -> bool {
     if let NestedMetaItem::MetaItem(mi) = &nmi {
         mi.is_word() && mi.check_name(expected)
     } else {
@@ -488,47 +485,22 @@ fn is_word(nmi: &NestedMetaItem, expected: &str) -> bool {
     }
 }
 
-// If the snippet is empty, it's an attribute that was inserted during macro
-// expansion and we want to ignore those, because they could come from external
-// sources that the user has no control over.
-// For some reason these attributes don't have any expansion info on them, so
-// we have to check it this way until there is a better way.
-fn is_present_in_source(cx: &LateContext<'_, '_>, span: Span) -> bool {
-    if let Some(snippet) = snippet_opt(cx, span) {
-        if snippet.is_empty() {
-            return false;
-        }
-    }
-    true
-}
-
-#[derive(Copy, Clone)]
-pub struct CfgAttrPass;
-
-impl LintPass for CfgAttrPass {
-    fn get_lints(&self) -> LintArray {
-        lint_array!(DEPRECATED_CFG_ATTR,)
-    }
-
-    fn name(&self) -> &'static str {
-        "DeprecatedCfgAttribute"
-    }
-}
+declare_lint_pass!(DeprecatedCfgAttribute => [DEPRECATED_CFG_ATTR]);
 
-impl EarlyLintPass for CfgAttrPass {
+impl EarlyLintPass for DeprecatedCfgAttribute {
     fn check_attribute(&mut self, cx: &EarlyContext<'_>, attr: &Attribute) {
         if_chain! {
             // check cfg_attr
-            if attr.check_name("cfg_attr");
+            if attr.check_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("rustfmt");
+            if feature_item.check_name(sym!(rustfmt));
             // check for `rustfmt_skip` and `rustfmt::skip`
             if let Some(skip_item) = &items[1].meta_item();
-            if skip_item.check_name("rustfmt_skip") ||
-                skip_item.path.segments.last().expect("empty path in attribute").ident.name == "skip";
+            if skip_item.check_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
             if let AttrStyle::Outer = attr.style;