From 406b23d9350777514e499cefdc080212de596e8d Mon Sep 17 00:00:00 2001 From: Dan Sinclair Date: Mon, 5 Mar 2018 21:04:51 +0000 Subject: [formcalc] Cleanup m_error handling This CL cleans up the setting of m_error. In most cases we don't need to set m_error it will be set when we bubble up the nullptr return from the various parse methods. The m_error was set inconsitently previously and was confusing on if it needed to be set or not. Change-Id: I8648b6296ef15239bd2663e6543a960b88177721 Reviewed-on: https://pdfium-review.googlesource.com/27910 Reviewed-by: Ryan Harrison Commit-Queue: dsinclair --- xfa/fxfa/fm2js/cxfa_fmparser.cpp | 109 ++++++++---------------------- xfa/fxfa/fm2js/cxfa_fmparser_unittest.cpp | 14 ++-- 2 files changed, 37 insertions(+), 86 deletions(-) diff --git a/xfa/fxfa/fm2js/cxfa_fmparser.cpp b/xfa/fxfa/fm2js/cxfa_fmparser.cpp index 30746a39c4..a91cab3e84 100644 --- a/xfa/fxfa/fm2js/cxfa_fmparser.cpp +++ b/xfa/fxfa/fm2js/cxfa_fmparser.cpp @@ -100,10 +100,8 @@ std::unique_ptr CXFA_FMParser::ParseFunction() { if (!CheckThenNext(TOKfunc)) return nullptr; - if (m_token.m_type != TOKidentifier) { - m_error = true; + if (m_token.m_type != TOKidentifier) return nullptr; - } ident = m_token.m_string; if (!NextToken()) @@ -117,10 +115,9 @@ std::unique_ptr CXFA_FMParser::ParseFunction() { return nullptr; } else { while (1) { - if (m_token.m_type != TOKidentifier) { - m_error = true; + if (m_token.m_type != TOKidentifier) return nullptr; - } + arguments.push_back(m_token.m_string); if (!NextToken()) return nullptr; @@ -200,7 +197,6 @@ std::unique_ptr CXFA_FMParser::ParseExpression() { return nullptr; break; default: - m_error = true; return nullptr; } return expr; @@ -217,10 +213,8 @@ std::unique_ptr CXFA_FMParser::ParseDeclarationExpression() { WideStringView ident; if (!NextToken()) return nullptr; - if (m_token.m_type != TOKidentifier) { - m_error = true; + if (m_token.m_type != TOKidentifier) return nullptr; - } ident = m_token.m_string; if (!NextToken()) @@ -596,10 +590,7 @@ std::unique_ptr CXFA_FMParser::ParseUnaryExpression() { expr = pdfium::MakeUnique(std::move(expr)); break; default: - expr = ParsePrimaryExpression(); - if (!expr) - return nullptr; - break; + return ParsePrimaryExpression(); } return expr; } @@ -643,13 +634,9 @@ CXFA_FMParser::ParsePrimaryExpression() { return nullptr; break; default: - m_error = true; return nullptr; } - expr = ParsePostExpression(std::move(expr)); - if (!expr) - return nullptr; - return expr; + return ParsePostExpression(std::move(expr)); } // Literal := String | Number | Null @@ -704,10 +691,8 @@ std::unique_ptr CXFA_FMParser::ParsePostExpression( break; } } - if (m_token.m_type != TOKrparen) { - m_error = true; + if (m_token.m_type != TOKrparen) return nullptr; - } } expr = pdfium::MakeUnique( std::move(expr), std::move(expressions), false); @@ -727,10 +712,9 @@ std::unique_ptr CXFA_FMParser::ParsePostExpression( case TOKdot: { if (!NextToken()) return nullptr; - if (m_token.m_type != TOKidentifier) { - m_error = true; + if (m_token.m_type != TOKidentifier) return nullptr; - } + WideStringView tempStr = m_token.m_string; if (!NextToken()) return nullptr; @@ -756,10 +740,8 @@ std::unique_ptr CXFA_FMParser::ParsePostExpression( break; } } - if (m_token.m_type != TOKrparen) { - m_error = true; + if (m_token.m_type != TOKrparen) return nullptr; - } } std::unique_ptr pIdentifier = pdfium::MakeUnique(tempStr); @@ -797,10 +779,8 @@ std::unique_ptr CXFA_FMParser::ParsePostExpression( case TOKdotdot: { if (!NextToken()) return nullptr; - if (m_token.m_type != TOKidentifier) { - m_error = true; + if (m_token.m_type != TOKidentifier) return nullptr; - } WideStringView tempStr = m_token.m_string; if (!NextToken()) @@ -824,10 +804,8 @@ std::unique_ptr CXFA_FMParser::ParsePostExpression( case TOKdotscream: { if (!NextToken()) return nullptr; - if (m_token.m_type != TOKidentifier) { - m_error = true; + if (m_token.m_type != TOKidentifier) return nullptr; - } WideStringView tempStr = m_token.m_string; if (!NextToken()) @@ -881,10 +859,8 @@ std::unique_ptr CXFA_FMParser::ParseIndexExpression() { // TODO(dsinclair): This should CheckThenNext(TOKrbracket) but need to clean // up the callsites. - if (m_token.m_type != TOKrbracket) { - m_error = true; + if (m_token.m_type != TOKrbracket) return nullptr; - } return pExp; } @@ -902,10 +878,9 @@ std::unique_ptr CXFA_FMParser::ParseIndexExpression() { std::unique_ptr s = ParseSimpleExpression(); if (!s) return nullptr; - if (m_token.m_type != TOKrbracket) { - m_error = true; + if (m_token.m_type != TOKrbracket) return nullptr; - } + return pdfium::MakeUnique(accessorIndex, std::move(s), false); } @@ -918,11 +893,8 @@ std::unique_ptr CXFA_FMParser::ParseParenExpression() { if (!CheckThenNext(TOKlparen)) return nullptr; - - if (m_token.m_type == TOKrparen) { - m_error = true; + if (m_token.m_type == TOKrparen) return nullptr; - } std::unique_ptr pExp1 = ParseSimpleExpression(); if (!pExp1) @@ -997,10 +969,9 @@ std::unique_ptr CXFA_FMParser::ParseWhileExpression() { return nullptr; auto exprs = ParseExpressionList(); - if (exprs.empty() || !CheckThenNext(TOKendwhile)) { - m_error = true; + if (exprs.empty() || !CheckThenNext(TOKendwhile)) return nullptr; - } + return pdfium::MakeUnique( std::move(pCondition), pdfium::MakeUnique(std::move(exprs))); @@ -1015,20 +986,13 @@ std::unique_ptr CXFA_FMParser::ParseForExpression() { return nullptr; if (!CheckThenNext(TOKfor)) return nullptr; - - if (m_token.m_type != TOKidentifier) { - m_error = true; + if (m_token.m_type != TOKidentifier) return nullptr; - } + WideStringView wsVariant = m_token.m_string; if (!NextToken()) return nullptr; - - if (m_token.m_type != TOKassign) { - m_error = true; - return nullptr; - } - if (!NextToken()) + if (!CheckThenNext(TOKassign)) return nullptr; std::unique_ptr pAssignment = @@ -1037,14 +1001,12 @@ std::unique_ptr CXFA_FMParser::ParseForExpression() { return nullptr; int32_t iDirection = 0; - if (m_token.m_type == TOKupto) { + if (m_token.m_type == TOKupto) iDirection = 1; - } else if (m_token.m_type == TOKdownto) { + else if (m_token.m_type == TOKdownto) iDirection = -1; - } else { - m_error = true; + else return nullptr; - } if (!NextToken()) return nullptr; @@ -1065,10 +1027,8 @@ std::unique_ptr CXFA_FMParser::ParseForExpression() { return nullptr; auto exprs = ParseExpressionList(); - if (exprs.empty() || !CheckThenNext(TOKendfor)) { - m_error = true; + if (exprs.empty() || !CheckThenNext(TOKendfor)) return nullptr; - } return pdfium::MakeUnique( wsVariant, std::move(pAssignment), std::move(pAccessor), iDirection, @@ -1087,11 +1047,9 @@ std::unique_ptr CXFA_FMParser::ParseForeachExpression() { return nullptr; if (!CheckThenNext(TOKforeach)) return nullptr; - - if (m_token.m_type != TOKidentifier) { - m_error = true; + if (m_token.m_type != TOKidentifier) return nullptr; - } + WideStringView wsIdentifier = m_token.m_string; if (!NextToken() || !CheckThenNext(TOKin) || !CheckThenNext(TOKlparen)) return nullptr; @@ -1109,19 +1067,14 @@ std::unique_ptr CXFA_FMParser::ParseForeachExpression() { return nullptr; } // We must have arguments. - if (pArgumentList.empty()) { - m_error = true; + if (pArgumentList.empty()) return nullptr; - } - if (!CheckThenNext(TOKrparen)) return nullptr; auto exprs = ParseExpressionList(); - if (exprs.empty() || !CheckThenNext(TOKendfor)) { - m_error = true; + if (exprs.empty() || !CheckThenNext(TOKendfor)) return nullptr; - } return pdfium::MakeUnique( wsIdentifier, std::move(pArgumentList), @@ -1140,10 +1093,8 @@ std::unique_ptr CXFA_FMParser::ParseDoExpression() { return nullptr; auto exprs = ParseExpressionList(); - if (exprs.empty() || !CheckThenNext(TOKend)) { - m_error = true; + if (exprs.empty() || !CheckThenNext(TOKend)) return nullptr; - } return pdfium::MakeUnique( pdfium::MakeUnique(std::move(exprs))); diff --git a/xfa/fxfa/fm2js/cxfa_fmparser_unittest.cpp b/xfa/fxfa/fm2js/cxfa_fmparser_unittest.cpp index a7f7006fcc..d1e5919324 100644 --- a/xfa/fxfa/fm2js/cxfa_fmparser_unittest.cpp +++ b/xfa/fxfa/fm2js/cxfa_fmparser_unittest.cpp @@ -15,7 +15,7 @@ TEST(CXFA_FMParserTest, Empty) { auto parser = pdfium::MakeUnique(L""); std::unique_ptr ast = parser->Parse(); - ASSERT(ast != nullptr); + ASSERT_TRUE(ast); EXPECT_FALSE(parser->HasError()); CXFA_FMToJavaScriptDepth::Reset(); @@ -28,7 +28,7 @@ TEST(CXFA_FMParserTest, Empty) { TEST(CXFA_FMParserTest, CommentOnlyIsError) { auto parser = pdfium::MakeUnique(L"; Just comment"); std::unique_ptr ast = parser->Parse(); - ASSERT(ast != nullptr); + ASSERT_TRUE(ast); // TODO(dsinclair): This isn't allowed per the spec. EXPECT_FALSE(parser->HasError()); // EXPECT_TRUE(parser->HasError()); @@ -49,7 +49,7 @@ TEST(CXFA_FMParserTest, CommentThenValue) { auto parser = pdfium::MakeUnique(L"; Just comment\n12"); std::unique_ptr ast = parser->Parse(); - ASSERT(ast != nullptr); + ASSERT_TRUE(ast); EXPECT_FALSE(parser->HasError()); CXFA_FMToJavaScriptDepth::Reset(); @@ -118,7 +118,7 @@ TEST(CXFA_FMParserTest, Parse) { auto parser = pdfium::MakeUnique(input); std::unique_ptr ast = parser->Parse(); - ASSERT(ast != nullptr); + ASSERT_TRUE(ast); EXPECT_FALSE(parser->HasError()); CXFA_FMToJavaScriptDepth::Reset(); @@ -147,7 +147,7 @@ TEST(CXFA_FMParserTest, MultipleAssignmentIsNotAllowed) { auto parser = pdfium::MakeUnique(L"(a=(b=t))=u"); std::unique_ptr ast = parser->Parse(); - ASSERT(ast == nullptr); + ASSERT_TRUE(!ast); EXPECT_TRUE(parser->HasError()); } @@ -170,7 +170,7 @@ TEST(CXFA_FMParserTest, ParseFuncWithParams) { auto parser = pdfium::MakeUnique(input); std::unique_ptr ast = parser->Parse(); - ASSERT(ast != nullptr); + ASSERT_TRUE(ast); EXPECT_FALSE(parser->HasError()); CXFA_FMToJavaScriptDepth::Reset(); @@ -198,7 +198,7 @@ TEST(CXFA_FMParserTest, ParseFuncWithoutParams) { auto parser = pdfium::MakeUnique(input); std::unique_ptr ast = parser->Parse(); - ASSERT(ast != nullptr); + ASSERT_TRUE(ast); EXPECT_FALSE(parser->HasError()); CXFA_FMToJavaScriptDepth::Reset(); -- cgit v1.2.3