Skip to content

Commit 8ffbf6f

Browse files
DevinR528DevinR528
committed
use hashset not map for keeping track of seen macro refs
remove stdout, fix clippy warnings, fmtcar
1 parent ede366b commit 8ffbf6f

File tree

5 files changed

+97
-100
lines changed

5 files changed

+97
-100
lines changed

‎clippy_lints/src/macro_use.rs

Lines changed: 48 additions & 83 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ use crate::utils::{in_macro, snippet, span_lint_and_sugg};
22
use hir::def::{DefKind, Res};
33
use if_chain::if_chain;
44
use rustc_ast::ast;
5-
use rustc_data_structures::fx::FxHashMap;
5+
use rustc_data_structures::fx::FxHashSet;
66
use rustc_errors::Applicability;
77
use rustc_hir as hir;
88
use rustc_lint::{LateContext, LateLintPass, LintContext};
@@ -29,7 +29,7 @@ declare_clippy_lint! {
2929

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

32-
/// MacroRefData includes the name of the macro
32+
/// `MacroRefData` includes the name of the macro
3333
/// and the path from `SourceMap::span_to_filename`.
3434
#[derive(Debug, Clone)]
3535
pub struct MacroRefData {
@@ -38,11 +38,11 @@ pub struct MacroRefData {
3838
}
3939

4040
impl MacroRefData {
41-
pub fn new(name: String, span: Span, ecx: &LateContext<'_, '_>) -> Self {
41+
pub fn new(name: &str, span: Span, ecx: &LateContext<'_, '_>) -> Self {
4242
let mut path = ecx.sess().source_map().span_to_filename(span).to_string();
4343

4444
// std lib paths are <::std::module::file type>
45-
// so remove brackets and space
45+
// so remove brackets, space and type.
4646
if path.contains('<') {
4747
path = path.replace(BRACKETS, "");
4848
}
@@ -57,13 +57,12 @@ impl MacroRefData {
5757
}
5858

5959
#[derive(Default)]
60+
#[allow(clippy::module_name_repetitions)]
6061
pub struct MacroUseImports {
6162
/// the actual import path used and the span of the attribute above it.
6263
imports: Vec<(String, Span)>,
63-
/// the span of the macro reference and the `MacroRefData`
64-
/// for the use of the macro.
65-
/// TODO make this FxHashSet<Span> to guard against inserting already found macros
66-
collected: FxHashMap<Span, MacroRefData>,
64+
/// the span of the macro reference, kept to ensure only one reference is used per macro call.
65+
collected: FxHashSet<Span>,
6766
mac_refs: Vec<(Span, MacroRefData)>,
6867
}
6968

@@ -72,34 +71,28 @@ impl_lint_pass!(MacroUseImports => [MACRO_USE_IMPORTS]);
7271
impl<'l, 'txc> LateLintPass<'l, 'txc> for MacroUseImports {
7372
fn check_item(&mut self, lcx: &LateContext<'_, '_>, item: &hir::Item<'_>) {
7473
if_chain! {
75-
if lcx.sess().opts.edition == Edition::Edition2018;
76-
if let hir::ItemKind::Use(path, _kind) = &item.kind;
77-
if let Some(mac_attr) = item
78-
.attrs
79-
.iter()
80-
.find(|attr| attr.ident().map(|s| s.to_string()) == Some("macro_use".to_string()));
81-
if let Res::Def(DefKind::Mod, id) = path.res;
82-
then {
83-
// println!("{:#?}", lcx.tcx.def_path_str(id));
84-
for kid in lcx.tcx.item_children(id).iter() {
85-
// println!("{:#?}", kid);
86-
if let Res::Def(DefKind::Macro(_mac_type), mac_id) = kid.res {
87-
let span = mac_attr.span.clone();
88-
89-
// println!("{:#?}", lcx.tcx.def_path_str(mac_id));
90-
91-
self.imports.push((lcx.tcx.def_path_str(mac_id), span));
74+
if lcx.sess().opts.edition == Edition::Edition2018;
75+
if let hir::ItemKind::Use(path, _kind) = &item.kind;
76+
if let Some(mac_attr) = item
77+
.attrs
78+
.iter()
79+
.find(|attr| attr.ident().map(|s| s.to_string()) == Some("macro_use".to_string()));
80+
if let Res::Def(DefKind::Mod, id) = path.res;
81+
then {
82+
for kid in lcx.tcx.item_children(id).iter() {
83+
if let Res::Def(DefKind::Macro(_mac_type), mac_id) = kid.res {
84+
let span = mac_attr.span;
85+
self.imports.push((lcx.tcx.def_path_str(mac_id), span));
86+
}
9287
}
93-
}
94-
} else {
88+
} else {
9589
if in_macro(item.span) {
9690
let call_site = item.span.source_callsite();
9791
let name = snippet(lcx, lcx.sess().source_map().span_until_char(call_site, '!'), "_");
9892
if let Some(callee) = item.span.source_callee() {
99-
if !self.collected.contains_key(&call_site) {
100-
let mac = MacroRefData::new(name.to_string(), callee.def_site, lcx);
101-
self.mac_refs.push((call_site, mac.clone()));
102-
self.collected.insert(call_site, mac);
93+
if !self.collected.contains(&call_site) {
94+
self.mac_refs.push((call_site, MacroRefData::new(&name, callee.def_site, lcx)));
95+
self.collected.insert(call_site);
10396
}
10497
}
10598
}
@@ -111,18 +104,16 @@ impl<'l, 'txc> LateLintPass<'l, 'txc> for MacroUseImports {
111104
let call_site = attr.span.source_callsite();
112105
let name = snippet(lcx, lcx.sess().source_map().span_until_char(call_site, '!'), "_");
113106
if let Some(callee) = attr.span.source_callee() {
114-
if !self.collected.contains_key(&call_site) {
115-
println!("{:?}\n{:#?}", call_site, attr);
116-
107+
if !self.collected.contains(&call_site) {
117108
let name = if name.contains("::") {
118109
name.split("::").last().unwrap().to_string()
119110
} else {
120111
name.to_string()
121112
};
122113

123-
let mac = MacroRefData::new(name, callee.def_site, lcx);
124-
self.mac_refs.push((call_site, mac.clone()));
125-
self.collected.insert(call_site, mac);
114+
self.mac_refs
115+
.push((call_site, MacroRefData::new(&name, callee.def_site, lcx)));
116+
self.collected.insert(call_site);
126117
}
127118
}
128119
}
@@ -132,16 +123,16 @@ impl<'l, 'txc> LateLintPass<'l, 'txc> for MacroUseImports {
132123
let call_site = expr.span.source_callsite();
133124
let name = snippet(lcx, lcx.sess().source_map().span_until_char(call_site, '!'), "_");
134125
if let Some(callee) = expr.span.source_callee() {
135-
if !self.collected.contains_key(&call_site) {
126+
if !self.collected.contains(&call_site) {
136127
let name = if name.contains("::") {
137128
name.split("::").last().unwrap().to_string()
138129
} else {
139130
name.to_string()
140131
};
141132

142-
let mac = MacroRefData::new(name, callee.def_site, lcx);
143-
self.mac_refs.push((call_site, mac.clone()));
144-
self.collected.insert(call_site, mac);
133+
self.mac_refs
134+
.push((call_site, MacroRefData::new(&name, callee.def_site, lcx)));
135+
self.collected.insert(call_site);
145136
}
146137
}
147138
}
@@ -151,16 +142,16 @@ impl<'l, 'txc> LateLintPass<'l, 'txc> for MacroUseImports {
151142
let call_site = stmt.span.source_callsite();
152143
let name = snippet(lcx, lcx.sess().source_map().span_until_char(call_site, '!'), "_");
153144
if let Some(callee) = stmt.span.source_callee() {
154-
if !self.collected.contains_key(&call_site) {
145+
if !self.collected.contains(&call_site) {
155146
let name = if name.contains("::") {
156147
name.split("::").last().unwrap().to_string()
157148
} else {
158149
name.to_string()
159150
};
160151

161-
let mac = MacroRefData::new(name, callee.def_site, lcx);
162-
self.mac_refs.push((call_site, mac.clone()));
163-
self.collected.insert(call_site, mac);
152+
self.mac_refs
153+
.push((call_site, MacroRefData::new(&name, callee.def_site, lcx)));
154+
self.collected.insert(call_site);
164155
}
165156
}
166157
}
@@ -170,10 +161,10 @@ impl<'l, 'txc> LateLintPass<'l, 'txc> for MacroUseImports {
170161
let call_site = pat.span.source_callsite();
171162
let name = snippet(lcx, lcx.sess().source_map().span_until_char(call_site, '!'), "_");
172163
if let Some(callee) = pat.span.source_callee() {
173-
if !self.collected.contains_key(&call_site) {
174-
let mac = MacroRefData::new(name.to_string(), callee.def_site, lcx);
175-
self.mac_refs.push((call_site, mac.clone()));
176-
self.collected.insert(call_site, mac);
164+
if !self.collected.contains(&call_site) {
165+
self.mac_refs
166+
.push((call_site, MacroRefData::new(&name, callee.def_site, lcx)));
167+
self.collected.insert(call_site);
177168
}
178169
}
179170
}
@@ -183,22 +174,18 @@ impl<'l, 'txc> LateLintPass<'l, 'txc> for MacroUseImports {
183174
let call_site = ty.span.source_callsite();
184175
let name = snippet(lcx, lcx.sess().source_map().span_until_char(call_site, '!'), "_");
185176
if let Some(callee) = ty.span.source_callee() {
186-
if !self.collected.contains_key(&call_site) {
187-
let mac = MacroRefData::new(name.to_string(), callee.def_site, lcx);
188-
self.mac_refs.push((call_site, mac.clone()));
189-
self.collected.insert(call_site, mac);
177+
if !self.collected.contains(&call_site) {
178+
self.mac_refs
179+
.push((call_site, MacroRefData::new(&name, callee.def_site, lcx)));
180+
self.collected.insert(call_site);
190181
}
191182
}
192183
}
193184
}
194185

195186
fn check_crate_post(&mut self, lcx: &LateContext<'_, '_>, _krate: &hir::Crate<'_>) {
196-
for (import, span) in self.imports.iter() {
197-
let matched = self
198-
.mac_refs
199-
.iter()
200-
.find(|(_span, mac)| import.ends_with(&mac.name))
201-
.is_some();
187+
for (import, span) in &self.imports {
188+
let matched = self.mac_refs.iter().any(|(_span, mac)| import.ends_with(&mac.name));
202189

203190
if matched {
204191
self.mac_refs.retain(|(_span, mac)| !import.ends_with(&mac.name));
@@ -218,30 +205,8 @@ impl<'l, 'txc> LateLintPass<'l, 'txc> for MacroUseImports {
218205
if !self.mac_refs.is_empty() {
219206
// TODO if not empty we found one we could not make a suggestion for
220207
// such as std::prelude::v1 or something else I haven't thought of.
221-
// println!("{:#?}", self.mac_refs);
208+
// If we defer the calling of span_lint_and_sugg we can make a decision about its
209+
// applicability?
222210
}
223211
}
224212
}
225-
226-
const PRELUDE: &[&str] = &[
227-
"marker", "ops", "convert", "iter", "option", "result", "borrow", "boxed", "string", "vec", "macros",
228-
];
229-
230-
/// This is somewhat of a fallback for imports from `std::prelude` because they
231-
/// are not recognized by `LateLintPass::check_item` `lcx.tcx.item_children(id)`
232-
fn make_path(mac: &MacroRefData, use_path: &str) -> String {
233-
let segs = mac.path.split("::").filter(|s| *s != "").collect::<Vec<_>>();
234-
235-
if segs.starts_with(&["std"]) && PRELUDE.iter().any(|m| segs.contains(m)) {
236-
return format!(
237-
"std::prelude::{} is imported by default, remove `use` statement",
238-
mac.name
239-
);
240-
}
241-
242-
if use_path.split("::").count() == 1 {
243-
return format!("{}::{}", use_path, mac.name);
244-
}
245-
246-
mac.path.clone()
247-
}

‎tests/ui/auxiliary/macro_use_helper.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ pub mod inner {
1717

1818
// ITEM
1919
#[macro_export]
20-
macro_rules! inner_mod {
20+
macro_rules! inner_mod_macro {
2121
() => {
2222
#[allow(dead_code)]
2323
pub struct Tardis;
@@ -27,7 +27,7 @@ pub mod inner {
2727

2828
// EXPR
2929
#[macro_export]
30-
macro_rules! function {
30+
macro_rules! function_macro {
3131
() => {
3232
if true {
3333
} else {
@@ -37,7 +37,7 @@ macro_rules! function {
3737

3838
// TYPE
3939
#[macro_export]
40-
macro_rules! ty_mac {
40+
macro_rules! ty_macro {
4141
() => {
4242
Vec<u8>
4343
};
@@ -46,7 +46,7 @@ macro_rules! ty_mac {
4646
mod extern_exports {
4747
pub(super) mod private_inner {
4848
#[macro_export]
49-
macro_rules! pub_in_private {
49+
macro_rules! pub_in_private_macro {
5050
($name:ident) => {
5151
let $name = String::from("secrets and lies");
5252
};

‎tests/ui/macro_use_import.stdout

Whitespace-only changes.

‎tests/ui/macro_use_imports.rs

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,6 @@ extern crate macro_use_helper as mac;
1212
extern crate clippy_mini_macro_test as mini_mac;
1313

1414
mod a {
15-
#[macro_use]
16-
use std::prelude;
1715
#[macro_use]
1816
use mac;
1917
#[macro_use]
@@ -26,15 +24,13 @@ mod a {
2624

2725
fn main() {
2826
pub_macro!();
29-
inner_mod!();
30-
pub_in_private!(_var);
31-
function!();
32-
let v: ty_mac!() = Vec::default();
27+
inner_mod_macro!();
28+
pub_in_private_macro!(_var);
29+
function_macro!();
30+
let v: ty_macro!() = Vec::default();
3331

3432
inner::try_err!();
3533
}
3634
}
3735

38-
fn main() {
39-
println!();
40-
}
36+
fn main() {}

‎tests/ui/macro_use_imports.stderr

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

9-
error: aborting due to previous error
9+
error: `macro_use` attributes are no longer needed in the Rust 2018 edition
10+
--> $DIR/macro_use_imports.rs:15:5
11+
|
12+
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`
44+
45+
error: aborting due to 7 previous errors
1046

0 commit comments

Comments
 (0)