diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index cd258c7b506..89f55986f63 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -60,6 +60,7 @@ extern crate rustc_trait_selection; #[allow(unused_extern_crates)] extern crate rustc_typeck; +use rustc::session::Session; use rustc_data_structures::fx::FxHashSet; use rustc_lint::LintId; use rustc_session::Session; @@ -1060,9 +1061,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: let max_struct_bools = conf.max_struct_bools; 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); - let warn_on_all_wildcard_imports = conf.warn_on_all_wildcard_imports; - store.register_late_pass(move || box wildcard_imports::WildcardImports::new(warn_on_all_wildcard_imports)); - store.register_early_pass(|| box macro_use::MacroUseImports); + store.register_late_pass(|| box wildcard_imports::WildcardImports); store.register_late_pass(|| box verbose_file_reads::VerboseFileReads); store.register_late_pass(|| box redundant_pub_crate::RedundantPubCrate::default()); store.register_late_pass(|| box unnamed_address::UnnamedAddress); @@ -1080,6 +1079,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: single_char_binding_names_threshold, }); store.register_early_pass(|| box unnested_or_patterns::UnnestedOrPatterns); + 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 cf526409374..4c89647a574 100644 --- a/clippy_lints/src/macro_use.rs +++ b/clippy_lints/src/macro_use.rs @@ -1,10 +1,12 @@ -use crate::utils::{snippet, span_lint_and_sugg, in_macro}; +use crate::utils::{in_macro, snippet, span_lint_and_sugg}; +use hir::def::{DefKind, Res}; use if_chain::if_chain; use rustc_ast::ast; -use rustc_data_structures::fx::FxHashSet; +use rustc_data_structures::fx::FxHashMap; use rustc_errors::Applicability; -use rustc_lint::{EarlyContext, EarlyLintPass}; -use rustc_session::{impl_lint_pass, declare_tool_lint}; +use rustc_hir as hir; +use rustc_lint::{LateContext, LateLintPass, LintContext}; +use rustc_session::{declare_tool_lint, impl_lint_pass}; use rustc_span::{edition::Edition, Span}; declare_clippy_lint! { @@ -20,82 +22,226 @@ declare_clippy_lint! { /// #[macro_use] /// use lazy_static; /// ``` - pub MACRO_USE_IMPORT, + pub MACRO_USE_IMPORTS, pedantic, "#[macro_use] is no longer needed" } -#[derive(Default)] -pub struct MacroUseImport { - collected: FxHashSet, +const BRACKETS: &[char] = &['<', '>']; + +/// 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_lint_pass!(MacroUseImport => [MACRO_USE_IMPORT]); +impl MacroRefData { + pub fn new(name: String, span: Span, ecx: &LateContext<'_, '_>) -> Self { + let mut path = ecx.sess().source_map().span_to_filename(span).to_string(); -impl EarlyLintPass for MacroUseImport { + // std lib paths are <::std::module::file type> + // so remove brackets and space + if path.contains('<') { + path = path.replace(BRACKETS, ""); + } + if path.contains(' ') { + path = path.split(' ').next().unwrap().to_string(); + } + Self { + name: name.to_string(), + path, + } + } +} - fn check_item(&mut self, ecx: &EarlyContext<'_>, item: &ast::Item) { +#[derive(Default)] +pub struct MacroUseImports { + /// the actual import path used and the span of the attribute above it. + imports: Vec<(String, Span)>, + /// the span of the macro reference and the `MacroRefData` + /// for the use of the macro. + /// TODO make this FxHashSet to guard against inserting already found macros + collected: FxHashMap, + mac_refs: Vec<(Span, MacroRefData)>, +} + +impl_lint_pass!(MacroUseImports => [MACRO_USE_IMPORTS]); + +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 mac_names = find_used_macros(ecx, &import_path); - let msg = "`macro_use` attributes are no longer needed in the Rust 2018 edition"; - let help = format!("use {}::", import_path); - span_lint_and_sugg( - ecx, - MACRO_USE_IMPORT, - mac_attr.span, - msg, - // "remove the attribute and import the macro directly, try", - "", - help, - Applicability::HasPlaceholders, - ); - } - } - } + // println!("{:#?}", lcx.tcx.def_path_str(id)); + for kid in lcx.tcx.item_children(id).iter() { + // println!("{:#?}", kid); + if let Res::Def(DefKind::Macro(_mac_type), mac_id) = kid.res { + let span = mac_attr.span.clone(); - fn check_expr(&mut self, ecx: &EarlyContext<'_>, expr: &ast::Expr) { - if in_macro(expr.span) { - let name = snippet(ecx, ecx.sess.source_map().span_until_char(expr.span.source_callsite(), '!'), "_"); - if let Some(callee) = expr.span.source_callee() { - if self.collected.insert(callee.def_site) { - println!("EXPR {:#?}", name); + // 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); + } + } } } } } - fn check_stmt(&mut self, ecx: &EarlyContext<'_>, stmt: &ast::Stmt) { - if in_macro(stmt.span) { - let name = snippet(ecx, ecx.sess.source_map().span_until_char(stmt.span.source_callsite(), '!'), "_"); - if let Some(callee) = stmt.span.source_callee() { - println!("EXPR {:#?}", name); + fn check_attribute(&mut self, lcx: &LateContext<'_, '_>, attr: &ast::Attribute) { + if in_macro(attr.span) { + let call_site = attr.span.source_callsite(); + let name = snippet(lcx, lcx.sess().source_map().span_until_char(call_site, '!'), "_"); + if let Some(callee) = attr.span.source_callee() { + if !self.collected.contains_key(&call_site) { + println!("{:?}\n{:#?}", call_site, attr); + + let name = if name.contains("::") { + name.split("::").last().unwrap().to_string() + } else { + name.to_string() + }; + + let mac = MacroRefData::new(name, callee.def_site, lcx); + self.mac_refs.push((call_site, mac.clone())); + self.collected.insert(call_site, mac); + } } } } - fn check_pat(&mut self, ecx: &EarlyContext<'_>, pat: &ast::Pat) { - if in_macro(pat.span) { - let name = snippet(ecx, ecx.sess.source_map().span_until_char(pat.span.source_callsite(), '!'), "_"); - if let Some(callee) = pat.span.source_callee() { - println!("EXPR {:#?}", name); + 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(lcx, lcx.sess().source_map().span_until_char(call_site, '!'), "_"); + if let Some(callee) = expr.span.source_callee() { + if !self.collected.contains_key(&call_site) { + let name = if name.contains("::") { + name.split("::").last().unwrap().to_string() + } else { + name.to_string() + }; + + let mac = MacroRefData::new(name, callee.def_site, lcx); + self.mac_refs.push((call_site, mac.clone())); + self.collected.insert(call_site, mac); + } } } } + 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(lcx, lcx.sess().source_map().span_until_char(call_site, '!'), "_"); + if let Some(callee) = stmt.span.source_callee() { + if !self.collected.contains_key(&call_site) { + let name = if name.contains("::") { + name.split("::").last().unwrap().to_string() + } else { + name.to_string() + }; + + let mac = MacroRefData::new(name, callee.def_site, lcx); + self.mac_refs.push((call_site, mac.clone())); + self.collected.insert(call_site, mac); + } + } + } + } + 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(lcx, lcx.sess().source_map().span_until_char(call_site, '!'), "_"); + if let Some(callee) = pat.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); + } + } + } + } + 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(lcx, lcx.sess().source_map().span_until_char(call_site, '!'), "_"); + if let Some(callee) = ty.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); + } + } + } + } + + 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, + ) + } + } + if !self.mac_refs.is_empty() { + // TODO if not empty we found one we could not make a suggestion for + // such as std::prelude::v1 or something else I haven't thought of. + // println!("{:#?}", self.mac_refs); + } + } } -fn find_used_macros(ecx: &EarlyContext<'_>, path: &str) { - for it in ecx.krate.module.items.iter() { - if in_macro(it.span) { - // println!("{:#?}", it) - } +const PRELUDE: &[&str] = &[ + "marker", "ops", "convert", "iter", "option", "result", "borrow", "boxed", "string", "vec", "macros", +]; + +/// 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"]) && PRELUDE.iter().any(|m| segs.contains(m)) { + return format!( + "std::prelude::{} is imported by default, remove `use` statement", + mac.name + ); } - for x in ecx.sess.imported_macro_spans.borrow().iter() { - // println!("{:?}", x); + + if use_path.split("::").count() == 1 { + return format!("{}::{}", use_path, mac.name); } + + mac.path.clone() } diff --git a/macro_use_import b/macro_use_import deleted file mode 100755 index 61d3a827f1f..00000000000 Binary files a/macro_use_import and /dev/null differ diff --git a/tests/ui/auxiliary/macro_use_helper.rs b/tests/ui/auxiliary/macro_use_helper.rs new file mode 100644 index 00000000000..c63149a6819 --- /dev/null +++ b/tests/ui/auxiliary/macro_use_helper.rs @@ -0,0 +1,55 @@ +extern crate macro_rules; + +// STMT +#[macro_export] +macro_rules! pub_macro { + () => { + let _ = "hello Mr. Vonnegut"; + }; +} + +pub mod inner { + pub use super::*; + + // RE-EXPORT + // this will stick in `inner` module + pub use macro_rules::try_err; + + // ITEM + #[macro_export] + macro_rules! inner_mod { + () => { + #[allow(dead_code)] + pub struct Tardis; + }; + } +} + +// EXPR +#[macro_export] +macro_rules! function { + () => { + if true { + } else { + } + }; +} + +// TYPE +#[macro_export] +macro_rules! ty_mac { + () => { + Vec + }; +} + +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_import.rs b/tests/ui/macro_use_import.rs deleted file mode 100644 index 6490a2107d5..00000000000 --- a/tests/ui/macro_use_import.rs +++ /dev/null @@ -1,12 +0,0 @@ -// compile-flags: --edition 2018 -#![warn(clippy::macro_use_import)] - -use std::collections::HashMap; -#[macro_use] -use std::prelude; - -fn main() { - let _ = HashMap::::new(); - serde_if_integer128!(""); - println!(); -} diff --git a/tests/ui/macro_use_import.stderr b/tests/ui/macro_use_import.stderr deleted file mode 100644 index 1d86ba58441..00000000000 --- a/tests/ui/macro_use_import.stderr +++ /dev/null @@ -1,10 +0,0 @@ -error: `macro_use` attributes are no longer needed in the Rust 2018 edition - --> $DIR/macro_use_import.rs:5:1 - | -LL | #[macro_use] - | ^^^^^^^^^^^^ help: remove the attribute and import the macro directly, try: `use std::prelude::` - | - = note: `-D clippy::macro-use-import` implied by `-D warnings` - -error: aborting due to previous error - diff --git a/tests/ui/macro_use_imports.rs b/tests/ui/macro_use_imports.rs index 60c64ee8146..76911b0c565 100644 --- a/tests/ui/macro_use_imports.rs +++ b/tests/ui/macro_use_imports.rs @@ -1,11 +1,40 @@ -// edition:2018 +// compile-flags: --edition 2018 +// aux-build:macro_rules.rs +// aux-build:macro_use_helper.rs + +#![allow(clippy::single_component_path_imports)] #![warn(clippy::macro_use_imports)] -use std::collections::HashMap; #[macro_use] -use std::prelude; +extern crate macro_use_helper as mac; + +#[macro_use] +extern crate clippy_mini_macro_test as mini_mac; + +mod a { + #[macro_use] + use std::prelude; + #[macro_use] + use mac; + #[macro_use] + use mini_mac; + #[macro_use] + use mac::inner; + + #[derive(ClippyMiniMacroTest)] + struct Test; + + fn main() { + pub_macro!(); + inner_mod!(); + pub_in_private!(_var); + function!(); + let v: ty_mac!() = Vec::default(); + + inner::try_err!(); + } +} fn main() { - let _ = HashMap::::new(); println!(); }