Skip to content

Commit

Permalink
Check for unnecessary visibility qualifier in trait items. (FuelLabs#…
Browse files Browse the repository at this point in the history
…2276)

This introduces a new check for unnecessary visibility qualifiers
when defining trait items.

For code like:

```
trait A {} {
    pub fn f(self) -> bool {
        false
    }
}
```

We now emit:

```
error
 --> main.sw:4:5
  |
2 |
3 | trait A {} {
4 |     pub fn f(self) -> bool {
  |     ^^^ Unnecessary visibility qualifier.
5 |         false
6 |     }
  |
____
```

Also adds some tests for both trait interface and method blocks.

Co-authored-by: Mohammad Fawaz <[email protected]>
  • Loading branch information
tritao and mohammadfawaz authored Jul 11, 2022
1 parent e51e639 commit f588d8a
Show file tree
Hide file tree
Showing 12 changed files with 72 additions and 4 deletions.
2 changes: 1 addition & 1 deletion sway-lib-std/src/u128.sw
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ pub enum U128Error {

pub trait From {
/// Function for creating U128 from its u64 components.
pub fn from(upper: u64, lower: u64) -> Self;
fn from(upper: u64, lower: u64) -> Self;
fn into(self) -> (u64, u64);
}

Expand Down
2 changes: 1 addition & 1 deletion sway-lib-std/src/u256.sw
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ pub enum U256Error {

pub trait From {
/// Function for creating a U256 from its u64 components.
pub fn from(a: u64, b: u64, c: u64, d: u64) -> Self;
fn from(a: u64, b: u64, c: u64, d: u64) -> Self;
fn into(self) -> (u64, u64, u64, u64);
}

Expand Down
2 changes: 2 additions & 0 deletions sway-parse/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,8 @@ pub enum ParseErrorKind {
UnexpectedRestPattern,
#[error("Identifiers cannot be a reserved keyword.")]
ReservedKeywordIdentifier,
#[error("Unnecessary visibility qualifier, `{}` is implied here.", visibility)]
UnnecessaryVisibilityQualifier { visibility: Ident },
}

#[derive(Debug, Error, Clone, PartialEq, Hash)]
Expand Down
30 changes: 28 additions & 2 deletions sway-parse/src/item/item_trait.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,34 @@ impl Parse for ItemTrait {
}
None => None,
};
let trait_items = parser.parse()?;
let trait_defs_opt = Braces::try_parse(parser)?;

let trait_items: Braces<Vec<(Annotated<FnSignature>, _)>> = parser.parse()?;
for item in trait_items.get().iter() {
let (fn_sig, _) = item;
if let Some(token) = &fn_sig.value.visibility {
return Err(parser.emit_error_with_span(
ParseErrorKind::UnnecessaryVisibilityQualifier {
visibility: token.ident(),
},
token.span(),
));
}
}

let trait_defs_opt: Option<Braces<Vec<Annotated<ItemFn>>>> = Braces::try_parse(parser)?;
if let Some(trait_defs) = &trait_defs_opt {
for item in trait_defs.get().iter() {
if let Some(token) = &item.value.fn_signature.visibility {
return Err(parser.emit_error_with_span(
ParseErrorKind::UnnecessaryVisibilityQualifier {
visibility: token.ident(),
},
token.span(),
));
}
}
}

Ok(ItemTrait {
visibility,
trait_token,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
[[package]]
name = 'trait_pub_fn_interface'
source = 'root'
dependencies = []
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
[project]
name = "trait_pub_fn_interface"
authors = ["Fuel Labs <[email protected]>"]
entry = "main.sw"
license = "Apache-2.0"
implicit-std = false
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
contract;

trait A {
pub fn f(self) -> bool;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
category = "fail"

# check: pub fn f(self) -> bool;
# nextln: $()Unnecessary visibility qualifier, `pub` is implied here.
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
[[package]]
name = 'trait_pub_fn_method'
source = 'root'
dependencies = []
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
[project]
name = "trait_pub_fn_method"
authors = ["Fuel Labs <[email protected]>"]
entry = "main.sw"
license = "Apache-2.0"
implicit-std = false
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
contract;

trait A {} {
pub fn f(self) -> bool {
false
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
category = "fail"

# check: pub fn f(self) -> bool {
# nextln: $()Unnecessary visibility qualifier, `pub` is implied here.

0 comments on commit f588d8a

Please sign in to comment.