Skip to content

Commit

Permalink
Disables usage of parenthesis in unit enum variants. (FuelLabs#3743)
Browse files Browse the repository at this point in the history
With these changes it is no longer possible to do `Option::None()`
instead it should be done with `Option::None`.

This changes also fixes enum turbofish without parenthesis so
`Option::None::<T>` can be used.

Ref FuelLabs#3585 (Does not close but addresses other issue mentioned on the
comments)
Closes FuelLabs#3375

Co-authored-by: Joshua Batty <[email protected]>
  • Loading branch information
esdrubal and JoshuaBatty authored Jan 18, 2023
1 parent 1c4f3e2 commit a97c97f
Show file tree
Hide file tree
Showing 26 changed files with 174 additions and 32 deletions.
2 changes: 1 addition & 1 deletion examples/ownership/src/main.sw
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ impl OwnershipExample for Contract {
// ANCHOR: revoke_owner_example
#[storage(write)]
fn revoke_ownership() {
storage.owner = Option::None();
storage.owner = Option::None;
}
// ANCHOR_END: revoke_owner_example
// ANCHOR: set_owner_example
Expand Down
5 changes: 4 additions & 1 deletion sway-core/src/language/parsed/expression/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,10 @@ pub struct AmbiguousPathExpression {
#[derive(Debug, Clone)]
pub struct DelineatedPathExpression {
pub call_path_binding: TypeBinding<CallPath>,
pub args: Vec<Expression>,
/// When args is equal to Option::None then it means that the
/// [DelineatedPathExpression] was initialized from an expression
/// that does not end with parenthesis.
pub args: Option<Vec<Expression>>,
}

#[derive(Debug, Clone)]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -490,7 +490,7 @@ impl ty::TyExpression {
ctx,
function_decl,
call_path_binding.inner,
arguments,
Some(arguments),
span,
)
}
Expand Down Expand Up @@ -1033,7 +1033,8 @@ impl ty::TyExpression {
type_arguments,
span: path_span,
};
let mut res = Self::type_check_delineated_path(ctx, call_path_binding, span, args);
let mut res =
Self::type_check_delineated_path(ctx, call_path_binding, span, Some(args));

// In case `before` has type args, this would be e.g., `foo::bar::<TyArgs>::baz(...)`.
// So, we would need, but don't have, parametric modules to apply arguments to.
Expand Down Expand Up @@ -1069,7 +1070,7 @@ impl ty::TyExpression {
mut ctx: TypeCheckContext,
call_path_binding: TypeBinding<CallPath>,
span: Span,
args: Vec<Expression>,
args: Option<Vec<Expression>>,
) -> CompileResult<ty::TyExpression> {
let mut warnings = vec![];
let mut errors = vec![];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ pub(crate) fn instantiate_enum(
enum_decl: ty::TyEnumDeclaration,
enum_name: Ident,
enum_variant_name: Ident,
args: Vec<Expression>,
args_opt: Option<Vec<Expression>>,
span: &Span,
) -> CompileResult<ty::TyExpression> {
let mut warnings = vec![];
Expand All @@ -35,6 +35,17 @@ pub(crate) fn instantiate_enum(
errors
);

// Return an error if enum variant is of type unit and it is called with parenthesis.
// args_opt.is_some() returns true when this variant was called with parenthesis.
if type_engine.get(enum_variant.initial_type_id).is_unit() && args_opt.is_some() {
errors.push(CompileError::UnitVariantWithParenthesesEnumInstantiator {
span: enum_variant_name.span(),
ty: enum_variant.name.as_str().to_string(),
});
return err(warnings, errors);
}
let args = args_opt.unwrap_or_default();

// If there is an instantiator, it must match up with the type. If there is not an
// instantiator, then the type of the enum is necessarily the unit type.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ pub(crate) fn instantiate_function_application(
mut ctx: TypeCheckContext,
mut function_decl: ty::TyFunctionDeclaration,
call_path: CallPath,
arguments: Vec<Expression>,
arguments: Option<Vec<Expression>>,
span: Span,
) -> CompileResult<ty::TyExpression> {
let mut warnings = vec![];
Expand All @@ -22,6 +22,15 @@ pub(crate) fn instantiate_function_application(
let decl_engine = ctx.decl_engine;
let engines = ctx.engines();

if arguments.is_none() {
errors.push(CompileError::MissingParenthesesForFunction {
method_name: call_path.suffix.clone(),
span: call_path.span(),
});
return err(warnings, errors);
}
let arguments = arguments.unwrap_or_default();

// 'purity' is that of the callee, 'opts.purity' of the caller.
if !ctx.purity().can_call(function_decl.purity) {
errors.push(CompileError::StorageAccessMismatch {
Expand Down
3 changes: 2 additions & 1 deletion sway-core/src/semantic_analysis/node_dependencies.rs
Original file line number Diff line number Diff line change
Expand Up @@ -493,9 +493,10 @@ impl Dependencies {
// It's either a module path which we can ignore, or an enum variant path, in which
// case we're interested in the enum name and initialiser args, ignoring the
// variant name.
let args_vec = args.clone().unwrap_or_default();
self.gather_from_call_path(&call_path_binding.inner, true, false)
.gather_from_type_arguments(type_engine, &call_path_binding.type_arguments)
.gather_from_iter(args.iter(), |deps, arg| {
.gather_from_iter(args_vec.iter(), |deps, arg| {
deps.gather_from_expr(type_engine, arg)
})
}
Expand Down
12 changes: 4 additions & 8 deletions sway-core/src/transform/to_parsed_lang/convert_parse_tree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1391,7 +1391,7 @@ fn expr_to_expression(
kind: ExpressionKind::Error(part_spans),
span,
},
Expr::Path(path_expr) => path_expr_to_expression(handler, path_expr)?,
Expr::Path(path_expr) => path_expr_to_expression(handler, engines, path_expr)?,
Expr::Literal(literal) => Expression {
kind: ExpressionKind::Literal(literal_to_literal(handler, literal)?),
span,
Expand Down Expand Up @@ -2153,6 +2153,7 @@ fn path_expr_segment_to_ident(

fn path_expr_to_expression(
handler: &Handler,
engines: Engines<'_>,
path_expr: PathExpr,
) -> Result<Expression, ErrorEmitted> {
let span = path_expr.span();
Expand All @@ -2163,16 +2164,11 @@ fn path_expr_to_expression(
span,
}
} else {
let call_path = path_expr_to_call_path(handler, path_expr)?;
let call_path_binding = TypeBinding {
inner: call_path.clone(),
type_arguments: vec![],
span: call_path.span(),
};
let call_path_binding = path_expr_to_call_path_binding(handler, engines, path_expr)?;
Expression {
kind: ExpressionKind::DelineatedPath(Box::new(DelineatedPathExpression {
call_path_binding,
args: Vec::new(),
args: None,
})),
span,
}
Expand Down
6 changes: 6 additions & 0 deletions sway-error/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -416,6 +416,8 @@ pub enum CompileError {
MoreThanOneEnumInstantiator { span: Span, ty: String },
#[error("This enum variant represents the unit type, so it should not be instantiated with any value.")]
UnnecessaryEnumInstantiator { span: Span },
#[error("The enum variant `{ty}` is of type `unit`, so its constructor does not take arguments or parentheses. Try removing the ().")]
UnitVariantWithParenthesesEnumInstantiator { span: Span, ty: String },
#[error("Cannot find trait \"{name}\" in this scope.")]
TraitNotFound { name: String, span: Span },
#[error("This expression is not valid on the left hand side of a reassignment.")]
Expand All @@ -442,6 +444,8 @@ pub enum CompileError {
expected: usize,
received: usize,
},
#[error("The function \"{method_name}\" was called without parentheses. Try adding ().")]
MissingParenthesesForFunction { span: Span, method_name: Ident },
#[error("This type is invalid in a function selector. A contract ABI function selector must be a known sized type, not generic.")]
InvalidAbiType { span: Span },
#[error("This is a {actually_is}, not an ABI. An ABI cast requires a valid ABI to cast the address to.")]
Expand Down Expand Up @@ -813,10 +817,12 @@ impl Spanned for CompileError {
ImportMustBeLibrary { span, .. } => span.clone(),
MoreThanOneEnumInstantiator { span, .. } => span.clone(),
UnnecessaryEnumInstantiator { span, .. } => span.clone(),
UnitVariantWithParenthesesEnumInstantiator { span, .. } => span.clone(),
TraitNotFound { span, .. } => span.clone(),
InvalidExpressionOnLhs { span, .. } => span.clone(),
TooManyArgumentsForFunction { span, .. } => span.clone(),
TooFewArgumentsForFunction { span, .. } => span.clone(),
MissingParenthesesForFunction { span, .. } => span.clone(),
InvalidAbiType { span, .. } => span.clone(),
NotAnAbi { span, .. } => span.clone(),
ImplAbiForNonContract { span, .. } => span.clone(),
Expand Down
2 changes: 1 addition & 1 deletion sway-lib-std/src/auth.sw
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ pub fn msg_sender() -> Result<Identity, AuthError> {
/// TransactionScript if they all share the same owner.
fn inputs_owner() -> Result<Identity, AuthError> {
let inputs = input_count();
let mut candidate = Option::None::<Address>();
let mut candidate = Option::None::<Address>;
let mut i = 0u8;

// Note: `inputs_count` is guaranteed to be at least 1 for any valid tx.
Expand Down
2 changes: 1 addition & 1 deletion sway-lib-std/src/vec.sw
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ impl<T> Vec<T> {
pub fn get(self, index: u64) -> Option<T> {
// First check that index is within bounds.
if self.len <= index {
return Option::None::<T>();
return Option::None::<T>;
};

// Get a pointer to the desired element using `index`
Expand Down
6 changes: 4 additions & 2 deletions sway-lsp/src/traverse/parsed_tree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -645,8 +645,10 @@ impl<'a> ParsedTree<'a> {
self.collect_type_arg(type_arg, &token);
}

for exp in args {
self.handle_expression(exp);
if let Some(args_vec) = args.as_ref() {
args_vec.iter().for_each(|exp| {
self.handle_expression(exp);
});
}
}
ExpressionKind::AbiCast(abi_cast_expression) => {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
[[package]]
name = 'core'
source = 'path+from-root-2C926E0DE5E7A78F'

[[package]]
name = 'enum_variant_unit'
source = 'member'
dependencies = ['std']

[[package]]
name = 'std'
source = 'path+from-root-2C926E0DE5E7A78F'
dependencies = ['core']
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
[project]
authors = ["Fuel Labs <[email protected]>"]
entry = "main.sw"
license = "Apache-2.0"
name = "enum_variant_unit"

[dependencies]
std = { path = "../../../../../../sway-lib-std" }
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
[]
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
library inner_lib;

pub enum MyEnum {
VariantA: (),
VariantB: u64
}

pub fn func() -> bool {
true
}

pub struct S2 {}

impl S2 {
pub fn new2() -> Self {
S2 {}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
library lib_a;

dep inner_lib;

use inner_lib::*;
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
script;

dep lib_a;

fn func() -> bool {
true
}

struct S {}

impl S {
fn new2() -> Self {
S {}
}
}

fn main() -> u64 {

// check that calling function and methods with no parameters still requires parenthesis
let b = func;
let b = func();


let s = S::new;
let s = S::new();


let b = lib_a::inner_lib::func;
let b = lib_a::inner_lib::func();


let s = lib_a::inner_lib::S2::new2;
let s = lib_a::inner_lib::S2::new2();


let n: Option<u64> = Option::None();


let n = Option::None::<u64>();


let n = lib_a::inner_lib::MyEnum::VariantA;
let n = lib_a::inner_lib::MyEnum::VariantA();

5
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
category = "fail"

# check: $()let b = func;
# nextln: $()Identifier "func" was used as a variable, but it is actually a function.

# check: $()let s = S::new;
# nextln: $()Could not find symbol "new" in this scope.

# check: $()let b = lib_a::inner_lib::func;
# nextln: $()The function "func" was called without parentheses. Try adding ().

# check: $()let s = lib_a::inner_lib::S2::new2;
# nextln: $()Could not find symbol "new2" in this scope.

# check: $()let n: Option<u64> = Option::None();
# nextln: $()The enum variant `None` is of type `unit`, so its constructor does not take arguments or parentheses. Try removing the ().

# check: $()let n = Option::None::<u64>();
# nextln: $()The enum variant `None` is of type `unit`, so its constructor does not take arguments or parentheses. Try removing the ().

# check: $()let n = lib_a::inner_lib::MyEnum::VariantA();
# nextln: $()The enum variant `VariantA` is of type `unit`, so its constructor does not take arguments or parentheses. Try removing the ().
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
script;

fn main() -> bool {
let o = Option::None();
let o = Option::None;
true
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
category = "fail"

# check: Option::None()
# check: Option::None
# nextln: $()Cannot infer type for type parameter "T". Insufficient type information provided. Try annotating its type.
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,6 @@ dep colors1;
dep colors2;

fn main() {
let c1 = colors1::Colors::Red();
let c2 = colors2::Colors::Red();
let c1 = colors1::Colors::Red;
let c2 = colors2::Colors::Red;
}
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ impl<T> MyOption<T> {
}

fn none() -> Self {
MyOption::None::<T>(())
MyOption::None::<T>
}

fn to_result(self) -> MyResult<T> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ storage {
impl TicTacToe for Contract {
#[storage(write)]
fn new_game() {
storage.game_boards.insert(1, Option::None::<Identity>());
storage.game_boards.insert(1, Option::None::<Identity>);
storage.game_boards.insert(1, Option::None);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ fn main() -> bool {
assert(foo);

let param2 = (
Location::Earth(()),
Location::Earth,
3u64
);
let bar = the_abi.bug2(param2);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ impl Eq for Error {
}

fn test_none_ok_or<T, E>(_val: T, default: E) where E: Eq {
match Option::None::<T>().ok_or(default) {
match Option::None::<T>.ok_or(default) {
Result::Ok(_) => revert(0),
Result::Err(e) => assert(default == e),
}
Expand Down
Loading

0 comments on commit a97c97f

Please sign in to comment.