Skip to content

Commit

Permalink
Auto merge of #51750 - zackmdavis:superstructure, r=oli-obk
Browse files Browse the repository at this point in the history
three diagnostics upgrades

 * reword `...` expression syntax error to not imply that you should use it in patterns either (#51043) and make it a structured suggestion
 * shorten the top-line message for the trivial-casts lint by tucking the advisory sentence into a help note
 * structured suggestion for pattern-named-the-same-as-variant warning

r? @oli-obk
  • Loading branch information
bors committed Jun 25, 2018
2 parents ecfe370 + 0b39a82 commit 8acec1f
Show file tree
Hide file tree
Showing 14 changed files with 218 additions and 56 deletions.
12 changes: 7 additions & 5 deletions src/librustc_mir/hair/pattern/check_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ use rustc::session::Session;
use rustc::ty::{self, Ty, TyCtxt};
use rustc::ty::subst::Substs;
use rustc::lint;
use rustc_errors::DiagnosticBuilder;
use rustc_errors::{Applicability, DiagnosticBuilder};
use rustc::util::common::ErrorReported;

use rustc::hir::def::*;
Expand Down Expand Up @@ -328,10 +328,12 @@ fn check_for_bindings_named_the_same_as_variants(cx: &MatchVisitor, pat: &Pat) {
"pattern binding `{}` is named the same as one \
of the variants of the type `{}`",
name.node, ty_path);
help!(err,
"if you meant to match on a variant, \
consider making the path in the pattern qualified: `{}::{}`",
ty_path, name.node);
err.span_suggestion_with_applicability(
p.span,
"to match on the variant, qualify the path",
format!("{}::{}", ty_path, name.node),
Applicability::MachineApplicable
);
err.emit();
}
}
Expand Down
41 changes: 20 additions & 21 deletions src/librustc_typeck/check/cast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -365,28 +365,27 @@ impl<'a, 'gcx, 'tcx> CastCheck<'tcx> {
fn trivial_cast_lint(&self, fcx: &FnCtxt<'a, 'gcx, 'tcx>) {
let t_cast = self.cast_ty;
let t_expr = self.expr_ty;
if t_cast.is_numeric() && t_expr.is_numeric() {
fcx.tcx.lint_node(
lint::builtin::TRIVIAL_NUMERIC_CASTS,
self.expr.id,
self.span,
&format!("trivial numeric cast: `{}` as `{}`. Cast can be \
replaced by coercion, this might require type \
ascription or a temporary variable",
fcx.ty_to_string(t_expr),
fcx.ty_to_string(t_cast)));
let type_asc_or = if fcx.tcx.features().type_ascription {
"type ascription or "
} else {
fcx.tcx.lint_node(
lint::builtin::TRIVIAL_CASTS,
self.expr.id,
self.span,
&format!("trivial cast: `{}` as `{}`. Cast can be \
replaced by coercion, this might require type \
ascription or a temporary variable",
fcx.ty_to_string(t_expr),
fcx.ty_to_string(t_cast)));
}

""
};
let (adjective, lint) = if t_cast.is_numeric() && t_expr.is_numeric() {
("numeric ", lint::builtin::TRIVIAL_NUMERIC_CASTS)
} else {
("", lint::builtin::TRIVIAL_CASTS)
};
let mut err = fcx.tcx.struct_span_lint_node(
lint,
self.expr.id,
self.span,
&format!("trivial {}cast: `{}` as `{}`",
adjective,
fcx.ty_to_string(t_expr),
fcx.ty_to_string(t_cast)));
err.help(&format!("cast can be replaced by coercion; this might \
require {}a temporary variable", type_asc_or));
err.emit();
}

pub fn check(mut self, fcx: &FnCtxt<'a, 'gcx, 'tcx>) {
Expand Down
14 changes: 8 additions & 6 deletions src/libsyntax/parse/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4800,12 +4800,14 @@ impl<'a> Parser<'a> {

fn err_dotdotdot_syntax(&self, span: Span) {
self.diagnostic().struct_span_err(span, {
"`...` syntax cannot be used in expressions"
}).help({
"Use `..` if you need an exclusive range (a < b)"
}).help({
"or `..=` if you need an inclusive range (a <= b)"
}).emit();
"unexpected token: `...`"
}).span_suggestion_with_applicability(
span, "use `..` for an exclusive range", "..".to_owned(),
Applicability::MaybeIncorrect
).span_suggestion_with_applicability(
span, "or `..=` for an inclusive range", "..=".to_owned(),
Applicability::MaybeIncorrect
).emit();
}

// Parse bounds of a type parameter `BOUND + BOUND + BOUND`, possibly with trailing `+`.
Expand Down
25 changes: 12 additions & 13 deletions src/test/parse-fail/range_inclusive_dotdotdot.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,22 +15,21 @@
use std::ops::RangeToInclusive;

fn return_range_to() -> RangeToInclusive<i32> {
return ...1; //~ERROR `...` syntax cannot be used in expressions
//~^HELP Use `..` if you need an exclusive range (a < b)
//~^^HELP or `..=` if you need an inclusive range (a <= b)
return ...1; //~ERROR unexpected token: `...`
//~^HELP use `..` for an exclusive range
//~^^HELP or `..=` for an inclusive range
}

pub fn main() {
let x = ...0; //~ERROR `...` syntax cannot be used in expressions
//~^HELP Use `..` if you need an exclusive range (a < b)
//~^^HELP or `..=` if you need an inclusive range (a <= b)
let x = ...0; //~ERROR unexpected token: `...`
//~^HELP use `..` for an exclusive range
//~^^HELP or `..=` for an inclusive range

let x = 5...5; //~ERROR `...` syntax cannot be used in expressions
//~^HELP Use `..` if you need an exclusive range (a < b)
//~^^HELP or `..=` if you need an inclusive range (a <= b)
let x = 5...5; //~ERROR unexpected token: `...`
//~^HELP use `..` for an exclusive range
//~^^HELP or `..=` for an inclusive range

for _ in 0...1 {} //~ERROR `...` syntax cannot be used in expressions
//~^HELP Use `..` if you need an exclusive range (a < b)
//~^^HELP or `..=` if you need an inclusive range (a <= b)
for _ in 0...1 {} //~ERROR unexpected token: `...`
//~^HELP use `..` for an exclusive range
//~^^HELP or `..=` for an inclusive range
}

40 changes: 40 additions & 0 deletions src/test/ui/issue-19100.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
// Copyright 2014 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.

// run-pass
// run-rustfix

#![allow(non_snake_case)]
#![allow(dead_code)]
#![allow(unused_variables)]

#[derive(Copy, Clone)]
enum Foo {
Bar,
Baz
}

impl Foo {
fn foo(&self) {
match self {
&
Foo::Bar if true
//~^ WARN pattern binding `Bar` is named the same as one of the variants of the type `Foo`
=> println!("bar"),
&
Foo::Baz if false
//~^ WARN pattern binding `Baz` is named the same as one of the variants of the type `Foo`
=> println!("baz"),
_ => ()
}
}
}

fn main() {}
1 change: 1 addition & 0 deletions src/test/ui/issue-19100.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
// except according to those terms.

// run-pass
// run-rustfix

#![allow(non_snake_case)]
#![allow(dead_code)]
Expand Down
12 changes: 4 additions & 8 deletions src/test/ui/issue-19100.stderr
Original file line number Diff line number Diff line change
@@ -1,16 +1,12 @@
warning[E0170]: pattern binding `Bar` is named the same as one of the variants of the type `Foo`
--> $DIR/issue-19100.rs:27:1
--> $DIR/issue-19100.rs:28:1
|
LL | Bar if true
| ^^^
|
= help: if you meant to match on a variant, consider making the path in the pattern qualified: `Foo::Bar`
| ^^^ help: to match on the variant, qualify the path: `Foo::Bar`

warning[E0170]: pattern binding `Baz` is named the same as one of the variants of the type `Foo`
--> $DIR/issue-19100.rs:31:1
--> $DIR/issue-19100.rs:32:1
|
LL | Baz if false
| ^^^
|
= help: if you meant to match on a variant, consider making the path in the pattern qualified: `Foo::Baz`
| ^^^ help: to match on the variant, qualify the path: `Foo::Baz`

4 changes: 1 addition & 3 deletions src/test/ui/issue-30302.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,7 @@ warning[E0170]: pattern binding `Nil` is named the same as one of the variants o
--> $DIR/issue-30302.rs:23:9
|
LL | Nil => true,
| ^^^
|
= help: if you meant to match on a variant, consider making the path in the pattern qualified: `Stack::Nil`
| ^^^ help: to match on the variant, qualify the path: `Stack::Nil`

error: unreachable pattern
--> $DIR/issue-30302.rs:25:9
Expand Down
20 changes: 20 additions & 0 deletions src/test/ui/lint/trivial-casts-featuring-type-ascription.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// Copyright 2018 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.

#![deny(trivial_casts, trivial_numeric_casts)]
#![feature(type_ascription)]

fn main() {
let lugubrious = 12i32 as i32;
//~^ ERROR trivial numeric cast
let haunted: &u32 = &99;
let _ = haunted as *const u32;
//~^ ERROR trivial cast
}
28 changes: 28 additions & 0 deletions src/test/ui/lint/trivial-casts-featuring-type-ascription.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
error: trivial numeric cast: `i32` as `i32`
--> $DIR/trivial-casts-featuring-type-ascription.rs:15:22
|
LL | let lugubrious = 12i32 as i32;
| ^^^^^^^^^^^^
|
note: lint level defined here
--> $DIR/trivial-casts-featuring-type-ascription.rs:11:24
|
LL | #![deny(trivial_casts, trivial_numeric_casts)]
| ^^^^^^^^^^^^^^^^^^^^^
= help: cast can be replaced by coercion; this might require type ascription or a temporary variable

error: trivial cast: `&u32` as `*const u32`
--> $DIR/trivial-casts-featuring-type-ascription.rs:18:13
|
LL | let _ = haunted as *const u32;
| ^^^^^^^^^^^^^^^^^^^^^
|
note: lint level defined here
--> $DIR/trivial-casts-featuring-type-ascription.rs:11:9
|
LL | #![deny(trivial_casts, trivial_numeric_casts)]
| ^^^^^^^^^^^^^
= help: cast can be replaced by coercion; this might require type ascription or a temporary variable

error: aborting due to 2 previous errors

19 changes: 19 additions & 0 deletions src/test/ui/lint/trivial-casts.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// Copyright 2018 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.

#![deny(trivial_casts, trivial_numeric_casts)]

fn main() {
let lugubrious = 12i32 as i32;
//~^ ERROR trivial numeric cast
let haunted: &u32 = &99;
let _ = haunted as *const u32;
//~^ ERROR trivial cast
}
28 changes: 28 additions & 0 deletions src/test/ui/lint/trivial-casts.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
error: trivial numeric cast: `i32` as `i32`
--> $DIR/trivial-casts.rs:14:22
|
LL | let lugubrious = 12i32 as i32;
| ^^^^^^^^^^^^
|
note: lint level defined here
--> $DIR/trivial-casts.rs:11:24
|
LL | #![deny(trivial_casts, trivial_numeric_casts)]
| ^^^^^^^^^^^^^^^^^^^^^
= help: cast can be replaced by coercion; this might require a temporary variable

error: trivial cast: `&u32` as `*const u32`
--> $DIR/trivial-casts.rs:17:13
|
LL | let _ = haunted as *const u32;
| ^^^^^^^^^^^^^^^^^^^^^
|
note: lint level defined here
--> $DIR/trivial-casts.rs:11:9
|
LL | #![deny(trivial_casts, trivial_numeric_casts)]
| ^^^^^^^^^^^^^
= help: cast can be replaced by coercion; this might require a temporary variable

error: aborting due to 2 previous errors

14 changes: 14 additions & 0 deletions src/test/ui/suggestions/dotdotdot-expr.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// Copyright 2018 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.

fn main() {
let _redemptive = 1...21;
//~^ ERROR unexpected token
}
16 changes: 16 additions & 0 deletions src/test/ui/suggestions/dotdotdot-expr.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
error: unexpected token: `...`
--> $DIR/dotdotdot-expr.rs:12:24
|
LL | let _redemptive = 1...21;
| ^^^
help: use `..` for an exclusive range
|
LL | let _redemptive = 1..21;
| ^^
help: or `..=` for an inclusive range
|
LL | let _redemptive = 1..=21;
| ^^^

error: aborting due to previous error

0 comments on commit 8acec1f

Please sign in to comment.