Skip to content

Commit 609ffa1

Browse files
estebankestebank
committed
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
1 parent 02f5786 commit 609ffa1

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

55 files changed

+375
-272
lines changed

‎src/librustc/lint/levels.rs

Lines changed: 20 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -191,7 +191,7 @@ impl<'a> LintLevelsBuilder<'a> {
191191
let store = self.sess.lint_store.borrow();
192192
let sess = self.sess;
193193
let bad_attr = |span| {
194-
struct_span_err!(sess, span, E0452, "malformed lint attribute")
194+
struct_span_err!(sess, span, E0452, "malformed lint attribute input")
195195
};
196196
for attr in attrs {
197197
let level = match Level::from_symbol(attr.name_or_empty()) {
@@ -238,18 +238,20 @@ impl<'a> LintLevelsBuilder<'a> {
238238
}
239239
reason = Some(rationale);
240240
} else {
241-
let mut err = bad_attr(name_value.span);
242-
err.help("reason must be a string literal");
243-
err.emit();
241+
bad_attr(name_value.span)
242+
.span_label(name_value.span, "reason must be a string literal")
243+
.emit();
244244
}
245245
} else {
246-
let mut err = bad_attr(item.span);
247-
err.emit();
246+
bad_attr(item.span)
247+
.span_label(item.span, "bad attribute argument")
248+
.emit();
248249
}
249250
},
250251
ast::MetaItemKind::List(_) => {
251-
let mut err = bad_attr(item.span);
252-
err.emit();
252+
bad_attr(item.span)
253+
.span_label(item.span, "bad attribute argument")
254+
.emit();
253255
}
254256
}
255257
}
@@ -258,14 +260,20 @@ impl<'a> LintLevelsBuilder<'a> {
258260
let meta_item = match li.meta_item() {
259261
Some(meta_item) if meta_item.is_word() => meta_item,
260262
_ => {
261-
let mut err = bad_attr(li.span());
263+
let sp = li.span();
264+
let mut err = bad_attr(sp);
265+
let mut add_label = true;
262266
if let Some(item) = li.meta_item() {
263267
if let ast::MetaItemKind::NameValue(_) = item.node {
264268
if item.path == sym::reason {
265-
err.help("reason in lint attribute must come last");
269+
err.span_label(sp, "reason in lint attribute must come last");
270+
add_label = false;
266271
}
267272
}
268273
}
274+
if add_label {
275+
err.span_label(sp, "bad attribute argument");
276+
}
269277
err.emit();
270278
continue;
271279
}
@@ -318,15 +326,14 @@ impl<'a> LintLevelsBuilder<'a> {
318326
Also `cfg_attr(cargo-clippy)` won't be necessary anymore",
319327
name
320328
);
321-
let mut err = lint::struct_lint_level(
329+
lint::struct_lint_level(
322330
self.sess,
323331
lint,
324332
lvl,
325333
src,
326334
Some(li.span().into()),
327335
&msg,
328-
);
329-
err.span_suggestion(
336+
).span_suggestion(
330337
li.span(),
331338
"change it to",
332339
new_lint_name.to_string(),

‎src/librustc_plugin/load.rs

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ use std::env;
1010
use std::mem;
1111
use std::path::PathBuf;
1212
use syntax::ast;
13-
use syntax::span_err;
13+
use syntax::struct_span_err;
1414
use syntax::symbol::{Symbol, kw, sym};
1515
use syntax_pos::{Span, DUMMY_SP};
1616

@@ -29,8 +29,10 @@ struct PluginLoader<'a> {
2929
plugins: Vec<PluginRegistrar>,
3030
}
3131

32-
fn call_malformed_plugin_attribute(a: &Session, b: Span) {
33-
span_err!(a, b, E0498, "malformed plugin attribute");
32+
fn call_malformed_plugin_attribute(sess: &Session, span: Span) {
33+
struct_span_err!(sess, span, E0498, "malformed `plugin` attribute")
34+
.span_label(span, "malformed attribute")
35+
.emit();
3436
}
3537

3638
/// Read plugin metadata and dynamically load registrar functions.

‎src/librustc_typeck/collect.rs

Lines changed: 21 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,8 @@ use rustc::hir::intravisit::{self, NestedVisitorMap, Visitor};
4848
use rustc::hir::GenericParamKind;
4949
use rustc::hir::{self, CodegenFnAttrFlags, CodegenFnAttrs, Unsafety};
5050

51+
use errors::Applicability;
52+
5153
use std::iter;
5254

5355
struct OnlySelfBounds(bool);
@@ -2400,23 +2402,26 @@ fn from_target_feature(
24002402
Some(list) => list,
24012403
None => return,
24022404
};
2405+
let bad_item = |span| {
2406+
let msg = "malformed `target_feature` attribute input";
2407+
let code = "enable = \"..\"".to_owned();
2408+
tcx.sess.struct_span_err(span, &msg)
2409+
.span_suggestion(span, "must be of the form", code, Applicability::HasPlaceholders)
2410+
.emit();
2411+
};
24032412
let rust_features = tcx.features();
24042413
for item in list {
24052414
// Only `enable = ...` is accepted in the meta item list
24062415
if !item.check_name(sym::enable) {
2407-
let msg = "#[target_feature(..)] only accepts sub-keys of `enable` \
2408-
currently";
2409-
tcx.sess.span_err(item.span(), &msg);
2416+
bad_item(item.span());
24102417
continue;
24112418
}
24122419

24132420
// Must be of the form `enable = "..."` ( a string)
24142421
let value = match item.value_str() {
24152422
Some(value) => value,
24162423
None => {
2417-
let msg = "#[target_feature] attribute must be of the form \
2418-
#[target_feature(enable = \"..\")]";
2419-
tcx.sess.span_err(item.span(), &msg);
2424+
bad_item(item.span());
24202425
continue;
24212426
}
24222427
};
@@ -2428,12 +2433,14 @@ fn from_target_feature(
24282433
Some(g) => g,
24292434
None => {
24302435
let msg = format!(
2431-
"the feature named `{}` is not valid for \
2432-
this target",
2436+
"the feature named `{}` is not valid for this target",
24332437
feature
24342438
);
24352439
let mut err = tcx.sess.struct_span_err(item.span(), &msg);
2436-
2440+
err.span_label(
2441+
item.span(),
2442+
format!("`{}` is not valid for this target", feature),
2443+
);
24372444
if feature.starts_with("+") {
24382445
let valid = whitelist.contains_key(&feature[1..]);
24392446
if valid {
@@ -2571,9 +2578,11 @@ fn codegen_fn_attrs<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, id: DefId) -> Codegen
25712578
}
25722579
} else if attr.check_name(sym::target_feature) {
25732580
if tcx.fn_sig(id).unsafety() == Unsafety::Normal {
2574-
let msg = "#[target_feature(..)] can only be applied to \
2575-
`unsafe` function";
2576-
tcx.sess.span_err(attr.span, msg);
2581+
let msg = "#[target_feature(..)] can only be applied to `unsafe` functions";
2582+
tcx.sess.struct_span_err(attr.span, msg)
2583+
.span_label(attr.span, "can only be applied to `unsafe` functions")
2584+
.span_label(tcx.def_span(id), "not an `unsafe` function")
2585+
.emit();
25772586
}
25782587
from_target_feature(
25792588
tcx,

‎src/libsyntax/attr/builtin.rs

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,15 @@ pub fn find_unwind_attr(diagnostic: Option<&Handler>, attrs: &[Attribute]) -> Op
9292
}
9393

9494
diagnostic.map(|d| {
95-
span_err!(d, attr.span, E0633, "malformed `#[unwind]` attribute");
95+
struct_span_err!(d, attr.span, E0633, "malformed `unwind` attribute input")
96+
.span_label(attr.span, "invalid argument")
97+
.span_suggestions(
98+
attr.span,
99+
"the allowed arguments are `allowed` and `aborts`",
100+
(vec!["allowed", "aborts"]).into_iter()
101+
.map(|s| format!("#[unwind({})]", s)),
102+
Applicability::MachineApplicable,
103+
).emit();
96104
});
97105
}
98106
}

‎src/libsyntax/config.rs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,22 @@ impl<'a> StripUnconfigured<'a> {
9494
if !attr.check_name(sym::cfg_attr) {
9595
return vec![attr];
9696
}
97+
if attr.tokens.len() == 0 {
98+
self.sess.span_diagnostic
99+
.struct_span_err(
100+
attr.span,
101+
"malformed `cfg_attr` attribute input",
102+
).span_suggestion(
103+
attr.span,
104+
"missing condition and attribute",
105+
"#[cfg_attr(condition, attribute, other_attribute, ...)]".to_owned(),
106+
Applicability::HasPlaceholders,
107+
).note("for more information, visit \
108+
<https://doc.rust-lang.org/reference/conditional-compilation.html\
109+
#the-cfg_attr-attribute>")
110+
.emit();
111+
return Vec::new();
112+
}
97113

98114
let (cfg_predicate, expanded_attrs) = match attr.parse(self.sess, |parser| {
99115
parser.expect(&token::OpenDelim(token::Paren))?;

‎src/libsyntax/ext/derive.rs

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ use crate::ext::base::ExtCtxt;
55
use crate::ext::build::AstBuilder;
66
use crate::parse::parser::PathStyle;
77
use crate::symbol::{Symbol, sym};
8+
use crate::errors::Applicability;
89

910
use syntax_pos::Span;
1011

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

‎src/libsyntax/feature_gate.rs

Lines changed: 41 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ use crate::parse::{token, ParseSess};
2525
use crate::symbol::{Symbol, kw, sym};
2626
use crate::tokenstream::TokenTree;
2727

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

18591859
match attr.parse_meta(self.context.parse_sess) {
18601860
Ok(meta) => if !should_skip(name) && !template.compatible(&meta.node) {
1861+
let error_msg = format!("malformed `{}` attribute input", name);
18611862
let mut msg = "attribute must be of the form ".to_owned();
1863+
let mut suggestions = vec![];
18621864
let mut first = true;
18631865
if template.word {
18641866
first = false;
1865-
msg.push_str(&format!("`#[{}{}]`", name, ""));
1867+
let code = format!("#[{}]", name);
1868+
msg.push_str(&format!("`{}`", &code));
1869+
suggestions.push(code);
18661870
}
18671871
if let Some(descr) = template.list {
18681872
if !first {
18691873
msg.push_str(" or ");
18701874
}
18711875
first = false;
1872-
msg.push_str(&format!("`#[{}({})]`", name, descr));
1876+
let code = format!("#[{}({})]", name, descr);
1877+
msg.push_str(&format!("`{}`", &code));
1878+
suggestions.push(code);
18731879
}
18741880
if let Some(descr) = template.name_value_str {
18751881
if !first {
18761882
msg.push_str(" or ");
18771883
}
1878-
msg.push_str(&format!("`#[{} = \"{}\"]`", name, descr));
1884+
let code = format!("#[{} = \"{}\"]", name, descr);
1885+
msg.push_str(&format!("`{}`", &code));
1886+
suggestions.push(code);
18791887
}
18801888
if should_warn(name) {
18811889
self.context.parse_sess.buffer_lint(
@@ -1885,7 +1893,17 @@ impl<'a> PostExpansionVisitor<'a> {
18851893
&msg,
18861894
);
18871895
} else {
1888-
self.context.parse_sess.span_diagnostic.span_err(meta.span, &msg);
1896+
self.context.parse_sess.span_diagnostic.struct_span_err(meta.span, &error_msg)
1897+
.span_suggestions(
1898+
meta.span,
1899+
if suggestions.len() == 1 {
1900+
"must be of the form"
1901+
} else {
1902+
"the following are the possible correct uses"
1903+
},
1904+
suggestions.into_iter(),
1905+
Applicability::HasPlaceholders,
1906+
).emit();
18891907
}
18901908
}
18911909
Err(mut err) => err.emit(),
@@ -2298,6 +2316,8 @@ pub fn get_features(span_handler: &Handler, krate_attrs: &[ast::Attribute],
22982316
let mut err = struct_span_err!(span_handler, span, E0557, "feature has been removed");
22992317
if let Some(reason) = reason {
23002318
err.span_note(span, reason);
2319+
} else {
2320+
err.span_label(span, "feature has been removed");
23012321
}
23022322
err.emit();
23032323
}
@@ -2379,12 +2399,24 @@ pub fn get_features(span_handler: &Handler, krate_attrs: &[ast::Attribute],
23792399
None => continue,
23802400
};
23812401

2402+
let bad_input = |span| {
2403+
struct_span_err!(span_handler, span, E0556, "malformed `feature` attribute input")
2404+
};
2405+
23822406
for mi in list {
23832407
let name = match mi.ident() {
23842408
Some(ident) if mi.is_word() => ident.name,
2385-
_ => {
2386-
span_err!(span_handler, mi.span(), E0556,
2387-
"malformed feature, expected just one word");
2409+
Some(ident) => {
2410+
bad_input(mi.span()).span_suggestion(
2411+
mi.span(),
2412+
"expected just one word",
2413+
format!("{}", ident.name),
2414+
Applicability::MaybeIncorrect,
2415+
).emit();
2416+
continue
2417+
}
2418+
None => {
2419+
bad_input(mi.span()).span_label(mi.span(), "expected just one word").emit();
23882420
continue
23892421
}
23902422
};

‎src/test/ui/deprecation/deprecated_no_stack_check.stderr

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ error[E0557]: feature has been removed
22
--> $DIR/deprecated_no_stack_check.rs:2:12
33
|
44
LL | #![feature(no_stack_check)]
5-
| ^^^^^^^^^^^^^^
5+
| ^^^^^^^^^^^^^^ feature has been removed
66

77
error: aborting due to previous error
88

‎src/test/ui/deprecation/invalid-literal.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
#[deprecated = b"test"] //~ ERROR attribute must be of the form
1+
#[deprecated = b"test"] //~ ERROR malformed `deprecated` attribute
22
fn foo() {}
33

44
fn main() {}

‎src/test/ui/deprecation/invalid-literal.stderr

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,16 @@
1-
error: attribute must be of the form `#[deprecated]` or `#[deprecated(/*opt*/ since = "version", /*opt*/ note = "reason)]` or `#[deprecated = "reason"]`
1+
error: malformed `deprecated` attribute input
22
--> $DIR/invalid-literal.rs:1:1
33
|
44
LL | #[deprecated = b"test"]
55
| ^^^^^^^^^^^^^^^^^^^^^^^
6+
help: the following are the possible correct uses
7+
|
8+
LL | #[deprecated]
9+
| ^^^^^^^^^^^^^
10+
LL | #[deprecated(/*opt*/ since = "version", /*opt*/ note = "reason")]
11+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
12+
LL | #[deprecated = "reason"]
13+
| ^^^^^^^^^^^^^^^^^^^^^^^^
614

715
error: aborting due to previous error
816

‎src/test/ui/error-codes/E0452.stderr

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
1-
error[E0452]: malformed lint attribute
1+
error[E0452]: malformed lint attribute input
22
--> $DIR/E0452.rs:1:10
33
|
44
LL | #![allow(foo = "")]
5-
| ^^^^^^^^
5+
| ^^^^^^^^ bad attribute argument
66

77
error: aborting due to previous error
88

0 commit comments

Comments
 (0)