Skip to content

Commit cdc96e1

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

File tree

1 file changed

+271
-0
lines changed

1 file changed

+271
-0
lines changed

‎clippy_lints/src/wildcard_imports.rs

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

0 commit comments

Comments
 (0)