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

Account for missing keyword in fn/struct definition #45997

Merged
merged 10 commits into from
Dec 1, 2017
4 changes: 3 additions & 1 deletion src/librustc/hir/lowering.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1757,7 +1757,9 @@ impl<'a> LoweringContext<'a> {
bounds,
items)
}
ItemKind::MacroDef(..) | ItemKind::Mac(..) => panic!("Shouldn't still be around"),
ItemKind::MacroDef(..) | ItemKind::Mac(..) => {
panic!("Shouldn't still be around")
}
}

// [1] `defaultness.has_value()` is never called for an `impl`, always `true` in order to
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 @@ -1155,7 +1155,7 @@ impl EmitterWriter {
let start = parts[0].snippet.len() - parts[0].snippet.trim_left().len();
let sub_len = parts[0].snippet.trim().len();
let underline_start = span_start_pos.col.0 + start;
let underline_end = span_start_pos.col.0 + sub_len;
let underline_end = span_start_pos.col.0 + start + sub_len;
for p in underline_start..underline_end {
buffer.putc(row_num,
max_line_num_len + 3 + p,
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_resolve/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1947,7 +1947,7 @@ impl<'a> Resolver<'a> {
}
}

ItemKind::ExternCrate(_) | ItemKind::MacroDef(..) | ItemKind::GlobalAsm(_)=> {
ItemKind::ExternCrate(_) | ItemKind::MacroDef(..) | ItemKind::GlobalAsm(_) => {
// do nothing, these are just around to be encoded
}

Expand Down
121 changes: 111 additions & 10 deletions src/libsyntax/parse/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1156,6 +1156,7 @@ impl<'a> Parser<'a> {
None => token::CloseDelim(self.token_cursor.frame.delim),
})
}

fn look_ahead_span(&self, dist: usize) -> Span {
if dist == 0 {
return self.span
Expand Down Expand Up @@ -4268,7 +4269,16 @@ impl<'a> Parser<'a> {
let mut stmts = vec![];

while !self.eat(&token::CloseDelim(token::Brace)) {
if let Some(stmt) = self.parse_full_stmt(false)? {
let stmt = match self.parse_full_stmt(false) {
Err(mut err) => {
err.emit();
self.recover_stmt_(SemiColonMode::Ignore, BlockMode::Break);
self.eat(&token::CloseDelim(token::Brace));
break;
}
Ok(stmt) => stmt,
};
if let Some(stmt) = stmt {
stmts.push(stmt);
} else if self.token == token::Eof {
break;
Expand All @@ -4277,7 +4287,6 @@ impl<'a> Parser<'a> {
continue;
};
}

Ok(P(ast::Block {
stmts,
id: ast::DUMMY_NODE_ID,
Expand Down Expand Up @@ -5325,18 +5334,45 @@ impl<'a> Parser<'a> {
Ok((class_name, ItemKind::Union(vdata, generics), None))
}

fn consume_block(&mut self, delim: token::DelimToken) {
let mut brace_depth = 0;
if !self.eat(&token::OpenDelim(delim)) {
return;
}
loop {
if self.eat(&token::OpenDelim(delim)) {
brace_depth += 1;
} else if self.eat(&token::CloseDelim(delim)) {
if brace_depth == 0 {
return;
} else {
brace_depth -= 1;
continue;
}
} else if self.eat(&token::Eof) || self.eat(&token::CloseDelim(token::NoDelim)) {
return;
} else {
self.bump();
}
}
}

pub fn parse_record_struct_body(&mut self) -> PResult<'a, Vec<StructField>> {
let mut fields = Vec::new();
if self.eat(&token::OpenDelim(token::Brace)) {
while self.token != token::CloseDelim(token::Brace) {
fields.push(self.parse_struct_decl_field().map_err(|e| {
let field = self.parse_struct_decl_field().map_err(|e| {
self.recover_stmt();
self.eat(&token::CloseDelim(token::Brace));
e
})?);
});
match field {
Ok(field) => fields.push(field),
Err(mut err) => {
err.emit();
}
}
}

self.bump();
self.eat(&token::CloseDelim(token::Brace));
} else {
let token_str = self.this_token_to_string();
return Err(self.fatal(&format!("expected `where`, or `{{` after struct \
Expand Down Expand Up @@ -5384,8 +5420,15 @@ impl<'a> Parser<'a> {
self.bump();
}
token::CloseDelim(token::Brace) => {}
token::DocComment(_) => return Err(self.span_fatal_err(self.span,
Error::UselessDocComment)),
token::DocComment(_) => {
let mut err = self.span_fatal_err(self.span, Error::UselessDocComment);
self.bump(); // consume the doc comment
if self.eat(&token::Comma) || self.token == token::CloseDelim(token::Brace) {
err.emit();
} else {
return Err(err);
}
}
_ => return Err(self.span_fatal_help(self.span,
&format!("expected `,`, or `}}`, found `{}`", self.this_token_to_string()),
"struct fields should be separated by commas")),
Expand Down Expand Up @@ -6236,7 +6279,65 @@ impl<'a> Parser<'a> {
return Ok(Some(macro_def));
}

self.parse_macro_use_or_failure(attrs,macros_allowed,attributes_allowed,lo,visibility)
// Verify wether we have encountered a struct or method definition where the user forgot to
// add the `struct` or `fn` keyword after writing `pub`: `pub S {}`
if visibility == Visibility::Public &&
self.check_ident() &&
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't it possible these days to have paths to macros? e.g., foo::bar!?

cc @jseyfried @petrochenkov

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The way the fall back happens, if the code isn't either pub foo ( or pub foo {, the current macro handling will happen. In the case of pub foo::bar this new branch will not be executed. As long as parse_macro_use_or_failure does the right thing with paths, this should be fine :)

self.look_ahead(1, |t| *t != token::Not)
{
// Space between `pub` keyword and the identifier
//
// pub S {}
// ^^^ `sp` points here
let sp = self.prev_span.between(self.span);
let full_sp = self.prev_span.to(self.span);
let ident_sp = self.span;
if self.look_ahead(1, |t| *t == token::OpenDelim(token::Brace)) {
// possible public struct definition where `struct` was forgotten
let ident = self.parse_ident().unwrap();
let msg = format!("add `struct` here to parse `{}` as a public struct",
ident);
let mut err = self.diagnostic()
.struct_span_err(sp, "missing `struct` for struct definition");
err.span_suggestion_short(sp, &msg, " struct ".into());
return Err(err);
} else if self.look_ahead(1, |t| *t == token::OpenDelim(token::Paren)) {
let ident = self.parse_ident().unwrap();
self.consume_block(token::Paren);
let (kw, kw_name, ambiguous) = if self.check(&token::RArrow) ||
self.check(&token::OpenDelim(token::Brace))
{
("fn", "method", false)
} else if self.check(&token::Colon) {
Copy link
Contributor

Choose a reason for hiding this comment

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

@estebank sorry for the enormous necropost, but do you know what the idea behind this was? It matches pub foo(stuff): and suggests adding struct, but where does that colon come from? Is it possible this was intended to be token::Semi instead? The behaviour isn't covered by any tests, unfortunately.

let kw = "struct";
(kw, kw, false)
} else {
("fn` or `struct", "method or struct", true)
};

let msg = format!("missing `{}` for {} definition", kw, kw_name);
let mut err = self.diagnostic().struct_span_err(sp, &msg);
if !ambiguous {
let suggestion = format!("add `{}` here to parse `{}` as a public {}",
kw,
ident,
kw_name);
err.span_suggestion_short(sp, &suggestion, format!(" {} ", kw));
} else {
if let Ok(snippet) = self.sess.codemap().span_to_snippet(ident_sp) {
err.span_suggestion(
full_sp,
"if you meant to call a macro, write instead",
format!("{}!", snippet));
} else {
err.help("if you meant to call a macro, remove the `pub` \
and add a trailing `!` after the identifier");
}
}
return Err(err);
}
}
self.parse_macro_use_or_failure(attrs, macros_allowed, attributes_allowed, lo, visibility)
}

/// Parse a foreign item.
Expand Down
8 changes: 7 additions & 1 deletion src/libsyntax_ext/deriving/custom.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,12 +96,18 @@ impl MultiItemModifier for ProcMacroDerive {
}
};

let error_count_before = ecx.parse_sess.span_diagnostic.err_count();
__internal::set_sess(ecx, || {
let msg = "proc-macro derive produced unparseable tokens";
match __internal::token_stream_parse_items(stream) {
// fail if there have been errors emitted
Ok(_) if ecx.parse_sess.span_diagnostic.err_count() > error_count_before => {
ecx.struct_span_fatal(span, msg).emit();
panic!(FatalError);
}
Ok(new_items) => new_items.into_iter().map(Annotatable::Item).collect(),
Err(_) => {
// FIXME: handle this better
let msg = "proc-macro derive produced unparseable tokens";
ecx.struct_span_fatal(span, msg).emit();
panic!(FatalError);
}
Expand Down
3 changes: 2 additions & 1 deletion src/test/compile-fail-fulldeps/proc-macro/derive-bad.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ extern crate derive_bad;
#[derive(
A
)]
//~^^ ERROR: proc-macro derive produced unparseable tokens
//~^^ ERROR proc-macro derive produced unparseable tokens
//~| ERROR expected `:`, found `}`
struct A;

fn main() {}
10 changes: 9 additions & 1 deletion src/test/parse-fail/doc-after-struct-field.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,20 @@
// except according to those terms.

// compile-flags: -Z continue-parse-after-error

struct X {
a: u8 /** document a */,
//~^ ERROR found a documentation comment that doesn't document anything
//~| HELP maybe a comment was intended
}

struct Y {
a: u8 /// document a
//~^ ERROR found a documentation comment that doesn't document anything
//~| HELP maybe a comment was intended
}

fn main() {
let y = X {a = 1};
let x = X { a: 1 };
let y = Y { a: 1 };
}
2 changes: 1 addition & 1 deletion src/test/parse-fail/doc-before-struct-rbrace-1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,5 +17,5 @@ struct X {
}

fn main() {
let y = X {a = 1};
let y = X {a: 1};
}
2 changes: 1 addition & 1 deletion src/test/parse-fail/doc-before-struct-rbrace-2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,5 +16,5 @@ struct X {
}

fn main() {
let y = X {a = 1};
let y = X {a: 1};
}
1 change: 1 addition & 0 deletions src/test/parse-fail/issue-22647.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ fn main() {
println!("Y {}",x);
return x;
};
//~^ ERROR expected item, found `;`

caller(bar_handler);
}
Expand Down
2 changes: 1 addition & 1 deletion src/test/parse-fail/issue-37234.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
macro_rules! failed {
() => {{
let x = 5 ""; //~ ERROR found `""`
}} //~ ERROR macro expansion ignores token `}`
}}
}

fn main() {
Expand Down
1 change: 1 addition & 0 deletions src/test/parse-fail/mut-patterns.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,5 @@
pub fn main() {
struct Foo { x: isize }
let mut Foo { x: x } = Foo { x: 3 }; //~ ERROR: expected one of `:`, `;`, `=`, or `@`, found `{`
//~^ ERROR expected item, found `=`
}
2 changes: 1 addition & 1 deletion src/test/parse-fail/pat-lt-bracket-5.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,5 @@
// except according to those terms.

fn main() {
let v[0] = v[1]; //~ error: expected one of `:`, `;`, `=`, or `@`, found `[`
let v[0] = v[1]; //~ ERROR expected one of `:`, `;`, `=`, or `@`, found `[`
}
2 changes: 1 addition & 1 deletion src/test/ui/pub/pub-restricted-error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,4 @@ struct Foo {
pub(crate) () foo: usize,
}


fn main() {}
16 changes: 16 additions & 0 deletions src/test/ui/suggestions/pub-ident-fn-2.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// Copyright 2017 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.

pub foo(s: usize) { bar() }
//~^ ERROR missing `fn` for method definition

fn main() {
foo(2);
}
12 changes: 12 additions & 0 deletions src/test/ui/suggestions/pub-ident-fn-2.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
error: missing `fn` for method definition
--> $DIR/pub-ident-fn-2.rs:11:4
|
11 | pub foo(s: usize) { bar() }
| ^
help: add `fn` here to parse `foo` as a public method
|
11 | pub fn foo(s: usize) { bar() }
| ^^

error: aborting due to previous error

14 changes: 14 additions & 0 deletions src/test/ui/suggestions/pub-ident-fn-or-struct-2.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// Copyright 2017 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.

pub S();
//~^ ERROR missing `fn` or `struct` for method or struct definition

fn main() {}
8 changes: 8 additions & 0 deletions src/test/ui/suggestions/pub-ident-fn-or-struct-2.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
error: missing `fn` or `struct` for method or struct definition
--> $DIR/pub-ident-fn-or-struct-2.rs:11:4
|
11 | pub S();
| ---^- help: if you meant to call a macro, write instead: `S!`

error: aborting due to previous error

14 changes: 14 additions & 0 deletions src/test/ui/suggestions/pub-ident-fn-or-struct.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// Copyright 2017 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.

pub S (foo) bar
//~^ ERROR missing `fn` or `struct` for method or struct definition

fn main() {}
8 changes: 8 additions & 0 deletions src/test/ui/suggestions/pub-ident-fn-or-struct.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
error: missing `fn` or `struct` for method or struct definition
--> $DIR/pub-ident-fn-or-struct.rs:11:4
|
11 | pub S (foo) bar
| ---^- help: if you meant to call a macro, write instead: `S!`

error: aborting due to previous error

16 changes: 16 additions & 0 deletions src/test/ui/suggestions/pub-ident-fn.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// Copyright 2017 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.

pub foo(s: usize) -> bool { true }
//~^ ERROR missing `fn` for method definition

fn main() {
foo(2);
}
Loading