Skip to content

Commit cba75c4

Browse files
flip1995flip1995
committed
Implement wildcard_imports lint
1 parent 4adbd9f commit cba75c4

File tree

1 file changed

+270
-0
lines changed

1 file changed

+270
-0
lines changed

‎clippy_lints/src/wildcard_imports.rs

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

0 commit comments

Comments
 (0)