Skip to content

Commit e237cbe

Browse files
flip1995flip1995
committed
Implement wildcard_imports lint
1 parent 49bcfc6 commit e237cbe

File tree

1 file changed

+268
-0
lines changed

1 file changed

+268
-0
lines changed

‎clippy_lints/src/wildcard_imports.rs

Lines changed: 268 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,268 @@
1+
use crate::utils::{in_macro, snippet_with_applicability, span_lint_and_sugg};
2+
use if_chain::if_chain;
3+
use rustc::ty::DefIdTree;
4+
use rustc_data_structures::fx::FxHashSet;
5+
use rustc_errors::Applicability;
6+
use rustc_hir::def_id::DefId;
7+
use rustc_hir::intravisit::{walk_item, NestedVisitorMap, Visitor};
8+
use rustc_hir::*;
9+
use rustc_lint::{LateContext, LateLintPass};
10+
use rustc_session::{declare_lint_pass, declare_tool_lint};
11+
use rustc_span::{symbol::Symbol, BytePos};
12+
13+
declare_clippy_lint! {
14+
/// **What it does:** Checks for wildcard imports `use _::*`.
15+
///
16+
/// **Why is this bad?** wildcard imports can polute the namespace. This is especially bad if
17+
/// the wildcard import shadows another import:
18+
///
19+
/// ```rust,ignore
20+
/// use crate1::foo; // Imports function named foo
21+
/// use crate2::*; // Has a function named foo and also imports it
22+
/// ```
23+
///
24+
/// This can lead to confusing error messages at best and to unexpected behavior at worst.
25+
///
26+
/// **Known problems:** If macros are imported through the wildcard, this macro is not included
27+
/// by the suggestion and has to be added by hand.
28+
///
29+
/// **Example:**
30+
///
31+
/// Bad:
32+
/// ```rust,ignore
33+
/// use crate1::*;
34+
///
35+
/// foo();
36+
/// ```
37+
///
38+
/// Good:
39+
/// ```rust,ignore
40+
/// use crate1::foo;
41+
///
42+
/// foo();
43+
/// ```
44+
pub WILDCARD_IMPORTS,
45+
pedantic,
46+
"lint `use _::*` statements"
47+
}
48+
49+
declare_lint_pass!(WildcardImports => [WILDCARD_IMPORTS]);
50+
51+
impl LateLintPass<'_, '_> for WildcardImports {
52+
fn check_item(&mut self, cx: &LateContext<'_, '_>, item: &Item<'_>) {
53+
if item.vis.node.is_pub() || item.vis.node.is_pub_restricted() {
54+
return;
55+
}
56+
if_chain! {
57+
if !in_macro(item.span);
58+
if let ItemKind::Use(use_path, UseKind::Glob) = &item.kind;
59+
if let Some(def_id) = use_path.res.opt_def_id();
60+
then {
61+
let hir = cx.tcx.hir();
62+
let parent_id = hir.get_parent_item(item.hir_id);
63+
let (items, in_module) = if parent_id == CRATE_HIR_ID {
64+
let items = hir
65+
.krate()
66+
.module
67+
.item_ids
68+
.iter()
69+
.map(|item_id| hir.get(item_id.id))
70+
.filter_map(|node| {
71+
if let Node::Item(item) = node {
72+
Some(item)
73+
} else {
74+
None
75+
}
76+
})
77+
.collect();
78+
(items, true)
79+
} else if let Node::Item(item) = hir.get(parent_id) {
80+
(vec![item], false)
81+
} else {
82+
(vec![], false)
83+
};
84+
85+
let mut import_used_visitor = ImportsUsedVisitor {
86+
cx,
87+
wildcard_def_id: def_id,
88+
in_module,
89+
used_imports: FxHashSet::default(),
90+
};
91+
for item in items {
92+
import_used_visitor.visit_item(item);
93+
}
94+
95+
if !import_used_visitor.used_imports.is_empty() {
96+
let module_name = use_path
97+
.segments
98+
.iter()
99+
.last()
100+
.expect("path has at least one segment")
101+
.ident
102+
.name;
103+
104+
let mut applicability = Applicability::MachineApplicable;
105+
let import_source = snippet_with_applicability(cx, use_path.span, "..", &mut applicability);
106+
let (span, braced_glob) = if import_source.is_empty() {
107+
// This is a `_::{_, *}` import
108+
// Probably it's `_::{self, *}`, in that case we don't want to suggest to
109+
// import `self`.
110+
// If it is something else, we also don't want to include `self` in the
111+
// suggestion, since either it was imported through another use statement:
112+
// ```
113+
// use foo::bar;
114+
// use foo::bar::{baz, *};
115+
// ```
116+
// or it couldn't be used anywhere.
117+
(
118+
use_path.span.with_hi(use_path.span.hi() + BytePos(1)),
119+
true,
120+
)
121+
} else {
122+
(
123+
use_path.span.with_hi(use_path.span.hi() + BytePos(3)),
124+
false,
125+
)
126+
};
127+
128+
let imports_string = if import_used_visitor.used_imports.len() == 1 {
129+
// We don't need to check for accidental suggesting the module name instead
130+
// of `self` here, since if `used_imports.len() == 1`, and the only usage
131+
// is `self`, then it's not through a `*` and if there is a `*`, it gets
132+
// already linted by `unused_imports` of rustc.
133+
import_used_visitor.used_imports.iter().next().unwrap().to_string()
134+
} else {
135+
let mut imports = import_used_visitor
136+
.used_imports
137+
.iter()
138+
.filter_map(|import_name| {
139+
if braced_glob && *import_name == module_name {
140+
None
141+
} else if *import_name == module_name {
142+
Some("self".to_string())
143+
} else {
144+
Some(import_name.to_string())
145+
}
146+
})
147+
.collect::<Vec<_>>();
148+
imports.sort();
149+
if braced_glob {
150+
imports.join(", ")
151+
} else {
152+
format!("{{{}}}", imports.join(", "))
153+
}
154+
};
155+
156+
let sugg = if import_source.is_empty() {
157+
imports_string
158+
} else {
159+
format!("{}::{}", import_source, imports_string)
160+
};
161+
162+
span_lint_and_sugg(
163+
cx,
164+
WILDCARD_IMPORTS,
165+
span,
166+
"usage of wildcard import",
167+
"try",
168+
sugg,
169+
applicability,
170+
);
171+
}
172+
}
173+
}
174+
}
175+
}
176+
177+
struct ImportsUsedVisitor<'a, 'tcx> {
178+
cx: &'a LateContext<'a, 'tcx>,
179+
wildcard_def_id: def_id::DefId,
180+
in_module: bool,
181+
used_imports: FxHashSet<Symbol>,
182+
}
183+
184+
impl<'a, 'tcx> Visitor<'tcx> for ImportsUsedVisitor<'a, 'tcx> {
185+
type Map = Map<'tcx>;
186+
187+
fn visit_item(&mut self, item: &'tcx Item<'_>) {
188+
match item.kind {
189+
ItemKind::Use(..) => {},
190+
ItemKind::Mod(..) if self.in_module => {},
191+
ItemKind::Mod(..) => self.in_module = true,
192+
_ => walk_item(self, item),
193+
}
194+
}
195+
196+
fn visit_path(&mut self, path: &Path<'_>, _: HirId) {
197+
if let Some(def_id) = self.first_path_segment_def_id(path) {
198+
// Check if the function/enum/... was exported
199+
if let Some(exports) = self.cx.tcx.module_exports(self.wildcard_def_id) {
200+
for export in exports {
201+
if let Some(export_def_id) = export.res.opt_def_id() {
202+
if export_def_id == def_id {
203+
self.used_imports.insert(
204+
path.segments
205+
.iter()
206+
.next()
207+
.expect("path has at least one segment")
208+
.ident
209+
.name,
210+
);
211+
return;
212+
}
213+
}
214+
}
215+
}
216+
217+
// Check if it is directly in the module
218+
if let Some(parent_def_id) = self.cx.tcx.parent(def_id) {
219+
if self.wildcard_def_id == parent_def_id {
220+
self.used_imports.insert(
221+
path.segments
222+
.iter()
223+
.next()
224+
.expect("path has at least one segment")
225+
.ident
226+
.name,
227+
);
228+
}
229+
}
230+
}
231+
}
232+
233+
fn nested_visit_map(&mut self) -> NestedVisitorMap<'_, Self::Map> {
234+
NestedVisitorMap::All(&self.cx.tcx.hir())
235+
}
236+
}
237+
238+
impl ImportsUsedVisitor<'_, '_> {
239+
fn skip_def_id(&self, def_id: DefId) -> DefId {
240+
let def_key = self.cx.tcx.def_key(def_id);
241+
match def_key.disambiguated_data.data {
242+
DefPathData::Ctor => {
243+
if let Some(def_id) = self.cx.tcx.parent(def_id) {
244+
self.skip_def_id(def_id)
245+
} else {
246+
def_id
247+
}
248+
},
249+
_ => def_id,
250+
}
251+
}
252+
253+
fn first_path_segment_def_id(&self, path: &Path<'_>) -> Option<DefId> {
254+
path.res.opt_def_id().and_then(|mut def_id| {
255+
def_id = self.skip_def_id(def_id);
256+
for _ in path.segments.iter().skip(1) {
257+
def_id = self.skip_def_id(def_id);
258+
if let Some(parent_def_id) = self.cx.tcx.parent(def_id) {
259+
def_id = parent_def_id;
260+
} else {
261+
return None;
262+
}
263+
}
264+
265+
Some(def_id)
266+
})
267+
}
268+
}

0 commit comments

Comments
 (0)