From 2dd5776b1110a24d9ceb1d1f89dd684d0e63fbf7 Mon Sep 17 00:00:00 2001 From: Daniel J Rollins Date: Sat, 5 Mar 2016 00:26:45 +0000 Subject: [PATCH 1/5] Add note if method is called on a function object Fixes issue #29124. If method is called on a function type a note is generated to suggest that the developer may have forgotten to call it. e.g. fn main() { let mut guess = String::new(); std::io::stdin.read_line(&mut guess); } will generate the note: note: called method on function type. did you mean `std::io::stdin().read_line(..)`? --- src/librustc_typeck/check/method/suggest.rs | 112 +++++++++++--------- 1 file changed, 59 insertions(+), 53 deletions(-) diff --git a/src/librustc_typeck/check/method/suggest.rs b/src/librustc_typeck/check/method/suggest.rs index cdf7e1bd33a..fa55da8b6ca 100644 --- a/src/librustc_typeck/check/method/suggest.rs +++ b/src/librustc_typeck/check/method/suggest.rs @@ -37,6 +37,42 @@ use std::cmp::Ordering; use super::{MethodError, NoMatchData, CandidateSource, impl_item, trait_item}; use super::probe::Mode; +fn is_fn_ty<'a, 'tcx>(ty: &Ty<'tcx>, fcx: &FnCtxt<'a, 'tcx>, span: Span) -> bool { + let cx = fcx.tcx(); + println!("{:?}", ty); + match ty.sty { + // Not all of these (e.g. unsafe fns) implement FnOnce + // so we look for these beforehand + ty::TyClosure(..) | ty::TyFnDef(..) | ty::TyFnPtr(_) => true, + // If it's not a simple function, look for things which implement FnOnce + _ => { + if let Ok(fn_once_trait_did) = + cx.lang_items.require(FnOnceTraitLangItem) { + let infcx = fcx.infcx(); + infcx.probe(|_| { + let fn_once_substs = + Substs::new_trait(vec![infcx.next_ty_var()], + Vec::new(), + ty); + let trait_ref = + ty::TraitRef::new(fn_once_trait_did, + cx.mk_substs(fn_once_substs)); + let poly_trait_ref = trait_ref.to_poly_trait_ref(); + let obligation = Obligation::misc(span, + fcx.body_id, + poly_trait_ref + .to_predicate()); + let mut selcx = SelectionContext::new(infcx); + + return selcx.evaluate_obligation(&obligation) + }) + } else { + false + } + } + } +} + pub fn report_error<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>, span: Span, rcvr_ty: Ty<'tcx>, @@ -79,65 +115,35 @@ pub fn report_error<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>, // snippet }; - macro_rules! span_stored_function { - () => { - err.span_note(span, - &format!("use `({0}.{1})(...)` if you meant to call \ - the function stored in the `{1}` field", - expr_string, item_name)); - } - } - - macro_rules! span_did_you_mean { - () => { - err.span_note(span, &format!("did you mean to write `{0}.{1}`?", - expr_string, item_name)); - } - } - - // Determine if the field can be used as a function in some way let field_ty = field.ty(cx, substs); - match field_ty.sty { - // Not all of these (e.g. unsafe fns) implement FnOnce - // so we look for these beforehand - ty::TyClosure(..) | ty::TyFnDef(..) | ty::TyFnPtr(_) => { - span_stored_function!(); - } - // If it's not a simple function, look for things which implement FnOnce - _ => { - if let Ok(fn_once_trait_did) = - cx.lang_items.require(FnOnceTraitLangItem) { - let infcx = fcx.infcx(); - infcx.probe(|_| { - let fn_once_substs = - Substs::new_trait(vec![infcx.next_ty_var()], - Vec::new(), - field_ty); - let trait_ref = - ty::TraitRef::new(fn_once_trait_did, - cx.mk_substs(fn_once_substs)); - let poly_trait_ref = trait_ref.to_poly_trait_ref(); - let obligation = Obligation::misc(span, - fcx.body_id, - poly_trait_ref - .to_predicate()); - let mut selcx = SelectionContext::new(infcx); - - if selcx.evaluate_obligation(&obligation) { - span_stored_function!(); - } else { - span_did_you_mean!(); - } - }); - } else { - span_did_you_mean!(); - } - } + if is_fn_ty(&field_ty, &fcx, span) { + err.span_note(span, + &format!("use `({0}.{1})(...)` if you meant to call \ + the function stored in the `{1}` field", + expr_string, item_name)); + } else { + err.span_note(span, &format!("did you mean to write `{0}.{1}`?", + expr_string, item_name)); } } } + if is_fn_ty(&rcvr_ty, &fcx, span) { + let expr_string = match rcvr_expr { + Some(expr) => match cx.sess.codemap().span_to_snippet(expr.span) { + Ok(expr_string) => expr_string, + _ => "s".into() + }, + _ => "s".into() + }; + err.fileline_note( + span, + &format!("method invoked on function type. did you \ + mean `{}().{}(...)`?", + expr_string, item_name)); + } + if !static_sources.is_empty() { err.fileline_note( span, From 5e3b36c100c298f64bb947ea8771f3f38f94fad8 Mon Sep 17 00:00:00 2001 From: Daniel J Rollins Date: Sat, 5 Mar 2016 17:17:36 +0000 Subject: [PATCH 2/5] Fix code review actions in PR #32053 Split `fileline_note` into a `file_line note` and `span_suggestion` as per @Manishearth's suggestions. Change nested `match`es to `if let`s. --- src/librustc_typeck/check/method/suggest.rs | 24 ++++++++++----------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/librustc_typeck/check/method/suggest.rs b/src/librustc_typeck/check/method/suggest.rs index fa55da8b6ca..3585f1e9c88 100644 --- a/src/librustc_typeck/check/method/suggest.rs +++ b/src/librustc_typeck/check/method/suggest.rs @@ -130,18 +130,18 @@ pub fn report_error<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>, } if is_fn_ty(&rcvr_ty, &fcx, span) { - let expr_string = match rcvr_expr { - Some(expr) => match cx.sess.codemap().span_to_snippet(expr.span) { - Ok(expr_string) => expr_string, - _ => "s".into() - }, - _ => "s".into() - }; - err.fileline_note( - span, - &format!("method invoked on function type. did you \ - mean `{}().{}(...)`?", - expr_string, item_name)); + if let Some(expr) = rcvr_expr { + if let Ok (expr_string) = cx.sess.codemap().span_to_snippet(expr.span) { + err.fileline_note( + expr.span, + &format!("{} is a function, perhaps you wish to call it?", + expr_string)); + err.span_suggestion(expr.span, + "try calling the base function:", + format!("{}()", + expr_string)); + } + } } if !static_sources.is_empty() { From fa0efa69c5df0164a5abcebfbb2434cc1f27ecb1 Mon Sep 17 00:00:00 2001 From: Daniel J Rollins Date: Sat, 5 Mar 2016 18:34:17 +0000 Subject: [PATCH 3/5] Add test for issue 29124 --- src/librustc_typeck/check/method/suggest.rs | 2 +- src/test/compile-fail/issue-29124.rs | 31 +++++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) create mode 100644 src/test/compile-fail/issue-29124.rs diff --git a/src/librustc_typeck/check/method/suggest.rs b/src/librustc_typeck/check/method/suggest.rs index 3585f1e9c88..f3724298fe5 100644 --- a/src/librustc_typeck/check/method/suggest.rs +++ b/src/librustc_typeck/check/method/suggest.rs @@ -134,7 +134,7 @@ pub fn report_error<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>, if let Ok (expr_string) = cx.sess.codemap().span_to_snippet(expr.span) { err.fileline_note( expr.span, - &format!("{} is a function, perhaps you wish to call it?", + &format!("{} is a function, perhaps you wish to call it", expr_string)); err.span_suggestion(expr.span, "try calling the base function:", diff --git a/src/test/compile-fail/issue-29124.rs b/src/test/compile-fail/issue-29124.rs new file mode 100644 index 00000000000..69c4dbd7f16 --- /dev/null +++ b/src/test/compile-fail/issue-29124.rs @@ -0,0 +1,31 @@ +// Copyright 2015 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +struct ret; +struct obj; + +impl obj { + fn func() -> ret { + ret + } +} + +fn func() -> ret { + ret +} + +fn main() { + obj::func.x(); + //~^ ERROR no method named `x` found for type `fn() -> ret {obj::func}` in the current scope + //~^^ NOTE obj::func is a function, perhaps you wish to call it + func.x(); + //~^ ERROR no method named `x` found for type `fn() -> ret {func}` in the current scope + //~^^ NOTE func is a function, perhaps you wish to call it +} From 234371216e0eb717ddc9cf00fb222885f4228798 Mon Sep 17 00:00:00 2001 From: Daniel J Rollins Date: Sat, 5 Mar 2016 19:50:34 +0000 Subject: [PATCH 4/5] Use last path segment for uncalled method note if span_to_segment fails PR: #32053 --- src/librustc_typeck/check/method/suggest.rs | 21 +++++++++++++++++---- src/test/compile-fail/issue-29124.rs | 4 ++-- 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/src/librustc_typeck/check/method/suggest.rs b/src/librustc_typeck/check/method/suggest.rs index f3724298fe5..44724ca26b1 100644 --- a/src/librustc_typeck/check/method/suggest.rs +++ b/src/librustc_typeck/check/method/suggest.rs @@ -25,11 +25,13 @@ use middle::subst::Substs; use middle::traits::{Obligation, SelectionContext}; use util::nodemap::{FnvHashSet}; + use syntax::ast; use syntax::codemap::Span; use syntax::errors::DiagnosticBuilder; use rustc_front::print::pprust; use rustc_front::hir; +use rustc_front::hir::Expr_; use std::cell; use std::cmp::Ordering; @@ -130,17 +132,28 @@ pub fn report_error<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>, } if is_fn_ty(&rcvr_ty, &fcx, span) { + macro_rules! report_function { + ($span:expr, $name:expr) => { + err.fileline_note( + $span, + &format!("{} is a function, perhaps you wish to call it", + $name)); + } + } + if let Some(expr) = rcvr_expr { if let Ok (expr_string) = cx.sess.codemap().span_to_snippet(expr.span) { - err.fileline_note( - expr.span, - &format!("{} is a function, perhaps you wish to call it", - expr_string)); + report_function!(expr.span, expr_string); err.span_suggestion(expr.span, "try calling the base function:", format!("{}()", expr_string)); } + else if let Expr_::ExprPath(_, path) = expr.node.clone() { + if let Some(segment) = path.segments.last() { + report_function!(expr.span, segment.identifier.name); + } + } } } diff --git a/src/test/compile-fail/issue-29124.rs b/src/test/compile-fail/issue-29124.rs index 69c4dbd7f16..a72dac0d5dd 100644 --- a/src/test/compile-fail/issue-29124.rs +++ b/src/test/compile-fail/issue-29124.rs @@ -22,10 +22,10 @@ fn func() -> ret { } fn main() { - obj::func.x(); + obj::func.x(); //~^ ERROR no method named `x` found for type `fn() -> ret {obj::func}` in the current scope //~^^ NOTE obj::func is a function, perhaps you wish to call it - func.x(); + func.x(); //~^ ERROR no method named `x` found for type `fn() -> ret {func}` in the current scope //~^^ NOTE func is a function, perhaps you wish to call it } From 88ad22998bc3cf22c28273756f48c169910bfad5 Mon Sep 17 00:00:00 2001 From: Daniel J Rollins Date: Sun, 6 Mar 2016 15:47:05 +0000 Subject: [PATCH 5/5] Add Help and Suggestion to issue-29124 tests --- src/test/compile-fail/issue-29124.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/test/compile-fail/issue-29124.rs b/src/test/compile-fail/issue-29124.rs index a72dac0d5dd..b3dc043f502 100644 --- a/src/test/compile-fail/issue-29124.rs +++ b/src/test/compile-fail/issue-29124.rs @@ -25,7 +25,11 @@ fn main() { obj::func.x(); //~^ ERROR no method named `x` found for type `fn() -> ret {obj::func}` in the current scope //~^^ NOTE obj::func is a function, perhaps you wish to call it + //~^^^ HELP try calling the base function: + //~| SUGGESTION obj::func().x(); func.x(); //~^ ERROR no method named `x` found for type `fn() -> ret {func}` in the current scope //~^^ NOTE func is a function, perhaps you wish to call it + //~^^^ HELP try calling the base function: + //~| SUGGESTION func().x(); }