From 9321efd8f7138e99c69d134fda24b7bf9dc0c95b Mon Sep 17 00:00:00 2001 From: Ivan Tham Date: Mon, 7 Sep 2020 22:27:00 +0800 Subject: [PATCH] Detect pub fn attr wrong order like `async pub` Redirects `const? async? unsafe? pub` to `pub const? async? unsafe?`. Fix #76437 --- compiler/rustc_parse/src/parser/item.rs | 27 ++++++++++++++-- src/test/ui/parser/default.rs | 4 +-- src/test/ui/parser/default.stderr | 31 ++++++++----------- src/test/ui/parser/duplicate-visibility.rs | 5 +-- .../ui/parser/duplicate-visibility.stderr | 25 ++++++--------- src/test/ui/parser/issue-63116.stderr | 4 +-- src/test/ui/parser/issue-76437-async.rs | 6 ++++ src/test/ui/parser/issue-76437-async.stderr | 11 +++++++ .../parser/issue-76437-const-async-unsafe.rs | 6 ++++ .../issue-76437-const-async-unsafe.stderr | 11 +++++++ src/test/ui/parser/issue-76437-const-async.rs | 6 ++++ .../ui/parser/issue-76437-const-async.stderr | 11 +++++++ src/test/ui/parser/issue-76437-const.rs | 6 ++++ src/test/ui/parser/issue-76437-const.stderr | 11 +++++++ .../ui/parser/issue-76437-pub-crate-unsafe.rs | 6 ++++ .../issue-76437-pub-crate-unsafe.stderr | 11 +++++++ src/test/ui/parser/issue-76437-unsafe.rs | 6 ++++ src/test/ui/parser/issue-76437-unsafe.stderr | 11 +++++++ 18 files changed, 156 insertions(+), 42 deletions(-) create mode 100644 src/test/ui/parser/issue-76437-async.rs create mode 100644 src/test/ui/parser/issue-76437-async.stderr create mode 100644 src/test/ui/parser/issue-76437-const-async-unsafe.rs create mode 100644 src/test/ui/parser/issue-76437-const-async-unsafe.stderr create mode 100644 src/test/ui/parser/issue-76437-const-async.rs create mode 100644 src/test/ui/parser/issue-76437-const-async.stderr create mode 100644 src/test/ui/parser/issue-76437-const.rs create mode 100644 src/test/ui/parser/issue-76437-const.stderr create mode 100644 src/test/ui/parser/issue-76437-pub-crate-unsafe.rs create mode 100644 src/test/ui/parser/issue-76437-pub-crate-unsafe.stderr create mode 100644 src/test/ui/parser/issue-76437-unsafe.rs create mode 100644 src/test/ui/parser/issue-76437-unsafe.stderr diff --git a/compiler/rustc_parse/src/parser/item.rs b/compiler/rustc_parse/src/parser/item.rs index a28595e6fae..2dc4edf9a14 100644 --- a/compiler/rustc_parse/src/parser/item.rs +++ b/compiler/rustc_parse/src/parser/item.rs @@ -1639,7 +1639,8 @@ fn parse_fn_body( pub(super) fn check_fn_front_matter(&mut self) -> bool { // We use an over-approximation here. // `const const`, `fn const` won't parse, but we're not stepping over other syntax either. - const QUALS: [Symbol; 4] = [kw::Const, kw::Async, kw::Unsafe, kw::Extern]; + // `pub` is added in case users got confused with the ordering like `async pub fn`. + const QUALS: [Symbol; 5] = [kw::Pub, kw::Const, kw::Async, kw::Unsafe, kw::Extern]; self.check_keyword(kw::Fn) // Definitely an `fn`. // `$qual fn` or `$qual $qual`: || QUALS.iter().any(|&kw| self.check_keyword(kw)) @@ -1668,6 +1669,7 @@ pub(super) fn check_fn_front_matter(&mut self) -> bool { /// FnFrontMatter = FnQual "fn" ; /// ``` pub(super) fn parse_fn_front_matter(&mut self) -> PResult<'a, FnHeader> { + let sp_start = self.token.span; let constness = self.parse_constness(); let asyncness = self.parse_asyncness(); let unsafety = self.parse_unsafety(); @@ -1681,8 +1683,27 @@ pub(super) fn parse_fn_front_matter(&mut self) -> PResult<'a, FnHeader> { // It is possible for `expect_one_of` to recover given the contents of // `self.expected_tokens`, therefore, do not use `self.unexpected()` which doesn't // account for this. - if !self.expect_one_of(&[], &[])? { - unreachable!() + match self.expect_one_of(&[], &[]) { + Ok(true) => {} + Ok(false) => unreachable!(), + Err(mut err) => { + // Recover incorrect visibility order such as `async pub`. + 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 = self.parse_visibility(FollowedByType::No)?; + let vs = pprust::vis_to_string(&vis); + let vs = vs.trim_end(); + err.span_suggestion( + sp_start.to(self.prev_token.span), + &format!("visibility `{}` must come before `{}`", vs, snippet), + format!("{} {}", vs, snippet), + Applicability::MachineApplicable, + ); + } + } + return Err(err); + } } } diff --git a/src/test/ui/parser/default.rs b/src/test/ui/parser/default.rs index 52338c1f13a..1e31a026d94 100644 --- a/src/test/ui/parser/default.rs +++ b/src/test/ui/parser/default.rs @@ -1,3 +1,4 @@ +// ignore-tidy-linelength // Test successful and unsuccessful parsing of the `default` contextual keyword #![feature(specialization)] @@ -21,8 +22,7 @@ impl Foo for u16 { impl Foo for u32 { //~ ERROR not all trait items implemented, missing: `foo` default pub fn foo() -> T { T::default() } - //~^ ERROR `default` is not followed by an item - //~| ERROR non-item in item list + //~^ ERROR expected one of `async`, `const`, `extern`, `fn`, `pub`, `unsafe`, or `use`, found keyword `pub` } fn main() {} diff --git a/src/test/ui/parser/default.stderr b/src/test/ui/parser/default.stderr index 5b763ae72f5..489613b1e09 100644 --- a/src/test/ui/parser/default.stderr +++ b/src/test/ui/parser/default.stderr @@ -1,30 +1,25 @@ -error: `default` is not followed by an item - --> $DIR/default.rs:23:5 - | -LL | default pub fn foo() -> T { T::default() } - | ^^^^^^^ the `default` qualifier - | - = note: only `fn`, `const`, `type`, or `impl` items may be prefixed by `default` - -error: non-item in item list - --> $DIR/default.rs:23:13 +error: expected one of `async`, `const`, `extern`, `fn`, `pub`, `unsafe`, or `use`, found keyword `pub` + --> $DIR/default.rs:24:13 | LL | impl Foo for u32 { - | - item list starts here + | - while parsing this item list starting here LL | default pub fn foo() -> T { T::default() } - | ^^^ non-item starts here -... + | ^^^ + | | + | expected one of 7 possible tokens + | help: visibility `pub` must come before `default pub`: `pub default pub` +LL | LL | } - | - item list ends here + | - the item list ends here error[E0449]: unnecessary visibility qualifier - --> $DIR/default.rs:17:5 + --> $DIR/default.rs:18:5 | LL | pub default fn foo() -> T { | ^^^ `pub` not permitted here because it's implied warning: the feature `specialization` is incomplete and may not be safe to use and/or cause compiler crashes - --> $DIR/default.rs:3:12 + --> $DIR/default.rs:4:12 | LL | #![feature(specialization)] | ^^^^^^^^^^^^^^ @@ -34,7 +29,7 @@ LL | #![feature(specialization)] = help: consider using `min_specialization` instead, which is more stable and complete error[E0046]: not all trait items implemented, missing: `foo` - --> $DIR/default.rs:22:1 + --> $DIR/default.rs:23:1 | LL | fn foo() -> T; | -------------------------- `foo` from trait @@ -42,7 +37,7 @@ LL | fn foo() -> T; LL | impl Foo for u32 { | ^^^^^^^^^^^^^^^^ missing `foo` in implementation -error: aborting due to 4 previous errors; 1 warning emitted +error: aborting due to 3 previous errors; 1 warning emitted Some errors have detailed explanations: E0046, E0449. For more information about an error, try `rustc --explain E0046`. diff --git a/src/test/ui/parser/duplicate-visibility.rs b/src/test/ui/parser/duplicate-visibility.rs index 547329cfb1b..97f19b3da45 100644 --- a/src/test/ui/parser/duplicate-visibility.rs +++ b/src/test/ui/parser/duplicate-visibility.rs @@ -1,7 +1,8 @@ +// ignore-tidy-linelength + fn main() {} extern "C" { pub pub fn foo(); - //~^ ERROR visibility `pub` is not followed by an item - //~| ERROR non-item in item list + //~^ ERROR expected one of `(`, `async`, `const`, `default`, `extern`, `fn`, `pub`, `unsafe`, or `use`, found keyword `pub` } diff --git a/src/test/ui/parser/duplicate-visibility.stderr b/src/test/ui/parser/duplicate-visibility.stderr index 8d8122292ae..6ac27078ea3 100644 --- a/src/test/ui/parser/duplicate-visibility.stderr +++ b/src/test/ui/parser/duplicate-visibility.stderr @@ -1,21 +1,16 @@ -error: visibility `pub` is not followed by an item - --> $DIR/duplicate-visibility.rs:4:5 - | -LL | pub pub fn foo(); - | ^^^ the visibility - | - = help: you likely meant to define an item, e.g., `pub fn foo() {}` - -error: non-item in item list - --> $DIR/duplicate-visibility.rs:4:9 +error: expected one of `(`, `async`, `const`, `default`, `extern`, `fn`, `pub`, `unsafe`, or `use`, found keyword `pub` + --> $DIR/duplicate-visibility.rs:6:9 | LL | extern "C" { - | - item list starts here + | - while parsing this item list starting here LL | pub pub fn foo(); - | ^^^ non-item starts here -... + | ^^^ + | | + | expected one of 9 possible tokens + | help: visibility `pub` must come before `pub pub`: `pub pub pub` +LL | LL | } - | - item list ends here + | - the item list ends here -error: aborting due to 2 previous errors +error: aborting due to previous error diff --git a/src/test/ui/parser/issue-63116.stderr b/src/test/ui/parser/issue-63116.stderr index e249a93df92..8d46f9c0fc4 100644 --- a/src/test/ui/parser/issue-63116.stderr +++ b/src/test/ui/parser/issue-63116.stderr @@ -1,4 +1,4 @@ -error: this file contains an unclosed delimiter +=rror: this file contains an unclosed delimiter --> $DIR/issue-63116.rs:3:18 | LL | impl W `, `...`, `::`, `:`, `<`, `=`, `>`, `?`, `[`, `_`, `async`, `const`, `dyn`, `extern`, `fn`, `for`, `impl`, `unsafe`, lifetime, or path, found `;` +error: expected one of `!`, `&&`, `&`, `(`, `)`, `*`, `+`, `,`, `->`, `...`, `::`, `<`, `>`, `?`, `[`, `_`, `async`, `const`, `dyn`, `extern`, `fn`, `for`, `impl`, `pub`, `unsafe`, lifetime, or path, found `;` --> $DIR/issue-63116.rs:3:15 | LL | impl W $DIR/issue-76437-async.rs:4:11 + | +LL | async pub fn t() {} + | ------^^^ + | | | + | | expected one of `extern`, `fn`, or `unsafe` + | help: visibility `pub` must come before `async`: `pub async` + +error: aborting due to previous error + diff --git a/src/test/ui/parser/issue-76437-const-async-unsafe.rs b/src/test/ui/parser/issue-76437-const-async-unsafe.rs new file mode 100644 index 00000000000..f9c0c6dbed8 --- /dev/null +++ b/src/test/ui/parser/issue-76437-const-async-unsafe.rs @@ -0,0 +1,6 @@ +// edition:2018 + +mod t { + const async unsafe pub fn t() {} + //~^ ERROR expected one of `extern` or `fn`, found keyword `pub` +} diff --git a/src/test/ui/parser/issue-76437-const-async-unsafe.stderr b/src/test/ui/parser/issue-76437-const-async-unsafe.stderr new file mode 100644 index 00000000000..2e91beda116 --- /dev/null +++ b/src/test/ui/parser/issue-76437-const-async-unsafe.stderr @@ -0,0 +1,11 @@ +error: expected one of `extern` or `fn`, found keyword `pub` + --> $DIR/issue-76437-const-async-unsafe.rs:4:24 + | +LL | const async unsafe pub fn t() {} + | -------------------^^^ + | | | + | | expected one of `extern` or `fn` + | help: visibility `pub` must come before `const async unsafe`: `pub const async unsafe` + +error: aborting due to previous error + diff --git a/src/test/ui/parser/issue-76437-const-async.rs b/src/test/ui/parser/issue-76437-const-async.rs new file mode 100644 index 00000000000..79bf801ece3 --- /dev/null +++ b/src/test/ui/parser/issue-76437-const-async.rs @@ -0,0 +1,6 @@ +// edition:2018 + +mod t { + const async pub fn t() {} + //~^ ERROR expected one of `extern`, `fn`, or `unsafe`, found keyword `pub` +} diff --git a/src/test/ui/parser/issue-76437-const-async.stderr b/src/test/ui/parser/issue-76437-const-async.stderr new file mode 100644 index 00000000000..21b96c14d7d --- /dev/null +++ b/src/test/ui/parser/issue-76437-const-async.stderr @@ -0,0 +1,11 @@ +error: expected one of `extern`, `fn`, or `unsafe`, found keyword `pub` + --> $DIR/issue-76437-const-async.rs:4:17 + | +LL | const async pub fn t() {} + | ------------^^^ + | | | + | | expected one of `extern`, `fn`, or `unsafe` + | help: visibility `pub` must come before `const async`: `pub const async` + +error: aborting due to previous error + diff --git a/src/test/ui/parser/issue-76437-const.rs b/src/test/ui/parser/issue-76437-const.rs new file mode 100644 index 00000000000..5dd151d8b76 --- /dev/null +++ b/src/test/ui/parser/issue-76437-const.rs @@ -0,0 +1,6 @@ +// edition:2018 + +mod t { + const pub fn t() {} + //~^ ERROR expected one of `async`, `extern`, `fn`, or `unsafe`, found keyword `pub` +} diff --git a/src/test/ui/parser/issue-76437-const.stderr b/src/test/ui/parser/issue-76437-const.stderr new file mode 100644 index 00000000000..cf80d9a9037 --- /dev/null +++ b/src/test/ui/parser/issue-76437-const.stderr @@ -0,0 +1,11 @@ +error: expected one of `async`, `extern`, `fn`, or `unsafe`, found keyword `pub` + --> $DIR/issue-76437-const.rs:4:11 + | +LL | const pub fn t() {} + | ------^^^ + | | | + | | expected one of `async`, `extern`, `fn`, or `unsafe` + | help: visibility `pub` must come before `const`: `pub const` + +error: aborting due to previous error + diff --git a/src/test/ui/parser/issue-76437-pub-crate-unsafe.rs b/src/test/ui/parser/issue-76437-pub-crate-unsafe.rs new file mode 100644 index 00000000000..e9269ef8e0f --- /dev/null +++ b/src/test/ui/parser/issue-76437-pub-crate-unsafe.rs @@ -0,0 +1,6 @@ +// edition:2018 + +mod t { + unsafe pub(crate) fn t() {} + //~^ ERROR expected one of `extern` or `fn`, found keyword `pub` +} diff --git a/src/test/ui/parser/issue-76437-pub-crate-unsafe.stderr b/src/test/ui/parser/issue-76437-pub-crate-unsafe.stderr new file mode 100644 index 00000000000..fa8f13721c8 --- /dev/null +++ b/src/test/ui/parser/issue-76437-pub-crate-unsafe.stderr @@ -0,0 +1,11 @@ +error: expected one of `extern` or `fn`, found keyword `pub` + --> $DIR/issue-76437-pub-crate-unsafe.rs:4:12 + | +LL | unsafe pub(crate) fn t() {} + | -------^^^------- + | | | + | | expected one of `extern` or `fn` + | help: visibility `pub(crate)` must come before `unsafe`: `pub(crate) unsafe` + +error: aborting due to previous error + diff --git a/src/test/ui/parser/issue-76437-unsafe.rs b/src/test/ui/parser/issue-76437-unsafe.rs new file mode 100644 index 00000000000..95538aaea69 --- /dev/null +++ b/src/test/ui/parser/issue-76437-unsafe.rs @@ -0,0 +1,6 @@ +// edition:2018 + +mod t { + unsafe pub fn t() {} + //~^ ERROR expected one of `extern` or `fn`, found keyword `pub` +} diff --git a/src/test/ui/parser/issue-76437-unsafe.stderr b/src/test/ui/parser/issue-76437-unsafe.stderr new file mode 100644 index 00000000000..c63292ef853 --- /dev/null +++ b/src/test/ui/parser/issue-76437-unsafe.stderr @@ -0,0 +1,11 @@ +error: expected one of `extern` or `fn`, found keyword `pub` + --> $DIR/issue-76437-unsafe.rs:4:12 + | +LL | unsafe pub fn t() {} + | -------^^^ + | | | + | | expected one of `extern` or `fn` + | help: visibility `pub` must come before `unsafe`: `pub unsafe` + +error: aborting due to previous error + -- 2.44.0