Fix parameter names not being checked

In 2018, when we
[removed case functions](#2542) from
Pony, we also removed the checking to make sure that parameters names were
valid.

We've added checking of parameter names to make sure they match the naming
rules for parameters in Pony.

Fixes #4054

.release-notes/4054.md

@@ -0,0 +1,5 @@
+## Fixed parameter names not being checked
+
+In 2018, when we [removed case functions](https://github.com/ponylang/ponyc/pull/2542) from Pony, we also removed the checking to make sure that parameters names were valid.
+
+We've added checking of parameter names to make sure they match the naming rules for parameters in Pony.

src/libponyc/pass/sugar.c

@@ -321,6 +321,51 @@ static ast_result_t sugar_typeparam(ast_t* ast)
 }
 
 
+static ast_result_t check_params(pass_opt_t* opt, ast_t* params)
+{
+  pony_assert(params != NULL);
+  ast_result_t result = AST_OK;
+
+  // Check each parameter.
+  for(ast_t* p = ast_child(params); p != NULL; p = ast_sibling(p))
+  {
+    if(ast_id(p) == TK_ELLIPSIS)
+      continue;
+
+    AST_GET_CHILDREN(p, id, type, def_arg);
+
+    if(ast_id(id) != TK_ID)
+    {
+      ast_error(opt->check.errors, p, "expected parameter name");
+      result = AST_ERROR;
+    }
+    else if(!is_name_internal_test(ast_name(id)) && !check_id_param(opt, id))
+    {
+      result = AST_ERROR;
+    }
+
+    if(ast_id(type) == TK_NONE)
+    {
+      ast_error(opt->check.errors, type, "expected parameter type");
+      result = AST_ERROR;
+    }
+  }
+
+  return result;
+}
+
+
+static ast_result_t check_method(pass_opt_t* opt, ast_t* method)
+{
+  pony_assert(method != NULL);
+
+  ast_result_t result = AST_OK;
+  ast_t* params = ast_childidx(method, 3);
+  result = check_params(opt, params);
+
+  return result;
+}
+
 
 static ast_result_t sugar_new(pass_opt_t* opt, ast_t* ast)
 {
@@ -348,7 +393,7 @@ static ast_result_t sugar_new(pass_opt_t* opt, ast_t* ast)
   }
 
   sugar_docstring(ast);
-  return AST_OK;
+  return check_method(opt, ast);
 }
 
 
@@ -367,7 +412,7 @@ static ast_result_t sugar_be(pass_opt_t* opt, ast_t* ast)
   }
 
   sugar_docstring(ast);
-  return AST_OK;
+  return check_method(opt, ast);
 }
 
 
@@ -405,10 +450,9 @@ void fun_defaults(ast_t* ast)
 
 static ast_result_t sugar_fun(pass_opt_t* opt, ast_t* ast)
 {
-  (void)opt;
   fun_defaults(ast);
   sugar_docstring(ast);
-  return AST_OK;
+  return check_method(opt, ast);
 }
 
 
@@ -835,6 +879,9 @@ static ast_result_t sugar_ffi(pass_opt_t* opt, ast_t* ast)
   ast_t* new_id = ast_from_string(id, stringtab_consume(new_name, len + 2));
   ast_replace(&id, new_id);
 
+//  if(ast_id(ast) == TK_FFIDECL)
+//    return check_params(opt, args);
+
   return AST_OK;
 }
 

test/libponyc/sugar.cc

@@ -553,31 +553,115 @@ TEST_F(SugarTest, FunctionNoReturnBody)
 }
 
 
-TEST_F(SugarTest, FunctionParamMustBeId)
+TEST_F(SugarTest, MethodParamMustBeId)
 {
   const char* good1 =
     "trait Foo\n"
     "  fun foo(x: U64) => 3";
 
   TEST_COMPILE(good1);
 
-  const char* good2 =
+  const char* good1_be =
     "trait Foo\n"
-    "  fun foo(_: U64) => 3";
+    "  be foo(x: U64) => 3";
 
-  TEST_COMPILE(good2);
+  TEST_COMPILE(good1_be);
+
+  const char* good1_new =
+    "class Foo\n"
+    "  new create(x: U64) => 3";
+
+  TEST_COMPILE(good1_new);
 
   const char* bad1 =
     "trait Foo\n"
     "  fun foo(x.y(): U64) => 3";
 
   TEST_ERROR(bad1);
 
+  const char* bad1_be =
+    "trait Foo\n"
+    "  be foo(x.y(): U64) => 3";
+
+  TEST_ERROR(bad1_be);
+
+  const char* bad1_new =
+    "class Foo\n"
+    "  new create(x.y(): U64) => 3";
+
+  TEST_ERROR(bad1_new);
+
   const char* bad2 =
     "trait Foo\n"
     "  fun foo($: U64) => 3";
 
   TEST_ERROR(bad2);
+
+  const char* bad2_be =
+    "trait Foo\n"
+    "  be foo($: U64) => 3";
+
+  TEST_ERROR(bad2_be);
+
+  const char* bad2_new =
+    "class Foo\n"
+    "  new create($: U64) => 3";
+
+  TEST_ERROR(bad2_new);
+
+  const char* bad3 =
+    "trait Foo\n"
+    "  fun foo(__: U64) => 3";
+
+  TEST_ERROR(bad3);
+
+  const char* bad3_be =
+    "trait Foo\n"
+    "  be foo(__: U64) => 3";
+
+  TEST_ERROR(bad3_be);
+
+  const char* bad3_new =
+    "class Foo\n"
+    "  new create(__: U64) => 3";
+
+  TEST_ERROR(bad3_new);
+
+  const char* bad4 =
+    "trait Foo\n"
+    "  fun foo(_x: U64) => 3";
+
+  TEST_ERROR(bad4);
+
+  const char* bad4_be =
+    "trait Foo\n"
+    "  be foo(_x: U64) => 3";
+
+  TEST_ERROR(bad4_be);
+
+  const char* bad4_new =
+    "class Foo\n"
+    "  new create(_x: U64) => 3";
+
+  TEST_ERROR(bad4_new);
+
+  const char* bad5 =
+    "trait Foo\n"
+    "  fun foo(_x: U64) => 3";
+
+  TEST_ERROR(bad5);
+
+  const char* bad5_be =
+    "trait Foo\n"
+    "  be foo(_x: U64) => 3";
+
+  TEST_ERROR(bad5_be);
+
+  const char* bad5_new =
+    "class Foo\n"
+    "  new create(_x: U64) => 3";
+
+  TEST_ERROR(bad5_new);
 }