Auto merge of #3570 - muth:master, r=phansch

panic at map_unit_fn.rs:202 for map() without args

map_unit_fn.rs accessed map() arguments before type check which ensures type is Option or Result.

Boiled it down to a simple test case.

FWIW: Found this panic when running clippy against code which used a gtk::Window's map() fn inherited from gtk::WidgetExt   http://gtk-rs.org/docs/gtk/trait.WidgetExt.html#tymethod.map

3 files changed, 21 insertions, 211 deletions

diff --git a/clippy_lints/src/map_unit_fn.rs b/clippy_lints/src/map_unit_fn.rs
index 39450deb84c..e5c17beb404 100644
--- a/clippy_lints/src/map_unit_fn.rs
+++ b/clippy_lints/src/map_unit_fn.rs
@@ -199,7 +199,6 @@ fn suggestion_msg(function_type: &str, map_type: &str) -> String {
 
 fn lint_map_unit_fn(cx: &LateContext<'_, '_>, stmt: &hir::Stmt, expr: &hir::Expr, map_args: &[hir::Expr]) {
     let var_arg = &map_args[0];
-    let fn_arg = &map_args[1];
 
     let (map_type, variant, lint) = if match_type(cx, cx.tables.expr_ty(var_arg), &paths::OPTION) {
         ("Option", "Some", OPTION_MAP_UNIT_FN)
@@ -208,6 +207,7 @@ fn lint_map_unit_fn(cx: &LateContext<'_, '_>, stmt: &hir::Stmt, expr: &hir::Expr
     } else {
         return;
     };
+    let fn_arg = &map_args[1];
 
     if is_unit_function(cx, fn_arg) {
         let msg = suggestion_msg("function", map_type);
diff --git a/tests/ui/map_unit_fn.rs b/tests/ui/map_unit_fn.rs
new file mode 100644
index 00000000000..1d203a147ba
--- /dev/null
+++ b/tests/ui/map_unit_fn.rs
@@ -0,0 +1,20 @@
+// Copyright 2014-2018 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+#![allow(unused)]
+struct Mappable {}
+
+impl Mappable {
+    pub fn map(&self) {}
+}
+
+fn main() {
+    let m = Mappable {};
+    m.map();
+}
diff --git a/tests/ui/map_unit_fn.stderr b/tests/ui/map_unit_fn.stderr
deleted file mode 100644
index c4ee0ce9238..00000000000
--- a/tests/ui/map_unit_fn.stderr
+++ /dev/null
@@ -1,210 +0,0 @@
-error: called `map(f)` on an Option value where `f` is a unit function
-  --> $DIR/map_unit_fn.rs:33:5
-   |
-33 |     x.field.map(do_nothing);
-   |     ^^^^^^^^^^^^^^^^^^^^^^^-
-   |     |
-   |     help: try this: `if let Some(x_field) = x.field { do_nothing(...) }`
-   |
-   = note: `-D option-map-unit-fn` implied by `-D warnings`
-
-error: called `map(f)` on an Option value where `f` is a unit function
-  --> $DIR/map_unit_fn.rs:35:5
-   |
-35 |     x.field.map(do_nothing);
-   |     ^^^^^^^^^^^^^^^^^^^^^^^-
-   |     |
-   |     help: try this: `if let Some(x_field) = x.field { do_nothing(...) }`
-
-error: called `map(f)` on an Option value where `f` is a unit function
-  --> $DIR/map_unit_fn.rs:37:5
-   |
-37 |     x.field.map(diverge);
-   |     ^^^^^^^^^^^^^^^^^^^^-
-   |     |
-   |     help: try this: `if let Some(x_field) = x.field { diverge(...) }`
-
-error: called `map(f)` on an Option value where `f` is a unit closure
-  --> $DIR/map_unit_fn.rs:43:5
-   |
-43 |     x.field.map(|value| x.do_option_nothing(value + captured));
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^-
-   |     |
-   |     help: try this: `if let Some(value) = x.field { x.do_option_nothing(value + captured) }`
-
-error: called `map(f)` on an Option value where `f` is a unit closure
-  --> $DIR/map_unit_fn.rs:45:5
-   |
-45 |     x.field.map(|value| { x.do_option_plus_one(value + captured); });
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^-
-   |     |
-   |     help: try this: `if let Some(value) = x.field { x.do_option_plus_one(value + captured); }`
-
-error: called `map(f)` on an Option value where `f` is a unit closure
-  --> $DIR/map_unit_fn.rs:48:5
-   |
-48 |     x.field.map(|value| do_nothing(value + captured));
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^-
-   |     |
-   |     help: try this: `if let Some(value) = x.field { do_nothing(value + captured) }`
-
-error: called `map(f)` on an Option value where `f` is a unit closure
-  --> $DIR/map_unit_fn.rs:50:5
-   |
-50 |     x.field.map(|value| { do_nothing(value + captured) });
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^-
-   |     |
-   |     help: try this: `if let Some(value) = x.field { do_nothing(value + captured) }`
-
-error: called `map(f)` on an Option value where `f` is a unit closure
-  --> $DIR/map_unit_fn.rs:52:5
-   |
-52 |     x.field.map(|value| { do_nothing(value + captured); });
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^-
-   |     |
-   |     help: try this: `if let Some(value) = x.field { do_nothing(value + captured); }`
-
-error: called `map(f)` on an Option value where `f` is a unit closure
-  --> $DIR/map_unit_fn.rs:54:5
-   |
-54 |     x.field.map(|value| { { do_nothing(value + captured); } });
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^-
-   |     |
-   |     help: try this: `if let Some(value) = x.field { do_nothing(value + captured); }`
-
-error: called `map(f)` on an Option value where `f` is a unit closure
-  --> $DIR/map_unit_fn.rs:57:5
-   |
-57 |     x.field.map(|value| diverge(value + captured));
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^-
-   |     |
-   |     help: try this: `if let Some(value) = x.field { diverge(value + captured) }`
-
-error: called `map(f)` on an Option value where `f` is a unit closure
-  --> $DIR/map_unit_fn.rs:59:5
-   |
-59 |     x.field.map(|value| { diverge(value + captured) });
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^-
-   |     |
-   |     help: try this: `if let Some(value) = x.field { diverge(value + captured) }`
-
-error: called `map(f)` on an Option value where `f` is a unit closure
-  --> $DIR/map_unit_fn.rs:61:5
-   |
-61 |     x.field.map(|value| { diverge(value + captured); });
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^-
-   |     |
-   |     help: try this: `if let Some(value) = x.field { diverge(value + captured); }`
-
-error: called `map(f)` on an Option value where `f` is a unit closure
-  --> $DIR/map_unit_fn.rs:63:5
-   |
-63 |     x.field.map(|value| { { diverge(value + captured); } });
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^-
-   |     |
-   |     help: try this: `if let Some(value) = x.field { diverge(value + captured); }`
-
-error: called `map(f)` on an Option value where `f` is a unit closure
-  --> $DIR/map_unit_fn.rs:68:5
-   |
-68 |     x.field.map(|value| { let y = plus_one(value + captured); });
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^-
-   |     |
-   |     help: try this: `if let Some(value) = x.field { let y = plus_one(value + captured); }`
-
-error: called `map(f)` on an Option value where `f` is a unit closure
-  --> $DIR/map_unit_fn.rs:70:5
-   |
-70 |     x.field.map(|value| { plus_one(value + captured); });
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^-
-   |     |
-   |     help: try this: `if let Some(value) = x.field { plus_one(value + captured); }`
-
-error: called `map(f)` on an Option value where `f` is a unit closure
-  --> $DIR/map_unit_fn.rs:72:5
-   |
-72 |     x.field.map(|value| { { plus_one(value + captured); } });
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^-
-   |     |
-   |     help: try this: `if let Some(value) = x.field { plus_one(value + captured); }`
-
-error: called `map(f)` on an Option value where `f` is a unit closure
-  --> $DIR/map_unit_fn.rs:75:5
-   |
-75 |     x.field.map(|ref value| { do_nothing(value + captured) });
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^-
-   |     |
-   |     help: try this: `if let Some(ref value) = x.field { do_nothing(value + captured) }`
-
-error: called `map(f)` on an Option value where `f` is a unit closure
-  --> $DIR/map_unit_fn.rs:78:5
-   |
-78 |     x.field.map(|value| { do_nothing(value); do_nothing(value) });
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^-
-   |     |
-   |     help: try this: `if let Some(value) = x.field { ... }`
-
-error: called `map(f)` on an Option value where `f` is a unit closure
-  --> $DIR/map_unit_fn.rs:80:5
-   |
-80 |     x.field.map(|value| if value > 0 { do_nothing(value); do_nothing(value) });
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^-
-   |     |
-   |     help: try this: `if let Some(value) = x.field { ... }`
-
-error: called `map(f)` on an Option value where `f` is a unit closure
-  --> $DIR/map_unit_fn.rs:84:5
-   |
-84 |        x.field.map(|value| {
-   |   _____^
-   |  |_____|
-   | ||
-85 | ||         do_nothing(value);
-86 | ||         do_nothing(value)
-87 | ||     });
-   | ||______^- help: try this: `if let Some(value) = x.field { ... }`
-   | |_______|
-   | 
-
-error: called `map(f)` on an Option value where `f` is a unit closure
-  --> $DIR/map_unit_fn.rs:88:5
-   |
-88 |     x.field.map(|value| { do_nothing(value); do_nothing(value); });
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^-
-   |     |
-   |     help: try this: `if let Some(value) = x.field { ... }`
-
-error: called `map(f)` on an Option value where `f` is a unit function
-  --> $DIR/map_unit_fn.rs:91:5
-   |
-91 |     Some(42).map(diverge);
-   |     ^^^^^^^^^^^^^^^^^^^^^-
-   |     |
-   |     help: try this: `if let Some(_) = Some(42) { diverge(...) }`
-
-error: called `map(f)` on an Option value where `f` is a unit function
-  --> $DIR/map_unit_fn.rs:92:5
-   |
-92 |     "12".parse::<i32>().ok().map(diverge);
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^-
-   |     |
-   |     help: try this: `if let Some(_) = "12".parse::<i32>().ok() { diverge(...) }`
-
-error: called `map(f)` on an Option value where `f` is a unit function
-  --> $DIR/map_unit_fn.rs:93:5
-   |
-93 |     Some(plus_one(1)).map(do_nothing);
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^-
-   |     |
-   |     help: try this: `if let Some(_) = Some(plus_one(1)) { do_nothing(...) }`
-
-error: called `map(f)` on an Option value where `f` is a unit function
-  --> $DIR/map_unit_fn.rs:97:5
-   |
-97 |     y.map(do_nothing);
-   |     ^^^^^^^^^^^^^^^^^-
-   |     |
-   |     help: try this: `if let Some(_y) = y { do_nothing(...) }`
-
-error: aborting due to 25 previous errors
-