Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Detect missing ; on methods with return type () #36409

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 18 additions & 5 deletions src/librustc_typeck/check/demand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,10 @@


use check::FnCtxt;
use rustc::ty::Ty;
use rustc::infer::{InferOk};
use rustc::infer::{InferOk, TypeTrace};
use rustc::traits::ObligationCause;
use rustc::ty::Ty;
use errors;

use syntax_pos::Span;
use rustc::hir;
Expand Down Expand Up @@ -51,13 +52,25 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
}
}

// Checks that the type of `expr` can be coerced to `expected`.
pub fn demand_coerce(&self, expr: &hir::Expr, checked_ty: Ty<'tcx>, expected: Ty<'tcx>) {
pub fn demand_coerce_diag(&self, expr: &hir::Expr, checked_ty: Ty<'tcx>, expected: Ty<'tcx>)
-> Option<errors::DiagnosticBuilder<'tcx>>
{
let expected = self.resolve_type_vars_with_obligations(expected);
if let Err(e) = self.try_coerce(expr, checked_ty, expected) {
let cause = self.misc(expr.span);
let expr_ty = self.resolve_type_vars_with_obligations(checked_ty);
self.report_mismatched_types(&cause, expected, expr_ty, e);
let trace = TypeTrace::types(&cause, true, expected, expr_ty);

Some(self.report_and_explain_type_error(trace, &e))
} else {
None
}
}

// Checks that the type of `expr` can be coerced to `expected`.
pub fn demand_coerce(&self, expr: &hir::Expr, checked_ty: Ty<'tcx>, expected: Ty<'tcx>) {
if let Some(mut err) = self.demand_coerce_diag(expr, checked_ty, expected) {
err.emit();
}
}
}
58 changes: 54 additions & 4 deletions src/librustc_typeck/check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ use rustc::hir::intravisit::{self, Visitor};
use rustc::hir::itemlikevisit::ItemLikeVisitor;
use rustc::hir::{self, PatKind};
use rustc::hir::print as pprust;
use rustc::hir::map::Node;
use rustc_back::slice;
use rustc_const_eval::eval_length;

Expand Down Expand Up @@ -3666,7 +3667,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
self.check_expr_closure(expr, capture, &decl, &body, expected)
}
hir::ExprBlock(ref b) => {
self.check_block_with_expected(&b, expected)
self.check_block_with_expected(&b, expected)
}
hir::ExprCall(ref callee, ref args) => {
self.check_call(expr, &callee, &args[..], expected)
Expand Down Expand Up @@ -4085,7 +4086,57 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
} else if let ExpectHasType(ety) = expected {
if let Some(ref e) = blk.expr {
// Coerce the tail expression to the right type.
self.demand_coerce(e, ty, ety);
if let Some(mut err) = self.demand_coerce_diag(e, ty, ety) {
// Be helpful when the user wrote `{... expr}` and
// adding a `;` is enough to fix the error.
if ety.is_nil() {
let span = Span {
lo: e.span.hi,
hi: e.span.hi + BytePos(1),
expn_id: e.span.expn_id
};
err.span_label(span, &"consider adding a semicolon here");
}

// Is the block part of a fn?
let parent = self.tcx.map.get(self.tcx.map.get_parent(blk.id));
let fn_decl = if let Node::NodeItem(&hir::Item {
name, node: hir::ItemFn(ref decl, ..), ..
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this is the only part I'm wary about anymore: doing it based solely on syntactical nesting is bound to be either too specific, or wrong. The only way I can see this working as intended is if ExpectHasType carried the extra information that it corresponds to a () implicit return type. cc @rust-lang/compiler

}) = parent {
// `fn main` must return `()`
if name.as_str() != "main" {
decl.clone().and_then(|decl| {
Some(decl)
})
} else {
None
}
} else if let Node::NodeTraitItem(&hir::TraitItem {
node: hir::TraitItem_::MethodTraitItem(hir::MethodSig {
ref decl, ..
}, ..), ..
}) = parent {
decl.clone().and_then(|decl| {
Some(decl)
})
} else {
// Do not recomend changing return type of `ImplItemKind::Method`
None
};

// Only recommend changing the return type for methods that
// haven't set a return type at all.
if let Some(hir::FnDecl {
output: hir::FunctionRetTy::DefaultReturn(span),
..
}) = fn_decl {
err.span_label(span,
&format!("possibly return type `{}` \
missing here",
ty));
}
err.emit();
}
} else {
// We're not diverging and there's an expected type, which,
// in case it's not `()`, could result in an error higher-up.
Expand Down Expand Up @@ -4118,9 +4169,8 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
hi: original_span.hi,
expn_id: original_span.expn_id
};
err.span_help(span_semi, "consider removing this semicolon:");
err.span_label(span_semi, &"consider removing this semicolon");
}

err.emit();
}
}
Expand Down
7 changes: 6 additions & 1 deletion src/test/compile-fail/block-must-not-have-result-do.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,11 @@

fn main() {
loop {
true //~ ERROR mismatched types
true
//~^ ERROR mismatched types
//~| NOTE: consider adding a semicolon here
//~| NOTE: expected (), found bool
//~| NOTE: expected type `()`
//~| NOTE: found type `bool`
}
}
7 changes: 6 additions & 1 deletion src/test/compile-fail/block-must-not-have-result-res.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,12 @@ struct r;

impl Drop for r {
fn drop(&mut self) {
true //~ ERROR mismatched types
true
//~^ ERROR: mismatched types
//~| NOTE: consider adding a semicolon here
//~| NOTE: expected (), found bool
//~| NOTE: expected type `()`
//~| NOTE: found type `bool`
}
}

Expand Down
10 changes: 6 additions & 4 deletions src/test/compile-fail/block-must-not-have-result-while.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,11 @@

fn main() {
while true {
true //~ ERROR mismatched types
//~| expected type `()`
//~| found type `bool`
//~| expected (), found bool
true
//~^ ERROR: mismatched types
//~| NOTE: consider adding a semicolon here
//~| NOTE: expected (), found bool
//~| NOTE: expected type `()`
//~| NOTE: found type `bool`
}
}
16 changes: 12 additions & 4 deletions src/test/compile-fail/consider-removing-last-semi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,22 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

fn f() -> String { //~ ERROR mismatched types
fn f() -> String {
//~^ ERROR mismatched types
//~| NOTE expected struct `std::string::String`, found ()
//~| NOTE expected type `std::string::String`
//~| NOTE found type `()`
0u8;
"bla".to_string(); //~ HELP consider removing this semicolon
"bla".to_string(); //~ NOTE consider removing this semicolon
}

fn g() -> String { //~ ERROR mismatched types
fn g() -> String {
//~^ ERROR mismatched types
//~| NOTE expected struct `std::string::String`, found ()
//~| NOTE expected type `std::string::String`
//~| NOTE found type `()`
"this won't work".to_string();
"removeme".to_string(); //~ HELP consider removing this semicolon
"removeme".to_string(); //~ NOTE consider removing this semicolon
}

fn main() {}
19 changes: 19 additions & 0 deletions src/test/compile-fail/expected-return-on-unit.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// Copyright 2016 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 <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.

// Test that we do some basic error correcton in the tokeniser (and don't spew
// too many bogus errors).

fn main() {
return 1;
//~^ mismatched types
//~| expected type `()`
//~| found type `{integer}`
}
8 changes: 6 additions & 2 deletions src/test/compile-fail/issue-11714.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,14 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

fn blah() -> i32 { //~ ERROR mismatched types
fn blah() -> i32 {
//~^ ERROR mismatched types
//~| NOTE expected i32, found ()
//~| NOTE expected type `i32`
//~| NOTE found type `()`
1

; //~ HELP consider removing this semicolon:
; //~ NOTE consider removing this semicolon
}

fn main() { }
16 changes: 12 additions & 4 deletions src/test/compile-fail/issue-13428.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,25 @@

// Regression test for #13428

fn foo() -> String { //~ ERROR mismatched types
fn foo() -> String {
//~^ ERROR mismatched types
//~| NOTE expected struct `std::string::String`, found ()
//~| NOTE expected type `std::string::String`
//~| NOTE found type `()`
format!("Hello {}",
"world")
// Put the trailing semicolon on its own line to test that the
// note message gets the offending semicolon exactly
; //~ HELP consider removing this semicolon
; //~ NOTE consider removing this semicolon
}

fn bar() -> String { //~ ERROR mismatched types
fn bar() -> String {
//~^ ERROR mismatched types
//~| NOTE expected struct `std::string::String`, found ()
//~| NOTE expected type `std::string::String`
//~| NOTE found type `()`
"foobar".to_string()
; //~ HELP consider removing this semicolon
; //~ NOTE consider removing this semicolon
}

pub fn main() {}
17 changes: 9 additions & 8 deletions src/test/compile-fail/issue-13624.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,11 @@ mod a {

pub fn get_enum_struct_variant() -> () {
Enum::EnumStructVariant { x: 1, y: 2, z: 3 }
//~^ ERROR mismatched types
//~| expected type `()`
//~| found type `a::Enum`
//~| expected (), found enum `a::Enum`
//~^ ERROR: mismatched types
//~| NOTE: consider adding a semicolon here
//~| NOTE: expected (), found enum `a::Enum`
//~| NOTE: expected type `()`
//~| NOTE: found type `a::Enum`
}
}

Expand All @@ -30,10 +31,10 @@ mod b {
let enum_struct_variant = ::a::get_enum_struct_variant();
match enum_struct_variant {
a::Enum::EnumStructVariant { x, y, z } => {
//~^ ERROR mismatched types
//~| expected type `()`
//~| found type `a::Enum`
//~| expected (), found enum `a::Enum`
//~^ ERROR: mismatched types
//~| NOTE: expected (), found enum `a::Enum`
//~| NOTE: expected type `()`
//~| NOTE: found type `a::Enum`
}
}
}
Expand Down
10 changes: 6 additions & 4 deletions src/test/compile-fail/issue-19109.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,13 @@
trait Trait { }

fn function(t: &mut Trait) {
//~^ NOTE possibly return type `*mut Trait` missing here
t as *mut Trait
//~^ ERROR: mismatched types
//~| NOTE: expected type `()`
//~| NOTE: found type `*mut Trait`
//~| NOTE: expected (), found *-ptr
//~^ ERROR mismatched types
//~| NOTE consider adding a semicolon here
//~| NOTE expected (), found *-ptr
//~| NOTE expected type `()`
//~| NOTE found type `*mut Trait`
}

fn main() { }
10 changes: 8 additions & 2 deletions src/test/compile-fail/issue-20862.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,17 @@
// except according to those terms.

fn foo(x: i32) {
//~^ NOTE possibly return type
|y| x + y
//~^ ERROR: mismatched types
//~^ ERROR mismatched types
//~| NOTE consider adding a semicolon here
//~| NOTE expected (), found closure
//~| NOTE expected type `()`
//~| NOTE found type
}


fn main() {
let x = foo(5)(2);
//~^ ERROR: expected function, found `()`
//~^ ERROR expected function, found `()`
}
6 changes: 6 additions & 0 deletions src/test/compile-fail/issue-22645.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,4 +24,10 @@ fn main() {
let b = Bob + 3.5;
b + 3 //~ ERROR E0277
//~^ ERROR: mismatched types
//~| NOTE consider adding a semicolon here
//~| NOTE expected (), found
//~| NOTE expected type `()`
//~| NOTE found type
//~| NOTE the trait `Scalar` is not implemented for `{integer}`
//~| NOTE required because of the requirements on the impl of `std::ops::Add<{integer}>`
}
6 changes: 5 additions & 1 deletion src/test/compile-fail/issue-3563.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,14 @@
// except according to those terms.

trait A {
fn a(&self) {
fn a(&self) { //~ possibly return type
|| self.b()
//~^ ERROR no method named `b` found for type `&Self` in the current scope
//~| ERROR mismatched types
//~| NOTE consider adding a semicolon here
//~| NOTE expected (), found closure
//~| NOTE expected type `()`
//~| NOTE found type
}
}
fn main() {}
6 changes: 3 additions & 3 deletions src/test/compile-fail/issue-5500.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
fn main() {
&panic!()
//~^ ERROR mismatched types
//~| expected type `()`
//~| found type `&_`
//~| expected (), found reference
//~| NOTE expected (), found reference
//~| NOTE expected type `()`
//~| NOTE found type `&_`
}
8 changes: 6 additions & 2 deletions src/test/compile-fail/issue-6458-4.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,12 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

fn foo(b: bool) -> Result<bool,String> { //~ ERROR mismatched types
Err("bar".to_string()); //~ HELP consider removing this semicolon
fn foo(b: bool) -> Result<bool,String> {
//~^ ERROR mismatched types
//~| NOTE expected enum `std::result::Result`, found ()
//~| NOTE expected type `std::result::Result<bool, std::string::String>`
//~| NOTE found type `()`
Err("bar".to_string()); //~ NOTE consider removing this semicolon
}

fn main() {
Expand Down
Loading