diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 12be44caeb65..33871f1515e3 100644 --- a/clippy_lints/src/lib.rs +++ b/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), diff --git a/clippy_lints/src/macro_use.rs b/clippy_lints/src/macro_use.rs index 35dcb8e13c84..58d7c3f579a2 100644 --- a/clippy_lints/src/macro_use.rs +++ b/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, + 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!("{}::", use_path) - } -} - -fn paths_match(mac: &MacroRefData, use_path: &str) -> bool { - let segs = mac.path.split("::") - .filter(|s| *s != "") - .collect::>(); +/// 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::>(); - 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::>()) -} -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::>(); - - 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, diff --git a/tests/ui/auxiliary/macro_use_helper.rs b/tests/ui/auxiliary/macro_use_helper.rs new file mode 100644 index 000000000000..f5b69115e5e8 --- /dev/null +++ b/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"); + }; + } + } +} diff --git a/tests/ui/macro_use_imports.rs b/tests/ui/macro_use_imports.rs index db274831e3d6..1e43b1f999f0 100644 --- a/tests/ui/macro_use_imports.rs +++ b/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::::new(); + println!(); + pub_macro!(); + inner_mod!(); + pub_in_private!(_var); + } +} fn main() { - let _ = Mutex::new(8_u8); - let _ = HashMap::::new(); println!(); - declare_tool_lint! { - pub clippy::TEST_LINT, - Warn, - "", - report_in_external_macro: true - } }