[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 <rharrison@chromium.org>
Commit-Queue: dsinclair <dsinclair@chromium.org>
diff --git a/xfa/fxfa/fm2js/cxfa_fmparser.cpp b/xfa/fxfa/fm2js/cxfa_fmparser.cpp
index 30746a3..a91cab3 100644
--- a/xfa/fxfa/fm2js/cxfa_fmparser.cpp
+++ b/xfa/fxfa/fm2js/cxfa_fmparser.cpp
@@ -100,10 +100,8 @@
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 @@
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 @@
return nullptr;
break;
default:
- m_error = true;
return nullptr;
}
return expr;
@@ -217,10 +213,8 @@
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 @@
expr = pdfium::MakeUnique<CXFA_FMNotExpression>(std::move(expr));
break;
default:
- expr = ParsePrimaryExpression();
- if (!expr)
- return nullptr;
- break;
+ return ParsePrimaryExpression();
}
return expr;
}
@@ -643,13 +634,9 @@
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 @@
break;
}
}
- if (m_token.m_type != TOKrparen) {
- m_error = true;
+ if (m_token.m_type != TOKrparen)
return nullptr;
- }
}
expr = pdfium::MakeUnique<CXFA_FMCallExpression>(
std::move(expr), std::move(expressions), false);
@@ -727,10 +712,9 @@
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 @@
break;
}
}
- if (m_token.m_type != TOKrparen) {
- m_error = true;
+ if (m_token.m_type != TOKrparen)
return nullptr;
- }
}
std::unique_ptr<CXFA_FMSimpleExpression> pIdentifier =
pdfium::MakeUnique<CXFA_FMIdentifierExpression>(tempStr);
@@ -797,10 +779,8 @@
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 @@
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 @@
// 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_FMSimpleExpression> 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<CXFA_FMIndexExpression>(accessorIndex, std::move(s),
false);
}
@@ -918,11 +893,8 @@
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<CXFA_FMSimpleExpression> pExp1 = ParseSimpleExpression();
if (!pExp1)
@@ -997,10 +969,9 @@
return nullptr;
auto exprs = ParseExpressionList();
- if (exprs.empty() || !CheckThenNext(TOKendwhile)) {
- m_error = true;
+ if (exprs.empty() || !CheckThenNext(TOKendwhile))
return nullptr;
- }
+
return pdfium::MakeUnique<CXFA_FMWhileExpression>(
std::move(pCondition),
pdfium::MakeUnique<CXFA_FMBlockExpression>(std::move(exprs)));
@@ -1015,20 +986,13 @@
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<CXFA_FMSimpleExpression> pAssignment =
@@ -1037,14 +1001,12 @@
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 @@
return nullptr;
auto exprs = ParseExpressionList();
- if (exprs.empty() || !CheckThenNext(TOKendfor)) {
- m_error = true;
+ if (exprs.empty() || !CheckThenNext(TOKendfor))
return nullptr;
- }
return pdfium::MakeUnique<CXFA_FMForExpression>(
wsVariant, std::move(pAssignment), std::move(pAccessor), iDirection,
@@ -1087,11 +1047,9 @@
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 @@
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<CXFA_FMForeachExpression>(
wsIdentifier, std::move(pArgumentList),
@@ -1140,10 +1093,8 @@
return nullptr;
auto exprs = ParseExpressionList();
- if (exprs.empty() || !CheckThenNext(TOKend)) {
- m_error = true;
+ if (exprs.empty() || !CheckThenNext(TOKend))
return nullptr;
- }
return pdfium::MakeUnique<CXFA_FMDoExpression>(
pdfium::MakeUnique<CXFA_FMBlockExpression>(std::move(exprs)));
diff --git a/xfa/fxfa/fm2js/cxfa_fmparser_unittest.cpp b/xfa/fxfa/fm2js/cxfa_fmparser_unittest.cpp
index a7f7006..d1e5919 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<CXFA_FMParser>(L"");
std::unique_ptr<CXFA_FMAST> ast = parser->Parse();
- ASSERT(ast != nullptr);
+ ASSERT_TRUE(ast);
EXPECT_FALSE(parser->HasError());
CXFA_FMToJavaScriptDepth::Reset();
@@ -28,7 +28,7 @@
TEST(CXFA_FMParserTest, CommentOnlyIsError) {
auto parser = pdfium::MakeUnique<CXFA_FMParser>(L"; Just comment");
std::unique_ptr<CXFA_FMAST> 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 @@
auto parser = pdfium::MakeUnique<CXFA_FMParser>(L"; Just comment\n12");
std::unique_ptr<CXFA_FMAST> ast = parser->Parse();
- ASSERT(ast != nullptr);
+ ASSERT_TRUE(ast);
EXPECT_FALSE(parser->HasError());
CXFA_FMToJavaScriptDepth::Reset();
@@ -118,7 +118,7 @@
auto parser = pdfium::MakeUnique<CXFA_FMParser>(input);
std::unique_ptr<CXFA_FMAST> ast = parser->Parse();
- ASSERT(ast != nullptr);
+ ASSERT_TRUE(ast);
EXPECT_FALSE(parser->HasError());
CXFA_FMToJavaScriptDepth::Reset();
@@ -147,7 +147,7 @@
auto parser = pdfium::MakeUnique<CXFA_FMParser>(L"(a=(b=t))=u");
std::unique_ptr<CXFA_FMAST> ast = parser->Parse();
- ASSERT(ast == nullptr);
+ ASSERT_TRUE(!ast);
EXPECT_TRUE(parser->HasError());
}
@@ -170,7 +170,7 @@
auto parser = pdfium::MakeUnique<CXFA_FMParser>(input);
std::unique_ptr<CXFA_FMAST> ast = parser->Parse();
- ASSERT(ast != nullptr);
+ ASSERT_TRUE(ast);
EXPECT_FALSE(parser->HasError());
CXFA_FMToJavaScriptDepth::Reset();
@@ -198,7 +198,7 @@
auto parser = pdfium::MakeUnique<CXFA_FMParser>(input);
std::unique_ptr<CXFA_FMAST> ast = parser->Parse();
- ASSERT(ast != nullptr);
+ ASSERT_TRUE(ast);
EXPECT_FALSE(parser->HasError());
CXFA_FMToJavaScriptDepth::Reset();
