Swift-style argument labels (#666)

.github/workflows/main.yml

@@ -68,8 +68,8 @@ jobs:
         uses: actions-rs/cargo@v1
         with:
           command: clippy
-          args: --workspace --all-targets --all-features -- -D warnings -A clippy::upper-case-acronyms -A clippy::large-enum-variant -W clippy::redundant_closure_for_method_calls
-            
+          args: --workspace --all-targets --all-features -- -D warnings -W clippy::redundant_closure_for_method_calls
+
   book:
     runs-on: ubuntu-latest
     steps:

crates/abi/src/builder.rs

@@ -130,28 +130,32 @@ mod tests {
     use crate::builder;
     use fe_analyzer::namespace::items::ModuleId;
     use fe_analyzer::TestDb;
+    use fe_common::diagnostics::print_diagnostics;
 
     #[test]
     fn build_contract_abi() {
         let contract = r#"
-pub fn add(x: u256, y: u256) -> u256:
+pub fn add(_ x: u256, _ y: u256) -> u256:
   return x + y
 
 contract Foo:
   event Food:
     idx barge: u256
   pub fn __init__(x: address):
     pass
-  fn baz(x: address) -> u256:
+  fn baz(_ x: address) -> u256:
     add(10, 20)
     revert
-  pub fn bar(x: u256) -> Array<u256, 10>:
+  pub fn bar(_ x: u256) -> Array<u256, 10>:
     revert"#;
 
         let mut db = TestDb::default();
         let module = ModuleId::new_standalone(&mut db, "test_module", contract);
 
-        fe_analyzer::analyze_module(&db, module).expect("failed to analyze source");
+        if !module.diagnostics(&db).is_empty() {
+            print_diagnostics(&db, &module.diagnostics(&db));
+            panic!("failed to analyze source")
+        }
         let abis = builder::module(&db, module).expect("unable to build ABI");
 
         if let Some(abi) = abis.get("Foo") {

crates/analyzer/src/db/queries/functions.rs

@@ -41,6 +41,8 @@ pub fn function_signature(
     let mut self_decl = None;
     let mut ctx_decl = None;
     let mut names = HashMap::new();
+    let mut labels = HashMap::new();
+
     let params = def
         .args
         .iter()
@@ -65,15 +67,12 @@ pub fn function_signature(
                 }
                 None
             }
-            ast::FunctionArg::Regular(ast::RegularFunctionArg {
-                name,
-                typ: typ_node,
-            }) => {
-                let typ = type_desc(&mut scope, typ_node).and_then(|typ| match typ.try_into() {
+            ast::FunctionArg::Regular(reg) => {
+                let typ = type_desc(&mut scope, &reg.typ).and_then(|typ| match typ.try_into() {
                     Ok(typ) => Ok(typ),
                     Err(_) => Err(TypeError::new(scope.error(
                         "function parameter types must have fixed size",
-                        typ_node.span,
+                        reg.typ.span,
                         "`Map` type can't be used as a function parameter",
                     ))),
                 });
@@ -110,30 +109,50 @@ pub fn function_signature(
                     }
                 }
 
-                if let Ok(Some(named_item)) = scope.resolve_name(&name.kind, name.span) {
+                if let Some(label) = &reg.label {
+                    if_chain! {
+                        if label.kind != "_";
+                        if let Some(dup_idx) = labels.get(&label.kind);
+                        then {
+                            let dup_arg: &Node<ast::FunctionArg> = &def.args[*dup_idx];
+                            scope.fancy_error(
+                                &format!("duplicate parameter labels in function `{}`", def.name.kind),
+                                vec![
+                                    Label::primary(dup_arg.span, "the label `{}` was first used here"),
+                                    Label::primary(label.span, "label `{}` used again here"),
+                                ], vec![]);
+                            return None;
+                        } else {
+                            labels.insert(&label.kind, index);
+                        }
+                    }
+                }
+
+                if let Ok(Some(named_item)) = scope.resolve_name(&reg.name.kind, reg.name.span) {
                     scope.name_conflict_error(
                         "function parameter",
-                        &name.kind,
+                        &reg.name.kind,
                         &named_item,
                         named_item.name_span(db),
-                        name.span,
+                        reg.name.span,
                     );
                     None
-                } else if let Some(dup_idx) = names.get(&name.kind) {
+                } else if let Some(dup_idx) = names.get(&reg.name.kind) {
                     let dup_arg: &Node<ast::FunctionArg> = &def.args[*dup_idx];
                     scope.duplicate_name_error(
                         &format!("duplicate parameter names in function `{}`", def.name.kind),
-                        &name.kind,
+                        &reg.name.kind,
                         dup_arg.span,
                         arg.span,
                     );
                     None
                 } else {
-                    names.insert(&name.kind, index);
-                    Some(types::FunctionParam {
-                        name: name.kind.clone(),
+                    names.insert(&reg.name.kind, index);
+                    Some(types::FunctionParam::new(
+                        reg.label.as_ref().map(|s| s.kind.as_str()),
+                        &reg.name.kind,
                         typ,
-                    })
+                    ))
                 }
             }
         })

crates/analyzer/src/namespace/types.rs

@@ -174,9 +174,26 @@ pub enum CtxDecl {
 
 #[derive(Clone, Debug, PartialEq, Eq, Hash)]
 pub struct FunctionParam {
+    label: Option<SmolStr>,
     pub name: SmolStr,
     pub typ: Result<FixedSize, TypeError>,
 }
+impl FunctionParam {
+    pub fn new(label: Option<&str>, name: &str, typ: Result<FixedSize, TypeError>) -> Self {
+        Self {
+            label: label.map(SmolStr::new),
+            name: name.into(),
+            typ,
+        }
+    }
+    pub fn label(&self) -> Option<&str> {
+        match &self.label {
+            Some(label) if label == "_" => None,
+            Some(label) => Some(label),
+            None => Some(&self.name),
+        }
+    }
+}
 
 #[derive(
     Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash, EnumString, AsRefStr, EnumIter,

crates/analyzer/src/traversal/call_args.rs

@@ -15,7 +15,7 @@ pub trait LabeledParameter {
 
 impl LabeledParameter for FunctionParam {
     fn label(&self) -> Option<&str> {
-        Some(&self.name)
+        self.label()
     }
     fn typ(&self) -> Result<FixedSize, TypeError> {
         self.typ.clone()
@@ -40,20 +40,6 @@ impl LabeledParameter for (SmolStr, Result<FixedSize, TypeError>) {
     }
 }
 
-pub fn validate_named_args(
-    context: &mut dyn AnalyzerContext,
-    name: &str,
-    name_span: Span,
-    args: &Node<Vec<Node<fe::CallArg>>>,
-    params: &[impl LabeledParameter],
-    label_policy: LabelPolicy,
-) -> Result<(), FatalError> {
-    validate_arg_count(context, name, name_span, args, params.len(), "argument");
-    validate_arg_labels(context, args, params, label_policy);
-    validate_arg_types(context, name, args, params)?;
-    Ok(())
-}
-
 pub fn validate_arg_count(
     context: &mut dyn AnalyzerContext,
     name: &str,
@@ -102,22 +88,18 @@ pub fn validate_arg_count(
     }
 }
 
-pub enum LabelPolicy {
-    AllowAnyUnlabeled,
-    AllowUnlabeledIfNameEqual,
-}
-
-pub fn validate_arg_labels(
+// TODO: missing label error should suggest adding `_` to fn def
+pub fn validate_named_args(
     context: &mut dyn AnalyzerContext,
+    name: &str,
+    name_span: Span,
     args: &Node<Vec<Node<fe::CallArg>>>,
     params: &[impl LabeledParameter],
-    label_policy: LabelPolicy,
-) {
-    for (expected_label, arg) in params
-        .iter()
-        .map(LabeledParameter::label)
-        .zip(args.kind.iter())
-    {
+) -> Result<(), FatalError> {
+    validate_arg_count(context, name, name_span, args, params.len(), "argument");
+
+    for (index, (param, arg)) in params.iter().zip(args.kind.iter()).enumerate() {
+        let expected_label = param.label();
         let arg_val = &arg.kind.value;
         match (expected_label, &arg.kind.label) {
             (Some(expected_label), Some(actual_label)) => {
@@ -143,8 +125,7 @@ pub fn validate_arg_labels(
             (Some(expected_label), None) => match &arg_val.kind {
                 fe::Expr::Name(var_name) if var_name == expected_label => {}
                 _ => {
-                    if let LabelPolicy::AllowUnlabeledIfNameEqual = label_policy {
-                        context.fancy_error(
+                    context.fancy_error(
                             "missing argument label",
                             vec![Label::primary(
                                 Span::new(arg_val.span.file_id, arg_val.span.start, arg_val.span.start),
@@ -155,7 +136,6 @@ pub fn validate_arg_labels(
                                 expected_label
                             )],
                         );
-                    }
                 }
             },
             (None, Some(actual_label)) => {
@@ -167,16 +147,7 @@ pub fn validate_arg_labels(
             }
             (None, None) => {}
         }
-    }
-}
 
-pub fn validate_arg_types(
-    context: &mut dyn AnalyzerContext,
-    name: &str,
-    args: &Node<Vec<Node<fe::CallArg>>>,
-    params: &[impl LabeledParameter],
-) -> Result<(), FatalError> {
-    for (index, (param, arg)) in params.iter().zip(args.kind.iter()).enumerate() {
         let param_type = param.typ()?;
         let val_attrs =
             assignable_expr(context, &arg.kind.value, Some(&param_type.clone().into()))?;

crates/analyzer/src/traversal/expressions.rs

@@ -7,7 +7,7 @@ use crate::namespace::types::{
     Array, Base, Contract, FeString, Integer, Struct, Tuple, Type, TypeDowncast, U256,
 };
 use crate::operations;
-use crate::traversal::call_args::{validate_arg_count, validate_named_args, LabelPolicy};
+use crate::traversal::call_args::{validate_arg_count, validate_named_args};
 use crate::traversal::types::apply_generic_type_args;
 use crate::traversal::utils::{add_bin_operations_errors, types_to_fixed_sizes};
 use fe_common::diagnostics::Label;
@@ -1097,14 +1097,7 @@ fn expr_call_pure(
 
     let sig = function.signature(context.db());
     let name_span = function.name_span(context.db());
-    validate_named_args(
-        context,
-        &fn_name,
-        name_span,
-        args,
-        &sig.params,
-        LabelPolicy::AllowAnyUnlabeled,
-    )?;
+    validate_named_args(context, &fn_name, name_span, args, &sig.params)?;
 
     let return_type = sig.return_type.clone()?;
     let return_location = Location::assign_location(&return_type);
@@ -1319,14 +1312,7 @@ fn expr_call_struct_constructor(
         .iter()
         .map(|(name, field)| (name.clone(), field.typ(db)))
         .collect::<Vec<_>>();
-    validate_named_args(
-        context,
-        name,
-        name_span,
-        args,
-        &fields,
-        LabelPolicy::AllowUnlabeledIfNameEqual,
-    )?;
+    validate_named_args(context, name, name_span, args, &fields)?;
 
     Ok((
         ExpressionAttributes::new(Type::Struct(struct_.clone()), Location::Memory),
@@ -1405,19 +1391,13 @@ fn expr_call_method(
             }
 
             let sig = method.signature(context.db());
+
             let params = if is_self {
                 &sig.params
             } else {
                 sig.external_params()
             };
-            validate_named_args(
-                context,
-                &field.kind,
-                field.span,
-                args,
-                params,
-                LabelPolicy::AllowAnyUnlabeled,
-            )?;
+            validate_named_args(context, &field.kind, field.span, args, params)?;
 
             let calltype = match class {
                 Class::Contract(contract) => {

crates/analyzer/src/traversal/functions.rs

@@ -3,7 +3,6 @@ use crate::errors::FatalError;
 use crate::namespace::items::Item;
 use crate::namespace::scopes::{BlockScope, BlockScopeType};
 use crate::namespace::types::{Base, EventField, FixedSize, Type};
-use crate::traversal::call_args::LabelPolicy;
 use crate::traversal::{assignments, call_args, declarations, expressions};
 use fe_common::diagnostics::Label;
 use fe_parser::ast as fe;
@@ -165,6 +164,7 @@ fn emit(scope: &mut BlockScope, stmt: &Node<fe::FuncStmt>) -> Result<(), FatalEr
             }
             Some(NamedThing::Item(Item::Event(event))) => {
                 scope.root.add_emit(stmt, event);
+
                 // Check visibility of event.
                 if !event.is_public(scope.db()) && event.module(scope.db()) != scope.module() {
                     let module_name = event.module(scope.db()).name(scope.db());
@@ -203,7 +203,6 @@ fn emit(scope: &mut BlockScope, stmt: &Node<fe::FuncStmt>) -> Result<(), FatalEr
                         name.span,
                         args,
                         &params_with_ctx,
-                        LabelPolicy::AllowUnlabeledIfNameEqual,
                     )?;
                 } else {
                     scope.fancy_error(