Skip to content

Commit 690cbb7

Browse files
poliorceticspoliorcetics
committed
Better message for invalid keyword placement in fn
After this commit, `unsafe async fn ...` now suggests the `async unsafe` fix instead of misunderstanding the issue. This is not perfect for repeated keywords (`const async const`) and for keywords that are misplaced after `extern "some abi"` because of the way `check_fn_font_matter` works, but changing it breaks so many tests and diagnostics it has been judged too high a cost for this PR.
1 parent f381e77 commit 690cbb7

File tree

13 files changed

+184
-4
lines changed

13 files changed

+184
-4
lines changed

‎compiler/rustc_parse/src/parser/item.rs

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1771,8 +1771,14 @@ impl<'a> Parser<'a> {
17711771
pub(super) fn parse_fn_front_matter(&mut self) -> PResult<'a, FnHeader> {
17721772
let sp_start = self.token.span;
17731773
let constness = self.parse_constness();
1774+
1775+
let async_start_sp = self.token.span;
17741776
let asyncness = self.parse_asyncness();
1777+
1778+
let unsafe_start_sp = self.token.span;
17751779
let unsafety = self.parse_unsafety();
1780+
1781+
let ext_start_sp = self.token.span;
17761782
let ext = self.parse_extern();
17771783

17781784
if let Async::Yes { span, .. } = asyncness {
@@ -1787,8 +1793,35 @@ impl<'a> Parser<'a> {
17871793
Ok(true) => {}
17881794
Ok(false) => unreachable!(),
17891795
Err(mut err) => {
1796+
// Qualifier keywords ordering check
1797+
1798+
// This will allow the machine fix to directly place the keyword in the correct place
1799+
let current_qual_sp = if self.check_keyword(kw::Const) {
1800+
Some(async_start_sp)
1801+
} else if self.check_keyword(kw::Async) {
1802+
Some(unsafe_start_sp)
1803+
} else if self.check_keyword(kw::Unsafe) {
1804+
Some(ext_start_sp)
1805+
} else {
1806+
None
1807+
};
1808+
1809+
if let Some(current_qual_sp) = current_qual_sp {
1810+
let current_qual_sp = current_qual_sp.to(self.prev_token.span);
1811+
if let Ok(current_qual) = self.span_to_snippet(current_qual_sp) {
1812+
let invalid_qual_sp = self.token.uninterpolated_span();
1813+
let invalid_qual = self.span_to_snippet(invalid_qual_sp).unwrap();
1814+
1815+
err.span_suggestion(
1816+
current_qual_sp.to(invalid_qual_sp),
1817+
&format!("`{}` must come before `{}`", invalid_qual, current_qual),
1818+
format!("{} {}", invalid_qual, current_qual),
1819+
Applicability::MachineApplicable,
1820+
).note("keyword order for functions declaration is `default`, `pub`, `const`, `async`, `unsafe`, `extern`");
1821+
}
1822+
}
17901823
// Recover incorrect visibility order such as `async pub`.
1791-
if self.check_keyword(kw::Pub) {
1824+
else if self.check_keyword(kw::Pub) {
17921825
let sp = sp_start.to(self.prev_token.span);
17931826
if let Ok(snippet) = self.span_to_snippet(sp) {
17941827
let vis = match self.parse_visibility(FollowedByType::No) {

‎src/test/ui/async-await/no-async-const.stderr

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,12 @@ error: expected one of `extern`, `fn`, or `unsafe`, found keyword `const`
22
--> $DIR/no-async-const.rs:4:11
33
|
44
LL | pub async const fn x() {}
5-
| ^^^^^ expected one of `extern`, `fn`, or `unsafe`
5+
| ------^^^^^
6+
| | |
7+
| | expected one of `extern`, `fn`, or `unsafe`
8+
| help: `const` must come before `async`: `const async`
9+
|
10+
= note: keyword order for functions declaration is `default`, `pub`, `const`, `async`, `unsafe`, `extern`
611

712
error: aborting due to previous error
813

‎src/test/ui/async-await/no-unsafe-async.stderr

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,15 +5,25 @@ LL | impl S {
55
| - while parsing this item list starting here
66
LL | #[cfg(FALSE)]
77
LL | unsafe async fn g() {}
8-
| ^^^^^ expected one of `extern` or `fn`
8+
| -------^^^^^
9+
| | |
10+
| | expected one of `extern` or `fn`
11+
| help: `async` must come before `unsafe`: `async unsafe`
912
LL | }
1013
| - the item list ends here
14+
|
15+
= note: keyword order for functions declaration is `default`, `pub`, `const`, `async`, `unsafe`, `extern`
1116

1217
error: expected one of `extern` or `fn`, found keyword `async`
1318
--> $DIR/no-unsafe-async.rs:11:8
1419
|
1520
LL | unsafe async fn f() {}
16-
| ^^^^^ expected one of `extern` or `fn`
21+
| -------^^^^^
22+
| | |
23+
| | expected one of `extern` or `fn`
24+
| help: `async` must come before `unsafe`: `async unsafe`
25+
|
26+
= note: keyword order for functions declaration is `default`, `pub`, `const`, `async`, `unsafe`, `extern`
1727

1828
error: aborting due to 2 previous errors
1929

‎src/test/ui/parser/issue-87217-keyword-order/const-async-const.rs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
// edition:2018
2+
3+
// Test that even when `const` is already present, the proposed fix is `const const async`,
4+
// like for `pub pub`.
5+
6+
const async const fn test() {}
7+
//~^ ERROR expected one of `extern`, `fn`, or `unsafe`, found keyword `const`
8+
//~| NOTE expected one of `extern`, `fn`, or `unsafe`
9+
//~| HELP `const` must come before `async`
10+
//~| SUGGESTION const async
11+
//~| NOTE keyword order for functions declaration is `default`, `pub`, `const`, `async`, `unsafe`, `extern`

‎src/test/ui/parser/issue-87217-keyword-order/const-async-const.stderr

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
error: expected one of `extern`, `fn`, or `unsafe`, found keyword `const`
2+
--> $DIR/const-async-const.rs:6:13
3+
|
4+
LL | const async const fn test() {}
5+
| ------^^^^^
6+
| | |
7+
| | expected one of `extern`, `fn`, or `unsafe`
8+
| help: `const` must come before `async`: `const async`
9+
|
10+
= note: keyword order for functions declaration is `default`, `pub`, `const`, `async`, `unsafe`, `extern`
11+
12+
error: aborting due to previous error
13+

‎src/test/ui/parser/issue-87217-keyword-order/several-kw-jump.rs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
// edition:2018
2+
3+
// There is an order to respect for keywords before a function:
4+
// `<visibility>, const, async, unsafe, extern, "<ABI>"`
5+
//
6+
// This test ensures the compiler is helpful about them being misplaced.
7+
// Visibilities are tested elsewhere.
8+
9+
async unsafe const fn test() {}
10+
//~^ ERROR expected one of `extern` or `fn`, found keyword `const`
11+
//~| NOTE expected one of `extern` or `fn`
12+
//~| HELP `const` must come before `async unsafe`
13+
//~| SUGGESTION const async unsafe
14+
//~| NOTE keyword order for functions declaration is `default`, `pub`, `const`, `async`, `unsafe`, `extern`

‎src/test/ui/parser/issue-87217-keyword-order/several-kw-jump.stderr

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
error: expected one of `extern` or `fn`, found keyword `const`
2+
--> $DIR/several-kw-jump.rs:9:14
3+
|
4+
LL | async unsafe const fn test() {}
5+
| -------------^^^^^
6+
| | |
7+
| | expected one of `extern` or `fn`
8+
| help: `const` must come before `async unsafe`: `const async unsafe`
9+
|
10+
= note: keyword order for functions declaration is `default`, `pub`, `const`, `async`, `unsafe`, `extern`
11+
12+
error: aborting due to previous error
13+

‎src/test/ui/parser/issue-87217-keyword-order/wrong-async.rs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
// edition:2018
2+
3+
// There is an order to respect for keywords before a function:
4+
// `<visibility>, const, async, unsafe, extern, "<ABI>"`
5+
//
6+
// This test ensures the compiler is helpful about them being misplaced.
7+
// Visibilities are tested elsewhere.
8+
9+
unsafe async fn test() {}
10+
//~^ ERROR expected one of `extern` or `fn`, found keyword `async`
11+
//~| NOTE expected one of `extern` or `fn`
12+
//~| HELP `async` must come before `unsafe`
13+
//~| SUGGESTION async unsafe
14+
//~| NOTE keyword order for functions declaration is `default`, `pub`, `const`, `async`, `unsafe`, `extern`

‎src/test/ui/parser/issue-87217-keyword-order/wrong-async.stderr

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
error: expected one of `extern` or `fn`, found keyword `async`
2+
--> $DIR/wrong-async.rs:9:8
3+
|
4+
LL | unsafe async fn test() {}
5+
| -------^^^^^
6+
| | |
7+
| | expected one of `extern` or `fn`
8+
| help: `async` must come before `unsafe`: `async unsafe`
9+
|
10+
= note: keyword order for functions declaration is `default`, `pub`, `const`, `async`, `unsafe`, `extern`
11+
12+
error: aborting due to previous error
13+

‎src/test/ui/parser/issue-87217-keyword-order/wrong-const.rs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
// edition:2018
2+
3+
// There is an order to respect for keywords before a function:
4+
// `<visibility>, const, async, unsafe, extern, "<ABI>"`
5+
//
6+
// This test ensures the compiler is helpful about them being misplaced.
7+
// Visibilities are tested elsewhere.
8+
9+
unsafe const fn test() {}
10+
//~^ ERROR expected one of `extern` or `fn`, found keyword `const`
11+
//~| NOTE expected one of `extern` or `fn`
12+
//~| HELP `const` must come before `unsafe`
13+
//~| SUGGESTION const unsafe
14+
//~| NOTE keyword order for functions declaration is `default`, `pub`, `const`, `async`, `unsafe`, `extern`

‎src/test/ui/parser/issue-87217-keyword-order/wrong-const.stderr

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
error: expected one of `extern` or `fn`, found keyword `const`
2+
--> $DIR/wrong-const.rs:9:8
3+
|
4+
LL | unsafe const fn test() {}
5+
| -------^^^^^
6+
| | |
7+
| | expected one of `extern` or `fn`
8+
| help: `const` must come before `unsafe`: `const unsafe`
9+
|
10+
= note: keyword order for functions declaration is `default`, `pub`, `const`, `async`, `unsafe`, `extern`
11+
12+
error: aborting due to previous error
13+

‎src/test/ui/parser/issue-87217-keyword-order/wrong-unsafe.rs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
// edition:2018
2+
3+
// There is an order to respect for keywords before a function:
4+
// `<visibility>, const, async, unsafe, extern, "<ABI>"`
5+
//
6+
// This test ensures the compiler is helpful about them being misplaced.
7+
// Visibilities are tested elsewhere.
8+
9+
extern unsafe fn test() {}
10+
//~^ ERROR expected `fn`, found keyword `unsafe`
11+
//~| NOTE expected `fn`
12+
//~| HELP `unsafe` must come before `extern`
13+
//~| SUGGESTION unsafe extern
14+
//~| NOTE keyword order for functions declaration is `default`, `pub`, `const`, `async`, `unsafe`, `extern`

‎src/test/ui/parser/issue-87217-keyword-order/wrong-unsafe.stderr

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
error: expected `fn`, found keyword `unsafe`
2+
--> $DIR/wrong-unsafe.rs:9:8
3+
|
4+
LL | extern unsafe fn test() {}
5+
| -------^^^^^^
6+
| | |
7+
| | expected `fn`
8+
| help: `unsafe` must come before `extern`: `unsafe extern`
9+
|
10+
= note: keyword order for functions declaration is `default`, `pub`, `const`, `async`, `unsafe`, `extern`
11+
12+
error: aborting due to previous error
13+

0 commit comments

Comments
 (0)