diff --git a/clippy_lints/src/macro_use.rs b/clippy_lints/src/macro_use.rs index 1e1f27e9430c..089ae79b02c2 100644 --- a/clippy_lints/src/macro_use.rs +++ b/clippy_lints/src/macro_use.rs @@ -29,6 +29,12 @@ declare_clippy_lint! { const BRACKETS: &[char] = &['<', '>']; +#[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)] @@ -110,7 +116,8 @@ impl<'l, 'txc> LateLintPass<'l, 'txc> for MacroUseImports { 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; - self.imports.push((cx.tcx.def_path_str(mac_id), span)); + let def_path = cx.tcx.def_path_str(mac_id); + self.imports.push((def_path, span)); } } } else { @@ -147,127 +154,69 @@ impl<'l, 'txc> LateLintPass<'l, 'txc> for MacroUseImports { } #[allow(clippy::too_many_lines)] fn check_crate_post(&mut self, cx: &LateContext<'_, '_>, _krate: &hir::Crate<'_>) { - let mut import_map = FxHashMap::default(); + 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); - proccess_macro_path(*span, import, &mut import_map); - } - } - // println!("{:#?}", import_map); - let mut imports = vec![]; - for (root, rest) in import_map { - let mut path = format!("use {}::", root); - let mut attr_span = None; - // when a multiple nested paths are found one may be written to the string - // before it is found in this loop so we make note and skip it when this - // loop finds it - let mut found_nested = vec![]; - let mut count = 1; - let rest_len = rest.len(); - - if rest_len > 1 { - path.push_str("{"); - } - - for m in &rest { - if attr_span.is_none() { - attr_span = Some(m.span()); - } - if found_nested.contains(&m) { - continue; - } - let comma = if rest_len == count { "" } else { ", " }; - match m { - ModPath::Item { item, .. } => { - path.push_str(&format!("{}{}", item, comma)); + let seg = import.split("::").collect::>(); + + match seg.as_slice() { + [] => unreachable!("this should never be empty"), + [_] => unreachable!("path must have two segments ?"), + [root, item] => { + if !check_dup.contains(&item.to_string()) { + used.entry((root.to_string(), span)) + .or_insert(vec![]) + .push(item.to_string()); + check_dup.push(item.to_string()); + } }, - ModPath::Nested { segments, item, .. } => { - // do any other Nested paths match the current one - let nested = rest - .iter() - // filter "self" out - .filter(|other_m| other_m != &m) - // filters out Nested we have previously seen - .filter(|other_m| !found_nested.contains(other_m)) - // this matches the first path segment and filters non ModPath::Nested items - .filter(|other_m| other_m.matches(0, m)) - .collect::>(); - - if nested.is_empty() { - path.push_str(&format!("{}::{}{}", segments.join("::").to_string(), item, comma)) - // use mod_a::{mod_b::{one, two}, mod_c::item} + [root, rest @ ..] => { + if !rest.iter().all(|item| !check_dup.contains(&item.to_string())) { + let mut rest = rest.to_vec(); + rest.sort(); + used.entry((root.to_string(), span)) + .or_insert(vec![]) + .push(rest.join("::")); + check_dup.extend(rest.iter().map(ToString::to_string)); } else { - found_nested.extend(nested.iter()); - found_nested.push(&m); - // we check each segment for matches with other import paths if - // one differs we have to open a new `{}` - for (idx, seg) in segments.iter().enumerate() { - path.push_str(&format!("{}::", seg)); - if nested.iter().all(|other_m| other_m.matches(idx, &m)) { - continue; - } - - path.push_str("{"); - let matched_seg_items = nested - .iter() - .filter(|other_m| !other_m.matches(idx, &m)) - .collect::>(); - for item in matched_seg_items { - if let ModPath::Nested { item, .. } = item { - path.push_str(&format!( - "{}{}", - item, - if nested.len() == idx + 1 { "" } else { ", " } - )); - } - } - path.push_str("}"); - } - path.push_str(&format!("{{{}{}", item, comma)); - for (i, item) in nested.iter().enumerate() { - if let ModPath::Nested { item, segments: matched_seg, .. } = item { - path.push_str(&format!( - "{}{}{}", - if matched_seg > segments { - format!("{}::", matched_seg[segments.len()..].join("::")) - } else { - String::new() - }, - item, - if nested.len() == i + 1 { "" } else { ", " } - )); - } - } - path.push_str("}"); + let mut filtered = rest + .iter() + .filter(|item| !check_dup.contains(&item.to_string())) + .map(ToString::to_string) + .collect::>(); + filtered.sort(); + used.entry((root.to_string(), span)) + .or_insert(vec![]) + .push(filtered.join("::")); + check_dup.extend(filtered); } }, } - count += 1; } - if rest_len > 1 { - path.push_str("};"); - } else { - path.push_str(";"); - } - if let Some(span) = attr_span { - imports.push((span, path)) + } + + let mut suggestions = vec![]; + for ((root, span), path) in used { + if path.len() == 1 { + suggestions.push((span, format!("{}::{}", root, path[0]))) } else { - unreachable!("a span must always be attached to a macro_use attribute") + 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 imports { + for (span, import) in suggestions { let help = format!("use {}", import); span_lint_and_sugg( cx, MACRO_USE_IMPORTS, - span, + *span, "`macro_use` attributes are no longer needed in the Rust 2018 edition", "remove the attribute and import the macro directly, try", help, @@ -277,78 +226,3 @@ impl<'l, 'txc> LateLintPass<'l, 'txc> for MacroUseImports { } } } - -#[derive(Debug, PartialEq)] -enum ModPath { - Item { - item: String, - span: Span, - }, - Nested { - segments: Vec, - item: String, - span: Span, - }, -} - -impl ModPath { - fn span(&self) -> Span { - match self { - Self::Item { span, .. } | Self::Nested { span, .. } => *span, - } - } - - fn item(&self) -> &str { - match self { - Self::Item { item, .. } | Self::Nested { item, .. } => item, - } - } - - fn matches(&self, idx: usize, other: &ModPath) -> bool { - match (self, other) { - (Self::Item { item, .. }, Self::Item { item: other_item, .. }) => item == other_item, - ( - Self::Nested { segments, .. }, - Self::Nested { - segments: other_names, .. - }, - ) => match (segments.get(idx), other_names.get(idx)) { - (Some(seg), Some(other_seg)) => seg == other_seg, - (_, _) => false, - }, - (_, _) => false, - } - } -} - -#[allow(clippy::comparison_chain)] -fn proccess_macro_path(span: Span, import: &str, import_map: &mut FxHashMap>) { - let mut mod_path = import.split("::").collect::>(); - - if mod_path.len() == 2 { - let item_list = import_map.entry(mod_path[0].to_string()).or_insert_with(Vec::new); - - if !item_list.iter().any(|mods| mods.item() == mod_path[1]) { - item_list.push(ModPath::Item { - item: mod_path[1].to_string(), - span, - }); - } - } else if mod_path.len() > 2 { - let first = mod_path.remove(0); - let name = mod_path.remove(mod_path.len() - 1); - - let nested = ModPath::Nested { - segments: mod_path.into_iter().map(ToString::to_string).collect(), - item: name.to_string(), - span, - }; - // CLIPPY NOTE: this told me to use `or_insert_with(vec![])` - // import_map.entry(first.to_string()).or_insert(vec![]).push(nested); - // which failed as `vec!` is not a closure then told me to add `||` which failed - // with the redundant_closure lint so I finally gave up and used this. - import_map.entry(first.to_string()).or_insert_with(Vec::new).push(nested); - } else { - unreachable!("test to see if code path hit TODO REMOVE") - } -} diff --git a/tests/ui/macro_use_imports.stderr b/tests/ui/macro_use_imports.stderr index 00c76c19ea9e..83c8ebe6ab9e 100644 --- a/tests/ui/macro_use_imports.stderr +++ b/tests/ui/macro_use_imports.stderr @@ -2,15 +2,27 @@ error: `macro_use` attributes are no longer needed in the Rust 2018 edition --> $DIR/macro_use_imports.rs:17:5 | LL | #[macro_use] - | ^^^^^^^^^^^^ help: remove the attribute and import the macro directly, try: `use use mini_mac::ClippyMiniMacroTest;` + | ^^^^^^^^^^^^ help: remove the attribute and import the macro directly, try: `use mini_mac::ClippyMiniMacroTest` | = note: `-D clippy::macro-use-imports` implied by `-D warnings` +error: `macro_use` attributes are no longer needed in the Rust 2018 edition + --> $DIR/macro_use_imports.rs:21:5 + | +LL | #[macro_use] + | ^^^^^^^^^^^^ help: remove the attribute and import the macro directly, try: `use mac::inner::nested::string_add` + +error: `macro_use` attributes are no longer needed in the Rust 2018 edition + --> $DIR/macro_use_imports.rs:19:5 + | +LL | #[macro_use] + | ^^^^^^^^^^^^ help: remove the attribute and import the macro directly, try: `use mac::{foofoo::inner, inner::try_err}` + error: `macro_use` attributes are no longer needed in the Rust 2018 edition --> $DIR/macro_use_imports.rs:15:5 | LL | #[macro_use] - | ^^^^^^^^^^^^ help: remove the attribute and import the macro directly, try: `use use mac::{pub_macro, inner_mod_macro, function_macro, ty_macro, pub_in_private_macro, inner::{foofoo, try_err, nested::string_add}};` + | ^^^^^^^^^^^^ help: remove the attribute and import the macro directly, try: `use mac::{pub_macro, inner_mod_macro, function_macro, ty_macro, pub_in_private_macro}` -error: aborting due to 2 previous errors +error: aborting due to 4 previous errors