[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>

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_FMExpression> 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_FMExpression> 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_FMExpression> CXFA_FMParser::ParseExpression() {
         return nullptr;
       break;
     default:
-      m_error = true;
       return nullptr;
   }
   return expr;
@@ -217,10 +213,8 @@ std::unique_ptr<CXFA_FMExpression> 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_FMSimpleExpression> CXFA_FMParser::ParseUnaryExpression() {
       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 @@ 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_FMSimpleExpression> CXFA_FMParser::ParsePostExpression(
               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 @@ std::unique_ptr<CXFA_FMSimpleExpression> 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_FMSimpleExpression> CXFA_FMParser::ParsePostExpression(
                 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 @@ std::unique_ptr<CXFA_FMSimpleExpression> 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_FMSimpleExpression> 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_FMSimpleExpression> 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_FMSimpleExpression> CXFA_FMParser::ParseIndexExpression() {
   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 @@ std::unique_ptr<CXFA_FMSimpleExpression> 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<CXFA_FMSimpleExpression> pExp1 = ParseSimpleExpression();
   if (!pExp1)
@@ -997,10 +969,9 @@ std::unique_ptr<CXFA_FMExpression> 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<CXFA_FMWhileExpression>(
       std::move(pCondition),
       pdfium::MakeUnique<CXFA_FMBlockExpression>(std::move(exprs)));
@@ -1015,20 +986,13 @@ std::unique_ptr<CXFA_FMExpression> 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<CXFA_FMSimpleExpression> pAssignment =
@@ -1037,14 +1001,12 @@ std::unique_ptr<CXFA_FMExpression> 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_FMExpression> 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<CXFA_FMForExpression>(
       wsVariant, std::move(pAssignment), std::move(pAccessor), iDirection,
@@ -1087,11 +1047,9 @@ std::unique_ptr<CXFA_FMExpression> 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_FMExpression> 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<CXFA_FMForeachExpression>(
       wsIdentifier, std::move(pArgumentList),
@@ -1140,10 +1093,8 @@ std::unique_ptr<CXFA_FMExpression> 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<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 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<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, Empty) {
 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 @@ TEST(CXFA_FMParserTest, CommentThenValue) {
 
   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 @@ TEST(CXFA_FMParserTest, Parse) {
 
   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 @@ TEST(CXFA_FMParserTest, MultipleAssignmentIsNotAllowed) {
   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 @@ TEST(CXFA_FMParserTest, ParseFuncWithParams) {
 
   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 @@ TEST(CXFA_FMParserTest, ParseFuncWithoutParams) {
 
   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();