Skip to content

Commit 0b7700e

Browse files
DevinR528DevinR528
committed
cleaned up import suggestion formatter, look into code reuse with wildcard impotrs
1 parent 3f20d1b commit 0b7700e

File tree

2 files changed

+64
-178
lines changed

2 files changed

+64
-178
lines changed

‎clippy_lints/src/macro_use.rs

Lines changed: 49 additions & 175 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,12 @@ declare_clippy_lint! {
2929

3030
const BRACKETS: &[char] = &['<', '>'];
3131

32+
#[derive(Clone, Debug, PartialEq, Eq)]
33+
struct PathAndSpan {
34+
path: String,
35+
span: Span,
36+
}
37+
3238
/// `MacroRefData` includes the name of the macro
3339
/// and the path from `SourceMap::span_to_filename`.
3440
#[derive(Debug, Clone)]
@@ -110,7 +116,8 @@ impl<'l, 'txc> LateLintPass<'l, 'txc> for MacroUseImports {
110116
for kid in cx.tcx.item_children(id).iter() {
111117
if let Res::Def(DefKind::Macro(_mac_type), mac_id) = kid.res {
112118
let span = mac_attr.span;
113-
self.imports.push((cx.tcx.def_path_str(mac_id), span));
119+
let def_path = cx.tcx.def_path_str(mac_id);
120+
self.imports.push((def_path, span));
114121
}
115122
}
116123
} else {
@@ -147,127 +154,69 @@ impl<'l, 'txc> LateLintPass<'l, 'txc> for MacroUseImports {
147154
}
148155
#[allow(clippy::too_many_lines)]
149156
fn check_crate_post(&mut self, cx: &LateContext<'_, '_>, _krate: &hir::Crate<'_>) {
150-
let mut import_map = FxHashMap::default();
157+
let mut used = FxHashMap::default();
158+
let mut check_dup = vec![];
151159
for (import, span) in &self.imports {
152160
let found_idx = self.mac_refs.iter().position(|mac| import.ends_with(&mac.name));
153161

154162
if let Some(idx) = found_idx {
155163
let _ = self.mac_refs.remove(idx);
156-
proccess_macro_path(*span, import, &mut import_map);
157-
}
158-
}
159-
// println!("{:#?}", import_map);
160-
let mut imports = vec![];
161-
for (root, rest) in import_map {
162-
let mut path = format!("use {}::", root);
163-
let mut attr_span = None;
164-
// when a multiple nested paths are found one may be written to the string
165-
// before it is found in this loop so we make note and skip it when this
166-
// loop finds it
167-
let mut found_nested = vec![];
168-
let mut count = 1;
169-
let rest_len = rest.len();
170-
171-
if rest_len > 1 {
172-
path.push_str("{");
173-
}
174-
175-
for m in &rest {
176-
if attr_span.is_none() {
177-
attr_span = Some(m.span());
178-
}
179-
if found_nested.contains(&m) {
180-
continue;
181-
}
182-
let comma = if rest_len == count { "" } else { ", " };
183-
match m {
184-
ModPath::Item { item, .. } => {
185-
path.push_str(&format!("{}{}", item, comma));
164+
let seg = import.split("::").collect::<Vec<_>>();
165+
166+
match seg.as_slice() {
167+
[] => unreachable!("this should never be empty"),
168+
[_] => unreachable!("path must have two segments ?"),
169+
[root, item] => {
170+
if !check_dup.contains(&item.to_string()) {
171+
used.entry((root.to_string(), span))
172+
.or_insert(vec![])
173+
.push(item.to_string());
174+
check_dup.push(item.to_string());
175+
}
186176
},
187-
ModPath::Nested { segments, item, .. } => {
188-
// do any other Nested paths match the current one
189-
let nested = rest
190-
.iter()
191-
// filter "self" out
192-
.filter(|other_m| other_m != &m)
193-
// filters out Nested we have previously seen
194-
.filter(|other_m| !found_nested.contains(other_m))
195-
// this matches the first path segment and filters non ModPath::Nested items
196-
.filter(|other_m| other_m.matches(0, m))
197-
.collect::<Vec<_>>();
198-
199-
if nested.is_empty() {
200-
path.push_str(&format!("{}::{}{}", segments.join("::").to_string(), item, comma))
201-
// use mod_a::{mod_b::{one, two}, mod_c::item}
177+
[root, rest @ ..] => {
178+
if !rest.iter().all(|item| !check_dup.contains(&item.to_string())) {
179+
let mut rest = rest.to_vec();
180+
rest.sort();
181+
used.entry((root.to_string(), span))
182+
.or_insert(vec![])
183+
.push(rest.join("::"));
184+
check_dup.extend(rest.iter().map(ToString::to_string));
202185
} else {
203-
found_nested.extend(nested.iter());
204-
found_nested.push(&m);
205-
// we check each segment for matches with other import paths if
206-
// one differs we have to open a new `{}`
207-
for (idx, seg) in segments.iter().enumerate() {
208-
path.push_str(&format!("{}::", seg));
209-
if nested.iter().all(|other_m| other_m.matches(idx, &m)) {
210-
continue;
211-
}
212-
213-
path.push_str("{");
214-
let matched_seg_items = nested
215-
.iter()
216-
.filter(|other_m| !other_m.matches(idx, &m))
217-
.collect::<Vec<_>>();
218-
for item in matched_seg_items {
219-
if let ModPath::Nested { item, .. } = item {
220-
path.push_str(&format!(
221-
"{}{}",
222-
item,
223-
if nested.len() == idx + 1 { "" } else { ", " }
224-
));
225-
}
226-
}
227-
path.push_str("}");
228-
}
229-
path.push_str(&format!("{{{}{}", item, comma));
230-
for (i, item) in nested.iter().enumerate() {
231-
if let ModPath::Nested { item, segments: matched_seg, .. } = item {
232-
path.push_str(&format!(
233-
"{}{}{}",
234-
if matched_seg > segments {
235-
format!("{}::", matched_seg[segments.len()..].join("::"))
236-
} else {
237-
String::new()
238-
},
239-
item,
240-
if nested.len() == i + 1 { "" } else { ", " }
241-
));
242-
}
243-
}
244-
path.push_str("}");
186+
let mut filtered = rest
187+
.iter()
188+
.filter(|item| !check_dup.contains(&item.to_string()))
189+
.map(ToString::to_string)
190+
.collect::<Vec<_>>();
191+
filtered.sort();
192+
used.entry((root.to_string(), span))
193+
.or_insert(vec![])
194+
.push(filtered.join("::"));
195+
check_dup.extend(filtered);
245196
}
246197
},
247198
}
248-
count += 1;
249199
}
250-
if rest_len > 1 {
251-
path.push_str("};");
252-
} else {
253-
path.push_str(";");
254-
}
255-
if let Some(span) = attr_span {
256-
imports.push((span, path))
200+
}
201+
202+
let mut suggestions = vec![];
203+
for ((root, span), path) in used {
204+
if path.len() == 1 {
205+
suggestions.push((span, format!("{}::{}", root, path[0])))
257206
} else {
258-
unreachable!("a span must always be attached to a macro_use attribute")
207+
suggestions.push((span, format!("{}::{{{}}}", root, path.join(", "))))
259208
}
260209
}
261210

262211
// If mac_refs is not empty we have encountered an import we could not handle
263212
// such as `std::prelude::v1::foo` or some other macro that expands to an import.
264213
if self.mac_refs.is_empty() {
265-
for (span, import) in imports {
214+
for (span, import) in suggestions {
266215
let help = format!("use {}", import);
267216
span_lint_and_sugg(
268217
cx,
269218
MACRO_USE_IMPORTS,
270-
span,
219+
*span,
271220
"`macro_use` attributes are no longer needed in the Rust 2018 edition",
272221
"remove the attribute and import the macro directly, try",
273222
help,
@@ -277,78 +226,3 @@ impl<'l, 'txc> LateLintPass<'l, 'txc> for MacroUseImports {
277226
}
278227
}
279228
}
280-
281-
#[derive(Debug, PartialEq)]
282-
enum ModPath {
283-
Item {
284-
item: String,
285-
span: Span,
286-
},
287-
Nested {
288-
segments: Vec<String>,
289-
item: String,
290-
span: Span,
291-
},
292-
}
293-
294-
impl ModPath {
295-
fn span(&self) -> Span {
296-
match self {
297-
Self::Item { span, .. } | Self::Nested { span, .. } => *span,
298-
}
299-
}
300-
301-
fn item(&self) -> &str {
302-
match self {
303-
Self::Item { item, .. } | Self::Nested { item, .. } => item,
304-
}
305-
}
306-
307-
fn matches(&self, idx: usize, other: &ModPath) -> bool {
308-
match (self, other) {
309-
(Self::Item { item, .. }, Self::Item { item: other_item, .. }) => item == other_item,
310-
(
311-
Self::Nested { segments, .. },
312-
Self::Nested {
313-
segments: other_names, ..
314-
},
315-
) => match (segments.get(idx), other_names.get(idx)) {
316-
(Some(seg), Some(other_seg)) => seg == other_seg,
317-
(_, _) => false,
318-
},
319-
(_, _) => false,
320-
}
321-
}
322-
}
323-
324-
#[allow(clippy::comparison_chain)]
325-
fn proccess_macro_path(span: Span, import: &str, import_map: &mut FxHashMap<String, Vec<ModPath>>) {
326-
let mut mod_path = import.split("::").collect::<Vec<_>>();
327-
328-
if mod_path.len() == 2 {
329-
let item_list = import_map.entry(mod_path[0].to_string()).or_insert_with(Vec::new);
330-
331-
if !item_list.iter().any(|mods| mods.item() == mod_path[1]) {
332-
item_list.push(ModPath::Item {
333-
item: mod_path[1].to_string(),
334-
span,
335-
});
336-
}
337-
} else if mod_path.len() > 2 {
338-
let first = mod_path.remove(0);
339-
let name = mod_path.remove(mod_path.len() - 1);
340-
341-
let nested = ModPath::Nested {
342-
segments: mod_path.into_iter().map(ToString::to_string).collect(),
343-
item: name.to_string(),
344-
span,
345-
};
346-
// CLIPPY NOTE: this told me to use `or_insert_with(vec![])`
347-
// import_map.entry(first.to_string()).or_insert(vec![]).push(nested);
348-
// which failed as `vec!` is not a closure then told me to add `||` which failed
349-
// with the redundant_closure lint so I finally gave up and used this.
350-
import_map.entry(first.to_string()).or_insert_with(Vec::new).push(nested);
351-
} else {
352-
unreachable!("test to see if code path hit TODO REMOVE")
353-
}
354-
}

‎tests/ui/macro_use_imports.stderr

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,27 @@ error: `macro_use` attributes are no longer needed in the Rust 2018 edition
22
--> $DIR/macro_use_imports.rs:17:5
33
|
44
LL | #[macro_use]
5-
| ^^^^^^^^^^^^ help: remove the attribute and import the macro directly, try: `use use mini_mac::ClippyMiniMacroTest;`
5+
| ^^^^^^^^^^^^ help: remove the attribute and import the macro directly, try: `use mini_mac::ClippyMiniMacroTest`
66
|
77
= note: `-D clippy::macro-use-imports` implied by `-D warnings`
88

9+
error: `macro_use` attributes are no longer needed in the Rust 2018 edition
10+
--> $DIR/macro_use_imports.rs:21:5
11+
|
12+
LL | #[macro_use]
13+
| ^^^^^^^^^^^^ help: remove the attribute and import the macro directly, try: `use mac::inner::nested::string_add`
14+
15+
error: `macro_use` attributes are no longer needed in the Rust 2018 edition
16+
--> $DIR/macro_use_imports.rs:19:5
17+
|
18+
LL | #[macro_use]
19+
| ^^^^^^^^^^^^ help: remove the attribute and import the macro directly, try: `use mac::{foofoo::inner, inner::try_err}`
20+
921
error: `macro_use` attributes are no longer needed in the Rust 2018 edition
1022
--> $DIR/macro_use_imports.rs:15:5
1123
|
1224
LL | #[macro_use]
13-
| ^^^^^^^^^^^^ 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}};`
25+
| ^^^^^^^^^^^^ 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}`
1426

15-
error: aborting due to 2 previous errors
27+
error: aborting due to 4 previous errors
1628

0 commit comments

Comments
 (0)