Skip to content

Commit 603d5a1

Browse files
borsbors
committed
Auto merge of rust-lang#13294 - WeiTheShinobi:new_lint_used_underscore_items, r=llogiq
Add new lint: `used_underscore_items` Closes rust-lang#13260 --- changelog: new [`used_underscore_items`] lint against using items with a single leading underscore
2 parents f194e68 + d40e04a commit 603d5a1

File tree

7 files changed

+333
-55
lines changed

7 files changed

+333
-55
lines changed

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6013,6 +6013,7 @@ Released 2018-09-13
60136013
[`use_debug`]: https://rust-lang.github.io/rust-clippy/master/index.html#use_debug
60146014
[`use_self`]: https://rust-lang.github.io/rust-clippy/master/index.html#use_self
60156015
[`used_underscore_binding`]: https://rust-lang.github.io/rust-clippy/master/index.html#used_underscore_binding
6016+
[`used_underscore_items`]: https://rust-lang.github.io/rust-clippy/master/index.html#used_underscore_items
60166017
[`useless_asref`]: https://rust-lang.github.io/rust-clippy/master/index.html#useless_asref
60176018
[`useless_attribute`]: https://rust-lang.github.io/rust-clippy/master/index.html#useless_attribute
60186019
[`useless_conversion`]: https://rust-lang.github.io/rust-clippy/master/index.html#useless_conversion

‎clippy_lints/src/declared_lints.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -486,6 +486,7 @@ pub static LINTS: &[&crate::LintInfo] = &[
486486
crate::misc::SHORT_CIRCUIT_STATEMENT_INFO,
487487
crate::misc::TOPLEVEL_REF_ARG_INFO,
488488
crate::misc::USED_UNDERSCORE_BINDING_INFO,
489+
crate::misc::USED_UNDERSCORE_ITEMS_INFO,
489490
crate::misc_early::BUILTIN_TYPE_SHADOW_INFO,
490491
crate::misc_early::DOUBLE_NEG_INFO,
491492
crate::misc_early::DUPLICATE_UNDERSCORE_ARGUMENT_INFO,

‎clippy_lints/src/misc.rs‎

Lines changed: 137 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ use clippy_utils::{
77
};
88
use rustc_errors::Applicability;
99
use rustc_hir::def::Res;
10+
use rustc_hir::def_id::LOCAL_CRATE;
1011
use rustc_hir::intravisit::FnKind;
1112
use rustc_hir::{
1213
BinOpKind, BindingMode, Body, ByRef, Expr, ExprKind, FnDecl, Mutability, PatKind, QPath, Stmt, StmtKind,
@@ -80,6 +81,45 @@ declare_clippy_lint! {
8081
"using a binding which is prefixed with an underscore"
8182
}
8283

84+
declare_clippy_lint! {
85+
/// ### What it does
86+
/// Checks for the use of item with a single leading
87+
/// underscore.
88+
///
89+
/// ### Why is this bad?
90+
/// A single leading underscore is usually used to indicate
91+
/// that a item will not be used. Using such a item breaks this
92+
/// expectation.
93+
///
94+
/// ### Example
95+
/// ```no_run
96+
/// fn _foo() {}
97+
///
98+
/// struct _FooStruct {}
99+
///
100+
/// fn main() {
101+
/// _foo();
102+
/// let _ = _FooStruct{};
103+
/// }
104+
/// ```
105+
///
106+
/// Use instead:
107+
/// ```no_run
108+
/// fn foo() {}
109+
///
110+
/// struct FooStruct {}
111+
///
112+
/// fn main() {
113+
/// foo();
114+
/// let _ = FooStruct{};
115+
/// }
116+
/// ```
117+
#[clippy::version = "pre 1.29.0"]
118+
pub USED_UNDERSCORE_ITEMS,
119+
pedantic,
120+
"using a item which is prefixed with an underscore"
121+
}
122+
83123
declare_clippy_lint! {
84124
/// ### What it does
85125
/// Checks for the use of short circuit boolean conditions as
@@ -104,6 +144,7 @@ declare_clippy_lint! {
104144
declare_lint_pass!(LintPass => [
105145
TOPLEVEL_REF_ARG,
106146
USED_UNDERSCORE_BINDING,
147+
USED_UNDERSCORE_ITEMS,
107148
SHORT_CIRCUIT_STATEMENT,
108149
]);
109150

@@ -205,51 +246,104 @@ impl<'tcx> LateLintPass<'tcx> for LintPass {
205246
{
206247
return;
207248
}
208-
let (definition_hir_id, ident) = match expr.kind {
209-
ExprKind::Path(ref qpath) => {
210-
if let QPath::Resolved(None, path) = qpath
211-
&& let Res::Local(id) = path.res
212-
&& is_used(cx, expr)
213-
{
214-
(id, last_path_segment(qpath).ident)
215-
} else {
216-
return;
217-
}
218-
},
219-
ExprKind::Field(recv, ident) => {
220-
if let Some(adt_def) = cx.typeck_results().expr_ty_adjusted(recv).ty_adt_def()
221-
&& let Some(field) = adt_def.all_fields().find(|field| field.name == ident.name)
222-
&& let Some(local_did) = field.did.as_local()
223-
&& !cx.tcx.type_of(field.did).skip_binder().is_phantom_data()
224-
{
225-
(cx.tcx.local_def_id_to_hir_id(local_did), ident)
226-
} else {
227-
return;
228-
}
249+
250+
used_underscore_binding(cx, expr);
251+
used_underscore_items(cx, expr);
252+
}
253+
}
254+
255+
fn used_underscore_items<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
256+
let (def_id, ident) = match expr.kind {
257+
ExprKind::Call(func, ..) => {
258+
if let ExprKind::Path(QPath::Resolved(.., path)) = func.kind
259+
&& let Some(last_segment) = path.segments.last()
260+
&& let Res::Def(_, def_id) = last_segment.res
261+
{
262+
(def_id, last_segment.ident)
263+
} else {
264+
return;
265+
}
266+
},
267+
ExprKind::MethodCall(path, ..) => {
268+
if let Some(def_id) = cx.typeck_results().type_dependent_def_id(expr.hir_id) {
269+
(def_id, path.ident)
270+
} else {
271+
return;
272+
}
273+
},
274+
ExprKind::Struct(QPath::Resolved(_, path), ..) => {
275+
if let Some(last_segment) = path.segments.last()
276+
&& let Res::Def(_, def_id) = last_segment.res
277+
{
278+
(def_id, last_segment.ident)
279+
} else {
280+
return;
281+
}
282+
},
283+
_ => return,
284+
};
285+
286+
let name = ident.name.as_str();
287+
let definition_span = cx.tcx.def_span(def_id);
288+
if name.starts_with('_')
289+
&& !name.starts_with("__")
290+
&& !definition_span.from_expansion()
291+
&& def_id.krate == LOCAL_CRATE
292+
{
293+
span_lint_and_then(
294+
cx,
295+
USED_UNDERSCORE_ITEMS,
296+
expr.span,
297+
"used underscore-prefixed item".to_string(),
298+
|diag| {
299+
diag.span_note(definition_span, "item is defined here".to_string());
229300
},
230-
_ => return,
231-
};
301+
);
302+
}
303+
}
232304

233-
let name = ident.name.as_str();
234-
if name.starts_with('_')
235-
&& !name.starts_with("__")
236-
&& let definition_span = cx.tcx.hir().span(definition_hir_id)
237-
&& !definition_span.from_expansion()
238-
&& !fulfill_or_allowed(cx, USED_UNDERSCORE_BINDING, [expr.hir_id, definition_hir_id])
239-
{
240-
span_lint_and_then(
241-
cx,
242-
USED_UNDERSCORE_BINDING,
243-
expr.span,
244-
format!(
245-
"used binding `{name}` which is prefixed with an underscore. A leading \
246-
underscore signals that a binding will not be used"
247-
),
248-
|diag| {
249-
diag.span_note(definition_span, format!("`{name}` is defined here"));
250-
},
251-
);
252-
}
305+
fn used_underscore_binding<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
306+
let (definition_hir_id, ident) = match expr.kind {
307+
ExprKind::Path(ref qpath) => {
308+
if let QPath::Resolved(None, path) = qpath
309+
&& let Res::Local(id) = path.res
310+
&& is_used(cx, expr)
311+
{
312+
(id, last_path_segment(qpath).ident)
313+
} else {
314+
return;
315+
}
316+
},
317+
ExprKind::Field(recv, ident) => {
318+
if let Some(adt_def) = cx.typeck_results().expr_ty_adjusted(recv).ty_adt_def()
319+
&& let Some(field) = adt_def.all_fields().find(|field| field.name == ident.name)
320+
&& let Some(local_did) = field.did.as_local()
321+
&& !cx.tcx.type_of(field.did).skip_binder().is_phantom_data()
322+
{
323+
(cx.tcx.local_def_id_to_hir_id(local_did), ident)
324+
} else {
325+
return;
326+
}
327+
},
328+
_ => return,
329+
};
330+
331+
let name = ident.name.as_str();
332+
if name.starts_with('_')
333+
&& !name.starts_with("__")
334+
&& let definition_span = cx.tcx.hir().span(definition_hir_id)
335+
&& !definition_span.from_expansion()
336+
&& !fulfill_or_allowed(cx, USED_UNDERSCORE_BINDING, [expr.hir_id, definition_hir_id])
337+
{
338+
span_lint_and_then(
339+
cx,
340+
USED_UNDERSCORE_BINDING,
341+
expr.span,
342+
"used underscore-prefixed binding".to_string(),
343+
|diag| {
344+
diag.span_note(definition_span, "binding is defined here".to_string());
345+
},
346+
);
253347
}
254348
}
255349

‎tests/ui/auxiliary/external_item.rs‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
pub struct _ExternalStruct {}
2+
3+
impl _ExternalStruct {
4+
pub fn _foo(self) {}
5+
}
6+
7+
pub fn _exernal_foo() {}

‎tests/ui/used_underscore_binding.stderr‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,72 +1,72 @@
1-
error: used binding `_foo` which is prefixed with an underscore. A leading underscore signals that a binding will not be used
1+
error: used underscore-prefixed binding
22
--> tests/ui/used_underscore_binding.rs:23:5
33
|
44
LL | _foo + 1
55
| ^^^^
66
|
7-
note: `_foo` is defined here
7+
note: binding is defined here
88
--> tests/ui/used_underscore_binding.rs:22:22
99
|
1010
LL | fn prefix_underscore(_foo: u32) -> u32 {
1111
| ^^^^
1212
= note: `-D clippy::used-underscore-binding` implied by `-D warnings`
1313
= help: to override `-D warnings` add `#[allow(clippy::used_underscore_binding)]`
1414

15-
error: used binding `_foo` which is prefixed with an underscore. A leading underscore signals that a binding will not be used
15+
error: used underscore-prefixed binding
1616
--> tests/ui/used_underscore_binding.rs:28:20
1717
|
1818
LL | println!("{}", _foo);
1919
| ^^^^
2020
|
21-
note: `_foo` is defined here
21+
note: binding is defined here
2222
--> tests/ui/used_underscore_binding.rs:27:24
2323
|
2424
LL | fn in_macro_or_desugar(_foo: u32) {
2525
| ^^^^
2626

27-
error: used binding `_foo` which is prefixed with an underscore. A leading underscore signals that a binding will not be used
27+
error: used underscore-prefixed binding
2828
--> tests/ui/used_underscore_binding.rs:29:16
2929
|
3030
LL | assert_eq!(_foo, _foo);
3131
| ^^^^
3232
|
33-
note: `_foo` is defined here
33+
note: binding is defined here
3434
--> tests/ui/used_underscore_binding.rs:27:24
3535
|
3636
LL | fn in_macro_or_desugar(_foo: u32) {
3737
| ^^^^
3838

39-
error: used binding `_foo` which is prefixed with an underscore. A leading underscore signals that a binding will not be used
39+
error: used underscore-prefixed binding
4040
--> tests/ui/used_underscore_binding.rs:29:22
4141
|
4242
LL | assert_eq!(_foo, _foo);
4343
| ^^^^
4444
|
45-
note: `_foo` is defined here
45+
note: binding is defined here
4646
--> tests/ui/used_underscore_binding.rs:27:24
4747
|
4848
LL | fn in_macro_or_desugar(_foo: u32) {
4949
| ^^^^
5050

51-
error: used binding `_underscore_field` which is prefixed with an underscore. A leading underscore signals that a binding will not be used
51+
error: used underscore-prefixed binding
5252
--> tests/ui/used_underscore_binding.rs:42:5
5353
|
5454
LL | s._underscore_field += 1;
5555
| ^^^^^^^^^^^^^^^^^^^
5656
|
57-
note: `_underscore_field` is defined here
57+
note: binding is defined here
5858
--> tests/ui/used_underscore_binding.rs:36:5
5959
|
6060
LL | _underscore_field: u32,
6161
| ^^^^^^^^^^^^^^^^^^^^^^
6262

63-
error: used binding `_i` which is prefixed with an underscore. A leading underscore signals that a binding will not be used
63+
error: used underscore-prefixed binding
6464
--> tests/ui/used_underscore_binding.rs:103:16
6565
|
6666
LL | uses_i(_i);
6767
| ^^
6868
|
69-
note: `_i` is defined here
69+
note: binding is defined here
7070
--> tests/ui/used_underscore_binding.rs:102:13
7171
|
7272
LL | let _i = 5;

‎tests/ui/used_underscore_items.rs‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
//@aux-build:external_item.rs
2+
#![allow(unused)]
3+
#![warn(clippy::used_underscore_items)]
4+
5+
extern crate external_item;
6+
7+
// should not lint macro
8+
macro_rules! macro_wrap_func {
9+
() => {
10+
fn _marco_foo() {}
11+
};
12+
}
13+
14+
macro_wrap_func!();
15+
16+
struct _FooStruct {}
17+
18+
impl _FooStruct {
19+
fn _method_call(self) {}
20+
}
21+
22+
fn _foo1() {}
23+
24+
fn _foo2() -> i32 {
25+
0
26+
}
27+
28+
mod a {
29+
pub mod b {
30+
pub mod c {
31+
pub fn _foo3() {}
32+
33+
pub struct _FooStruct2 {}
34+
35+
impl _FooStruct2 {
36+
pub fn _method_call(self) {}
37+
}
38+
}
39+
}
40+
}
41+
42+
fn main() {
43+
_foo1();
44+
let _ = _foo2();
45+
a::b::c::_foo3();
46+
let _ = &_FooStruct {};
47+
let _ = _FooStruct {};
48+
49+
let foo_struct = _FooStruct {};
50+
foo_struct._method_call();
51+
52+
let foo_struct2 = a::b::c::_FooStruct2 {};
53+
foo_struct2._method_call();
54+
}
55+
56+
// should not lint exteranl crate.
57+
// user cannot control how others name their items
58+
fn external_item_call() {
59+
let foo_struct3 = external_item::_ExternalStruct {};
60+
foo_struct3._foo();
61+
62+
external_item::_exernal_foo();
63+
}

0 commit comments

Comments
 (0)