Auto merge of #5279 - DevinR528:macro-use-sugg, r=flip1995

Macro use sugg

changelog: Add auto applicable suggstion to macro_use_imports

fixes #5275

<s>Where exactly is the `wildcard_imports_helper` I haven't been able to import anything ex.
`use lazy_static;` or something like for that I get version/compiler conflicts?</s>
Found it.

Should we also check for `#[macro_use] extern crate`, although this is still depended on for stuff like `rustc_private`?

clippy_lints/src/lib.rs

@@ -1062,7 +1062,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
  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 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),

clippy_lints/src/macro_use.rs

@@ -1,18 +1,21 @@
-use crate::utils::{snippet, span_lint_and_sugg};
+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::{FxHashMap, FxHashSet};
 use rustc_errors::Applicability;
-use rustc_lint::{EarlyContext, EarlyLintPass};
-use rustc_session::{declare_lint_pass, declare_tool_lint};
-use rustc_span::edition::Edition;
+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! {
  /// **What it does:** Checks for `#[macro_use] use...`.
  ///
  /// **Why is this bad?** Since the Rust 2018 edition you can import
  /// macro's directly, this is considered idiomatic.
  ///
- /// **Known problems:** This lint does not generate an auto-applicable suggestion.
+ /// **Known problems:** None.
  ///
  /// **Example:**
  /// ```rust
@@ -24,29 +27,205 @@ declare_clippy_lint! {
  "#[macro_use] is no longer needed"
 }
 
-declare_lint_pass!(MacroUseImports => [MACRO_USE_IMPORTS]);
+const BRACKETS: &[char] = &['<', '>'];
 
-impl EarlyLintPass for MacroUseImports {
- fn check_item(&mut self, ecx: &EarlyContext<'_>, item: &ast::Item) {
+#[derive(Clone, Debug, PartialEq, Eq)]
+struct PathAndSpan {
+ path: String,
+ span: Span,
+}
+
+/// `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, callee: Span, cx: &LateContext<'_, '_>) -> Self {
+ let mut path = cx.sess().source_map().span_to_filename(callee).to_string();
+
+ // std lib paths are <::std::module::file type>
+ // so remove brackets, space and type.
+ if path.contains('<') {
+ path = path.replace(BRACKETS, "");
+ }
+ if path.contains(' ') {
+ path = path.split(' ').next().unwrap().to_string();
+ }
+ Self { name, path }
+ }
+}
+
+#[derive(Default)]
+#[allow(clippy::module_name_repetitions)]
+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, kept to ensure only one reference is used per macro call.
+ collected: FxHashSet<Span>,
+ mac_refs: Vec<MacroRefData>,
+}
+
+impl_lint_pass!(MacroUseImports => [MACRO_USE_IMPORTS]);
+
+impl MacroUseImports {
+ fn push_unique_macro(&mut self, cx: &LateContext<'_, '_>, span: Span) {
+ let call_site = span.source_callsite();
+ let name = snippet(cx, cx.sess().source_map().span_until_char(call_site, '!'), "_");
+ if let Some(callee) = span.source_callee() {
+ if !self.collected.contains(&call_site) {
+ let name = if name.contains("::") {
+ name.split("::").last().unwrap().to_string()
+ } else {
+ name.to_string()
+ };
+
+ self.mac_refs.push(MacroRefData::new(name, callee.def_site, cx));
+ self.collected.insert(call_site);
+ }
+ }
+ }
+
+ fn push_unique_macro_pat_ty(&mut self, cx: &LateContext<'_, '_>, span: Span) {
+ let call_site = span.source_callsite();
+ let name = snippet(cx, cx.sess().source_map().span_until_char(call_site, '!'), "_");
+ if let Some(callee) = span.source_callee() {
+ if !self.collected.contains(&call_site) {
+ self.mac_refs
+ .push(MacroRefData::new(name.to_string(), callee.def_site, cx));
+ self.collected.insert(call_site);
+ }
+ }
+ }
+}
+
+impl<'l, 'txc> LateLintPass<'l, 'txc> for MacroUseImports {
+ fn check_item(&mut self, cx: &LateContext<'_, '_>, item: &hir::Item<'_>) {
  if_chain! {
- if ecx.sess.opts.edition == Edition::Edition2018;
- if let ast::ItemKind::Use(use_tree) = &item.kind;
+ if cx.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 msg = "`macro_use` attributes are no longer needed in the Rust 2018 edition";
- let help = format!("use {}::<macro name>", snippet(ecx, use_tree.span, "_"));
+ for kid in cx.tcx.item_children(id).iter() {
+ if let Res::Def(DefKind::Macro(_mac_type), mac_id) = kid.res {
+ let span = mac_attr.span;
+ let def_path = cx.tcx.def_path_str(mac_id);
+ self.imports.push((def_path, span));
+ }
+ }
+ } else {
+ if in_macro(item.span) {
+ self.push_unique_macro_pat_ty(cx, item.span);
+ }
+ }
+ }
+ }
+ fn check_attribute(&mut self, cx: &LateContext<'_, '_>, attr: &ast::Attribute) {
+ if in_macro(attr.span) {
+ self.push_unique_macro(cx, attr.span);
+ }
+ }
+ fn check_expr(&mut self, cx: &LateContext<'_, '_>, expr: &hir::Expr<'_>) {
+ if in_macro(expr.span) {
+ self.push_unique_macro(cx, expr.span);
+ }
+ }
+ fn check_stmt(&mut self, cx: &LateContext<'_, '_>, stmt: &hir::Stmt<'_>) {
+ if in_macro(stmt.span) {
+ self.push_unique_macro(cx, stmt.span);
+ }
+ }
+ fn check_pat(&mut self, cx: &LateContext<'_, '_>, pat: &hir::Pat<'_>) {
+ if in_macro(pat.span) {
+ self.push_unique_macro_pat_ty(cx, pat.span);
+ }
+ }
+ fn check_ty(&mut self, cx: &LateContext<'_, '_>, ty: &hir::Ty<'_>) {
+ if in_macro(ty.span) {
+ self.push_unique_macro_pat_ty(cx, ty.span);
+ }
+ }
+ #[allow(clippy::too_many_lines)]
+ fn check_crate_post(&mut self, cx: &LateContext<'_, '_>, _krate: &hir::Crate<'_>) {
+ let mut used = FxHashMap::default();
+ let mut check_dup = vec![];
+ for (import, span) in &self.imports {
+ let found_idx = self.mac_refs.iter().position(|mac| import.ends_with(&mac.name));
+
+ if let Some(idx) = found_idx {
+ let _ = self.mac_refs.remove(idx);
+ let seg = import.split("::").collect::<Vec<_>>();
+
+ match seg.as_slice() {
+ // an empty path is impossible
+ // a path should always consist of 2 or more segments
+ [] | [_] => return,
+ [root, item] => {
+ if !check_dup.contains(&(*item).to_string()) {
+ used.entry(((*root).to_string(), span))
+ .or_insert_with(Vec::new)
+ .push((*item).to_string());
+ check_dup.push((*item).to_string());
+ }
+ },
+ [root, rest @ ..] => {
+ if rest.iter().all(|item| !check_dup.contains(&(*item).to_string())) {
+ let filtered = rest
+ .iter()
+ .filter_map(|item| {
+ if check_dup.contains(&(*item).to_string()) {
+ None
+ } else {
+ Some((*item).to_string())
+ }
+ })
+ .collect::<Vec<_>>();
+ used.entry(((*root).to_string(), span))
+ .or_insert_with(Vec::new)
+ .push(filtered.join("::"));
+ check_dup.extend(filtered);
+ } else {
+ let rest = rest.to_vec();
+ used.entry(((*root).to_string(), span))
+ .or_insert_with(Vec::new)
+ .push(rest.join("::"));
+ check_dup.extend(rest.iter().map(ToString::to_string));
+ }
+ },
+ }
+ }
+ }
+
+ let mut suggestions = vec![];
+ for ((root, span), path) in used {
+ if path.len() == 1 {
+ suggestions.push((span, format!("{}::{}", root, path[0])))
+ } else {
+ suggestions.push((span, format!("{}::{{{}}}", root, path.join(", "))))
+ }
+ }
+
+ // If mac_refs is not empty we have encountered an import we could not handle
+ // such as `std::prelude::v1::foo` or some other macro that expands to an import.
+ if self.mac_refs.is_empty() {
+ for (span, import) in suggestions {
+ let help = format!("use {};", import);
  span_lint_and_sugg(
- ecx,
+ cx,
  MACRO_USE_IMPORTS,
- mac_attr.span,
- msg,
+ *span,
+ "`macro_use` attributes are no longer needed in the Rust 2018 edition",
  "remove the attribute and import the macro directly, try",
  help,
- Applicability::HasPlaceholders,
- );
+ Applicability::MaybeIncorrect,
+ )
  }
  }
  }

tests/ui/auxiliary/macro_use_helper.rs

@@ -0,0 +1,60 @@
+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::foofoo;
+ pub use macro_rules::try_err;
+
+ pub mod nested {
+ pub use macro_rules::string_add;
+ }
+
+ // ITEM
+ #[macro_export]
+ macro_rules! inner_mod_macro {
+ () => {
+ #[allow(dead_code)]
+ pub struct Tardis;
+ };
+ }
+}
+
+// EXPR
+#[macro_export]
+macro_rules! function_macro {
+ () => {
+ if true {
+ } else {
+ }
+ };
+}
+
+// TYPE
+#[macro_export]
+macro_rules! ty_macro {
+ () => {
+ Vec<u8>
+ };
+}
+
+mod extern_exports {
+ pub(super) mod private_inner {
+ #[macro_export]
+ macro_rules! pub_in_private_macro {
+ ($name:ident) => {
+ let $name = String::from("secrets and lies");
+ };
+ }
+ }
+}

tests/ui/macro_use_imports.fixed

@@ -0,0 +1,42 @@
+// compile-flags: --edition 2018
+// aux-build:macro_rules.rs
+// aux-build:macro_use_helper.rs
+// run-rustfix
+
+#![allow(unused_imports, unreachable_code, unused_variables, dead_code)]
+#![allow(clippy::single_component_path_imports)]
+#![warn(clippy::macro_use_imports)]
+
+#[macro_use]
+extern crate macro_use_helper as mac;
+
+#[macro_use]
+extern crate clippy_mini_macro_test as mini_mac;
+
+mod a {
+ use mac::{pub_macro, inner_mod_macro, function_macro, ty_macro, pub_in_private_macro};
+ use mac;
+ use mini_mac::ClippyMiniMacroTest;
+ use mini_mac;
+ use mac::{inner::foofoo, inner::try_err};
+ use mac::inner;
+ use mac::inner::nested::string_add;
+ use mac::inner::nested;
+
+ #[derive(ClippyMiniMacroTest)]
+ struct Test;
+
+ fn test() {
+ pub_macro!();
+ inner_mod_macro!();
+ pub_in_private_macro!(_var);
+ function_macro!();
+ let v: ty_macro!() = Vec::default();
+
+ inner::try_err!();
+ inner::foofoo!();
+ nested::string_add!();
+ }
+}
+
+fn main() {}