Skip to content

Commit

Permalink
Auto merge of #87235 - poliorcetics:issue-87217-fn-quali-order, r=nagisa
Browse files Browse the repository at this point in the history
Improve diagnostics for wrongly ordered keywords in function declaration

Fix #87217

`@rustbot` label A-diagnostics T-compiler
  • Loading branch information
bors committed Aug 8, 2021
2 parents ad981d5 + 690cbb7 commit 74a11c6
Show file tree
Hide file tree
Showing 13 changed files with 184 additions and 4 deletions.
35 changes: 34 additions & 1 deletion compiler/rustc_parse/src/parser/item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1768,8 +1768,14 @@ impl<'a> Parser<'a> {
pub(super) fn parse_fn_front_matter(&mut self) -> PResult<'a, FnHeader> {
let sp_start = self.token.span;
let constness = self.parse_constness();

let async_start_sp = self.token.span;
let asyncness = self.parse_asyncness();

let unsafe_start_sp = self.token.span;
let unsafety = self.parse_unsafety();

let ext_start_sp = self.token.span;
let ext = self.parse_extern();

if let Async::Yes { span, .. } = asyncness {
Expand All @@ -1784,8 +1790,35 @@ impl<'a> Parser<'a> {
Ok(true) => {}
Ok(false) => unreachable!(),
Err(mut err) => {
// Qualifier keywords ordering check

// This will allow the machine fix to directly place the keyword in the correct place
let current_qual_sp = if self.check_keyword(kw::Const) {
Some(async_start_sp)
} else if self.check_keyword(kw::Async) {
Some(unsafe_start_sp)
} else if self.check_keyword(kw::Unsafe) {
Some(ext_start_sp)
} else {
None
};

if let Some(current_qual_sp) = current_qual_sp {
let current_qual_sp = current_qual_sp.to(self.prev_token.span);
if let Ok(current_qual) = self.span_to_snippet(current_qual_sp) {
let invalid_qual_sp = self.token.uninterpolated_span();
let invalid_qual = self.span_to_snippet(invalid_qual_sp).unwrap();

err.span_suggestion(
current_qual_sp.to(invalid_qual_sp),
&format!("`{}` must come before `{}`", invalid_qual, current_qual),
format!("{} {}", invalid_qual, current_qual),
Applicability::MachineApplicable,
).note("keyword order for functions declaration is `default`, `pub`, `const`, `async`, `unsafe`, `extern`");
}
}
// Recover incorrect visibility order such as `async pub`.
if self.check_keyword(kw::Pub) {
else if self.check_keyword(kw::Pub) {
let sp = sp_start.to(self.prev_token.span);
if let Ok(snippet) = self.span_to_snippet(sp) {
let vis = match self.parse_visibility(FollowedByType::No) {
Expand Down
7 changes: 6 additions & 1 deletion src/test/ui/async-await/no-async-const.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,12 @@ error: expected one of `extern`, `fn`, or `unsafe`, found keyword `const`
--> $DIR/no-async-const.rs:4:11
|
LL | pub async const fn x() {}
| ^^^^^ expected one of `extern`, `fn`, or `unsafe`
| ------^^^^^
| | |
| | expected one of `extern`, `fn`, or `unsafe`
| help: `const` must come before `async`: `const async`
|
= note: keyword order for functions declaration is `default`, `pub`, `const`, `async`, `unsafe`, `extern`

error: aborting due to previous error

14 changes: 12 additions & 2 deletions src/test/ui/async-await/no-unsafe-async.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,25 @@ LL | impl S {
| - while parsing this item list starting here
LL | #[cfg(FALSE)]
LL | unsafe async fn g() {}
| ^^^^^ expected one of `extern` or `fn`
| -------^^^^^
| | |
| | expected one of `extern` or `fn`
| help: `async` must come before `unsafe`: `async unsafe`
LL | }
| - the item list ends here
|
= note: keyword order for functions declaration is `default`, `pub`, `const`, `async`, `unsafe`, `extern`

error: expected one of `extern` or `fn`, found keyword `async`
--> $DIR/no-unsafe-async.rs:11:8
|
LL | unsafe async fn f() {}
| ^^^^^ expected one of `extern` or `fn`
| -------^^^^^
| | |
| | expected one of `extern` or `fn`
| help: `async` must come before `unsafe`: `async unsafe`
|
= note: keyword order for functions declaration is `default`, `pub`, `const`, `async`, `unsafe`, `extern`

error: aborting due to 2 previous errors

11 changes: 11 additions & 0 deletions src/test/ui/parser/issue-87217-keyword-order/const-async-const.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// edition:2018

// Test that even when `const` is already present, the proposed fix is `const const async`,
// like for `pub pub`.

const async const fn test() {}
//~^ ERROR expected one of `extern`, `fn`, or `unsafe`, found keyword `const`
//~| NOTE expected one of `extern`, `fn`, or `unsafe`
//~| HELP `const` must come before `async`
//~| SUGGESTION const async
//~| NOTE keyword order for functions declaration is `default`, `pub`, `const`, `async`, `unsafe`, `extern`
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
error: expected one of `extern`, `fn`, or `unsafe`, found keyword `const`
--> $DIR/const-async-const.rs:6:13
|
LL | const async const fn test() {}
| ------^^^^^
| | |
| | expected one of `extern`, `fn`, or `unsafe`
| help: `const` must come before `async`: `const async`
|
= note: keyword order for functions declaration is `default`, `pub`, `const`, `async`, `unsafe`, `extern`

error: aborting due to previous error

14 changes: 14 additions & 0 deletions src/test/ui/parser/issue-87217-keyword-order/several-kw-jump.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// edition:2018

// There is an order to respect for keywords before a function:
// `<visibility>, const, async, unsafe, extern, "<ABI>"`
//
// This test ensures the compiler is helpful about them being misplaced.
// Visibilities are tested elsewhere.

async unsafe const fn test() {}
//~^ ERROR expected one of `extern` or `fn`, found keyword `const`
//~| NOTE expected one of `extern` or `fn`
//~| HELP `const` must come before `async unsafe`
//~| SUGGESTION const async unsafe
//~| NOTE keyword order for functions declaration is `default`, `pub`, `const`, `async`, `unsafe`, `extern`
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
error: expected one of `extern` or `fn`, found keyword `const`
--> $DIR/several-kw-jump.rs:9:14
|
LL | async unsafe const fn test() {}
| -------------^^^^^
| | |
| | expected one of `extern` or `fn`
| help: `const` must come before `async unsafe`: `const async unsafe`
|
= note: keyword order for functions declaration is `default`, `pub`, `const`, `async`, `unsafe`, `extern`

error: aborting due to previous error

14 changes: 14 additions & 0 deletions src/test/ui/parser/issue-87217-keyword-order/wrong-async.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// edition:2018

// There is an order to respect for keywords before a function:
// `<visibility>, const, async, unsafe, extern, "<ABI>"`
//
// This test ensures the compiler is helpful about them being misplaced.
// Visibilities are tested elsewhere.

unsafe async fn test() {}
//~^ ERROR expected one of `extern` or `fn`, found keyword `async`
//~| NOTE expected one of `extern` or `fn`
//~| HELP `async` must come before `unsafe`
//~| SUGGESTION async unsafe
//~| NOTE keyword order for functions declaration is `default`, `pub`, `const`, `async`, `unsafe`, `extern`
13 changes: 13 additions & 0 deletions src/test/ui/parser/issue-87217-keyword-order/wrong-async.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
error: expected one of `extern` or `fn`, found keyword `async`
--> $DIR/wrong-async.rs:9:8
|
LL | unsafe async fn test() {}
| -------^^^^^
| | |
| | expected one of `extern` or `fn`
| help: `async` must come before `unsafe`: `async unsafe`
|
= note: keyword order for functions declaration is `default`, `pub`, `const`, `async`, `unsafe`, `extern`

error: aborting due to previous error

14 changes: 14 additions & 0 deletions src/test/ui/parser/issue-87217-keyword-order/wrong-const.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// edition:2018

// There is an order to respect for keywords before a function:
// `<visibility>, const, async, unsafe, extern, "<ABI>"`
//
// This test ensures the compiler is helpful about them being misplaced.
// Visibilities are tested elsewhere.

unsafe const fn test() {}
//~^ ERROR expected one of `extern` or `fn`, found keyword `const`
//~| NOTE expected one of `extern` or `fn`
//~| HELP `const` must come before `unsafe`
//~| SUGGESTION const unsafe
//~| NOTE keyword order for functions declaration is `default`, `pub`, `const`, `async`, `unsafe`, `extern`
13 changes: 13 additions & 0 deletions src/test/ui/parser/issue-87217-keyword-order/wrong-const.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
error: expected one of `extern` or `fn`, found keyword `const`
--> $DIR/wrong-const.rs:9:8
|
LL | unsafe const fn test() {}
| -------^^^^^
| | |
| | expected one of `extern` or `fn`
| help: `const` must come before `unsafe`: `const unsafe`
|
= note: keyword order for functions declaration is `default`, `pub`, `const`, `async`, `unsafe`, `extern`

error: aborting due to previous error

14 changes: 14 additions & 0 deletions src/test/ui/parser/issue-87217-keyword-order/wrong-unsafe.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// edition:2018

// There is an order to respect for keywords before a function:
// `<visibility>, const, async, unsafe, extern, "<ABI>"`
//
// This test ensures the compiler is helpful about them being misplaced.
// Visibilities are tested elsewhere.

extern unsafe fn test() {}
//~^ ERROR expected `fn`, found keyword `unsafe`
//~| NOTE expected `fn`
//~| HELP `unsafe` must come before `extern`
//~| SUGGESTION unsafe extern
//~| NOTE keyword order for functions declaration is `default`, `pub`, `const`, `async`, `unsafe`, `extern`
13 changes: 13 additions & 0 deletions src/test/ui/parser/issue-87217-keyword-order/wrong-unsafe.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
error: expected `fn`, found keyword `unsafe`
--> $DIR/wrong-unsafe.rs:9:8
|
LL | extern unsafe fn test() {}
| -------^^^^^^
| | |
| | expected `fn`
| help: `unsafe` must come before `extern`: `unsafe extern`
|
= note: keyword order for functions declaration is `default`, `pub`, `const`, `async`, `unsafe`, `extern`

error: aborting due to previous error

0 comments on commit 74a11c6

Please sign in to comment.