in which the E0618 "expected function" diagnostic gets a makeover

zackmdavis · zackmdavis · commit f3e9b1a70320 · 2018-11-10T22:12:33.000-08:00
Now the main span focuses on the erroneous not-a-function callee, while showing the entire call expression is relegated to a secondary span. In the case where the erroneous callee is itself a call, we point out the definition, and, if the call expression spans multiple lines, tentatively suggest a semicolon (because we suspect that the "outer" call is actually supposed to be a tuple). The new `bug!` assertion is, in fact, safe (`confirm_builtin_call` is only called by `check_call`, which is only called with a first arg of kind `ExprKind::Call` in `check_expr_kind`). Resolves #51055.
diff --git a/src/librustc_typeck/check/callee.rs b/src/librustc_typeck/check/callee.rs
@@ -218,35 +218,62 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
                     }
                 }
 
-                let mut err = type_error_struct!(
-                    self.tcx.sess,
-                    call_expr.span,
-                    callee_ty,
-                    E0618,
-                    "expected function, found {}",
-                    match unit_variant {
-                        Some(ref path) => format!("enum variant `{}`", path),
-                        None => format!("`{}`", callee_ty),
-                    });
-
-                err.span_label(call_expr.span, "not a function");
+                if let hir::ExprKind::Call(ref callee, _) = call_expr.node {
+                    let mut err = type_error_struct!(
+                        self.tcx.sess,
+                        callee.span,
+                        callee_ty,
+                        E0618,
+                        "expected function, found {}",
+                        match unit_variant {
+                            Some(ref path) => format!("enum variant `{}`", path),
+                            None => format!("`{}`", callee_ty),
+                        });
 
-                if let Some(ref path) = unit_variant {
-                    err.span_suggestion_with_applicability(
-                        call_expr.span,
-                        &format!("`{}` is a unit variant, you need to write it \
-                                  without the parenthesis", path),
-                        path.to_string(),
-                        Applicability::MachineApplicable
-                    );
-                }
+                    if let Some(ref path) = unit_variant {
+                        err.span_suggestion_with_applicability(
+                            call_expr.span,
+                            &format!("`{}` is a unit variant, you need to write it \
+                                      without the parenthesis", path),
+                            path.to_string(),
+                            Applicability::MachineApplicable
+                        );
+                    }
 
-                if let hir::ExprKind::Call(ref expr, _) = call_expr.node {
-                    let def = if let hir::ExprKind::Path(ref qpath) = expr.node {
-                        self.tables.borrow().qpath_def(qpath, expr.hir_id)
-                    } else {
-                        Def::Err
+                    let mut inner_callee_path = None;
+                    let def = match callee.node {
+                        hir::ExprKind::Path(ref qpath) => {
+                            self.tables.borrow().qpath_def(qpath, callee.hir_id)
+                        },
+                        hir::ExprKind::Call(ref inner_callee, _) => {
+                            // If the call spans more than one line and the callee kind is
+                            // itself another `ExprCall`, that's a clue that we might just be
+                            // missing a semicolon (Issue #51055)
+                            let call_is_multiline = self.tcx.sess.source_map()
+                                .is_multiline(call_expr.span);
+                            if call_is_multiline {
+                                let span = self.tcx.sess.source_map().next_point(callee.span);
+                                err.span_suggestion_with_applicability(
+                                    span,
+                                    "try adding a semicolon",
+                                    ";".to_owned(),
+                                    Applicability::MaybeIncorrect
+                                );
+                            }
+                            if let hir::ExprKind::Path(ref inner_qpath) = inner_callee.node {
+                                inner_callee_path = Some(inner_qpath);
+                                self.tables.borrow().qpath_def(inner_qpath, inner_callee.hir_id)
+                            } else {
+                                Def::Err
+                            }
+                        },
+                        _ => {
+                            Def::Err
+                        }
                     };
+
+                    err.span_label(call_expr.span, "call expression requires function");
+
                     let def_span = match def {
                         Def::Err => None,
                         Def::Local(id) | Def::Upvar(id, ..) => {
@@ -255,16 +282,20 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
                         _ => self.tcx.hir.span_if_local(def.def_id())
                     };
                     if let Some(span) = def_span {
-                        let name = match unit_variant {
-                            Some(path) => path,
-                            None => callee_ty.to_string(),
+                        let label = match (unit_variant, inner_callee_path) {
+                            (Some(path), _) => format!("`{}` defined here", path),
+                            (_, Some(hir::QPath::Resolved(_, path))) => format!(
+                                "`{}` defined here returns `{}`", path, callee_ty.to_string()
+                            ),
+                            _ => format!("`{}` defined here", callee_ty.to_string()),
                         };
-                        err.span_label(span, format!("`{}` defined here", name));
+                        err.span_label(span, label);
                     }
+                    err.emit();
+                } else {
+                    bug!("call_expr.node should be an ExprKind::Call, got {:?}", call_expr.node);
                 }
 
-                err.emit();
-
                 // This is the "default" function signature, used in case of error.
                 // In that case, we check each argument against "error" in order to
                 // set up all the node type bindings.
diff --git a/src/test/ui/block-result/issue-20862.stderr b/src/test/ui/block-result/issue-20862.stderr
@@ -12,8 +12,16 @@ LL |     |y| x + y
 error[E0618]: expected function, found `()`
   --> $DIR/issue-20862.rs:17:13
    |
-LL |     let x = foo(5)(2);
-   |             ^^^^^^^^^ not a function
+LL | / fn foo(x: i32) {
+LL | |     |y| x + y
+LL | | //~^ ERROR: mismatched types
+LL | | }
+   | |_- `foo` defined here returns `()`
+...
+LL |       let x = foo(5)(2);
+   |               ^^^^^^---
+   |               |
+   |               call expression requires function
 
 error: aborting due to 2 previous errors
 
diff --git a/src/test/ui/empty/empty-struct-unit-expr.stderr b/src/test/ui/empty/empty-struct-unit-expr.stderr
@@ -5,7 +5,9 @@ LL | struct Empty2;
    | -------------- `Empty2` defined here
 ...
 LL |     let e2 = Empty2(); //~ ERROR expected function, found `Empty2`
-   |              ^^^^^^^^ not a function
+   |              ^^^^^^--
+   |              |
+   |              call expression requires function
 
 error[E0618]: expected function, found enum variant `E::Empty4`
   --> $DIR/empty-struct-unit-expr.rs:26:14
@@ -14,7 +16,9 @@ LL |     Empty4
    |     ------ `E::Empty4` defined here
 ...
 LL |     let e4 = E::Empty4();
-   |              ^^^^^^^^^^^ not a function
+   |              ^^^^^^^^^--
+   |              |
+   |              call expression requires function
 help: `E::Empty4` is a unit variant, you need to write it without the parenthesis
    |
 LL |     let e4 = E::Empty4;
@@ -24,13 +28,17 @@ error[E0618]: expected function, found `empty_struct::XEmpty2`
   --> $DIR/empty-struct-unit-expr.rs:28:15
    |
 LL |     let xe2 = XEmpty2(); //~ ERROR expected function, found `empty_struct::XEmpty2`
-   |               ^^^^^^^^^ not a function
+   |               ^^^^^^^--
+   |               |
+   |               call expression requires function
 
 error[E0618]: expected function, found enum variant `XE::XEmpty4`
   --> $DIR/empty-struct-unit-expr.rs:29:15
    |
 LL |     let xe4 = XE::XEmpty4();
-   |               ^^^^^^^^^^^^^ not a function
+   |               ^^^^^^^^^^^--
+   |               |
+   |               call expression requires function
 help: `XE::XEmpty4` is a unit variant, you need to write it without the parenthesis
    |
 LL |     let xe4 = XE::XEmpty4;
diff --git a/src/test/ui/error-codes/E0618.stderr b/src/test/ui/error-codes/E0618.stderr
@@ -5,7 +5,9 @@ LL |     Entry,
    |     ----- `X::Entry` defined here
 ...
 LL |     X::Entry();
-   |     ^^^^^^^^^^ not a function
+   |     ^^^^^^^^--
+   |     |
+   |     call expression requires function
 help: `X::Entry` is a unit variant, you need to write it without the parenthesis
    |
 LL |     X::Entry;
@@ -17,7 +19,9 @@ error[E0618]: expected function, found `i32`
 LL |     let x = 0i32;
    |         - `i32` defined here
 LL |     x();
-   |     ^^^ not a function
+   |     ^--
+   |     |
+   |     call expression requires function
 
 error: aborting due to 2 previous errors
 
diff --git a/src/test/ui/issues/issue-10969.stderr b/src/test/ui/issues/issue-10969.stderr
@@ -4,15 +4,19 @@ error[E0618]: expected function, found `i32`
 LL | fn func(i: i32) {
    |         - `i32` defined here
 LL |     i(); //~ERROR expected function, found `i32`
-   |     ^^^ not a function
+   |     ^--
+   |     |
+   |     call expression requires function
 
 error[E0618]: expected function, found `i32`
   --> $DIR/issue-10969.rs:16:5
    |
 LL |     let i = 0i32;
    |         - `i32` defined here
 LL |     i(); //~ERROR expected function, found `i32`
-   |     ^^^ not a function
+   |     ^--
+   |     |
+   |     call expression requires function
 
 error: aborting due to 2 previous errors
 
diff --git a/src/test/ui/issues/issue-18532.stderr b/src/test/ui/issues/issue-18532.stderr
@@ -2,7 +2,9 @@ error[E0618]: expected function, found `!`
   --> $DIR/issue-18532.rs:16:5
    |
 LL |     (return)((),()); //~ ERROR expected function, found `!`
-   |     ^^^^^^^^^^^^^^^ not a function
+   |     ^^^^^^^^-------
+   |     |
+   |     call expression requires function
 
 error: aborting due to previous error
 
diff --git a/src/test/ui/issues/issue-20714.stderr b/src/test/ui/issues/issue-20714.stderr
@@ -5,7 +5,9 @@ LL | struct G;
    | --------- `G` defined here
 ...
 LL |     let g = G(); //~ ERROR: expected function, found `G`
-   |             ^^^ not a function
+   |             ^--
+   |             |
+   |             call expression requires function
 
 error: aborting due to previous error
 
diff --git a/src/test/ui/issues/issue-21701.stderr b/src/test/ui/issues/issue-21701.stderr
@@ -4,7 +4,9 @@ error[E0618]: expected function, found `U`
 LL | fn foo<U>(t: U) {
    |           - `U` defined here
 LL |     let y = t();
-   |             ^^^ not a function
+   |             ^--
+   |             |
+   |             call expression requires function
 
 error[E0618]: expected function, found `Bar`
   --> $DIR/issue-21701.rs:19:13
@@ -13,7 +15,9 @@ LL | struct Bar;
    | ----------- `Bar` defined here
 ...
 LL |     let f = Bar();
-   |             ^^^^^ not a function
+   |             ^^^--
+   |             |
+   |             call expression requires function
 
 error: aborting due to 2 previous errors
 
diff --git a/src/test/ui/issues/issue-22468.stderr b/src/test/ui/issues/issue-22468.stderr
@@ -4,7 +4,9 @@ error[E0618]: expected function, found `&str`
 LL |     let foo = "bar";
    |         --- `&str` defined here
 LL |     let x = foo("baz");
-   |             ^^^^^^^^^^ not a function
+   |             ^^^-------
+   |             |
+   |             call expression requires function
 
 error: aborting due to previous error
 
diff --git a/src/test/ui/issues/issue-26237.rs b/src/test/ui/issues/issue-26237.rs
@@ -11,13 +11,12 @@
 macro_rules! macro_panic {
     ($not_a_function:expr, $some_argument:ident) => {
         $not_a_function($some_argument)
-        //~^ ERROR expected function, found `{integer}`
     }
 }
 
 fn main() {
     let mut value_a = 0;
     let mut value_b = 0;
     macro_panic!(value_a, value_b);
-    //~^ in this expansion of macro_panic!
+    //~^ ERROR expected function, found `{integer}`
 }
diff --git a/src/test/ui/issues/issue-26237.stderr b/src/test/ui/issues/issue-26237.stderr
@@ -1,14 +1,14 @@
 error[E0618]: expected function, found `{integer}`
-  --> $DIR/issue-26237.rs:13:9
+  --> $DIR/issue-26237.rs:20:18
    |
 LL |         $not_a_function($some_argument)
-   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not a function
+   |         ------------------------------- call expression requires function
 ...
 LL |     let mut value_a = 0;
    |         ----------- `{integer}` defined here
 LL |     let mut value_b = 0;
 LL |     macro_panic!(value_a, value_b);
-   |     ------------------------------- in this macro invocation
+   |                  ^^^^^^^
 
 error: aborting due to previous error
 
diff --git a/src/test/ui/issues/issue-45965.stderr b/src/test/ui/issues/issue-45965.stderr
@@ -2,7 +2,9 @@ error[E0618]: expected function, found `{float}`
   --> $DIR/issue-45965.rs:12:30
    |
 LL |     let a = |r: f64| if r != 0.0(r != 0.0) { 1.0 } else { 0.0 };
-   |                              ^^^^^^^^^^^^^ not a function
+   |                              ^^^----------
+   |                              |
+   |                              call expression requires function
 
 error: aborting due to previous error
 
diff --git a/src/test/ui/issues/issue-46771.stderr b/src/test/ui/issues/issue-46771.stderr
@@ -4,7 +4,9 @@ error[E0618]: expected function, found `main::Foo`
 LL |     struct Foo;
    |     ----------- `main::Foo` defined here
 LL |     (1 .. 2).find(|_| Foo(0) == 0); //~ ERROR expected function, found `main::Foo`
-   |                       ^^^^^^ not a function
+   |                       ^^^---
+   |                       |
+   |                       call expression requires function
 
 error: aborting due to previous error
 
diff --git a/src/test/ui/issues/issue-5100.stderr b/src/test/ui/issues/issue-5100.stderr
@@ -47,9 +47,9 @@ error[E0618]: expected function, found `(char, char)`
   --> $DIR/issue-5100.rs:58:14
    |
 LL |       let v = [('a', 'b')   //~ ERROR expected function, found `(char, char)`
-   |  ______________^
+   |  ______________-^^^^^^^^^
 LL | |              ('c', 'd'),
-   | |_______________________^ not a function
+   | |_______________________- call expression requires function
 
 error[E0308]: mismatched types
   --> $DIR/issue-5100.rs:65:19
diff --git a/src/test/ui/parse-error-correct.stderr b/src/test/ui/parse-error-correct.stderr
@@ -17,7 +17,9 @@ LL |     let y = 42;
    |         - `{integer}` defined here
 LL |     let x = y.;  //~ ERROR unexpected token
 LL |     let x = y.();  //~ ERROR unexpected token
-   |             ^^^^ not a function
+   |             ^---
+   |             |
+   |             call expression requires function
 
 error[E0610]: `{integer}` is a primitive type and therefore doesn't have fields
   --> $DIR/parse-error-correct.rs:21:15
diff --git a/src/test/ui/resolve/privacy-enum-ctor.stderr b/src/test/ui/resolve/privacy-enum-ctor.stderr
@@ -171,7 +171,9 @@ LL |             Unit,
    |             ---- `Z::Unit` defined here
 ...
 LL |         let _ = Z::Unit();
-   |                 ^^^^^^^^^ not a function
+   |                 ^^^^^^^--
+   |                 |
+   |                 call expression requires function
 help: `Z::Unit` is a unit variant, you need to write it without the parenthesis
    |
 LL |         let _ = Z::Unit;
@@ -193,7 +195,9 @@ LL |         Unit,
    |         ---- `m::E::Unit` defined here
 ...
 LL |     let _: E = m::E::Unit();
-   |                ^^^^^^^^^^^^ not a function
+   |                ^^^^^^^^^^--
+   |                |
+   |                call expression requires function
 help: `m::E::Unit` is a unit variant, you need to write it without the parenthesis
    |
 LL |     let _: E = m::E::Unit;
@@ -215,7 +219,9 @@ LL |         Unit,
    |         ---- `E::Unit` defined here
 ...
 LL |     let _: E = E::Unit();
-   |                ^^^^^^^^^ not a function
+   |                ^^^^^^^--
+   |                |
+   |                call expression requires function
 help: `E::Unit` is a unit variant, you need to write it without the parenthesis
    |
 LL |     let _: E = E::Unit;
diff --git a/src/test/ui/suggestions/issue-51055-missing-semicolon-between-call-and-tuple.rs b/src/test/ui/suggestions/issue-51055-missing-semicolon-between-call-and-tuple.rs
@@ -0,0 +1,8 @@
+fn vindictive() -> bool { true }
+
+fn perfidy() -> (i32, i32) {
+    vindictive() //~ ERROR expected function, found `bool`
+    (1, 2)
+}
+
+fn main() {}
diff --git a/src/test/ui/suggestions/issue-51055-missing-semicolon-between-call-and-tuple.stderr b/src/test/ui/suggestions/issue-51055-missing-semicolon-between-call-and-tuple.stderr

Original file line number	Diff line number	Diff line change
@@ -2,7 +2,9 @@ error[E0618]: expected function, found `!`
`2`	`2`	`--> $DIR/issue-18532.rs:16:5`
`3`	`3`	`\|`
`4`	`4`	LL \| (return)((),()); //~ ERROR expected function, found `!`
`5`		`- \| ^^^^^^^^^^^^^^^ not a function`
	`5`	`+ \| ^^^^^^^^-------`
	`6`	`+ \| \|`
	`7`	`+ \| call expression requires function`
`6`	`8`
`7`	`9`	`error: aborting due to previous error`
`8`	`10`
Original file line number	Diff line number	Diff line change
`@@ -11,13 +11,12 @@`
`11`	`11`	`macro_rules! macro_panic {`
`12`	`12`	`($not_a_function:expr, $some_argument:ident) => {`
`13`	`13`	`$not_a_function($some_argument)`
`14`		- //~^ ERROR expected function, found `{integer}`
`15`	`14`	`}`
`16`	`15`	`}`
`17`	`16`
`18`	`17`	`fn main() {`
`19`	`18`	`let mut value_a = 0;`
`20`	`19`	`let mut value_b = 0;`
`21`	`20`	`macro_panic!(value_a, value_b);`
`22`		`- //~^ in this expansion of macro_panic!`
	`21`	+ //~^ ERROR expected function, found `{integer}`
`23`	`22`	`}`