[formcalc] Simplify function signature parsingchromium/3363

This CL refactors the signature parsing for formcalc functions.

Change-Id: I69634968ab1cbd9698a3ce3ae321e14bafe918c6
Reviewed-on: https://pdfium-review.googlesource.com/27930
Commit-Queue: dsinclair <dsinclair@chromium.org>
Reviewed-by: Ryan Harrison <rharrison@chromium.org>

2 files changed, 37 insertions, 29 deletions

diff --git a/xfa/fxfa/fm2js/cxfa_fmparser.cpp b/xfa/fxfa/fm2js/cxfa_fmparser.cpp
index a91cab3e84..09cbee5a10 100644
--- a/xfa/fxfa/fm2js/cxfa_fmparser.cpp
+++ b/xfa/fxfa/fm2js/cxfa_fmparser.cpp
@@ -94,56 +94,52 @@ std::unique_ptr<CXFA_FMExpression> CXFA_FMParser::ParseFunction() {
   AutoRestorer<unsigned long> restorer(&m_parse_depth);
   if (HasError() || !IncrementParseDepthAndCheck())
     return nullptr;
-
-  WideStringView ident;
-  std::vector<std::unique_ptr<CXFA_FMExpression>> expressions;
   if (!CheckThenNext(TOKfunc))
     return nullptr;
-
   if (m_token.m_type != TOKidentifier)
     return nullptr;
 
-  ident = m_token.m_string;
+  WideStringView ident = m_token.m_string;
   if (!NextToken())
     return nullptr;
   if (!CheckThenNext(TOKlparen))
     return nullptr;
 
   std::vector<WideStringView> arguments;
-  if (m_token.m_type == TOKrparen) {
-    if (!NextToken())
+  bool last_was_comma = false;
+  while (1) {
+    if (m_token.m_type == TOKrparen)
+      break;
+    if (m_token.m_type != TOKidentifier)
       return nullptr;
-  } else {
-    while (1) {
-      if (m_token.m_type != TOKidentifier)
-        return nullptr;
 
-      arguments.push_back(m_token.m_string);
-      if (!NextToken())
-        return nullptr;
-      if (m_token.m_type == TOKcomma) {
-        if (!NextToken())
-          return nullptr;
-        continue;
-      }
-      if (!CheckThenNext(TOKrparen))
-        return nullptr;
+    last_was_comma = false;
 
-      break;
-    }
-  }
+    arguments.push_back(m_token.m_string);
+    if (!NextToken())
+      return nullptr;
+    if (m_token.m_type != TOKcomma)
+      continue;
 
-  if (!CheckThenNext(TOKdo))
-    return nullptr;
-  if (m_token.m_type == TOKendfunc) {
+    last_was_comma = true;
     if (!NextToken())
       return nullptr;
-  } else {
+  }
+  if (last_was_comma || !CheckThenNext(TOKrparen))
+    return nullptr;
+  if (!CheckThenNext(TOKdo))
+    return nullptr;
+
+  std::vector<std::unique_ptr<CXFA_FMExpression>> expressions;
+  if (m_token.m_type != TOKendfunc) {
     expressions = ParseExpressionList();
-    if (!expressions.size() || !CheckThenNext(TOKendfunc))
+    if (expressions.empty())
       return nullptr;
   }
 
+  if (!CheckThenNext(TOKendfunc))
+    return nullptr;
+
   return pdfium::MakeUnique<CXFA_FMFunctionDefinition>(
       ident, std::move(arguments), std::move(expressions));
 }
diff --git a/xfa/fxfa/fm2js/cxfa_fmparser_unittest.cpp b/xfa/fxfa/fm2js/cxfa_fmparser_unittest.cpp
index d1e5919324..52de964f65 100644
--- a/xfa/fxfa/fm2js/cxfa_fmparser_unittest.cpp
+++ b/xfa/fxfa/fm2js/cxfa_fmparser_unittest.cpp
@@ -206,3 +206,15 @@ TEST(CXFA_FMParserTest, ParseFuncWithoutParams) {
   EXPECT_TRUE(ast->ToJavaScript(buf));
   EXPECT_EQ(ret, buf.AsStringView());
 }
+
+TEST(CXFA_FMParserTest, ParseFuncWithBadParamsList) {
+  const wchar_t input[] = {
+      L"func MyFunction(param1,) do\n"
+      L"  param1 * param2\n"
+      L"endfunc"};
+
+  auto parser = pdfium::MakeUnique<CXFA_FMParser>(input);
+  std::unique_ptr<CXFA_FMAST> ast = parser->Parse();
+  ASSERT_TRUE(ast == nullptr);
+  EXPECT_TRUE(parser->HasError());
+}