From 26fc813975170de5040e9abe79a80b7034719944 Mon Sep 17 00:00:00 2001 From: Dan Sinclair Date: Thu, 22 Feb 2018 20:01:15 +0000 Subject: [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 Reviewed-by: Ryan Harrison --- fxjs/cfxjse_formcalc_context.cpp | 6 +- xfa/fxfa/fm2js/cxfa_fmexpression.cpp | 109 ++++++++++++++---------------- xfa/fxfa/fm2js/cxfa_fmexpression.h | 14 +++- xfa/fxfa/fm2js/cxfa_fmparser.cpp | 17 +++-- xfa/fxfa/fm2js/cxfa_fmparser.h | 2 +- xfa/fxfa/fm2js/cxfa_fmparser_unittest.cpp | 82 ++++++++++++++++++---- 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 func = parser.Parse(); - if (!func || parser.HasError()) + std::unique_ptr 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&& arguments, std::vector>&& 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> 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&& arguments, std::vector>&& expressions); @@ -38,7 +37,18 @@ class CXFA_FMFunctionDefinition : public CXFA_FMExpression { WideStringView m_wsName; std::vector m_pArguments; std::vector> m_pExpressions; - bool m_isGlobal; +}; + +class CXFA_FMAST { + public: + explicit CXFA_FMAST( + std::vector> expressions); + ~CXFA_FMAST(); + + bool ToJavaScript(CFX_WideTextBuf& javascript); + + private: + std::vector> 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_FMParser::Parse() { +std::unique_ptr CXFA_FMParser::Parse() { auto expressions = ParseExpressionList(); if (HasError()) return nullptr; - std::vector arguments; - return pdfium::MakeUnique( - true, L"", std::move(arguments), std::move(expressions)); + return pdfium::MakeUnique(std::move(expressions)); } bool CXFA_FMParser::NextToken() { @@ -101,14 +99,15 @@ std::unique_ptr CXFA_FMParser::ParseFunction() { std::vector> 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_FMParser::ParseFunction() { } return pdfium::MakeUnique( - 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 Parse(); + std::unique_ptr 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(L""); - std::unique_ptr ast = parser->Parse(); + std::unique_ptr 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(L"; Just comment"); - std::unique_ptr ast = parser->Parse(); + std::unique_ptr 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(L"; Just comment\n12"); - std::unique_ptr ast = parser->Parse(); + std::unique_ptr 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(input); - std::unique_ptr ast = parser->Parse(); + std::unique_ptr 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(L"(a=(b=t))=u"); - std::unique_ptr ast = parser->Parse(); + std::unique_ptr 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(input); + std::unique_ptr 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(input); + std::unique_ptr 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()); +} -- cgit v1.2.3