Skip to content

Commit d4f60b5

Browse files
DevinR528DevinR528
committed
wip: of handling nested import paths for multi-macro paths
1 parent 451363d commit d4f60b5

File tree

4 files changed

+122
-122
lines changed

4 files changed

+122
-122
lines changed

‎clippy_lints/src/macro_use.rs‎

Lines changed: 109 additions & 88 deletions
Original file line numberDiff line numberDiff line change
@@ -49,10 +49,7 @@ impl MacroRefData {
4949
if path.contains(' ') {
5050
path = path.split(' ').next().unwrap().to_string();
5151
}
52-
Self {
53-
name,
54-
path,
55-
}
52+
Self { name, path }
5653
}
5754
}
5855

@@ -79,7 +76,7 @@ impl MacroUseImports {
7976
} else {
8077
name.to_string()
8178
};
82-
79+
8380
self.mac_refs.push(MacroRefData::new(name, callee.def_site, cx));
8481
self.collected.insert(call_site);
8582
}
@@ -91,7 +88,8 @@ impl MacroUseImports {
9188
let name = snippet(cx, cx.sess().source_map().span_until_char(call_site, '!'), "_");
9289
if let Some(callee) = span.source_callee() {
9390
if !self.collected.contains(&call_site) {
94-
self.mac_refs.push(MacroRefData::new(name.to_string(), callee.def_site, cx));
91+
self.mac_refs
92+
.push(MacroRefData::new(name.to_string(), callee.def_site, cx));
9593
self.collected.insert(call_site);
9694
}
9795
}
@@ -147,78 +145,123 @@ impl<'l, 'txc> LateLintPass<'l, 'txc> for MacroUseImports {
147145
self.push_unique_macro_pat_ty(cx, ty.span);
148146
}
149147
}
150-
148+
#[allow(clippy::too_many_lines)]
151149
fn check_crate_post(&mut self, cx: &LateContext<'_, '_>, _krate: &hir::Crate<'_>) {
152150
let mut import_map = FxHashMap::default();
153151
for (import, span) in &self.imports {
154152
let found_idx = self.mac_refs.iter().position(|mac| import.ends_with(&mac.name));
155-
153+
156154
if let Some(idx) = found_idx {
157155
let _ = self.mac_refs.remove(idx);
158156
proccess_macro_path(*span, import, &mut import_map);
159157
}
160158
}
161-
println!("{:#?}", import_map);
159+
// println!("{:#?}", import_map);
162160
let mut imports = vec![];
163161
for (root, rest) in import_map {
164162
let mut path = format!("use {}::", root);
165-
let mut s = None;
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![];
166168
let mut count = 1;
167169
let rest_len = rest.len();
170+
168171
if rest_len > 1 {
169172
path.push_str("{");
170173
}
174+
171175
for m in &rest {
172-
println!("{} => {:?}", root, m);
173-
if count == 1 {
174-
s = Some(m.span());
176+
if attr_span.is_none() {
177+
attr_span = Some(m.span());
178+
}
179+
if found_nested.contains(&m) {
180+
continue;
175181
}
176-
177182
let comma = if rest_len == count { "" } else { ", " };
178183
match m {
179184
ModPath::Item { item, .. } => {
180185
path.push_str(&format!("{}{}", item, comma));
181-
}
182-
ModPath::Nested { names, item, span } => {
183-
let nested = rest.iter()
186+
},
187+
ModPath::Nested { segments, item, .. } => {
188+
// do any other Nested paths match the current one
189+
let nested = rest
190+
.iter()
184191
// filter "self" out
185192
.filter(|other_m| other_m != &m)
193+
// filters out Nested we have previously seen
194+
.filter(|other_m| !found_nested.contains(other_m))
186195
// this matches the first path segment and filters non ModPath::Nested items
187196
.filter(|other_m| other_m.matches(0, m))
188197
.collect::<Vec<_>>();
189198

190-
println!("{:#?}", nested);
191-
192199
if nested.is_empty() {
193-
path.push_str(&format!("{}::{}{}", names.join("::").to_string(), item, comma))
200+
path.push_str(&format!("{}::{}{}", segments.join("::").to_string(), item, comma))
201+
// use mod_a::{mod_b::{one, two}, mod_c::item}
194202
} else {
195-
// use mod_a::{mod_b::{one, two}, mod_c::item, item1, item2}
196-
let mod_path = if names.len() - 1 > 0 {
197-
ModPath::Nested { names: names.clone(), item: item.to_string(), span: *span, }
198-
} else {
199-
ModPath::Item { item: names[0].to_string(), span: *span, }
200-
};
201-
let names = recursive_path_push(mod_path, comma, &rest, String::new());
202-
path.push_str(&format!("{}::{{{}}}{}", names, item, comma))
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("}");
203245
}
204-
}
246+
},
205247
}
206-
count += 1;
248+
count += 1;
207249
}
208250
if rest_len > 1 {
209251
path.push_str("};");
252+
} else {
253+
path.push_str(";");
210254
}
211-
if let Some(span) = s {
255+
if let Some(span) = attr_span {
212256
imports.push((span, path))
257+
} else {
258+
unreachable!("a span must always be attached to a macro_use attribute")
213259
}
214260
}
215261

216-
if !self.mac_refs.is_empty() {
217-
// TODO if not empty we found one we could not make a suggestion for
218-
// such as std::prelude::v1 or something else I haven't thought of.
219-
// If we defer the calling of span_lint_and_sugg we can make a decision about its
220-
// applicability?
221-
} else {
262+
// If mac_refs is not empty we have encountered an import we could not handle
263+
// such as `std::prelude::v1::foo` or some other macro that expands to an import.
264+
if self.mac_refs.is_empty() {
222265
for (span, import) in imports {
223266
let help = format!("use {}", import);
224267
span_lint_and_sugg(
@@ -237,48 +280,56 @@ impl<'l, 'txc> LateLintPass<'l, 'txc> for MacroUseImports {
237280

238281
#[derive(Debug, PartialEq)]
239282
enum ModPath {
240-
Item { item: String, span: Span, },
241-
Nested { names: Vec<String>, item: String, span: Span, },
283+
Item {
284+
item: String,
285+
span: Span,
286+
},
287+
Nested {
288+
segments: Vec<String>,
289+
item: String,
290+
span: Span,
291+
},
242292
}
243293

244294
impl ModPath {
245295
fn span(&self) -> Span {
246296
match self {
247-
Self::Item { span, .. } => *span,
248-
Self::Nested { span, .. } => *span,
297+
Self::Item { span, .. } | Self::Nested { span, .. } => *span,
249298
}
250299
}
251300

252301
fn item(&self) -> &str {
253302
match self {
254-
Self::Item { item, .. } => item,
255-
Self::Nested { item, .. } => item,
303+
Self::Item { item, .. } | Self::Nested { item, .. } => item,
256304
}
257305
}
258306

259307
fn matches(&self, idx: usize, other: &ModPath) -> bool {
260308
match (self, other) {
261309
(Self::Item { item, .. }, Self::Item { item: other_item, .. }) => item == other_item,
262-
(Self::Nested { names, .. }, Self::Nested { names: other_names, .. }) => {
263-
match (names.get(idx), other_names.get(idx)) {
264-
(Some(seg), Some(other_seg)) => seg == other_seg,
265-
(_, _) => false,
266-
}
267-
}
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+
},
268319
(_, _) => false,
269320
}
270321
}
271322
}
272323

324+
#[allow(clippy::comparison_chain)]
273325
fn proccess_macro_path(span: Span, import: &str, import_map: &mut FxHashMap<String, Vec<ModPath>>) {
274326
let mut mod_path = import.split("::").collect::<Vec<_>>();
275327

276328
if mod_path.len() == 2 {
277-
let item_list = import_map.entry(mod_path[0].to_string())
278-
.or_insert(vec![]);
329+
let item_list = import_map.entry(mod_path[0].to_string()).or_insert_with(Vec::new);
279330

280331
if !item_list.iter().any(|mods| mods.item() == mod_path[1]) {
281-
item_list.push(ModPath::Item{
332+
item_list.push(ModPath::Item {
282333
item: mod_path[1].to_string(),
283334
span,
284335
});
@@ -288,46 +339,16 @@ fn proccess_macro_path(span: Span, import: &str, import_map: &mut FxHashMap<Stri
288339
let name = mod_path.remove(mod_path.len() - 1);
289340

290341
let nested = ModPath::Nested {
291-
names: mod_path.into_iter().map(ToString::to_string).collect(),
342+
segments: mod_path.into_iter().map(ToString::to_string).collect(),
292343
item: name.to_string(),
293344
span,
294345
};
295-
import_map.entry(first.to_string())
296-
.or_insert(vec![])
297-
.push(nested);
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);
298351
} else {
299352
unreachable!("test to see if code path hit TODO REMOVE")
300353
}
301354
}
302-
303-
fn recursive_path_push(module: ModPath, comma: &str, rest: &[ModPath], mut path: String) -> String {
304-
match &module {
305-
ModPath::Item { item, .. } => {
306-
path.push_str(&format!("{}{}", item, comma));
307-
}
308-
ModPath::Nested { names, item, span } => {
309-
let nested = rest.iter()
310-
// filter "self" out
311-
.filter(|other_m| other_m != &&module)
312-
// this matches the first path segment and filters non ModPath::Nested items
313-
.filter(|other_m| other_m.matches(0, &module))
314-
.collect::<Vec<_>>();
315-
316-
println!("{:#?}", nested);
317-
318-
if nested.is_empty() {
319-
path.push_str(&format!("{}::{}{}", names.join("::").to_string(), item, comma))
320-
} else {
321-
// use mod_a::{mod_b::{one, two}, mod_c::item, item1, item2}
322-
let mod_path = if names.len() - 1 > 0 {
323-
ModPath::Nested { names: names.clone(), item: item.to_string(), span: *span, }
324-
} else {
325-
ModPath::Item { item: names[0].to_string(), span: *span, }
326-
};
327-
let names = recursive_path_push(mod_path, comma, rest, path.to_string());
328-
// path.push_str(&format!("{}{}", item, comma));
329-
}
330-
}
331-
}
332-
path
333-
}

‎tests/ui/auxiliary/macro_use_helper.rs‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,13 @@ pub mod inner {
1313

1414
// RE-EXPORT
1515
// this will stick in `inner` module
16+
pub use macro_rules::foofoo;
1617
pub use macro_rules::try_err;
1718

19+
pub mod nested {
20+
pub use macro_rules::string_add;
21+
}
22+
1823
// ITEM
1924
#[macro_export]
2025
macro_rules! inner_mod_macro {

‎tests/ui/macro_use_imports.rs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ mod a {
1818
use mini_mac;
1919
#[macro_use]
2020
use mac::inner;
21+
#[macro_use]
22+
use mac::inner::nested;
2123

2224
#[derive(ClippyMiniMacroTest)]
2325
struct Test;
@@ -30,6 +32,8 @@ mod a {
3032
let v: ty_macro!() = Vec::default();
3133

3234
inner::try_err!();
35+
inner::foofoo!();
36+
nested::string_add!();
3337
}
3438
}
3539

‎tests/ui/macro_use_imports.stderr‎

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

99
error: `macro_use` attributes are no longer needed in the Rust 2018 edition
1010
--> $DIR/macro_use_imports.rs:15:5
1111
|
1212
LL | #[macro_use]
13-
| ^^^^^^^^^^^^ help: remove the attribute and import the macro directly, try: `use mac::inner_mod_macro`
14-
15-
error: `macro_use` attributes are no longer needed in the Rust 2018 edition
16-
--> $DIR/macro_use_imports.rs:15:5
17-
|
18-
LL | #[macro_use]
19-
| ^^^^^^^^^^^^ help: remove the attribute and import the macro directly, try: `use mac::function_macro`
20-
21-
error: `macro_use` attributes are no longer needed in the Rust 2018 edition
22-
--> $DIR/macro_use_imports.rs:15:5
23-
|
24-
LL | #[macro_use]
25-
| ^^^^^^^^^^^^ help: remove the attribute and import the macro directly, try: `use mac::ty_macro`
26-
27-
error: `macro_use` attributes are no longer needed in the Rust 2018 edition
28-
--> $DIR/macro_use_imports.rs:15:5
29-
|
30-
LL | #[macro_use]
31-
| ^^^^^^^^^^^^ help: remove the attribute and import the macro directly, try: `use mac::pub_in_private_macro`
32-
33-
error: `macro_use` attributes are no longer needed in the Rust 2018 edition
34-
--> $DIR/macro_use_imports.rs:17:5
35-
|
36-
LL | #[macro_use]
37-
| ^^^^^^^^^^^^ help: remove the attribute and import the macro directly, try: `use mini_mac::ClippyMiniMacroTest`
38-
39-
error: `macro_use` attributes are no longer needed in the Rust 2018 edition
40-
--> $DIR/macro_use_imports.rs:19:5
41-
|
42-
LL | #[macro_use]
43-
| ^^^^^^^^^^^^ help: remove the attribute and import the macro directly, try: `use mac::inner::try_err`
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}};`
4414

45-
error: aborting due to 7 previous errors
15+
error: aborting due to 2 previous errors
4616

0 commit comments

Comments
 (0)