[formcalc] Cleanup function handling

This CL creates a CXFA_FMAST root for the AST tree instead of
overloading the CXFA_FMFunctionDefinition. This Removes the m_global
from FunctionDefinition and simpifies the code.

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

6 files changed, 143 insertions, 87 deletions

diff --git a/fxjs/cfxjse_formcalc_context.cpp b/fxjs/cfxjse_formcalc_context.cpp
index 82884265cf..f773da1e3f 100644
--- a/fxjs/cfxjse_formcalc_context.cpp
+++ b/fxjs/cfxjse_formcalc_context.cpp
@@ -6175,12 +6175,12 @@ bool CFXJSE_FormCalcContext::Translate(const WideStringView& wsFormcalc,
   }
 
   CXFA_FMParser parser(wsFormcalc);
-  std::unique_ptr<CXFA_FMFunctionDefinition> func = parser.Parse();
-  if (!func || parser.HasError())
+  std::unique_ptr<CXFA_FMAST> ast = parser.Parse();
+  if (!ast || parser.HasError())
     return false;
 
   CXFA_FMToJavaScriptDepth::Reset();
-  if (!func->ToJavaScript(*wsJavascript, ReturnType::kInfered))
+  if (!ast->ToJavaScript(*wsJavascript))
     return false;
 
   wsJavascript->AppendChar(0);
diff --git a/xfa/fxfa/fm2js/cxfa_fmexpression.cpp b/xfa/fxfa/fm2js/cxfa_fmexpression.cpp
index db93a38e50..b2995cae4c 100644
--- a/xfa/fxfa/fm2js/cxfa_fmexpression.cpp
+++ b/xfa/fxfa/fm2js/cxfa_fmexpression.cpp
@@ -19,63 +19,48 @@ const wchar_t kGreaterEqual[] = L" >= ";
 const wchar_t kPlusEqual[] = L" += ";
 const wchar_t kMinusEqual[] = L" -= ";
 
+WideString IdentifierToName(WideStringView ident) {
+  if (ident.IsEmpty())
+    return L"";
+  if (ident[0] != L'!')
+    return WideString(ident);
+  return L"pfm__excl__" + ident.Right(ident.GetLength() - 1);
+}
+
 }  // namespace
 
 CXFA_FMExpression::CXFA_FMExpression() = default;
 
 CXFA_FMFunctionDefinition::CXFA_FMFunctionDefinition(
-    bool isGlobal,
     const WideStringView& wsName,
     std::vector<WideStringView>&& arguments,
     std::vector<std::unique_ptr<CXFA_FMExpression>>&& expressions)
     : CXFA_FMExpression(),
       m_wsName(wsName),
       m_pArguments(std::move(arguments)),
-      m_pExpressions(std::move(expressions)),
-      m_isGlobal(isGlobal) {}
+      m_pExpressions(std::move(expressions)) {
+  ASSERT(!wsName.IsEmpty());
+}
 
 CXFA_FMFunctionDefinition::~CXFA_FMFunctionDefinition() = default;
 
 bool CXFA_FMFunctionDefinition::ToJavaScript(CFX_WideTextBuf& js,
                                              ReturnType type) {
   CXFA_FMToJavaScriptDepth depthManager;
-  if (type == ReturnType::kImplied)
-    return !CXFA_IsTooBig(js) && depthManager.IsWithinMaxDepth();
-
   if (CXFA_IsTooBig(js) || !depthManager.IsWithinMaxDepth())
     return false;
 
-  if (m_isGlobal && m_pExpressions.empty()) {
-    js << L"// comments only";
-    return !CXFA_IsTooBig(js);
-  }
-  if (m_isGlobal)
-    js << L"(\n";
-
-  js << L"function ";
-  if (!m_wsName.IsEmpty() && m_wsName[0] == L'!') {
-    WideString tempName =
-        L"pfm__excl__" + m_wsName.Right(m_wsName.GetLength() - 1);
-    js << tempName;
-  } else {
-    js << m_wsName;
-  }
+  if (m_wsName.IsEmpty())
+    return false;
 
-  js << L"(";
-  bool bNeedComma = false;
+  js << L"function " << IdentifierToName(m_wsName) << L"(";
   for (const auto& identifier : m_pArguments) {
-    if (bNeedComma)
+    if (identifier != m_pArguments.front())
       js << L", ";
-    if (identifier[0] == L'!') {
-      WideString tempIdentifier =
-          L"pfm__excl__" + identifier.Right(identifier.GetLength() - 1);
-      js << tempIdentifier;
-    } else {
-      js << identifier;
-    }
-    bNeedComma = true;
+
+    js << IdentifierToName(identifier);
   }
-  js << L")\n{\n";
+  js << L") {\n";
 
   js << L"var pfm_ret = null;\n";
   for (const auto& expr : m_pExpressions) {
@@ -85,15 +70,36 @@ bool CXFA_FMFunctionDefinition::ToJavaScript(CFX_WideTextBuf& js,
       return false;
   }
 
-  js << L"return ";
-  if (m_isGlobal)
-    js << L"pfm_rt.get_val(pfm_ret);\n";
-  else
-    js << L"pfm_ret;\n";
-
+  js << L"return pfm_ret;\n";
   js << L"}\n";
-  if (m_isGlobal)
-    js << L").call(this);\n";
+
+  return !CXFA_IsTooBig(js);
+}
+
+CXFA_FMAST::CXFA_FMAST(
+    std::vector<std::unique_ptr<CXFA_FMExpression>> expressions)
+    : expressions_(std::move(expressions)) {}
+
+CXFA_FMAST::~CXFA_FMAST() = default;
+
+bool CXFA_FMAST::ToJavaScript(CFX_WideTextBuf& js) {
+  if (expressions_.empty()) {
+    js << L"// comments only";
+    return !CXFA_IsTooBig(js);
+  }
+
+  js << L"(function() {\n";
+  js << L"var pfm_ret = null;\n";
+
+  for (const auto& expr : expressions_) {
+    ReturnType ret_type = expr == expressions_.back() ? ReturnType::kImplied
+                                                      : ReturnType::kInfered;
+    if (!expr->ToJavaScript(js, ret_type))
+      return false;
+  }
+
+  js << L"return pfm_rt.get_val(pfm_ret);\n";
+  js << L"}).call(this);";
 
   return !CXFA_IsTooBig(js);
 }
@@ -110,10 +116,7 @@ bool CXFA_FMVarExpression::ToJavaScript(CFX_WideTextBuf& js, ReturnType type) {
   if (CXFA_IsTooBig(js) || !depthManager.IsWithinMaxDepth())
     return false;
 
-  WideString tempName(m_wsName);
-  if (m_wsName[0] == L'!')
-    tempName = L"pfm__excl__" + m_wsName.Right(m_wsName.GetLength() - 1);
-
+  WideString tempName = IdentifierToName(m_wsName);
   js << L"var " << tempName << L" = ";
   if (m_pInit) {
     if (!m_pInit->ToJavaScript(js, ReturnType::kInfered))
@@ -356,12 +359,7 @@ bool CXFA_FMForExpression::ToJavaScript(CFX_WideTextBuf& js, ReturnType type) {
 
   js << L"{\n";
 
-  WideString tmpName;
-  if (m_wsVariant[0] == L'!')
-    tmpName = L"pfm__excl__" + m_wsVariant.Right(m_wsVariant.GetLength() - 1);
-  else
-    tmpName = m_wsVariant;
-
+  WideString tmpName = IdentifierToName(m_wsVariant);
   js << L"var " << tmpName << L" = null;\n";
 
   CFX_WideTextBuf assign_txt;
@@ -418,14 +416,7 @@ bool CXFA_FMForeachExpression::ToJavaScript(CFX_WideTextBuf& js,
 
   js << L"{\n";
 
-  WideString tmpName;
-  if (m_wsIdentifier[0] == L'!') {
-    tmpName =
-        L"pfm__excl__" + m_wsIdentifier.Right(m_wsIdentifier.GetLength() - 1);
-  } else {
-    tmpName = m_wsIdentifier;
-  }
-
+  WideString tmpName = IdentifierToName(m_wsIdentifier);
   js << L"var " << tmpName << L" = null;\n";
   js << L"var pfm_ary = pfm_rt.concat_obj(";
   for (const auto& expr : m_pAccessors) {
diff --git a/xfa/fxfa/fm2js/cxfa_fmexpression.h b/xfa/fxfa/fm2js/cxfa_fmexpression.h
index 5024432df9..32cbf96c95 100644
--- a/xfa/fxfa/fm2js/cxfa_fmexpression.h
+++ b/xfa/fxfa/fm2js/cxfa_fmexpression.h
@@ -26,7 +26,6 @@ class CXFA_FMExpression {
 class CXFA_FMFunctionDefinition : public CXFA_FMExpression {
  public:
   CXFA_FMFunctionDefinition(
-      bool isGlobal,
       const WideStringView& wsName,
       std::vector<WideStringView>&& arguments,
       std::vector<std::unique_ptr<CXFA_FMExpression>>&& expressions);
@@ -38,7 +37,18 @@ class CXFA_FMFunctionDefinition : public CXFA_FMExpression {
   WideStringView m_wsName;
   std::vector<WideStringView> m_pArguments;
   std::vector<std::unique_ptr<CXFA_FMExpression>> m_pExpressions;
-  bool m_isGlobal;
+};
+
+class CXFA_FMAST {
+ public:
+  explicit CXFA_FMAST(
+      std::vector<std::unique_ptr<CXFA_FMExpression>> expressions);
+  ~CXFA_FMAST();
+
+  bool ToJavaScript(CFX_WideTextBuf& javascript);
+
+ private:
+  std::vector<std::unique_ptr<CXFA_FMExpression>> expressions_;
 };
 
 class CXFA_FMVarExpression : public CXFA_FMExpression {
diff --git a/xfa/fxfa/fm2js/cxfa_fmparser.cpp b/xfa/fxfa/fm2js/cxfa_fmparser.cpp
index 9beda2c564..30746a39c4 100644
--- a/xfa/fxfa/fm2js/cxfa_fmparser.cpp
+++ b/xfa/fxfa/fm2js/cxfa_fmparser.cpp
@@ -28,14 +28,12 @@ CXFA_FMParser::CXFA_FMParser(const WideStringView& wsFormcalc)
 
 CXFA_FMParser::~CXFA_FMParser() {}
 
-std::unique_ptr<CXFA_FMFunctionDefinition> CXFA_FMParser::Parse() {
+std::unique_ptr<CXFA_FMAST> CXFA_FMParser::Parse() {
   auto expressions = ParseExpressionList();
   if (HasError())
     return nullptr;
 
-  std::vector<WideStringView> arguments;
-  return pdfium::MakeUnique<CXFA_FMFunctionDefinition>(
-      true, L"", std::move(arguments), std::move(expressions));
+  return pdfium::MakeUnique<CXFA_FMAST>(std::move(expressions));
 }
 
 bool CXFA_FMParser::NextToken() {
@@ -101,14 +99,15 @@ std::unique_ptr<CXFA_FMExpression> CXFA_FMParser::ParseFunction() {
   std::vector<std::unique_ptr<CXFA_FMExpression>> expressions;
   if (!CheckThenNext(TOKfunc))
     return nullptr;
+
   if (m_token.m_type != TOKidentifier) {
     m_error = true;
     return nullptr;
-  } else {
-    ident = m_token.m_string;
-    if (!NextToken())
-      return nullptr;
   }
+
+  ident = m_token.m_string;
+  if (!NextToken())
+    return nullptr;
   if (!CheckThenNext(TOKlparen))
     return nullptr;
 
@@ -149,7 +148,7 @@ std::unique_ptr<CXFA_FMExpression> CXFA_FMParser::ParseFunction() {
   }
 
   return pdfium::MakeUnique<CXFA_FMFunctionDefinition>(
-      false, ident, std::move(arguments), std::move(expressions));
+      ident, std::move(arguments), std::move(expressions));
 }
 
 // Expression := IfExpression | WhileExpression | ForExpression |
diff --git a/xfa/fxfa/fm2js/cxfa_fmparser.h b/xfa/fxfa/fm2js/cxfa_fmparser.h
index 103667d878..2f72871c02 100644
--- a/xfa/fxfa/fm2js/cxfa_fmparser.h
+++ b/xfa/fxfa/fm2js/cxfa_fmparser.h
@@ -18,7 +18,7 @@ class CXFA_FMParser {
   explicit CXFA_FMParser(const WideStringView& wsFormcalc);
   ~CXFA_FMParser();
 
-  std::unique_ptr<CXFA_FMFunctionDefinition> Parse();
+  std::unique_ptr<CXFA_FMAST> Parse();
   bool HasError() const;
 
   void SetMaxParseDepthForTest(unsigned long max_depth) {
diff --git a/xfa/fxfa/fm2js/cxfa_fmparser_unittest.cpp b/xfa/fxfa/fm2js/cxfa_fmparser_unittest.cpp
index e939cd954f..a7f7006fcc 100644
--- a/xfa/fxfa/fm2js/cxfa_fmparser_unittest.cpp
+++ b/xfa/fxfa/fm2js/cxfa_fmparser_unittest.cpp
@@ -14,20 +14,20 @@
 
 TEST(CXFA_FMParserTest, Empty) {
   auto parser = pdfium::MakeUnique<CXFA_FMParser>(L"");
-  std::unique_ptr<CXFA_FMFunctionDefinition> ast = parser->Parse();
+  std::unique_ptr<CXFA_FMAST> ast = parser->Parse();
   ASSERT(ast != nullptr);
   EXPECT_FALSE(parser->HasError());
 
   CXFA_FMToJavaScriptDepth::Reset();
   CFX_WideTextBuf buf;
-  EXPECT_TRUE(ast->ToJavaScript(buf, ReturnType::kInfered));
+  EXPECT_TRUE(ast->ToJavaScript(buf));
   // TODO(dsinclair): This is a little weird .....
   EXPECT_EQ(L"// comments only", buf.AsStringView());
 }
 
 TEST(CXFA_FMParserTest, CommentOnlyIsError) {
   auto parser = pdfium::MakeUnique<CXFA_FMParser>(L"; Just comment");
-  std::unique_ptr<CXFA_FMFunctionDefinition> ast = parser->Parse();
+  std::unique_ptr<CXFA_FMAST> ast = parser->Parse();
   ASSERT(ast != nullptr);
   // TODO(dsinclair): This isn't allowed per the spec.
   EXPECT_FALSE(parser->HasError());
@@ -35,26 +35,26 @@ TEST(CXFA_FMParserTest, CommentOnlyIsError) {
 
   CXFA_FMToJavaScriptDepth::Reset();
   CFX_WideTextBuf buf;
-  EXPECT_TRUE(ast->ToJavaScript(buf, ReturnType::kInfered));
+  EXPECT_TRUE(ast->ToJavaScript(buf));
   EXPECT_EQ(L"// comments only", buf.AsStringView());
 }
 
 TEST(CXFA_FMParserTest, CommentThenValue) {
   const wchar_t ret[] =
-      L"(\nfunction ()\n{\n"
+      L"(function() {\n"
       L"var pfm_ret = null;\n"
       L"pfm_ret = 12;\n"
       L"return pfm_rt.get_val(pfm_ret);\n"
-      L"}\n).call(this);\n";
+      L"}).call(this);";
 
   auto parser = pdfium::MakeUnique<CXFA_FMParser>(L"; Just comment\n12");
-  std::unique_ptr<CXFA_FMFunctionDefinition> ast = parser->Parse();
+  std::unique_ptr<CXFA_FMAST> ast = parser->Parse();
   ASSERT(ast != nullptr);
   EXPECT_FALSE(parser->HasError());
 
   CXFA_FMToJavaScriptDepth::Reset();
   CFX_WideTextBuf buf;
-  EXPECT_TRUE(ast->ToJavaScript(buf, ReturnType::kInfered));
+  EXPECT_TRUE(ast->ToJavaScript(buf));
   EXPECT_EQ(ret, buf.AsStringView());
 }
 
@@ -74,7 +74,7 @@ TEST(CXFA_FMParserTest, Parse) {
       L"$";
 
   const wchar_t ret[] =
-      L"(\nfunction ()\n{\n"
+      L"(function() {\n"
       L"var pfm_ret = null;\n"
       L"if (pfm_rt.is_obj(this))\n{\n"
       L"pfm_rt.asgn_val_op(this, pfm_rt.Avg(pfm_rt.neg_op(3), 5, "
@@ -114,16 +114,16 @@ TEST(CXFA_FMParserTest, Parse) {
       L"}\n"
       L"pfm_ret = this;\n"
       L"return pfm_rt.get_val(pfm_ret);\n"
-      L"}\n).call(this);\n";
+      L"}).call(this);";
 
   auto parser = pdfium::MakeUnique<CXFA_FMParser>(input);
-  std::unique_ptr<CXFA_FMFunctionDefinition> ast = parser->Parse();
+  std::unique_ptr<CXFA_FMAST> ast = parser->Parse();
   ASSERT(ast != nullptr);
   EXPECT_FALSE(parser->HasError());
 
   CXFA_FMToJavaScriptDepth::Reset();
   CFX_WideTextBuf buf;
-  EXPECT_TRUE(ast->ToJavaScript(buf, ReturnType::kInfered));
+  EXPECT_TRUE(ast->ToJavaScript(buf));
   EXPECT_EQ(ret, buf.AsStringView());
 }
 
@@ -146,7 +146,63 @@ TEST(CFXA_FMParserTest, chromium752201) {
 TEST(CXFA_FMParserTest, MultipleAssignmentIsNotAllowed) {
   auto parser = pdfium::MakeUnique<CXFA_FMParser>(L"(a=(b=t))=u");
 
-  std::unique_ptr<CXFA_FMFunctionDefinition> ast = parser->Parse();
+  std::unique_ptr<CXFA_FMAST> ast = parser->Parse();
   ASSERT(ast == nullptr);
   EXPECT_TRUE(parser->HasError());
 }
+
+TEST(CXFA_FMParserTest, ParseFuncWithParams) {
+  const wchar_t input[] = {
+      L"func MyFunction(param1, param2) do\n"
+      L"  param1 * param2\n"
+      L"endfunc"};
+
+  const wchar_t ret[] = {
+      L"(function() {\n"
+      L"var pfm_ret = null;\n"
+      L"function MyFunction(param1, param2) {\n"
+      L"var pfm_ret = null;\n"
+      L"pfm_ret = pfm_rt.mul_op(param1, param2);\n"
+      L"return pfm_ret;\n"
+      L"}\n"
+      L"return pfm_rt.get_val(pfm_ret);\n"
+      L"}).call(this);"};
+
+  auto parser = pdfium::MakeUnique<CXFA_FMParser>(input);
+  std::unique_ptr<CXFA_FMAST> ast = parser->Parse();
+  ASSERT(ast != nullptr);
+  EXPECT_FALSE(parser->HasError());
+
+  CXFA_FMToJavaScriptDepth::Reset();
+  CFX_WideTextBuf buf;
+  EXPECT_TRUE(ast->ToJavaScript(buf));
+  EXPECT_EQ(ret, buf.AsStringView());
+}
+
+TEST(CXFA_FMParserTest, ParseFuncWithoutParams) {
+  const wchar_t input[] = {
+      L"func MyFunction() do\n"
+      L"  42\n"
+      L"endfunc"};
+
+  const wchar_t ret[] = {
+      L"(function() {\n"
+      L"var pfm_ret = null;\n"
+      L"function MyFunction() {\n"
+      L"var pfm_ret = null;\n"
+      L"pfm_ret = 42;\n"
+      L"return pfm_ret;\n"
+      L"}\n"
+      L"return pfm_rt.get_val(pfm_ret);\n"
+      L"}).call(this);"};
+
+  auto parser = pdfium::MakeUnique<CXFA_FMParser>(input);
+  std::unique_ptr<CXFA_FMAST> ast = parser->Parse();
+  ASSERT(ast != nullptr);
+  EXPECT_FALSE(parser->HasError());
+
+  CXFA_FMToJavaScriptDepth::Reset();
+  CFX_WideTextBuf buf;
+  EXPECT_TRUE(ast->ToJavaScript(buf));
+  EXPECT_EQ(ret, buf.AsStringView());
+}