WIP auto-applicable suggestion macro-use-imports

Author: DevinR528

clippy_lints/src/lib.rs

@@ -53,6 +53,11 @@ extern crate rustc_target;
 #[allow(unused_extern_crates)]
 extern crate rustc_typeck;
 
+#[allow(unused_extern_crates)]
+extern crate rustc_resolve;
+#[allow(unused_extern_crates)]
+extern crate rustc_codegen_llvm;
+
 use rustc::session::Session;
 use rustc_data_structures::fx::FxHashSet;
 use rustc_lint::LintId;
@@ -1014,7 +1019,8 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
     store.register_early_pass(move || box excessive_bools::ExcessiveBools::new(max_struct_bools, max_fn_params_bools));
     store.register_early_pass(|| box option_env_unwrap::OptionEnvUnwrap);
     store.register_late_pass(|| box wildcard_imports::WildcardImports);
-    store.register_early_pass(|| box macro_use::MacroUseImports::default());
+    // store.register_early_pass(|| box macro_use::MacroUseImports::default());
+    store.register_late_pass(|| box macro_use::MacroUseImports::default());
 
     store.register_group(true, "clippy::restriction", Some("clippy_restriction"), vec![
         LintId::of(&arithmetic::FLOAT_ARITHMETIC),

clippy_lints/src/macro_use.rs

@@ -1,16 +1,15 @@
-use crate::utils::{snippet, span_lint_and_sugg, in_macro};
+use crate::utils::{in_macro, snippet, span_lint_and_sugg};
 use if_chain::if_chain;
-use rustc_ast::ast;
 use rustc_data_structures::fx::FxHashMap;
 use rustc_errors::Applicability;
-use rustc_lint::{EarlyContext, EarlyLintPass, LintContext, Lint};
-use rustc_session::{impl_lint_pass, declare_tool_lint};
-use rustc_span::{edition::Edition, Span};
 use rustc_hir as hir;
+use hir::def::{Res, DefKind};
+use rustc_lint::{LintContext, LateLintPass, LateContext};
+use rustc_session::{declare_tool_lint, impl_lint_pass};
+use rustc_span::{edition::Edition, Span};
 
 const PRELUDE: &[&str] = &[
-    "marker", "ops", "convert", "iter", "option", "result", "borrow", "boxed", "string", "vec",
-    "macros"
+    "marker", "ops", "convert", "iter", "option", "result", "borrow", "boxed", "string", "vec", "macros",
 ];
 const BRACKETS: &[char] = &['<', '>'];
 
@@ -34,14 +33,15 @@ declare_clippy_lint! {
 
 /// MacroRefData includes the name of the macro
 /// and the path from `SourceMap::span_to_filename`.
+#[derive(Debug, Clone)]
 pub struct MacroRefData {
     name: String,
     path: String,
 }
 
 impl MacroRefData {
-    pub fn new(name: String, span: Span, ecx: &EarlyContext<'_>) -> Self {
-        let mut path = ecx.sess.source_map().span_to_filename(span).to_string();
+    pub fn new(name: String, span: Span, ecx: &LateContext<'_, '_>) -> Self {
+        let mut path = ecx.sess().source_map().span_to_filename(span).to_string();
 
         // std lib paths are <::std::module::file type>
         // so remove brackets and space
@@ -51,7 +51,10 @@ impl MacroRefData {
         if path.contains(' ') {
             path = path.split(' ').next().unwrap().to_string();
         }
-        Self { name: name.to_string(), path, }
+        Self {
+            name: name.to_string(),
+            path,
+        }
     }
 }
 
@@ -62,122 +65,144 @@ pub struct MacroUseImports {
     /// the span of the macro reference and the `MacroRefData`
     /// for the use of the macro.
     collected: FxHashMap<Span, MacroRefData>,
+    mac_refs: Vec<(Span, MacroRefData)>,
 }
 
-impl MacroUseImports {
-    fn import_path_mac(&self, use_path: &str) -> String {
-        for mac in self.collected.values() {
-            if paths_match(mac, use_path) {
-                return make_path(mac, use_path)
-            }
-        }
-        format!("{}::<macro name>", use_path)
-    }
-}
-
-fn paths_match(mac: &MacroRefData, use_path: &str) -> bool {
-    let segs = mac.path.split("::")
-        .filter(|s| *s != "")
-        .collect::<Vec<_>>();
+/// This is somewhat of a fallback for imports from `std::prelude` because they
+/// are not recognized by `LateLintPass::check_item` `lcx.tcx.item_children(id)`
+fn make_path(mac: &MacroRefData, use_path: &str) -> String {
+    let segs = mac.path.split("::").filter(|s| *s != "").collect::<Vec<_>>();
 
-    if segs.starts_with(&["std"]) {
-        return PRELUDE.iter().any(|m| segs.contains(m))
+    if segs.starts_with(&["std"]) && PRELUDE.iter().any(|m| segs.contains(m)) {
+        return format!("std::prelude::{} is imported by default, remove `use` statement", mac.name);
     }
-    
-    segs.starts_with(&use_path.split("::").collect::<Vec<_>>())
-}
 
-fn make_path(mac: &MacroRefData, use_path: &str) -> String {
     if use_path.split("::").count() == 1 {
         return format!("{}::{}", use_path, mac.name);
     }
 
-    let segs = mac.path.split("::")
-        .filter(|s| *s != "")
-        .collect::<Vec<_>>();
-    
-    if segs.starts_with(&["std"]) && PRELUDE.iter().any(|m| segs.contains(m)) {
-        return format!("std::prelude::{}", mac.name);
-    }
-    
     mac.path.clone()
 }
 
 impl_lint_pass!(MacroUseImports => [MACRO_USE_IMPORTS]);
 
-impl EarlyLintPass for MacroUseImports {
-    fn check_item(&mut self, ecx: &EarlyContext<'_>, item: &ast::Item) {
+impl<'l, 'txc> LateLintPass<'l, 'txc> for MacroUseImports {
+    fn check_item(&mut self, lcx: &LateContext<'_, '_>, item: &hir::Item<'_>) {
         if_chain! {
-            if ecx.sess.opts.edition == Edition::Edition2018;
-            if let ast::ItemKind::Use(use_tree) = &item.kind;
+            if lcx.sess().opts.edition == Edition::Edition2018;
+            if let hir::ItemKind::Use(path, _kind) = &item.kind;
             if let Some(mac_attr) = item
                 .attrs
                 .iter()
                 .find(|attr| attr.ident().map(|s| s.to_string()) == Some("macro_use".to_string()));
+            if let Res::Def(DefKind::Mod, id) = path.res;
             then {
-                let import_path = snippet(ecx, use_tree.span, "_");
-                let span = mac_attr.span.clone();
-                self.imports.push((import_path.to_string(), span));
+                for kid in lcx.tcx.item_children(id).iter() {
+                    if let Res::Def(DefKind::Macro(_mac_type), mac_id) = kid.res {
+                        let span = mac_attr.span.clone();
+                        println!("{:#?}", lcx.tcx.def_path_str(mac_id));
+                        self.imports.push((lcx.tcx.def_path_str(mac_id), span));
+                    }
+                }
+            } else {
+                if in_macro(item.span) {
+                    let call_site = item.span.source_callsite();
+                    let name = snippet(lcx, lcx.sess().source_map().span_until_char(call_site, '!'), "_");
+                    if let Some(callee) = item.span.source_callee() {
+                        if !self.collected.contains_key(&call_site) {
+                            let mac = MacroRefData::new(name.to_string(), callee.def_site, lcx);
+                            self.mac_refs.push((call_site, mac.clone()));
+                            self.collected.insert(call_site, mac);
+                            // println!("EXPR {:?} {:?}", name, lcx.sess().source_map().span_to_filename(callee.def_site));
+                        }
+                    }
+                }
             }
         }
     }
 
-    fn check_expr(&mut self, ecx: &EarlyContext<'_>, expr: &ast::Expr) {
+    fn check_expr(&mut self, lcx: &LateContext<'_, '_>, expr: &hir::Expr<'_>) {
         if in_macro(expr.span) {
             let call_site = expr.span.source_callsite();
-            let name = snippet(ecx, ecx.sess.source_map().span_until_char(call_site, '!'), "_");
+            let name = snippet(lcx, lcx.sess().source_map().span_until_char(call_site, '!'), "_");
             if let Some(callee) = expr.span.source_callee() {
-                self.collected.entry(call_site)
-                    .or_insert_with(|| {
-                        MacroRefData::new(name.to_string(), callee.def_site, ecx)
-                    });
+                if !self.collected.contains_key(&call_site) {
+                    let mac = MacroRefData::new(name.to_string(), callee.def_site, lcx);
+                    self.mac_refs.push((call_site, mac.clone()));
+                    self.collected.insert(call_site, mac);
+                    // println!("EXPR {:?} {:?}", name, lcx.sess().source_map().span_to_filename(callee.def_site));
+                }
             }
         }
     }
-    fn check_stmt(&mut self, ecx: &EarlyContext<'_>, stmt: &ast::Stmt) {
+    fn check_stmt(&mut self, lcx: &LateContext<'_, '_>, stmt: &hir::Stmt<'_>) {
         if in_macro(stmt.span) {
             let call_site = stmt.span.source_callsite();
-            let name = snippet(ecx, ecx.sess.source_map().span_until_char(call_site, '!'), "_");
+            let name = snippet(lcx, lcx.sess().source_map().span_until_char(call_site, '!'), "_");
             if let Some(callee) = stmt.span.source_callee() {
-                self.collected.entry(call_site)
-                    .or_insert_with(|| {
-                        MacroRefData::new(name.to_string(), callee.def_site, ecx)
-                    });
+                if !self.collected.contains_key(&call_site) {
+                    let mac = MacroRefData::new(name.to_string(), callee.def_site, lcx);
+                    self.mac_refs.push((call_site, mac.clone()));
+                    self.collected.insert(call_site, mac);
+                    // println!("STMT {:?} {:?}", name, lcx.sess().source_map().span_to_filename(callee.def_site));
+                }
             }
         }
     }
-    fn check_pat(&mut self, ecx: &EarlyContext<'_>, pat: &ast::Pat) {
+    fn check_pat(&mut self, lcx: &LateContext<'_, '_>, pat: &hir::Pat<'_>) {
         if in_macro(pat.span) {
             let call_site = pat.span.source_callsite();
-            let name = snippet(ecx, ecx.sess.source_map().span_until_char(call_site, '!'), "_");
+            let name = snippet(lcx, lcx.sess().source_map().span_until_char(call_site, '!'), "_");
             if let Some(callee) = pat.span.source_callee() {
-                self.collected.entry(call_site)
-                    .or_insert_with(|| {
-                        MacroRefData::new(name.to_string(), callee.def_site, ecx)
-                    });
+                if !self.collected.contains_key(&call_site) {
+                    let mac = MacroRefData::new(name.to_string(), callee.def_site, lcx);
+                    self.mac_refs.push((call_site, mac.clone()));
+                    self.collected.insert(call_site, mac);
+                    // println!("PAT {:?} {:?}", name, lcx.sess().source_map().span_to_filename(callee.def_site));
+                }
             }
         }
     }
-    fn check_ty(&mut self, ecx: &EarlyContext<'_>, ty: &ast::Ty) {
+    fn check_ty(&mut self, lcx: &LateContext<'_, '_>, ty: &hir::Ty<'_>) {
         if in_macro(ty.span) {
             let call_site = ty.span.source_callsite();
-            let name = snippet(ecx, ecx.sess.source_map().span_until_char(call_site, '!'), "_");
+            let name = snippet(lcx, lcx.sess().source_map().span_until_char(call_site, '!'), "_");
             if let Some(callee) = ty.span.source_callee() {
-                self.collected.entry(call_site)
-                    .or_insert_with(|| {
-                        MacroRefData::new(name.to_string(), callee.def_site, ecx)
-                    });
+                if !self.collected.contains_key(&call_site) {
+                    let mac = MacroRefData::new(name.to_string(), callee.def_site, lcx);
+                    self.mac_refs.push((call_site, mac.clone()));
+                    self.collected.insert(call_site, mac);
+                    // println!("TYPE {:?} {:?}", name, lcx.sess().source_map().span_to_filename(callee.def_site));
+                }
             }
         }
     }
 
-    fn check_crate_post(&mut self, ecx: &EarlyContext<'_>, _krate: &ast::Crate) {
-        for (name, span) in self.imports.iter() {
-            let import_path = self.import_path_mac(&name);
+    fn check_crate_post(&mut self, lcx: &LateContext<'_, '_>, _krate: &hir::Crate<'_>) {
+        for (import, span) in self.imports.iter() {
+
+            let matched = self.mac_refs.iter().find(|(_span, mac)| !import.ends_with(&mac.name)).is_some();
+            if matched {
+                self.mac_refs.retain(|(_span, mac)| !import.ends_with(&mac.name));
+                let msg = "`macro_use` attributes are no longer needed in the Rust 2018 edition";
+                let help = format!("use {}", import);
+                span_lint_and_sugg(
+                    lcx,
+                    MACRO_USE_IMPORTS,
+                    *span,
+                    msg,
+                    "remove the attribute and import the macro directly, try",
+                    help,
+                    Applicability::HasPlaceholders,
+                )
+            }
+        }
+
+        for (span, mac) in self.mac_refs.iter() {
             let msg = "`macro_use` attributes are no longer needed in the Rust 2018 edition";
-            let help = format!("use {}", import_path);
+            let help = make_path(mac, "hello");
             span_lint_and_sugg(
-                ecx,
+                lcx,
                 MACRO_USE_IMPORTS,
                 *span,
                 msg,

tests/ui/auxiliary/macro_use_helper.rs

@@ -0,0 +1,32 @@
+// pub use macro_rules::foofoo;
+// pub use crate::extern_exports::*;
+
+#[macro_export]
+macro_rules! pub_macro {
+    () => {
+        let _ = "hello Mr. Vonnegut";
+    };
+}
+
+
+pub mod inner {
+    #[macro_export]
+        macro_rules! inner_mod {
+            () => {
+                #[allow(dead_code)]
+                pub struct Tardis;
+            };
+        }
+}
+
+mod extern_exports {
+    
+    pub(super) mod private_inner {
+        #[macro_export]
+        macro_rules! pub_in_private {
+            ($name:ident) => {
+                let $name = String::from("secrets and lies");
+            };
+        }
+    }
+}

tests/ui/macro_use_imports.rs

@@ -1,21 +1,30 @@
 // compile-flags: --edition 2018
+// aux-build:macro_use_helper.rs
+
+// aux-build:macro_rules.rs
+#![allow(clippy::single_component_path_imports)]
 #![warn(clippy::macro_use_imports)]
 #![feature(rustc_private)]
 
-use std::collections::HashMap;
 #[macro_use]
-use std::{prelude, sync::Mutex};
+extern crate macro_use_helper as mac;
 #[macro_use]
-extern crate rustc_session;
+use std::prelude;
+
+mod a {
+    use std::collections::HashMap;
+    #[macro_use]
+    use mac;
+
+    fn main() {
+        let _ = HashMap::<u8, u8>::new();
+        println!();
+        pub_macro!();
+        inner_mod!();
+        pub_in_private!(_var);
+    }
+}
 
 fn main() {
-    let _ = Mutex::new(8_u8);
-    let _ = HashMap::<u8, u8>::new();
     println!();
-    declare_tool_lint! {
-        pub clippy::TEST_LINT,
-        Warn,
-        "",
-        report_in_external_macro: true
-    }
 }