Skip to content

Commit

Permalink
Rollup merge of rust-lang#61140 - estebank:attr-diagnostics, r=michae…
Browse files Browse the repository at this point in the history
…lwoerister

Reword malformed attribute input diagnostics

- Handle empty `cfg_attr` attribute
- Reword empty `derive` attribute error
- Use consistend error message: "malformed `attrname` attribute input"
- Provide suggestions when possible
- Move note/help to label/suggestion
- Use consistent wording "ill-formed" -> "malformed"
- Move diagnostic logic out of parser

Split up from rust-lang#61026, where there's prior conversation.
  • Loading branch information
Centril authored May 27, 2019
2 parents b2cca1b + 609ffa1 commit e3b66d6
Show file tree
Hide file tree
Showing 55 changed files with 375 additions and 272 deletions.
33 changes: 20 additions & 13 deletions src/librustc/lint/levels.rs
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ impl<'a> LintLevelsBuilder<'a> {
let store = self.sess.lint_store.borrow();
let sess = self.sess;
let bad_attr = |span| {
struct_span_err!(sess, span, E0452, "malformed lint attribute")
struct_span_err!(sess, span, E0452, "malformed lint attribute input")
};
for attr in attrs {
let level = match Level::from_symbol(attr.name_or_empty()) {
Expand Down Expand Up @@ -238,18 +238,20 @@ impl<'a> LintLevelsBuilder<'a> {
}
reason = Some(rationale);
} else {
let mut err = bad_attr(name_value.span);
err.help("reason must be a string literal");
err.emit();
bad_attr(name_value.span)
.span_label(name_value.span, "reason must be a string literal")
.emit();
}
} else {
let mut err = bad_attr(item.span);
err.emit();
bad_attr(item.span)
.span_label(item.span, "bad attribute argument")
.emit();
}
},
ast::MetaItemKind::List(_) => {
let mut err = bad_attr(item.span);
err.emit();
bad_attr(item.span)
.span_label(item.span, "bad attribute argument")
.emit();
}
}
}
Expand All @@ -258,14 +260,20 @@ impl<'a> LintLevelsBuilder<'a> {
let meta_item = match li.meta_item() {
Some(meta_item) if meta_item.is_word() => meta_item,
_ => {
let mut err = bad_attr(li.span());
let sp = li.span();
let mut err = bad_attr(sp);
let mut add_label = true;
if let Some(item) = li.meta_item() {
if let ast::MetaItemKind::NameValue(_) = item.node {
if item.path == sym::reason {
err.help("reason in lint attribute must come last");
err.span_label(sp, "reason in lint attribute must come last");
add_label = false;
}
}
}
if add_label {
err.span_label(sp, "bad attribute argument");
}
err.emit();
continue;
}
Expand Down Expand Up @@ -318,15 +326,14 @@ impl<'a> LintLevelsBuilder<'a> {
Also `cfg_attr(cargo-clippy)` won't be necessary anymore",
name
);
let mut err = lint::struct_lint_level(
lint::struct_lint_level(
self.sess,
lint,
lvl,
src,
Some(li.span().into()),
&msg,
);
err.span_suggestion(
).span_suggestion(
li.span(),
"change it to",
new_lint_name.to_string(),
Expand Down
8 changes: 5 additions & 3 deletions src/librustc_plugin/load.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use std::env;
use std::mem;
use std::path::PathBuf;
use syntax::ast;
use syntax::span_err;
use syntax::struct_span_err;
use syntax::symbol::{Symbol, kw, sym};
use syntax_pos::{Span, DUMMY_SP};

Expand All @@ -29,8 +29,10 @@ struct PluginLoader<'a> {
plugins: Vec<PluginRegistrar>,
}

fn call_malformed_plugin_attribute(a: &Session, b: Span) {
span_err!(a, b, E0498, "malformed plugin attribute");
fn call_malformed_plugin_attribute(sess: &Session, span: Span) {
struct_span_err!(sess, span, E0498, "malformed `plugin` attribute")
.span_label(span, "malformed attribute")
.emit();
}

/// Read plugin metadata and dynamically load registrar functions.
Expand Down
33 changes: 21 additions & 12 deletions src/librustc_typeck/collect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ use rustc::hir::intravisit::{self, NestedVisitorMap, Visitor};
use rustc::hir::GenericParamKind;
use rustc::hir::{self, CodegenFnAttrFlags, CodegenFnAttrs, Unsafety};

use errors::Applicability;

use std::iter;

struct OnlySelfBounds(bool);
Expand Down Expand Up @@ -2400,23 +2402,26 @@ fn from_target_feature(
Some(list) => list,
None => return,
};
let bad_item = |span| {
let msg = "malformed `target_feature` attribute input";
let code = "enable = \"..\"".to_owned();
tcx.sess.struct_span_err(span, &msg)
.span_suggestion(span, "must be of the form", code, Applicability::HasPlaceholders)
.emit();
};
let rust_features = tcx.features();
for item in list {
// Only `enable = ...` is accepted in the meta item list
if !item.check_name(sym::enable) {
let msg = "#[target_feature(..)] only accepts sub-keys of `enable` \
currently";
tcx.sess.span_err(item.span(), &msg);
bad_item(item.span());
continue;
}

// Must be of the form `enable = "..."` ( a string)
let value = match item.value_str() {
Some(value) => value,
None => {
let msg = "#[target_feature] attribute must be of the form \
#[target_feature(enable = \"..\")]";
tcx.sess.span_err(item.span(), &msg);
bad_item(item.span());
continue;
}
};
Expand All @@ -2428,12 +2433,14 @@ fn from_target_feature(
Some(g) => g,
None => {
let msg = format!(
"the feature named `{}` is not valid for \
this target",
"the feature named `{}` is not valid for this target",
feature
);
let mut err = tcx.sess.struct_span_err(item.span(), &msg);

err.span_label(
item.span(),
format!("`{}` is not valid for this target", feature),
);
if feature.starts_with("+") {
let valid = whitelist.contains_key(&feature[1..]);
if valid {
Expand Down Expand Up @@ -2571,9 +2578,11 @@ fn codegen_fn_attrs<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, id: DefId) -> Codegen
}
} else if attr.check_name(sym::target_feature) {
if tcx.fn_sig(id).unsafety() == Unsafety::Normal {
let msg = "#[target_feature(..)] can only be applied to \
`unsafe` function";
tcx.sess.span_err(attr.span, msg);
let msg = "#[target_feature(..)] can only be applied to `unsafe` functions";
tcx.sess.struct_span_err(attr.span, msg)
.span_label(attr.span, "can only be applied to `unsafe` functions")
.span_label(tcx.def_span(id), "not an `unsafe` function")
.emit();
}
from_target_feature(
tcx,
Expand Down
10 changes: 9 additions & 1 deletion src/libsyntax/attr/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,15 @@ pub fn find_unwind_attr(diagnostic: Option<&Handler>, attrs: &[Attribute]) -> Op
}

diagnostic.map(|d| {
span_err!(d, attr.span, E0633, "malformed `#[unwind]` attribute");
struct_span_err!(d, attr.span, E0633, "malformed `unwind` attribute input")
.span_label(attr.span, "invalid argument")
.span_suggestions(
attr.span,
"the allowed arguments are `allowed` and `aborts`",
(vec!["allowed", "aborts"]).into_iter()
.map(|s| format!("#[unwind({})]", s)),
Applicability::MachineApplicable,
).emit();
});
}
}
Expand Down
16 changes: 16 additions & 0 deletions src/libsyntax/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,22 @@ impl<'a> StripUnconfigured<'a> {
if !attr.check_name(sym::cfg_attr) {
return vec![attr];
}
if attr.tokens.len() == 0 {
self.sess.span_diagnostic
.struct_span_err(
attr.span,
"malformed `cfg_attr` attribute input",
).span_suggestion(
attr.span,
"missing condition and attribute",
"#[cfg_attr(condition, attribute, other_attribute, ...)]".to_owned(),
Applicability::HasPlaceholders,
).note("for more information, visit \
<https://doc.rust-lang.org/reference/conditional-compilation.html\
#the-cfg_attr-attribute>")
.emit();
return Vec::new();
}

let (cfg_predicate, expanded_attrs) = match attr.parse(self.sess, |parser| {
parser.expect(&token::OpenDelim(token::Paren))?;
Expand Down
10 changes: 8 additions & 2 deletions src/libsyntax/ext/derive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use crate::ext::base::ExtCtxt;
use crate::ext::build::AstBuilder;
use crate::parse::parser::PathStyle;
use crate::symbol::{Symbol, sym};
use crate::errors::Applicability;

use syntax_pos::Span;

Expand All @@ -17,8 +18,13 @@ pub fn collect_derives(cx: &mut ExtCtxt<'_>, attrs: &mut Vec<ast::Attribute>) ->
return true;
}
if !attr.is_meta_item_list() {
cx.span_err(attr.span,
"attribute must be of the form `#[derive(Trait1, Trait2, ...)]`");
cx.struct_span_err(attr.span, "malformed `derive` attribute input")
.span_suggestion(
attr.span,
"missing traits to be derived",
"#[derive(Trait1, Trait2, ...)]".to_owned(),
Applicability::HasPlaceholders,
).emit();
return false;
}

Expand Down
50 changes: 41 additions & 9 deletions src/libsyntax/feature_gate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ use crate::parse::{token, ParseSess};
use crate::symbol::{Symbol, kw, sym};
use crate::tokenstream::TokenTree;

use errors::{DiagnosticBuilder, Handler};
use errors::{Applicability, DiagnosticBuilder, Handler};
use rustc_data_structures::fx::FxHashMap;
use rustc_target::spec::abi::Abi;
use syntax_pos::{Span, DUMMY_SP};
Expand Down Expand Up @@ -1422,7 +1422,7 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[
Normal,
template!(
Word,
List: r#"/*opt*/ since = "version", /*opt*/ note = "reason"#,
List: r#"/*opt*/ since = "version", /*opt*/ note = "reason""#,
NameValueStr: "reason"
),
Ungated
Expand Down Expand Up @@ -1858,24 +1858,32 @@ impl<'a> PostExpansionVisitor<'a> {

match attr.parse_meta(self.context.parse_sess) {
Ok(meta) => if !should_skip(name) && !template.compatible(&meta.node) {
let error_msg = format!("malformed `{}` attribute input", name);
let mut msg = "attribute must be of the form ".to_owned();
let mut suggestions = vec![];
let mut first = true;
if template.word {
first = false;
msg.push_str(&format!("`#[{}{}]`", name, ""));
let code = format!("#[{}]", name);
msg.push_str(&format!("`{}`", &code));
suggestions.push(code);
}
if let Some(descr) = template.list {
if !first {
msg.push_str(" or ");
}
first = false;
msg.push_str(&format!("`#[{}({})]`", name, descr));
let code = format!("#[{}({})]", name, descr);
msg.push_str(&format!("`{}`", &code));
suggestions.push(code);
}
if let Some(descr) = template.name_value_str {
if !first {
msg.push_str(" or ");
}
msg.push_str(&format!("`#[{} = \"{}\"]`", name, descr));
let code = format!("#[{} = \"{}\"]", name, descr);
msg.push_str(&format!("`{}`", &code));
suggestions.push(code);
}
if should_warn(name) {
self.context.parse_sess.buffer_lint(
Expand All @@ -1885,7 +1893,17 @@ impl<'a> PostExpansionVisitor<'a> {
&msg,
);
} else {
self.context.parse_sess.span_diagnostic.span_err(meta.span, &msg);
self.context.parse_sess.span_diagnostic.struct_span_err(meta.span, &error_msg)
.span_suggestions(
meta.span,
if suggestions.len() == 1 {
"must be of the form"
} else {
"the following are the possible correct uses"
},
suggestions.into_iter(),
Applicability::HasPlaceholders,
).emit();
}
}
Err(mut err) => err.emit(),
Expand Down Expand Up @@ -2298,6 +2316,8 @@ pub fn get_features(span_handler: &Handler, krate_attrs: &[ast::Attribute],
let mut err = struct_span_err!(span_handler, span, E0557, "feature has been removed");
if let Some(reason) = reason {
err.span_note(span, reason);
} else {
err.span_label(span, "feature has been removed");
}
err.emit();
}
Expand Down Expand Up @@ -2379,12 +2399,24 @@ pub fn get_features(span_handler: &Handler, krate_attrs: &[ast::Attribute],
None => continue,
};

let bad_input = |span| {
struct_span_err!(span_handler, span, E0556, "malformed `feature` attribute input")
};

for mi in list {
let name = match mi.ident() {
Some(ident) if mi.is_word() => ident.name,
_ => {
span_err!(span_handler, mi.span(), E0556,
"malformed feature, expected just one word");
Some(ident) => {
bad_input(mi.span()).span_suggestion(
mi.span(),
"expected just one word",
format!("{}", ident.name),
Applicability::MaybeIncorrect,
).emit();
continue
}
None => {
bad_input(mi.span()).span_label(mi.span(), "expected just one word").emit();
continue
}
};
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/deprecation/deprecated_no_stack_check.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ error[E0557]: feature has been removed
--> $DIR/deprecated_no_stack_check.rs:2:12
|
LL | #![feature(no_stack_check)]
| ^^^^^^^^^^^^^^
| ^^^^^^^^^^^^^^ feature has been removed

error: aborting due to previous error

Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/deprecation/invalid-literal.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#[deprecated = b"test"] //~ ERROR attribute must be of the form
#[deprecated = b"test"] //~ ERROR malformed `deprecated` attribute
fn foo() {}

fn main() {}
10 changes: 9 additions & 1 deletion src/test/ui/deprecation/invalid-literal.stderr
Original file line number Diff line number Diff line change
@@ -1,8 +1,16 @@
error: attribute must be of the form `#[deprecated]` or `#[deprecated(/*opt*/ since = "version", /*opt*/ note = "reason)]` or `#[deprecated = "reason"]`
error: malformed `deprecated` attribute input
--> $DIR/invalid-literal.rs:1:1
|
LL | #[deprecated = b"test"]
| ^^^^^^^^^^^^^^^^^^^^^^^
help: the following are the possible correct uses
|
LL | #[deprecated]
| ^^^^^^^^^^^^^
LL | #[deprecated(/*opt*/ since = "version", /*opt*/ note = "reason")]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
LL | #[deprecated = "reason"]
| ^^^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to previous error

4 changes: 2 additions & 2 deletions src/test/ui/error-codes/E0452.stderr
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
error[E0452]: malformed lint attribute
error[E0452]: malformed lint attribute input
--> $DIR/E0452.rs:1:10
|
LL | #![allow(foo = "")]
| ^^^^^^^^
| ^^^^^^^^ bad attribute argument

error: aborting due to previous error

Expand Down
Loading

0 comments on commit e3b66d6

Please sign in to comment.