Skip to content

Commit

Permalink
Add diagnostic for incorrect pub (restriction)
Browse files Browse the repository at this point in the history
Given the following statement

```rust
pub (a) fn afn() {}
```

Provide the following diagnostic:

```rust
error: incorrect restriction in `pub`
  --> file.rs:15:1
   |
15 | pub (a) fn afn() {}
   | ^^^^^^^
   |
   = help: some valid visibility restrictions are:
           `pub(crate)`: visible only on the current crate
           `pub(super)`: visible only in the current module's parent
           `pub(in path::to::module)`: visible only on the specified path
help: to make this visible only to module `a`, add `in` before the path:
   | pub (in a) fn afn() {}
```

Remove cruft from old `pub(path)` syntax.
  • Loading branch information
estebank committed Mar 23, 2017
1 parent 8c4f2c6 commit 769b95d
Show file tree
Hide file tree
Showing 12 changed files with 205 additions and 35 deletions.
64 changes: 38 additions & 26 deletions src/libsyntax/parse/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4626,7 +4626,7 @@ impl<'a> Parser<'a> {

let mut attrs = self.parse_outer_attributes()?;
let lo = self.span.lo;
let vis = self.parse_visibility()?;
let vis = self.parse_visibility(false)?;
let defaultness = self.parse_defaultness()?;
let (name, node) = if self.eat_keyword(keywords::Type) {
let name = self.parse_ident()?;
Expand Down Expand Up @@ -4939,25 +4939,8 @@ impl<'a> Parser<'a> {
|p| {
let attrs = p.parse_outer_attributes()?;
let lo = p.span.lo;
let mut vis = p.parse_visibility()?;
let ty_is_interpolated =
p.token.is_interpolated() || p.look_ahead(1, |t| t.is_interpolated());
let mut ty = p.parse_ty()?;

// Handle `pub(path) type`, in which `vis` will be `pub` and `ty` will be `(path)`.
if vis == Visibility::Public && !ty_is_interpolated &&
p.token != token::Comma && p.token != token::CloseDelim(token::Paren) {
ty = if let TyKind::Paren(ref path_ty) = ty.node {
if let TyKind::Path(None, ref path) = path_ty.node {
vis = Visibility::Restricted { path: P(path.clone()), id: path_ty.id };
Some(p.parse_ty()?)
} else {
None
}
} else {
None
}.unwrap_or(ty);
}
let vis = p.parse_visibility(true)?;
let ty = p.parse_ty()?;
Ok(StructField {
span: mk_sp(lo, p.span.hi),
vis: vis,
Expand Down Expand Up @@ -4996,18 +4979,25 @@ impl<'a> Parser<'a> {
fn parse_struct_decl_field(&mut self) -> PResult<'a, StructField> {
let attrs = self.parse_outer_attributes()?;
let lo = self.span.lo;
let vis = self.parse_visibility()?;
let vis = self.parse_visibility(false)?;
self.parse_single_struct_field(lo, vis, attrs)
}

// Parse `pub`, `pub(crate)` and `pub(in path)` plus shortcuts
// `pub(self)` for `pub(in self)` and `pub(super)` for `pub(in super)`.
fn parse_visibility(&mut self) -> PResult<'a, Visibility> {
/// Parse `pub`, `pub(crate)` and `pub(in path)` plus shortcuts `pub(self)` for `pub(in self)`
/// and `pub(super)` for `pub(in super)`. If the following element can't be a tuple (i.e. it's
/// a function definition, it's not a tuple struct field) and the contents within the parens
/// isn't valid, emit a proper diagnostic.
fn parse_visibility(&mut self, can_take_tuple: bool) -> PResult<'a, Visibility> {
if !self.eat_keyword(keywords::Pub) {
return Ok(Visibility::Inherited)
}

if self.check(&token::OpenDelim(token::Paren)) {
let start_span = self.span;
// We don't `self.bump()` the `(` yet because this might be a struct definition where
// `()` or a tuple might be allowed. For example, `struct Struct(pub (), pub (usize));`.
// Because of this, we only `bump` the `(` if we're assured it is appropriate to do so
// by the following tokens.
if self.look_ahead(1, |t| t.is_keyword(keywords::Crate)) {
// `pub(crate)`
self.bump(); // `(`
Expand All @@ -5032,6 +5022,28 @@ impl<'a> Parser<'a> {
let vis = Visibility::Restricted { path: P(path), id: ast::DUMMY_NODE_ID };
self.expect(&token::CloseDelim(token::Paren))?; // `)`
return Ok(vis)
} else if !can_take_tuple { // Provide this diagnostic if this is not a tuple struct
// `pub(something) fn ...` or `struct X { pub(something) y: Z }`
self.bump(); // `(`
let msg = "incorrect visibility restriction";
let suggestion = r##"some possible visibility restrictions are:
`pub(crate)`: visible only on the current crate
`pub(super)`: visible only in the current module's parent
`pub(in path::to::module)`: visible only on the specified path"##;
let path = self.parse_path(PathStyle::Mod)?;
let path_span = self.prev_span;
let help_msg = format!("to make this visible only to module `{}`, add `in` before \
the path:",
path);
self.expect(&token::CloseDelim(token::Paren))?; // `)`
let sp = Span {
lo: start_span.lo,
hi: self.prev_span.hi,
expn_id: start_span.expn_id,
};
let mut err = self.span_fatal_help(sp, &msg, &suggestion);
err.span_suggestion(path_span, &help_msg, format!("in {}", path));
err.emit(); // emit diagnostic, but continue with public visibility
}
}

Expand Down Expand Up @@ -5508,7 +5520,7 @@ impl<'a> Parser<'a> {

let lo = self.span.lo;

let visibility = self.parse_visibility()?;
let visibility = self.parse_visibility(false)?;

if self.eat_keyword(keywords::Use) {
// USE ITEM
Expand Down Expand Up @@ -5787,7 +5799,7 @@ impl<'a> Parser<'a> {
fn parse_foreign_item(&mut self) -> PResult<'a, Option<ForeignItem>> {
let attrs = self.parse_outer_attributes()?;
let lo = self.span.lo;
let visibility = self.parse_visibility()?;
let visibility = self.parse_visibility(false)?;

if self.check_keyword(keywords::Static) {
// FOREIGN STATIC ITEM
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@

mod foo {
type T = ();
struct S1(pub(foo) (), pub(T), pub(crate) (), pub(((), T)));
struct S2(pub((foo)) ()); //~ ERROR expected `,`, found `(`
//~| ERROR expected one of `;` or `where`, found `(`
struct S1(pub(in foo) (), pub(T), pub(crate) (), pub(((), T)));
struct S2(pub((foo)) ());
//~^ ERROR expected `,`, found `(`
//~| ERROR expected one of `;` or `where`, found `(`
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,10 @@
macro_rules! define_struct {
($t:ty) => {
struct S1(pub $t);
struct S2(pub (foo) ());
struct S3(pub $t ()); //~ ERROR expected `,`, found `(`
//~| ERROR expected one of `;` or `where`, found `(`
struct S2(pub (in foo) ());
struct S3(pub $t ());
//~^ ERROR expected `,`, found `(`
//~| ERROR expected one of `;` or `where`, found `(`
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,10 @@
macro_rules! define_struct {
($t:ty) => {
struct S1(pub($t));
struct S2(pub (foo) ());
struct S3(pub($t) ()); //~ ERROR expected `,`, found `(`
//~| ERROR expected one of `;` or `where`, found `(`
struct S2(pub (in foo) ());
struct S3(pub($t) ());
//~^ ERROR expected `,`, found `(`
//~| ERROR expected one of `;` or `where`, found `(`
}
}

Expand Down
13 changes: 13 additions & 0 deletions src/test/ui/pub/pub-restricted-error-fn.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// 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.

#![feature(pub_restricted)]

pub(crate) () fn foo() {}
8 changes: 8 additions & 0 deletions src/test/ui/pub/pub-restricted-error-fn.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
error: unmatched visibility `pub`
--> $DIR/pub-restricted-error-fn.rs:13:10
|
13 | pub(crate) () fn foo() {}
| ^

error: aborting due to previous error

19 changes: 19 additions & 0 deletions src/test/ui/pub/pub-restricted-error.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// 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.

#![feature(pub_restricted)]

struct Bar(pub(()));

struct Foo {
pub(crate) () foo: usize,
}


8 changes: 8 additions & 0 deletions src/test/ui/pub/pub-restricted-error.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
error: expected identifier, found `(`
--> $DIR/pub-restricted-error.rs:16:16
|
16 | pub(crate) () foo: usize,
| ^

error: aborting due to previous error

15 changes: 15 additions & 0 deletions src/test/ui/pub/pub-restricted-non-path.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// 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.

#![feature(pub_restricted)]

pub (.) fn afn() {}

fn main() {}
8 changes: 8 additions & 0 deletions src/test/ui/pub/pub-restricted-non-path.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
error: expected identifier, found `.`
--> $DIR/pub-restricted-non-path.rs:13:6
|
13 | pub (.) fn afn() {}
| ^

error: aborting due to previous error

37 changes: 37 additions & 0 deletions src/test/ui/pub/pub-restricted.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
// 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.

#![feature(pub_restricted)]

mod a {}

pub (a) fn afn() {}
pub (b) fn bfn() {}
pub fn privfn() {}
mod x {
mod y {
pub (in x) fn foo() {}
pub (super) fn bar() {}
pub (crate) fn qux() {}
}
}

mod y {
struct Foo {
pub (crate) c: usize,
pub (super) s: usize,
valid_private: usize,
pub (in y) valid_in_x: usize,
pub (a) invalid: usize,
pub (in x) non_parent_invalid: usize,
}
}

fn main() {}
47 changes: 47 additions & 0 deletions src/test/ui/pub/pub-restricted.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
error: incorrect visibility restriction
--> $DIR/pub-restricted.rs:15:5
|
15 | pub (a) fn afn() {}
| ^^^
|
= help: some possible visibility restrictions are:
`pub(crate)`: visible only on the current crate
`pub(super)`: visible only in the current module's parent
`pub(in path::to::module)`: visible only on the specified path
help: to make this visible only to module `a`, add `in` before the path:
| pub (in a) fn afn() {}

error: incorrect visibility restriction
--> $DIR/pub-restricted.rs:16:5
|
16 | pub (b) fn bfn() {}
| ^^^
|
= help: some possible visibility restrictions are:
`pub(crate)`: visible only on the current crate
`pub(super)`: visible only in the current module's parent
`pub(in path::to::module)`: visible only on the specified path
help: to make this visible only to module `b`, add `in` before the path:
| pub (in b) fn bfn() {}

error: incorrect visibility restriction
--> $DIR/pub-restricted.rs:32:13
|
32 | pub (a) invalid: usize,
| ^^^
|
= help: some possible visibility restrictions are:
`pub(crate)`: visible only on the current crate
`pub(super)`: visible only in the current module's parent
`pub(in path::to::module)`: visible only on the specified path
help: to make this visible only to module `a`, add `in` before the path:
| pub (in a) invalid: usize,

error: visibilities can only be restricted to ancestor modules
--> $DIR/pub-restricted.rs:33:17
|
33 | pub (in x) non_parent_invalid: usize,
| ^

error: aborting due to 4 previous errors

0 comments on commit 769b95d

Please sign in to comment.