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

Change some notes into suggestions #42033

Merged
merged 3 commits into from
Jul 17, 2017
Merged
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
2 changes: 1 addition & 1 deletion src/librustc_borrowck/borrowck/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -991,7 +991,7 @@ impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> {
.span_suggestion(err.span,
&format!("to force the closure to take ownership of {} \
(and any other referenced variables), \
use the `move` keyword, as shown:",
use the `move` keyword",
cmt_path_or_string),
suggestion)
.emit();
Expand Down
12 changes: 12 additions & 0 deletions src/librustc_errors/diagnostic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,18 @@ impl Diagnostic {

/// Prints out a message with a suggested edit of the code.
///
/// In case of short messages and a simple suggestion,
/// rustc displays it as a label like
///
/// "try adding parentheses: `(tup.0).1`"
///
/// The message
/// * should not end in any punctuation (a `:` is added automatically)
/// * should not be a question
/// * should not contain any parts like "the following", "as shown"
/// * may look like "to do xyz, use" or "to do xyz, use abc"
/// * may contain a name of a function, variable or type, but not whole expressions
///
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added some guidelines for suggestion messages. This way we can keep appending the suggestion at the end. I like this solution, because it's easy to read on the command line, requires little effort from suggestion authors and works well with non-label suggestions and json output.

/// See `diagnostic::CodeSuggestion` for more information.
pub fn span_suggestion(&mut self, sp: Span, msg: &str, suggestion: String) -> &mut Self {
self.suggestions.push(CodeSuggestion {
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_errors/emitter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ impl Emitter for EmitterWriter {
// This substitution is only removal, don't show it
format!("help: {}", sugg.msg)
} else {
format!("help: {} `{}`", sugg.msg, substitution)
format!("help: {}: `{}`", sugg.msg, substitution)
};
primary_span.push_span_label(sugg.substitution_spans().next().unwrap(), msg);
} else {
Expand Down
20 changes: 12 additions & 8 deletions src/librustc_resolve/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2409,13 +2409,15 @@ impl<'a> Resolver<'a> {
.map(|suggestion| import_candidate_to_paths(&suggestion)).collect::<Vec<_>>();
enum_candidates.sort();
for (sp, variant_path, enum_path) in enum_candidates {
let msg = format!("there is an enum variant `{}`, did you mean to use `{}`?",
variant_path,
enum_path);
if sp == DUMMY_SP {
let msg = format!("there is an enum variant `{}`, \
try using `{}`?",
variant_path,
enum_path);
err.help(&msg);
} else {
err.span_help(sp, &msg);
err.span_suggestion(span, "you can try using the variant's enum",
enum_path);
}
}
}
Expand All @@ -2424,18 +2426,20 @@ impl<'a> Resolver<'a> {
let self_is_available = this.self_value_is_available(path[0].ctxt, span);
match candidate {
AssocSuggestion::Field => {
err.span_label(span, format!("did you mean `self.{}`?", path_str));
err.span_suggestion(span, "try",
format!("self.{}", path_str));
if !self_is_available {
err.span_label(span, format!("`self` value is only available in \
methods with `self` parameter"));
}
}
AssocSuggestion::MethodWithSelf if self_is_available => {
err.span_label(span, format!("did you mean `self.{}(...)`?",
path_str));
err.span_suggestion(span, "try",
format!("self.{}", path_str));
}
AssocSuggestion::MethodWithSelf | AssocSuggestion::AssocItem => {
err.span_label(span, format!("did you mean `Self::{}`?", path_str));
err.span_suggestion(span, "try",
format!("Self::{}", path_str));
}
}
return err;
Expand Down
5 changes: 3 additions & 2 deletions src/librustc_resolve/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -658,9 +658,10 @@ impl<'a> Resolver<'a> {
if let Some(suggestion) = suggestion {
if suggestion != name {
if let MacroKind::Bang = kind {
err.help(&format!("did you mean `{}!`?", suggestion));
err.span_suggestion(span, "you could try the macro",
format!("{}!", suggestion));
} else {
err.help(&format!("did you mean `{}`?", suggestion));
err.span_suggestion(span, "try", suggestion.to_string());
}
} else {
err.help("have you added the `#[macro_use]` on the module/import?");
Expand Down
4 changes: 2 additions & 2 deletions src/librustc_typeck/check/cast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,7 @@ impl<'a, 'gcx, 'tcx> CastCheck<'tcx> {
match fcx.tcx.sess.codemap().span_to_snippet(self.cast_span) {
Ok(s) => {
err.span_suggestion(self.cast_span,
"try casting to a reference instead:",
"try casting to a reference instead",
format!("&{}{}", mtstr, s));
}
Err(_) => {
Expand All @@ -272,7 +272,7 @@ impl<'a, 'gcx, 'tcx> CastCheck<'tcx> {
match fcx.tcx.sess.codemap().span_to_snippet(self.cast_span) {
Ok(s) => {
err.span_suggestion(self.cast_span,
"try casting to a `Box` instead:",
"try casting to a `Box` instead",
format!("Box<{}>", s));
}
Err(_) => span_help!(err, self.cast_span, "did you mean `Box<{}>`?", tstr),
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_typeck/check/op.rs
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
from a string reference. String concatenation \
appends the string on the right to the string \
on the left and may require reallocation. This \
requires ownership of the string on the left."), suggestion);
requires ownership of the string on the left"), suggestion);
is_string_addition = true;
}

Expand Down
4 changes: 2 additions & 2 deletions src/libsyntax/parse/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1490,7 +1490,7 @@ impl<'a> Parser<'a> {
s.print_bounds(" +", &bounds)?;
s.pclose()
});
err.span_suggestion(sum_span, "try adding parentheses:", sum_with_parens);
err.span_suggestion(sum_span, "try adding parentheses", sum_with_parens);
}
TyKind::Ptr(..) | TyKind::BareFn(..) => {
err.span_label(sum_span, "perhaps you forgot parentheses?");
Expand Down Expand Up @@ -5280,7 +5280,7 @@ impl<'a> Parser<'a> {
`pub(in path::to::module)`: visible only on the specified path"##;
let path = self.parse_path(PathStyle::Mod)?;
let path_span = self.prev_span;
let help_msg = format!("make this visible only to module `{}` with `in`:", path);
let help_msg = format!("make this visible only to module `{}` with `in`", path);
self.expect(&token::CloseDelim(token::Paren))?; // `)`
let mut err = self.span_fatal_help(path_span, msg, suggestion);
err.span_suggestion(path_span, &help_msg, format!("in {}", path));
Expand Down
6 changes: 3 additions & 3 deletions src/test/compile-fail/issue-35675.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,21 +12,21 @@
enum Fruit { //~ HELP possible candidate is found in another module, you can import it into scope
//~^ HELP possible candidate is found in another module, you can import it into scope
Apple(i64),
//~^ HELP there is an enum variant `Fruit::Apple`, did you mean to use `Fruit`?
//~| HELP there is an enum variant `Fruit::Apple`, did you mean to use `Fruit`?
Orange(i64),
}

fn should_return_fruit() -> Apple {
//~^ ERROR cannot find type `Apple` in this scope
//~| NOTE not found in this scope
//~| HELP you can try using the variant's enum
Apple(5)
//~^ ERROR cannot find function `Apple` in this scope
//~| NOTE not found in this scope
}

fn should_return_fruit_too() -> Fruit::Apple {
//~^ ERROR expected type, found variant `Fruit::Apple`
//~| HELP you can try using the variant's enum
//~| NOTE not a type
Apple(5)
//~^ ERROR cannot find function `Apple` in this scope
Expand All @@ -43,6 +43,7 @@ fn foo() -> Ok {

fn bar() -> Variant3 {
//~^ ERROR cannot find type `Variant3` in this scope
//~| HELP you can try using the variant's enum
//~| NOTE not found in this scope
}

Expand All @@ -61,7 +62,6 @@ mod x {
Variant1,
Variant2(),
Variant3(usize),
//~^ HELP there is an enum variant `x::Enum::Variant3`, did you mean to use `x::Enum`?
Variant4 {},
}
}
23 changes: 23 additions & 0 deletions src/test/ui-fulldeps/auxiliary/attr_proc_macro.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// 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.

// force-host
// no-prefer-dynamic
#![feature(proc_macro)]
#![crate_type = "proc-macro"]

extern crate proc_macro;

use proc_macro::TokenStream;

#[proc_macro_attribute]
pub fn attr_proc_macro(_: TokenStream, input: TokenStream) -> TokenStream {
input
}
23 changes: 23 additions & 0 deletions src/test/ui-fulldeps/auxiliary/bang_proc_macro.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// 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.

// force-host
// no-prefer-dynamic
#![feature(proc_macro)]
#![crate_type = "proc-macro"]

extern crate proc_macro;

use proc_macro::TokenStream;

#[proc_macro]
pub fn bang_proc_macro(input: TokenStream) -> TokenStream {
input
}
23 changes: 23 additions & 0 deletions src/test/ui-fulldeps/auxiliary/derive-clona.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// 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.

// force-host
// no-prefer-dynamic

#![crate_type = "proc-macro"]

extern crate proc_macro;

use proc_macro::TokenStream;

#[proc_macro_derive(Clona)]
pub fn derive_clonea(input: TokenStream) -> TokenStream {
"".parse().unwrap()
}
23 changes: 23 additions & 0 deletions src/test/ui-fulldeps/auxiliary/derive-foo.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// 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.

// force-host
// no-prefer-dynamic

#![crate_type = "proc-macro"]

extern crate proc_macro;

use proc_macro::TokenStream;

#[proc_macro_derive(FooWithLongName)]
pub fn derive_foo(input: TokenStream) -> TokenStream {
"".parse().unwrap()
}
Original file line number Diff line number Diff line change
Expand Up @@ -35,46 +35,29 @@ macro_rules! attr_proc_mac {
}

#[derive(FooWithLongNan)]
//~^ ERROR cannot find derive macro `FooWithLongNan` in this scope
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's better to keep these annotations.
UI and compile-fail tests are going to be unified and ERROR/WARN will be necessary again.

//~^^ HELP did you mean `FooWithLongName`?
struct Foo;

#[attr_proc_macra]
//~^ ERROR cannot find attribute macro `attr_proc_macra` in this scope
//~^^ HELP did you mean `attr_proc_macro`?
struct Bar;

#[FooWithLongNan]
//~^ ERROR cannot find attribute macro `FooWithLongNan` in this scope
struct Asdf;

#[derive(Dlone)]
//~^ ERROR cannot find derive macro `Dlone` in this scope
//~^^ HELP did you mean `Clone`?
struct A;

#[derive(Dlona)]
//~^ ERROR cannot find derive macro `Dlona` in this scope
//~^^ HELP did you mean `Clona`?
struct B;

#[derive(attr_proc_macra)]
//~^ ERROR cannot find derive macro `attr_proc_macra` in this scope
struct C;

fn main() {
FooWithLongNama!();
//~^ ERROR cannot find macro `FooWithLongNama!` in this scope
//~^^ HELP did you mean `FooWithLongNam!`?

attr_proc_macra!();
//~^ ERROR cannot find macro `attr_proc_macra!` in this scope
//~^^ HELP did you mean `attr_proc_mac!`?

Dlona!();
//~^ ERROR cannot find macro `Dlona!` in this scope

bang_proc_macrp!();
//~^ ERROR cannot find macro `bang_proc_macrp!` in this scope
//~^^ HELP did you mean `bang_proc_macro!`?
}
62 changes: 62 additions & 0 deletions src/test/ui-fulldeps/resolve-error.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
error: cannot find derive macro `FooWithLongNan` in this scope
--> $DIR/resolve-error.rs:37:10
|
37 | #[derive(FooWithLongNan)]
| ^^^^^^^^^^^^^^ help: try: `FooWithLongName`

error: cannot find attribute macro `attr_proc_macra` in this scope
--> $DIR/resolve-error.rs:40:3
|
40 | #[attr_proc_macra]
| ^^^^^^^^^^^^^^^ help: try: `attr_proc_macro`

error: cannot find attribute macro `FooWithLongNan` in this scope
--> $DIR/resolve-error.rs:43:3
|
43 | #[FooWithLongNan]
| ^^^^^^^^^^^^^^

error: cannot find derive macro `Dlone` in this scope
--> $DIR/resolve-error.rs:46:10
|
46 | #[derive(Dlone)]
| ^^^^^ help: try: `Clone`

error: cannot find derive macro `Dlona` in this scope
--> $DIR/resolve-error.rs:49:10
|
49 | #[derive(Dlona)]
| ^^^^^ help: try: `Clona`

error: cannot find derive macro `attr_proc_macra` in this scope
--> $DIR/resolve-error.rs:52:10
|
52 | #[derive(attr_proc_macra)]
| ^^^^^^^^^^^^^^^

error: cannot find macro `FooWithLongNama!` in this scope
--> $DIR/resolve-error.rs:56:5
|
56 | FooWithLongNama!();
| ^^^^^^^^^^^^^^^ help: you could try the macro: `FooWithLongNam!`

error: cannot find macro `attr_proc_macra!` in this scope
--> $DIR/resolve-error.rs:58:5
|
58 | attr_proc_macra!();
| ^^^^^^^^^^^^^^^ help: you could try the macro: `attr_proc_mac!`

error: cannot find macro `Dlona!` in this scope
--> $DIR/resolve-error.rs:60:5
|
60 | Dlona!();
| ^^^^^

error: cannot find macro `bang_proc_macrp!` in this scope
--> $DIR/resolve-error.rs:62:5
|
62 | bang_proc_macrp!();
| ^^^^^^^^^^^^^^^ help: you could try the macro: `bang_proc_macro!`

error: aborting due to 10 previous errors

Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,5 @@

fn main() {
&1 as Send;
//~^ ERROR cast to unsized type
//~| HELP try casting to a reference instead:
//~| SUGGESTION &1 as &Send;
Box::new(1) as Send;
//~^ ERROR cast to unsized type
//~| HELP try casting to a `Box` instead:
//~| SUGGESTION Box::new(1) as Box<Send>;
}
Loading