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

Update error [E0449]: unnecessary visibility qualifier to be more clear #109923

Merged
merged 2 commits into from
Apr 11, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions compiler/rustc_ast_passes/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,10 @@ ast_passes_keyword_lifetime =
ast_passes_invalid_label =
invalid label name `{$name}`

ast_passes_invalid_visibility =
unnecessary visibility qualifier
.implied = `pub` not permitted here because it's implied
ast_passes_visibility_not_permitted =
visibility qualifiers are not permitted here
.enum_variant = enum variants and their fields always share the visibility of the enum they are in
.trait_impl = trait items always share the visibility of their trait
.individual_impl_items = place qualifiers on individual impl items instead
.individual_foreign_items = place qualifiers on individual foreign items instead

Expand Down
33 changes: 19 additions & 14 deletions compiler/rustc_ast_passes/src/ast_validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -240,16 +240,12 @@ impl<'a> AstValidator<'a> {
}
}

fn invalid_visibility(&self, vis: &Visibility, note: Option<errors::InvalidVisibilityNote>) {
fn visibility_not_permitted(&self, vis: &Visibility, note: errors::VisibilityNotPermittedNote) {
if let VisibilityKind::Inherited = vis.kind {
return;
}

self.session.emit_err(errors::InvalidVisibility {
span: vis.span,
implied: vis.kind.is_pub().then_some(vis.span),
note,
});
self.session.emit_err(errors::VisibilityNotPermitted { span: vis.span, note });
}

fn check_decl_no_pat(decl: &FnDecl, mut report_err: impl FnMut(Span, Option<Ident>, bool)) {
Expand Down Expand Up @@ -819,7 +815,10 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
items,
}) => {
self.with_in_trait_impl(true, Some(*constness), |this| {
this.invalid_visibility(&item.vis, None);
this.visibility_not_permitted(
&item.vis,
errors::VisibilityNotPermittedNote::TraitImpl,
);
if let TyKind::Err = self_ty.kind {
this.err_handler().emit_err(errors::ObsoleteAuto { span: item.span });
}
Expand Down Expand Up @@ -866,9 +865,9 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
only_trait: only_trait.then_some(()),
};

self.invalid_visibility(
self.visibility_not_permitted(
&item.vis,
Some(errors::InvalidVisibilityNote::IndividualImplItems),
errors::VisibilityNotPermittedNote::IndividualImplItems,
);
if let &Unsafe::Yes(span) = unsafety {
self.err_handler().emit_err(errors::InherentImplCannotUnsafe {
Expand Down Expand Up @@ -924,9 +923,9 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
}
ItemKind::ForeignMod(ForeignMod { abi, unsafety, .. }) => {
let old_item = mem::replace(&mut self.extern_mod, Some(item));
self.invalid_visibility(
self.visibility_not_permitted(
&item.vis,
Some(errors::InvalidVisibilityNote::IndividualForeignItems),
errors::VisibilityNotPermittedNote::IndividualForeignItems,
);
if let &Unsafe::Yes(span) = unsafety {
self.err_handler().emit_err(errors::UnsafeItem { span, kind: "extern block" });
Expand All @@ -940,9 +939,15 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
}
ItemKind::Enum(def, _) => {
for variant in &def.variants {
self.invalid_visibility(&variant.vis, None);
self.visibility_not_permitted(
&variant.vis,
errors::VisibilityNotPermittedNote::EnumVariant,
);
for field in variant.data.fields() {
self.invalid_visibility(&field.vis, None);
self.visibility_not_permitted(
&field.vis,
errors::VisibilityNotPermittedNote::EnumVariant,
);
}
}
}
Expand Down Expand Up @@ -1303,7 +1308,7 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
}

if ctxt == AssocCtxt::Trait || self.in_trait_impl {
self.invalid_visibility(&item.vis, None);
self.visibility_not_permitted(&item.vis, errors::VisibilityNotPermittedNote::TraitImpl);
if let AssocItemKind::Fn(box Fn { sig, .. }) = &item.kind {
self.check_trait_fn_not_const(sig.header.constness);
}
Expand Down
14 changes: 8 additions & 6 deletions compiler/rustc_ast_passes/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,18 +42,20 @@ pub struct InvalidLabel {
}

#[derive(Diagnostic)]
#[diag(ast_passes_invalid_visibility, code = "E0449")]
pub struct InvalidVisibility {
#[diag(ast_passes_visibility_not_permitted, code = "E0449")]
pub struct VisibilityNotPermitted {
#[primary_span]
pub span: Span,
#[label(ast_passes_implied)]
pub implied: Option<Span>,
#[subdiagnostic]
pub note: Option<InvalidVisibilityNote>,
pub note: VisibilityNotPermittedNote,
}

#[derive(Subdiagnostic)]
pub enum InvalidVisibilityNote {
pub enum VisibilityNotPermittedNote {
#[note(ast_passes_enum_variant)]
EnumVariant,
#[note(ast_passes_trait_impl)]
TraitImpl,
#[note(ast_passes_individual_impl_items)]
IndividualImplItems,
#[note(ast_passes_individual_foreign_items)]
Expand Down
29 changes: 20 additions & 9 deletions compiler/rustc_error_codes/src/error_codes/E0449.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
A visibility qualifier was used when it was unnecessary.
A visibility qualifier was used where one is not permitted. Visibility
qualifiers are not permitted on enum variants, trait items, impl blocks, and
extern blocks, as they already share the visibility of the parent item.

Erroneous code examples:

Expand All @@ -9,15 +11,18 @@ trait Foo {
fn foo();
}

pub impl Bar {} // error: unnecessary visibility qualifier
enum Baz {
pub Qux, // error: visibility qualifiers are not permitted here
}

pub impl Bar {} // error: visibility qualifiers are not permitted here

pub impl Foo for Bar { // error: unnecessary visibility qualifier
pub fn foo() {} // error: unnecessary visibility qualifier
pub impl Foo for Bar { // error: visibility qualifiers are not permitted here
pub fn foo() {} // error: visibility qualifiers are not permitted here
}
```

To fix this error, please remove the visibility qualifier when it is not
required. Example:
To fix this error, simply remove the visibility qualifier. Example:

```
struct Bar;
Expand All @@ -26,12 +31,18 @@ trait Foo {
fn foo();
}

enum Baz {
// Enum variants share the visibility of the enum they are in, so
// `pub` is not allowed here
Qux,
}

// Directly implemented methods share the visibility of the type itself,
// so `pub` is unnecessary here
// so `pub` is not allowed here
impl Bar {}

// Trait methods share the visibility of the trait, so `pub` is
// unnecessary in either case
// Trait methods share the visibility of the trait, so `pub` is not
// allowed in either case
impl Foo for Bar {
fn foo() {}
}
Expand Down
16 changes: 10 additions & 6 deletions tests/ui/error-codes/E0449.stderr
Original file line number Diff line number Diff line change
@@ -1,22 +1,26 @@
error[E0449]: unnecessary visibility qualifier
error[E0449]: visibility qualifiers are not permitted here
--> $DIR/E0449.rs:7:1
|
LL | pub impl Bar {}
| ^^^ `pub` not permitted here because it's implied
| ^^^
|
= note: place qualifiers on individual impl items instead

error[E0449]: unnecessary visibility qualifier
error[E0449]: visibility qualifiers are not permitted here
--> $DIR/E0449.rs:9:1
|
LL | pub impl Foo for Bar {
| ^^^ `pub` not permitted here because it's implied
| ^^^
|
= note: trait items always share the visibility of their trait

error[E0449]: unnecessary visibility qualifier
error[E0449]: visibility qualifiers are not permitted here
--> $DIR/E0449.rs:10:5
|
LL | pub fn foo() {}
| ^^^ `pub` not permitted here because it's implied
| ^^^
|
= note: trait items always share the visibility of their trait

error: aborting due to 3 previous errors

Expand Down
4 changes: 2 additions & 2 deletions tests/ui/issues/issue-28433.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
enum Bird {
pub Duck,
//~^ ERROR unnecessary visibility qualifier
//~^ ERROR visibility qualifiers are not permitted here
Goose,
pub(crate) Dove
//~^ ERROR unnecessary visibility qualifier
//~^ ERROR visibility qualifiers are not permitted here
}


Expand Down
10 changes: 7 additions & 3 deletions tests/ui/issues/issue-28433.stderr
Original file line number Diff line number Diff line change
@@ -1,14 +1,18 @@
error[E0449]: unnecessary visibility qualifier
error[E0449]: visibility qualifiers are not permitted here
--> $DIR/issue-28433.rs:2:5
|
LL | pub Duck,
| ^^^ `pub` not permitted here because it's implied
| ^^^
|
= note: enum variants and their fields always share the visibility of the enum they are in

error[E0449]: unnecessary visibility qualifier
error[E0449]: visibility qualifiers are not permitted here
--> $DIR/issue-28433.rs:5:5
|
LL | pub(crate) Dove
| ^^^^^^^^^^
|
= note: enum variants and their fields always share the visibility of the enum they are in

error: aborting due to 2 previous errors

Expand Down
4 changes: 2 additions & 2 deletions tests/ui/parser/assoc-static-semantic-fail.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ trait T {
//~| ERROR a static item cannot be `default`
pub(crate) default static TD: u8;
//~^ ERROR associated `static` items are not allowed
//~| ERROR unnecessary visibility qualifier
//~| ERROR visibility qualifiers are not permitted here
//~| ERROR a static item cannot be `default`
}

Expand All @@ -47,6 +47,6 @@ impl T for S {
pub default static TD: u8;
//~^ ERROR associated `static` items are not allowed
//~| ERROR associated constant in `impl` without body
//~| ERROR unnecessary visibility qualifier
//~| ERROR visibility qualifiers are not permitted here
//~| ERROR a static item cannot be `default`
}
10 changes: 7 additions & 3 deletions tests/ui/parser/assoc-static-semantic-fail.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -134,11 +134,13 @@ LL | pub(crate) default static ID: u8;
| |
| help: provide a definition for the constant: `= <expr>;`

error[E0449]: unnecessary visibility qualifier
error[E0449]: visibility qualifiers are not permitted here
--> $DIR/assoc-static-semantic-fail.rs:32:5
|
LL | pub(crate) default static TD: u8;
| ^^^^^^^^^^
|
= note: trait items always share the visibility of their trait

error: associated constant in `impl` without body
--> $DIR/assoc-static-semantic-fail.rs:41:5
Expand All @@ -156,11 +158,13 @@ LL | pub default static TD: u8;
| |
| help: provide a definition for the constant: `= <expr>;`

error[E0449]: unnecessary visibility qualifier
error[E0449]: visibility qualifiers are not permitted here
--> $DIR/assoc-static-semantic-fail.rs:47:5
|
LL | pub default static TD: u8;
| ^^^ `pub` not permitted here because it's implied
| ^^^
|
= note: trait items always share the visibility of their trait

warning: the feature `specialization` is incomplete and may not be safe to use and/or cause compiler crashes
--> $DIR/assoc-static-semantic-fail.rs:3:12
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/parser/default.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ impl Foo for u8 {
}

impl Foo for u16 {
pub default fn foo<T: Default>() -> T { //~ ERROR unnecessary visibility qualifier
pub default fn foo<T: Default>() -> T { //~ ERROR visibility qualifiers are not permitted here
T::default()
}
}
Expand Down
6 changes: 4 additions & 2 deletions tests/ui/parser/default.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,13 @@ LL | default pub fn foo<T: Default>() -> T { T::default() }
LL | }
| - item list ends here

error[E0449]: unnecessary visibility qualifier
error[E0449]: visibility qualifiers are not permitted here
--> $DIR/default.rs:17:5
|
LL | pub default fn foo<T: Default>() -> T {
| ^^^ `pub` not permitted here because it's implied
| ^^^
|
= note: trait items always share the visibility of their trait

warning: the feature `specialization` is incomplete and may not be safe to use and/or cause compiler crashes
--> $DIR/default.rs:3:12
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/parser/trait-pub-assoc-const.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
trait Foo {
pub const Foo: u32;
//~^ ERROR unnecessary visibility qualifier
//~^ ERROR visibility qualifiers are not permitted here
}

fn main() {}
6 changes: 4 additions & 2 deletions tests/ui/parser/trait-pub-assoc-const.stderr
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
error[E0449]: unnecessary visibility qualifier
error[E0449]: visibility qualifiers are not permitted here
--> $DIR/trait-pub-assoc-const.rs:2:5
|
LL | pub const Foo: u32;
| ^^^ `pub` not permitted here because it's implied
| ^^^
|
= note: trait items always share the visibility of their trait

error: aborting due to previous error

Expand Down
2 changes: 1 addition & 1 deletion tests/ui/parser/trait-pub-assoc-ty.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
trait Foo {
pub type Foo;
//~^ ERROR unnecessary visibility qualifier
//~^ ERROR visibility qualifiers are not permitted here
}

fn main() {}
6 changes: 4 additions & 2 deletions tests/ui/parser/trait-pub-assoc-ty.stderr
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
error[E0449]: unnecessary visibility qualifier
error[E0449]: visibility qualifiers are not permitted here
--> $DIR/trait-pub-assoc-ty.rs:2:5
|
LL | pub type Foo;
| ^^^ `pub` not permitted here because it's implied
| ^^^
|
= note: trait items always share the visibility of their trait

error: aborting due to previous error

Expand Down
2 changes: 1 addition & 1 deletion tests/ui/parser/trait-pub-method.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
trait Foo {
pub fn foo();
//~^ ERROR unnecessary visibility qualifier
//~^ ERROR visibility qualifiers are not permitted here
}

fn main() {}
6 changes: 4 additions & 2 deletions tests/ui/parser/trait-pub-method.stderr
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
error[E0449]: unnecessary visibility qualifier
error[E0449]: visibility qualifiers are not permitted here
--> $DIR/trait-pub-method.rs:2:5
|
LL | pub fn foo();
| ^^^ `pub` not permitted here because it's implied
| ^^^
|
= note: trait items always share the visibility of their trait

error: aborting due to previous error

Expand Down
2 changes: 1 addition & 1 deletion tests/ui/privacy/issue-29161.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ mod a {
struct A;

impl Default for A {
pub fn default() -> A { //~ ERROR unnecessary visibility qualifier
pub fn default() -> A { //~ ERROR visibility qualifiers are not permitted here
A
}
}
Expand Down
Loading