Skip to content

Commit 8bc889d

Browse files
committed
Add suggestion on type signatures
1 parent ef95ca1 commit 8bc889d

File tree

4 files changed

+82
-18
lines changed

4 files changed

+82
-18
lines changed

clippy_lints/src/unnecessary_wrap.rs

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use rustc_errors::Applicability;
77
use rustc_hir::intravisit::{FnKind, NestedVisitorMap, Visitor};
88
use rustc_hir::*;
99
use rustc_lint::{LateContext, LateLintPass};
10-
use rustc_middle::hir::map::Map;
10+
use rustc_middle::{hir::map::Map, ty::subst::GenericArgKind};
1111
use rustc_session::{declare_lint_pass, declare_tool_lint};
1212
use rustc_span::Span;
1313

@@ -100,14 +100,27 @@ impl<'tcx> LateLintPass<'tcx> for UnnecessaryWrap {
100100
return;
101101
}
102102

103-
let suggs = result.iter().filter_map(|expr| {
104-
let snippet = if let ExprKind::Call(_, ref args) = expr.kind {
105-
Some(snippet(cx, args[0].span, "..").to_string())
106-
} else {
107-
None
108-
};
109-
snippet.map(|snip| (expr.span, snip))
110-
});
103+
let suggs = result
104+
.iter()
105+
.filter_map(|expr| {
106+
let snippet = if let ExprKind::Call(_, ref args) = expr.kind {
107+
Some(snippet(cx, args[0].span, "..").to_string())
108+
} else {
109+
None
110+
};
111+
snippet.map(|snip| (expr.span, snip))
112+
})
113+
.chain({
114+
let inner_ty = return_ty(cx, hir_id)
115+
.walk()
116+
.skip(1) // skip `std::option::Option` or `std::result::Result`
117+
.take(1) // first outermost inner type is needed
118+
.filter_map(|inner| match inner.unpack() {
119+
GenericArgKind::Type(inner_ty) => Some(inner_ty.to_string()),
120+
_ => None,
121+
});
122+
inner_ty.map(|inner_ty| (fn_decl.output.span(), inner_ty))
123+
});
111124

112125
span_lint_and_then(
113126
cx,

tests/ui/unnecessary_wrap.fixed

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
// run-rustfix
2+
23
#![warn(clippy::unnecessary_wrap)]
34
#![allow(clippy::no_effect)]
45
#![allow(clippy::needless_return)]
56
#![allow(clippy::if_same_then_else)]
7+
#![allow(dead_code)]
68

79
// should be linted
810
fn func1(a: bool, b: bool) -> Option<i32> {
@@ -37,7 +39,21 @@ fn func3(a: bool) -> Option<i32> {
3739

3840
// should be linted
3941
fn func4() -> Option<i32> {
40-
1
42+
Some(1)
43+
}
44+
45+
// should be linted
46+
fn func5() -> Result<i32, ()> {
47+
Ok(1)
48+
}
49+
50+
// should not be linted
51+
fn func6(a: bool) -> Result<i32, ()> {
52+
if a {
53+
Ok(1)
54+
} else {
55+
Err(())
56+
}
4157
}
4258

4359
fn main() {

tests/ui/unnecessary_wrap.rs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
// run-rustfix
2+
23
#![warn(clippy::unnecessary_wrap)]
34
#![allow(clippy::no_effect)]
45
#![allow(clippy::needless_return)]
56
#![allow(clippy::if_same_then_else)]
7+
#![allow(dead_code)]
68

79
// should be linted
810
fn func1(a: bool, b: bool) -> Option<i32> {
@@ -40,6 +42,20 @@ fn func4() -> Option<i32> {
4042
Some(1)
4143
}
4244

45+
// should be linted
46+
fn func5() -> Result<i32, ()> {
47+
Ok(1)
48+
}
49+
50+
// should not be linted
51+
fn func6(a: bool) -> Result<i32, ()> {
52+
if a {
53+
Ok(1)
54+
} else {
55+
Err(())
56+
}
57+
}
58+
4359
fn main() {
4460
// method calls are not linted
4561
func1(true, true);

tests/ui/unnecessary_wrap.stderr

Lines changed: 27 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
error: this function unnecessarily wrapping data
2-
--> $DIR/unnecessary_wrap.rs:8:1
1+
error: this function returns unnecessarily wrapping data
2+
--> $DIR/unnecessary_wrap.rs:10:1
33
|
44
LL | / fn func1(a: bool, b: bool) -> Option<i32> {
55
LL | | if a && b {
@@ -13,22 +13,41 @@ LL | | }
1313
= note: `-D clippy::unnecessary-wrap` implied by `-D warnings`
1414
help: factor this out to
1515
|
16+
LL | fn func1(a: bool, b: bool) -> i32 {
17+
LL | if a && b {
1618
LL | return 42;
1719
LL | }
1820
LL | if a {
1921
LL | Some(-1);
20-
LL | 2
21-
LL | } else {
2222
...
2323

24-
error: this function unnecessarily wrapping data
25-
--> $DIR/unnecessary_wrap.rs:39:1
24+
error: this function returns unnecessarily wrapping data
25+
--> $DIR/unnecessary_wrap.rs:41:1
2626
|
2727
LL | / fn func4() -> Option<i32> {
2828
LL | | Some(1)
29-
| | ------- help: factor this out to: `1`
3029
LL | | }
3130
| |_^
31+
|
32+
help: factor this out to
33+
|
34+
LL | fn func4() -> i32 {
35+
LL | 1
36+
|
37+
38+
error: this function returns unnecessarily wrapping data
39+
--> $DIR/unnecessary_wrap.rs:46:1
40+
|
41+
LL | / fn func5() -> Result<i32, ()> {
42+
LL | | Ok(1)
43+
LL | | }
44+
| |_^
45+
|
46+
help: factor this out to
47+
|
48+
LL | fn func5() -> i32 {
49+
LL | 1
50+
|
3251

33-
error: aborting due to 2 previous errors
52+
error: aborting due to 3 previous errors
3453

0 commit comments

Comments
 (0)