Refactor `JSONReactor::dictionaryItem`: remove redundant `QTC::TC` calls, enhance assert usage.

m-holger
1 parent 4c23d2c0
Showing 2 changed files with 115 additions and 125 deletions
libqpdf/QPDF_json.cc
qpdf/qpdf.testcov
@@ -460,104 +460,111 @@ QPDF::JSONReactor::dictionaryItem(std::string const&amp; key, JSON const&amp; value)
     next_state = st_ignore;
     auto state = stack.back().state;
     if (state == st_ignore) {
-        QTC::TC("qpdf", "QPDF_json ignoring in st_ignore");
-        // ignore
-    } else if (state == st_top) {
+        return true; // ignore
+    }
+    if (state == st_top) {
         if (key == "qpdf") {
-            this->saw_qpdf = true;
+            saw_qpdf = true;
             if (!value.isArray()) {
                 error(value.getStart(), "\"qpdf\" must be an array");
             } else {
                 next_state = st_qpdf;
             }
-        } else {
-            // Ignore all other fields.
-            QTC::TC("qpdf", "QPDF_json ignoring unknown top-level key");
+            return true;
         }
-    } else if (state == st_qpdf_meta) {
+        return true; // Ignore all other fields.
+    }
+
+    if (state == st_qpdf_meta) {
         if (key == "pdfversion") {
-            this->saw_pdf_version = true;
+            saw_pdf_version = true;
             std::string v;
-            bool okay = false;
             if (value.getString(v)) {
                 std::string version;
                 char const* p = v.c_str();
                 if (QPDF::validatePDFVersion(p, version) && (*p == '\0')) {
-                    this->pdf.m->pdf_version = version;
-                    okay = true;
+                    pdf.m->pdf_version = version;
+                    return true;
                 }
             }
-            if (!okay) {
-                error(value.getStart(), "invalid PDF version (must be \"x.y\")");
-            }
-        } else if (key == "jsonversion") {
-            this->saw_json_version = true;
+            error(value.getStart(), "invalid PDF version (must be \"x.y\")");
+            return true;
+        }
+        if (key == "jsonversion") {
+            saw_json_version = true;
             std::string v;
-            bool okay = false;
             if (value.getNumber(v)) {
                 std::string version;
                 if (QUtil::string_to_int(v.c_str()) == 2) {
-                    okay = true;
+                    return true;
                 }
             }
-            if (!okay) {
-                error(value.getStart(), "invalid JSON version (must be numeric value 2)");
-            }
-        } else if (key == "pushedinheritedpageresources") {
+            error(value.getStart(), "invalid JSON version (must be numeric value 2)");
+            return true;
+        }
+        if (key == "pushedinheritedpageresources") {
             bool v;
             if (value.getBool(v)) {
-                if (!this->must_be_complete && v) {
-                    this->pdf.pushInheritedAttributesToPage();
+                if (!must_be_complete && v) {
+                    pdf.pushInheritedAttributesToPage();
                 }
-            } else {
-                error(value.getStart(), "pushedinheritedpageresources must be a boolean");
+                return true;
             }
-        } else if (key == "calledgetallpages") {
+            error(value.getStart(), "pushedinheritedpageresources must be a boolean");
+            return true;
+        }
+        if (key == "calledgetallpages") {
             bool v;
             if (value.getBool(v)) {
-                if (!this->must_be_complete && v) {
+                if (!must_be_complete && v) {
                     (void)pdf.doc().pages().all();
                 }
-            } else {
-                error(value.getStart(), "calledgetallpages must be a boolean");
+                return true;
             }
-        } else {
-            // ignore unknown keys for forward compatibility and to skip keys we don't care about
-            // like "maxobjectid".
-            QTC::TC("qpdf", "QPDF_json ignore second-level key");
+            error(value.getStart(), "calledgetallpages must be a boolean");
+            return true;
         }
-    } else if (state == st_objects) {
-        int obj = 0;
-        int gen = 0;
+        // ignore unknown keys for forward compatibility and to skip keys we don't care about
+        // like "maxobjectid".
+        return true;
+    }
+
+    if (state == st_objects) {
         if (key == "trailer") {
-            this->saw_trailer = true;
-            this->cur_object = "trailer";
+            saw_trailer = true;
+            cur_object = "trailer";
             setNextStateIfDictionary(key, value, st_trailer);
-        } else if (is_obj_key(key, obj, gen)) {
-            this->cur_object = key;
+            return true;
+        }
+
+        int obj = 0;
+        int gen = 0;
+        if (is_obj_key(key, obj, gen)) {
+            cur_object = key;
             if (setNextStateIfDictionary(key, value, st_object_top)) {
                 next_obj = objects.getObjectForJSON(obj, gen);
             }
-        } else {
-            error(value.getStart(), "object key should be \"trailer\" or \"obj:n n R\"");
-        }
-    } else if (state == st_object_top) {
-        if (stack.empty()) {
-            throw std::logic_error("stack empty in st_object_top");
+            return true;
         }
+        error(value.getStart(), "object key should be \"trailer\" or \"obj:n n R\"");
+        return true;
+    }
+
+    if (state == st_object_top) {
+        util::assertion(!stack.empty(), "QPDF_json: stack empty in st_object_top");
         auto& tos = stack.back();
-        if (!tos.object) {
-            throw std::logic_error("current object uninitialized in st_object_top");
-        }
+        util::assertion(!!tos.object, "current object uninitialized in st_object_top");
         if (key == "value") {
             // Don't use setNextStateIfDictionary since this can have any type.
-            this->saw_value = true;
+            saw_value = true;
             replaceObject(makeObject(value), value);
             next_state = st_object;
-        } else if (key == "stream") {
-            this->saw_stream = true;
+            return true;
+        }
+        if (key == "stream") {
+            saw_stream = true;
             if (setNextStateIfDictionary(key, value, st_stream)) {
-                this->this_stream_needs_data = false;
+                this_stream_needs_data = false;
                 if (tos.object.isStream()) {
                     QTC::TC("qpdf", "QPDF_json updating existing stream");
                 } else {
@@ -568,95 +575,87 @@ QPDF::JSONReactor::dictionaryItem(std::string const&amp; key, JSON const&amp; value)
                         value);
                 }
                 next_obj = tos.object;
-            } else {
-                // Error message already given above
-                QTC::TC("qpdf", "QPDF_json stream not a dictionary");
+                return true;
             }
-        } else {
-            // Ignore unknown keys for forward compatibility
-            QTC::TC("qpdf", "QPDF_json ignore unknown key in object_top");
+            return true; // Error message already given above
         }
-    } else if (state == st_trailer) {
+        return true; // Ignore unknown keys for forward compatibility
+    }
+
+    if (state == st_trailer) {
         if (key == "value") {
-            this->saw_value = true;
+            saw_value = true;
             // The trailer must be a dictionary, so we can use setNextStateIfDictionary.
             if (setNextStateIfDictionary("trailer.value", value, st_object)) {
-                this->pdf.m->trailer = makeObject(value);
-                setObjectDescription(this->pdf.m->trailer, value);
+                pdf.m->trailer = makeObject(value);
+                setObjectDescription(pdf.m->trailer, value);
             }
-        } else if (key == "stream") {
+            return true;
+        }
+        if (key == "stream") {
             // Don't need to set saw_stream here since there's already an error.
             error(value.getStart(), "the trailer may not be a stream");
-        } else {
-            // Ignore unknown keys for forward compatibility
-            QTC::TC("qpdf", "QPDF_json ignore unknown key in trailer");
-        }
-    } else if (state == st_stream) {
-        if (stack.empty()) {
-            throw std::logic_error("stack empty in st_stream");
+            return true;
         }
+        return true; // Ignore unknown keys for forward compatibility
+    }
+
+    if (state == st_stream) {
+        util::assertion(!stack.empty(), "stack empty in st_stream");
         auto& tos = stack.back();
-        if (!tos.object.isStream()) {
-            throw std::logic_error("current object is not stream in st_stream");
-        }
+        util::assertion(tos.object.isStream(), "current object is not stream in st_stream");
         if (key == "dict") {
-            this->saw_dict = true;
+            saw_dict = true;
             if (setNextStateIfDictionary("stream.dict", value, st_object)) {
                 tos.object.replaceDict(makeObject(value));
-            } else {
-                // An error had already been given by setNextStateIfDictionary
-                QTC::TC("qpdf", "QPDF_json stream dict not dict");
+                return true;
             }
-        } else if (key == "data") {
-            this->saw_data = true;
+            return true; // An error had already been given by setNextStateIfDictionary
+        }
+        if (key == "data") {
+            saw_data = true;
             std::string v;
             if (!value.getString(v)) {
                 error(value.getStart(), "\"stream.data\" must be a string");
                 tos.object.replaceStreamData("", {}, {});
-            } else {
-                // The range includes the quotes.
-                auto start = value.getStart() + 1;
-                auto end = value.getEnd() - 1;
-                if (end < start) {
-                    throw std::logic_error("QPDF_json: JSON string length < 0");
-                }
-                tos.object.replaceStreamData(provide_data(is, start, end), {}, {});
+                return true;
             }
-        } else if (key == "datafile") {
-            this->saw_datafile = true;
+            // The range includes the quotes.
+            auto start = value.getStart() + 1;
+            auto end = value.getEnd() - 1;
+            util::assertion(end >= start, "QPDF_json: JSON string length < 0");
+            tos.object.replaceStreamData(provide_data(is, start, end), {}, {});
+            return true;
+        }
+        if (key == "datafile") {
+            saw_datafile = true;
             std::string filename;
             if (!value.getString(filename)) {
-                QTC::TC("qpdf", "QPDF_json stream datafile not string");
                 error(
                     value.getStart(),
                     "\"stream.datafile\" must be a string containing a file name");
                 tos.object.replaceStreamData("", {}, {});
-            } else {
-                tos.object.replaceStreamData(QUtil::file_provider(filename), {}, {});
+                return true;
             }
-        } else {
-            // Ignore unknown keys for forward compatibility.
-            QTC::TC("qpdf", "QPDF_json ignore unknown key in stream");
-        }
-    } else if (state == st_object) {
-        if (stack.empty()) {
-            throw std::logic_error("stack empty in st_object");
+            tos.object.replaceStreamData(QUtil::file_provider(filename), {}, {});
+            return true;
         }
-        auto& tos = stack.back();
-        auto dict = tos.object;
-        if (dict.isStream()) {
-            dict = dict.getDict();
-        }
-        if (!dict.isDictionary()) {
-            throw std::logic_error(
-                "current object is not stream or dictionary in st_object dictionary item");
-        }
-        dict.replaceKey(
-            is_pdf_name(key) ? QPDFObjectHandle::parse(key.substr(2)).getName() : key,
-            makeObject(value));
-    } else {
-        throw std::logic_error("QPDF_json: unknown state " + std::to_string(state));
+        return true; // Ignore unknown keys for forward compatibility.
     }
+
+    util::assertion(state == st_object, "QPDF_json: unknown state " + std::to_string(state));
+    util::assertion(!stack.empty(), "stack empty in st_object");
+    auto& tos = stack.back();
+    auto dict = tos.object;
+    if (dict.isStream()) {
+        dict = dict.getDict();
+    }
+    util::assertion(
+        dict.isDictionary(),
+        "current object is not stream or dictionary in st_object dictionary item");
+    dict.replaceKey(
+        is_pdf_name(key) ? QPDFObjectHandle::parse(key.substr(2)).getName() : key,
+        makeObject(value));
     return true;
 }
@@ -423,8 +423,6 @@ QPDF_json top-level scalar 0
 QPDF_json top-level array 0
 QPDF_json missing trailer 0
 QPDF_json missing objects 0
-QPDF_json ignoring in st_ignore 0
-QPDF_json stream dict not dict 0
 QPDF_json unrecognized string value 0
 QPDF_json data datafile both or neither 0
 QPDF_json stream no dict 0
@@ -433,11 +431,6 @@ QPDF_json value stream both or neither 0
 QPDFJob need json-stream-prefix for stdout 0
 QPDFJob write json to stdout 0
 QPDFJob write json to file 0
-QPDF_json ignoring unknown top-level key 0
-QPDF_json ignore second-level key 0
-QPDF_json ignore unknown key in object_top 0
-QPDF_json ignore unknown key in trailer 0
-QPDF_json ignore unknown key in stream 0
 QPDF_json data and datafile 0
 QPDF_json no stream data in update mode 0
 QPDF_json updating existing stream 0
@@ -448,5 +441,3 @@ QPDF skipping cache for known unchecked object 0
 QPDF recover xref stream 0
 QPDFJob json over/under no file 0
 QPDF_Array copy 1
-QPDF_json stream datafile not string 0
-QPDF_json stream not a dictionary 0