json-parser: warn about unknown nodes in our JSON format

kdudka · kdudka · commit 3bdd3c8809ba · 2020-10-15T17:47:31.000+02:00
Otherwise they would be silently ignored with even `--mode=json`,
which is supposed to preserve as much information as possible.
diff --git a/json-parser.cc b/json-parser.cc
@@ -21,6 +21,8 @@
 
 #include "csparser.hh"              // for KeyEventDigger
 
+#include <set>
+
 #include <boost/foreach.hpp>
 #include <boost/property_tree/json_parser.hpp>
 
@@ -44,10 +46,25 @@ class AbstractTreeDecoder {
 /// tree decoder of the native JSON format of csdiff
 class SimpleTreeDecoder: public AbstractTreeDecoder {
     public:
+        SimpleTreeDecoder(const std::string &fileName, bool silent);
         virtual void readNode(Defect *def, const pt::ptree &node);
 
     private:
-        KeyEventDigger              keDigger;
+        enum ENodeKind {
+            NK_DEFECT,
+            NK_EVENT,
+            NK_LAST
+        };
+
+        void reportUnknownNodes(ENodeKind, const pt::ptree &) const;
+
+        typedef std::set<std::string>       TNodeSet;
+        typedef std::vector<TNodeSet>       TNodeStore;
+
+        const std::string           fileName_;
+        const bool                  silent_;
+        TNodeStore                  nodeStore_;
+        KeyEventDigger              keDigger_;
 };
 
 /// tree decoder of the Coverity JSON format
@@ -145,7 +162,7 @@ JsonParser::JsonParser(
 
         if (findChildOf(&d->defList, d->root, "defects"))
             // csdiff-native JSON format
-            d->decoder = new SimpleTreeDecoder;
+            d->decoder = new SimpleTreeDecoder(fileName, silent);
         else if (findChildOf(&d->defList, d->root, "issues"))
             // Coverity JSON format
             d->decoder = new CovTreeDecoder;
@@ -214,13 +231,66 @@ bool JsonParser::getNext(Defect *def) {
     }
 }
 
+SimpleTreeDecoder::SimpleTreeDecoder(const std::string &fileName, bool silent):
+    fileName_(fileName),
+    silent_(silent)
+{
+    if (silent_)
+        // skip initialization of nodeStore_ because no lookup will ever happen
+        return;
+
+    nodeStore_.resize(NK_LAST);
+
+    // known per-defect subnodes
+    nodeStore_[NK_DEFECT] = {
+        "annotation",
+        "checker",
+        "cwe",
+        "defect_id",
+        "events",
+        "function",
+        "imp",
+        "key_event_idx",
+        "language",
+    };
+
+    // known per-event subnodes
+    nodeStore_[NK_EVENT] = {
+        "column",
+        "event",
+        "file_name",
+        "line",
+        "message",
+        "verbosity_level",
+    };
+}
+
+void SimpleTreeDecoder::reportUnknownNodes(ENodeKind nk, const pt::ptree &node)
+    const
+{
+    if (silent_)
+        return;
+
+    const TNodeSet &nodeSet = nodeStore_[nk];
+
+    BOOST_FOREACH(const pt::ptree::value_type &item, node) {
+        const std::string &name = item.first;
+        if (nodeSet.end() == nodeSet.find(name))
+            std::cerr << fileName_
+                << ": warning: unknown JSON node: " << name
+                << std::endl;
+    }
+}
+
 void SimpleTreeDecoder::readNode(
         Defect                      *def,
         const pt::ptree             &defNode)
 {
     // make sure the Defect structure is properly initialized
     (*def) = Defect();
 
+    this->reportUnknownNodes(NK_DEFECT, defNode);
+
     // the checker field is mandatory
     def->checker = defNode.get<std::string>("checker");
 
@@ -231,6 +301,7 @@ void SimpleTreeDecoder::readNode(
     const pt::ptree &evtListSrc = defNode.get_child("events");
     BOOST_FOREACH(const pt::ptree::value_type &evtItem, evtListSrc) {
         const pt::ptree &evtNode = evtItem.second;
+        this->reportUnknownNodes(NK_EVENT, evtNode);
 
         DefEvent evt;
         evt.fileName    = valueOf<std::string   >(evtNode, "file_name"  , "");
@@ -254,7 +325,7 @@ void SimpleTreeDecoder::readNode(
 
     if (defNode.not_found() == defNode.find("key_event_idx")) {
         // key event not specified, try to guess it
-        if (!this->keDigger.guessKeyEvent(def))
+        if (!keDigger_.guessKeyEvent(def))
             throw pt::ptree_error("failed to guess key event");
     }
     else {
@@ -269,7 +340,7 @@ void SimpleTreeDecoder::readNode(
 
     if (verbosityLevelNeedsInit)
         // missing or incomplete verbosity_level, initialize it over
-        this->keDigger.initVerbosity(def);
+        keDigger_.initVerbosity(def);
 
     // read annotation if available
     def->annotation = valueOf<std::string>(defNode, "annotation", "");
diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
@@ -28,7 +28,7 @@ set(jsfilter "sed")
 set(jsfilter "${jsfilter} -e ':a;N;$!ba;s|:\\\\\\\\n *\\\\\\\\[|: [|g'")
 
 macro(add_test_wrap test_name cmd)
-    add_test("${test_name}" sh -c "${cmd}")
+    add_test("${test_name}" bash -c "${cmd}")
     set_tests_properties(${test_name} PROPERTIES COST ${test_cost})
     math(EXPR test_cost "${test_cost} - 1")
 endmacro()
@@ -180,6 +180,7 @@ test_csgrep(csgrep      "60-gcc-parser-cppcheck-cwe"  )
 test_csgrep(csgrep      "61-json-parser-cov-v7-lang"  )
 test_csgrep(csgrep      "62-csparser-checker-lang"    )
 test_csgrep(csgrep      "63-gcc-parser-checker-lang"  )
+test_csgrep(csgrep      "64-json-parser-unknown-node" )
 test_csparser(csparser-5.8                          00)
 test_csparser(csparser-5.8                          01)
 test_csparser(csparser-5.8                          02)
diff --git a/tests/csgrep/64-json-parser-unknown-node-args.txt b/tests/csgrep/64-json-parser-unknown-node-args.txt
@@ -0,0 +1 @@
+--mode=json 2> >(sed "s|^.*/||") >/dev/null
diff --git a/tests/csgrep/64-json-parser-unknown-node-stdin.txt b/tests/csgrep/64-json-parser-unknown-node-stdin.txt
@@ -0,0 +1,64 @@
+{
+    "defects": [
+        {
+            "checker": "MISSING_MOVE_ASSIGNMENT",
+            "function": "null",
+            "lang": "c/c++",
+            "key_event_idx": 0,
+            "events": [
+                {
+                    "file_name": "/builddir/build/BUILD/systemtap-4.4/bpf-bitset.h",
+                    "line": 209,
+                    "event": "missing_move_assignment",
+                    "message": "Class \"bpf::bitset::set1\" may benefit from adding a move assignment operator. See other events which show the copy assignment operator being applied to rvalue(s), where a move assignment may be faster.",
+                    "verbosity_level": "0"
+                },
+                {
+                    "file_name": "/builddir/build/BUILD/systemtap-4.4/bpf-opt.cxx",
+                    "line": 633,
+                    "event": "copy_assignment",
+                    "message": "Example 1: Copy assignment is performed on an object (rvalue) of type \"bpf::bitset::set1\".",
+                    "verbosity_level": "1"
+                },
+                {
+                    "file_name": "/builddir/build/BUILD/systemtap-4.4/bpf-opt.cxx",
+                    "line": 528,
+                    "event": "copy_assignment",
+                    "message": "Example 2: Copy assignment is performed on an object (rvalue) of type \"bpf::bitset::set1\".",
+                    "verbosity_level": "1"
+                },
+                {
+                    "file_name": "/builddir/build/BUILD/systemtap-4.4/bpf-opt.cxx",
+                    "line": 533,
+                    "event": "copy_assignment",
+                    "message": "Example 3: Copy assignment is performed on an object (rvalue) of type \"bpf::bitset::set1\".",
+                    "verbosity_level": "1",
+                    "comment": "useless event"
+                }
+            ]
+        },
+        {
+            "checker": "UNINIT_CTOR",
+            "cwe": 457,
+            "function": "bpf::life_data::life_data(unsigned long, unsigned long)",
+            "language": "c/c++",
+            "key_event_idx": 1,
+            "events": [
+                {
+                    "file_name": "/builddir/build/BUILD/systemtap-4.4/bpf-opt.cxx",
+                    "line": 475,
+                    "event": "member_decl",
+                    "message": "Class member declaration for \"npartitions\".",
+                    "verbosity_level": "1"
+                },
+                {
+                    "file_name": "/builddir/build/BUILD/systemtap-4.4/bpf-opt.cxx",
+                    "line": 487,
+                    "event": "uninit_member",
+                    "message": "Non-static class member \"npartitions\" is not initialized in this constructor nor in any functions that it calls.",
+                    "verbosity_level": "0"
+                }
+            ]
+        }
+    ]
+}
diff --git a/tests/csgrep/64-json-parser-unknown-node-stdout.txt b/tests/csgrep/64-json-parser-unknown-node-stdout.txt
@@ -0,0 +1,2 @@
+64-json-parser-unknown-node-stdin.txt: warning: unknown JSON node: lang
+64-json-parser-unknown-node-stdin.txt: warning: unknown JSON node: comment

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	`+--mode=json 2> >(sed "s\|^.*/\|\|") >/dev/null`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,2 @@`
	`1`	`+64-json-parser-unknown-node-stdin.txt: warning: unknown JSON node: lang`
	`2`	`+64-json-parser-unknown-node-stdin.txt: warning: unknown JSON node: comment`