From b764b2a7aaef7af093f18aeed6e53a8287c05914 Mon Sep 17 00:00:00 2001 From: Andre Bogus <bogusandre@gmail.com> Date: Sun, 15 Jan 2017 00:31:20 +0100 Subject: [PATCH 1/3] extend or_fun_call lint to cover methods --- clippy_lints/src/methods.rs | 18 ++++++++++++------ tests/compile-fail/methods.rs | 7 ++++++- 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/clippy_lints/src/methods.rs b/clippy_lints/src/methods.rs index 23ceeec78e0..4d48ff45c92 100644 --- a/clippy_lints/src/methods.rs +++ b/clippy_lints/src/methods.rs @@ -724,7 +724,7 @@ fn lint_or_fun_call(cx: &LateContext, expr: &hir::Expr, name: &str, args: &[hir: fn check_general_case( cx: &LateContext, name: &str, - fun: &hir::Expr, + fun_span: Span, self_expr: &hir::Expr, arg: &hir::Expr, or_has_args: bool, @@ -765,7 +765,7 @@ fn lint_or_fun_call(cx: &LateContext, expr: &hir::Expr, name: &str, args: &[hir: let sugg: Cow<_> = match (fn_has_arguments, !or_has_args) { (true, _) => format!("|_| {}", snippet(cx, arg.span, "..")).into(), (false, false) => format!("|| {}", snippet(cx, arg.span, "..")).into(), - (false, true) => snippet(cx, fun.span, ".."), + (false, true) => snippet(cx, fun_span, ".."), }; span_lint_and_then(cx, @@ -780,11 +780,17 @@ fn lint_or_fun_call(cx: &LateContext, expr: &hir::Expr, name: &str, args: &[hir: } if args.len() == 2 { - if let hir::ExprCall(ref fun, ref or_args) = args[1].node { - let or_has_args = !or_args.is_empty(); - if !check_unwrap_or_default(cx, name, fun, &args[0], &args[1], or_has_args, expr.span) { - check_general_case(cx, name, fun, &args[0], &args[1], or_has_args, expr.span); + match args[1].node { + hir::ExprCall(ref fun, ref or_args) => { + let or_has_args = !or_args.is_empty(); + if !check_unwrap_or_default(cx, name, fun, &args[0], &args[1], or_has_args, expr.span) { + check_general_case(cx, name, fun.span, &args[0], &args[1], or_has_args, expr.span); + } } + hir::ExprMethodCall(fun, _, ref or_args) => { + check_general_case(cx, name, fun.span, &args[0], &args[1], !or_args.is_empty(), expr.span) + } + _ => {} } } } diff --git a/tests/compile-fail/methods.rs b/tests/compile-fail/methods.rs index 0f9a44b619f..cd04a3b6b97 100644 --- a/tests/compile-fail/methods.rs +++ b/tests/compile-fail/methods.rs @@ -127,7 +127,6 @@ fn option_methods() { ); // macro case let _ = opt_map!(opt, |x| x + 1).unwrap_or_else(|| 0); // should not lint - } /// Struct to generate false positives for things with .iter() @@ -340,6 +339,12 @@ fn or_fun_call() { //~^ERROR use of `or_insert` followed by a function call //~|HELP try this //~|SUGGESTION btree.entry(42).or_insert_with(String::new); + + let stringy = Some(String::from("")); + let _ = stringy.unwrap_or("".to_owned()); + //~^ERROR use of `unwrap_or` + //~|HELP try this + //~|SUGGESTION stringy.unwrap_or_else(|| "".to_owned()); } /// Checks implementation of `ITER_NTH` lint From dbde3b8e1c724580a450176f05cdd413ba535454 Mon Sep 17 00:00:00 2001 From: Andre Bogus <bogusandre@gmail.com> Date: Sun, 15 Jan 2017 00:33:29 +0100 Subject: [PATCH 2/3] dogfood fallout --- src/main.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main.rs b/src/main.rs index 0b91c79d21b..38610701808 100644 --- a/src/main.rs +++ b/src/main.rs @@ -234,7 +234,7 @@ pub fn main() { } else { option_env!("SYSROOT") .map(|s| s.to_owned()) - .or(Command::new("rustc") + .or_else(|| Command::new("rustc") .arg("--print") .arg("sysroot") .output() From 94c97d2ec9c2d6a88d3e9fabf3d9a9d640f8bd47 Mon Sep 17 00:00:00 2001 From: Andre Bogus <bogusandre@gmail.com> Date: Sun, 15 Jan 2017 01:03:46 +0100 Subject: [PATCH 3/3] formatting --- clippy_lints/src/methods.rs | 6 +++--- src/main.rs | 16 +++++++++------- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/clippy_lints/src/methods.rs b/clippy_lints/src/methods.rs index 4d48ff45c92..578fda8e791 100644 --- a/clippy_lints/src/methods.rs +++ b/clippy_lints/src/methods.rs @@ -786,11 +786,11 @@ fn lint_or_fun_call(cx: &LateContext, expr: &hir::Expr, name: &str, args: &[hir: if !check_unwrap_or_default(cx, name, fun, &args[0], &args[1], or_has_args, expr.span) { check_general_case(cx, name, fun.span, &args[0], &args[1], or_has_args, expr.span); } - } + }, hir::ExprMethodCall(fun, _, ref or_args) => { check_general_case(cx, name, fun.span, &args[0], &args[1], !or_args.is_empty(), expr.span) - } - _ => {} + }, + _ => {}, } } } diff --git a/src/main.rs b/src/main.rs index 38610701808..036adae8366 100644 --- a/src/main.rs +++ b/src/main.rs @@ -234,13 +234,15 @@ pub fn main() { } else { option_env!("SYSROOT") .map(|s| s.to_owned()) - .or_else(|| Command::new("rustc") - .arg("--print") - .arg("sysroot") - .output() - .ok() - .and_then(|out| String::from_utf8(out.stdout).ok()) - .map(|s| s.trim().to_owned())) + .or_else(|| { + Command::new("rustc") + .arg("--print") + .arg("sysroot") + .output() + .ok() + .and_then(|out| String::from_utf8(out.stdout).ok()) + .map(|s| s.trim().to_owned()) + }) .expect("need to specify SYSROOT env var during clippy compilation, or use rustup or multirust") };