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

Reword malformed attribute input diagnostics #61140

Merged
merged 1 commit into from
May 27, 2019
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
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);
Copy link
Contributor

Choose a reason for hiding this comment

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

AFAIK, "malformed" is usually used in context of biology, like "malformed foetus/limb".
"Ill-formed" is something not fitting into a language/grammar.

Copy link
Contributor

Choose a reason for hiding this comment

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

🤷‍♂ I've heard malformed being used wrt. syntax as well; a google search does give plenty of results for "malformed syntax". I've also used "malformed" in the context of grammar myself before.

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