Skip to content

Commit e6e3f24

Browse files
author
Michael Wright
committed
Fix #4019 
1 parent c44a882 commit e6e3f24

File tree

3 files changed

+80
-28
lines changed

3 files changed

+80
-28
lines changed

‎clippy_lints/src/methods/mod.rs‎

Lines changed: 50 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ use if_chain::if_chain;
99
use matches::matches;
1010
use rustc::hir;
1111
use rustc::hir::def::{DefKind, Res};
12+
use rustc::hir::intravisit::{self, Visitor};
1213
use rustc::lint::{in_external_macro, LateContext, LateLintPass, Lint, LintArray, LintContext, LintPass};
1314
use rustc::ty::{self, Predicate, Ty};
1415
use rustc::{declare_lint_pass, declare_tool_lint};
@@ -1044,7 +1045,49 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Methods {
10441045

10451046
/// Checks for the `OR_FUN_CALL` lint.
10461047
#[allow(clippy::too_many_lines)]
1047-
fn lint_or_fun_call(cx: &LateContext<'_, '_>, expr: &hir::Expr, method_span: Span, name: &str, args: &[hir::Expr]) {
1048+
fn lint_or_fun_call<'a, 'tcx: 'a>(
1049+
cx: &LateContext<'a, 'tcx>,
1050+
expr: &hir::Expr,
1051+
method_span: Span,
1052+
name: &str,
1053+
args: &'tcx [hir::Expr],
1054+
) {
1055+
// Searches an expression for method calls or function calls that aren't ctors
1056+
struct FunCallFinder<'a, 'tcx: 'a> {
1057+
cx: &'a LateContext<'a, 'tcx>,
1058+
found: bool,
1059+
}
1060+
1061+
impl<'a, 'tcx> intravisit::Visitor<'tcx> for FunCallFinder<'a, 'tcx> {
1062+
fn visit_expr(&mut self, expr: &'tcx hir::Expr) {
1063+
let found = match &expr.node {
1064+
hir::ExprKind::Call(..) => !is_ctor_function(self.cx, expr),
1065+
hir::ExprKind::MethodCall(..) => true,
1066+
_ => false,
1067+
};
1068+
1069+
if found {
1070+
let owner_def = self.cx.tcx.hir().get_parent_did_by_hir_id(expr.hir_id);
1071+
let promotable = self
1072+
.cx
1073+
.tcx
1074+
.rvalue_promotable_map(owner_def)
1075+
.contains(&expr.hir_id.local_id);
1076+
if !promotable {
1077+
self.found |= true;
1078+
}
1079+
}
1080+
1081+
if !self.found {
1082+
intravisit::walk_expr(self, expr);
1083+
}
1084+
}
1085+
1086+
fn nested_visit_map<'this>(&'this mut self) -> intravisit::NestedVisitorMap<'this, 'tcx> {
1087+
intravisit::NestedVisitorMap::None
1088+
}
1089+
}
1090+
10481091
/// Checks for `unwrap_or(T::new())` or `unwrap_or(T::default())`.
10491092
fn check_unwrap_or_default(
10501093
cx: &LateContext<'_, '_>,
@@ -1096,13 +1139,13 @@ fn lint_or_fun_call(cx: &LateContext<'_, '_>, expr: &hir::Expr, method_span: Spa
10961139

10971140
/// Checks for `*or(foo())`.
10981141
#[allow(clippy::too_many_arguments)]
1099-
fn check_general_case(
1100-
cx: &LateContext<'_, '_>,
1142+
fn check_general_case<'a, 'tcx: 'a>(
1143+
cx: &LateContext<'a, 'tcx>,
11011144
name: &str,
11021145
method_span: Span,
11031146
fun_span: Span,
11041147
self_expr: &hir::Expr,
1105-
arg: &hir::Expr,
1148+
arg: &'tcx hir::Expr,
11061149
or_has_args: bool,
11071150
span: Span,
11081151
) {
@@ -1120,14 +1163,9 @@ fn lint_or_fun_call(cx: &LateContext<'_, '_>, expr: &hir::Expr, method_span: Spa
11201163
}
11211164

11221165
// ignore enum and struct constructors
1123-
if is_ctor_function(cx, &arg) {
1124-
return;
1125-
}
1126-
1127-
// don't lint for constant values
1128-
let owner_def = cx.tcx.hir().get_parent_did_by_hir_id(arg.hir_id);
1129-
let promotable = cx.tcx.rvalue_promotable_map(owner_def).contains(&arg.hir_id.local_id);
1130-
if promotable {
1166+
let mut finder = FunCallFinder { cx: &cx, found: false };
1167+
finder.visit_expr(&arg);
1168+
if !finder.found {
11311169
return;
11321170
}
11331171

‎tests/ui/or_fun_call.rs‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
use std::collections::BTreeMap;
44
use std::collections::HashMap;
5+
use std::time::Duration;
56

67
/// Checks implementation of the `OR_FUN_CALL` lint.
78
fn or_fun_call() {
@@ -24,8 +25,8 @@ fn or_fun_call() {
2425
let with_enum = Some(Enum::A(1));
2526
with_enum.unwrap_or(Enum::A(5));
2627

27-
let with_const_fn = Some(::std::time::Duration::from_secs(1));
28-
with_const_fn.unwrap_or(::std::time::Duration::from_secs(5));
28+
let with_const_fn = Some(Duration::from_secs(1));
29+
with_const_fn.unwrap_or(Duration::from_secs(5));
2930

3031
let with_constructor = Some(vec![1]);
3132
with_constructor.unwrap_or(make());
@@ -71,6 +72,7 @@ fn or_fun_call() {
7172
}
7273

7374
struct Foo(u8);
75+
struct Bar(String, Duration);
7476
#[rustfmt::skip]
7577
fn test_or_with_ctors() {
7678
let opt = Some(1);
@@ -86,6 +88,12 @@ fn test_or_with_ctors() {
8688
let _ = opt.ok_or(Foo(two));
8789
let _ = opt.or(Some(2));
8890
let _ = opt.or(Some(two));
91+
92+
let _ = Some("a".to_string()).or(Some("b".to_string()));
93+
94+
let b = "b".to_string();
95+
let _ = Some(Bar("a".to_string(), Duration::from_secs(1)))
96+
.or(Some(Bar(b, Duration::from_secs(2))));
8997
}
9098

9199
fn main() {}

‎tests/ui/or_fun_call.stderr‎

Lines changed: 20 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,82 +1,88 @@
11
error: use of `unwrap_or` followed by a function call
2-
--> $DIR/or_fun_call.rs:31:22
2+
--> $DIR/or_fun_call.rs:32:22
33
|
44
LL | with_constructor.unwrap_or(make());
55
| ^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(make)`
66
|
77
= note: `-D clippy::or-fun-call` implied by `-D warnings`
88

99
error: use of `unwrap_or` followed by a call to `new`
10-
--> $DIR/or_fun_call.rs:34:5
10+
--> $DIR/or_fun_call.rs:35:5
1111
|
1212
LL | with_new.unwrap_or(Vec::new());
1313
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `with_new.unwrap_or_default()`
1414

1515
error: use of `unwrap_or` followed by a function call
16-
--> $DIR/or_fun_call.rs:37:21
16+
--> $DIR/or_fun_call.rs:38:21
1717
|
1818
LL | with_const_args.unwrap_or(Vec::with_capacity(12));
1919
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| Vec::with_capacity(12))`
2020

2121
error: use of `unwrap_or` followed by a function call
22-
--> $DIR/or_fun_call.rs:40:14
22+
--> $DIR/or_fun_call.rs:41:14
2323
|
2424
LL | with_err.unwrap_or(make());
2525
| ^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|_| make())`
2626

2727
error: use of `unwrap_or` followed by a function call
28-
--> $DIR/or_fun_call.rs:43:19
28+
--> $DIR/or_fun_call.rs:44:19
2929
|
3030
LL | with_err_args.unwrap_or(Vec::with_capacity(12));
3131
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|_| Vec::with_capacity(12))`
3232

3333
error: use of `unwrap_or` followed by a call to `default`
34-
--> $DIR/or_fun_call.rs:46:5
34+
--> $DIR/or_fun_call.rs:47:5
3535
|
3636
LL | with_default_trait.unwrap_or(Default::default());
3737
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `with_default_trait.unwrap_or_default()`
3838

3939
error: use of `unwrap_or` followed by a call to `default`
40-
--> $DIR/or_fun_call.rs:49:5
40+
--> $DIR/or_fun_call.rs:50:5
4141
|
4242
LL | with_default_type.unwrap_or(u64::default());
4343
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `with_default_type.unwrap_or_default()`
4444

4545
error: use of `unwrap_or` followed by a function call
46-
--> $DIR/or_fun_call.rs:52:14
46+
--> $DIR/or_fun_call.rs:53:14
4747
|
4848
LL | with_vec.unwrap_or(vec![]);
4949
| ^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| vec![])`
5050

5151
error: use of `unwrap_or` followed by a function call
52-
--> $DIR/or_fun_call.rs:57:21
52+
--> $DIR/or_fun_call.rs:58:21
5353
|
5454
LL | without_default.unwrap_or(Foo::new());
5555
| ^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(Foo::new)`
5656

5757
error: use of `or_insert` followed by a function call
58-
--> $DIR/or_fun_call.rs:60:19
58+
--> $DIR/or_fun_call.rs:61:19
5959
|
6060
LL | map.entry(42).or_insert(String::new());
6161
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `or_insert_with(String::new)`
6262

6363
error: use of `or_insert` followed by a function call
64-
--> $DIR/or_fun_call.rs:63:21
64+
--> $DIR/or_fun_call.rs:64:21
6565
|
6666
LL | btree.entry(42).or_insert(String::new());
6767
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `or_insert_with(String::new)`
6868

6969
error: use of `unwrap_or` followed by a function call
70-
--> $DIR/or_fun_call.rs:66:21
70+
--> $DIR/or_fun_call.rs:67:21
7171
|
7272
LL | let _ = stringy.unwrap_or("".to_owned());
7373
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| "".to_owned())`
7474

7575
error: use of `ok_or` followed by a function call
76-
--> $DIR/or_fun_call.rs:70:17
76+
--> $DIR/or_fun_call.rs:71:17
7777
|
7878
LL | let _ = opt.ok_or(format!("{} world.", hello));
7979
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `ok_or_else(|| format!("{} world.", hello))`
8080

81-
error: aborting due to 13 previous errors
81+
error: use of `or` followed by a function call
82+
--> $DIR/or_fun_call.rs:92:35
83+
|
84+
LL | let _ = Some("a".to_string()).or(Some("b".to_string()));
85+
| ^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `or_else(|| Some("b".to_string()))`
86+
87+
error: aborting due to 14 previous errors
8288

0 commit comments

Comments
 (0)