Support PartialKind::from_json

Yuh Shin Ong · facebook-github-bot · commit b2c283c59b69 · 2025-03-25T21:23:42.000-07:00
Summary:
Changes the JSON representation of PartialKind from the string "Partial:&lt;label&gt;:&lt;name&gt;" to a JSON structure (similar to TransformKind).

Makes parsing sharded models from JSON easier. Used in --replay-mode, and potentially JAR caching (--sharded-input-models).

Conversion to string representation is done in mariana_trench_parser_objects.py, similar to TransformKinds. Similar support for TriggeredPartialKind coming up next.

NOTE: There is now a `PartialKind::from_json()` and `PartialKind::from_rule_json()`. The latter is pre-existing and renamed. It supports parsing partial kinds from rules where the name and labels are nested in different objects:

```
{
    "name": "Blah",
    "code": 1,
    "description": "multi source rule",
    "multi_sources": {
      "a": [ "Source1" ],
      "b": [ "Source2" ]
    },
    "partial_sinks": [ "MyPartialSink" ]
}
```

Reviewed By: arthaud

Differential Revision: D71245841

fbshipit-source-id: e086e05c7d7bb61fcb5402f5eca3ff1cc115e1cc
diff --git a/source/Kind.cpp b/source/Kind.cpp
@@ -41,15 +41,28 @@ const Kind* Kind::from_json(const Json::Value& value, Context& context) {
   const auto leaf_kind =
       JsonValidation::object_or_string(value, /* field */ "kind");
 
-  // An object means it's a TransformKind.
+  // Some kinds are represented as an object. Use unique keys in them to
+  // determine the Kind.
   if (leaf_kind.isObject()) {
-    return TransformKind::from_json(leaf_kind, context);
+    if (leaf_kind.isMember("base")) {
+      return TransformKind::from_json(leaf_kind, context);
+    } else if (leaf_kind.isMember("partial_label")) {
+      return PartialKind::from_json(leaf_kind, context);
+    } else {
+      throw JsonValidationError(
+          value,
+          "kind",
+          /* expected */ "TransformKind or PartialKind nested in an object.");
+    }
   }
 
   if (value.isMember("partial_label")) {
-    return context.kind_factory->get_partial(
-        leaf_kind.asString(),
-        JsonValidation::string(value, /* field */ "partial_label"));
+    // "partial_label" in the outer object is a legacy format that is no longer
+    // supported. It should be nested within the "kind" object.
+    throw JsonValidationError(
+        value,
+        "partial_label",
+        /* expected */ "'partial_label' nested in a 'kind' object.");
   }
 
   return Kind::from_trace_string(leaf_kind.asString(), context);
@@ -81,11 +94,9 @@ const Kind* Kind::from_trace_string(const std::string& kind, Context& context) {
         kind,
         /* expected */ "Non-TriggeredPartial Kind");
   } else if (boost::starts_with(kind, "Partial:")) {
-    // Note that parsing partial kinds from the JSON is supported, but not
-    // from the string representation.
-    throw KindNotSupportedError(
-        kind,
-        /* expected */ "Non-Partial Kind");
+    // Parsing of PartialKinds from the JSON is supported, but it no longer uses
+    // the old string representation.
+    throw KindNotSupportedError(kind, /* expected */ "Non-Partial Kind");
   } else if (kind.find_first_of(":@") != std::string::npos) {
     // Note that parsing transform kinds from the JSON is supported, but it
     // no longer uses the old string representation.
diff --git a/source/MultiSourceMultiSinkRule.cpp b/source/MultiSourceMultiSinkRule.cpp
@@ -136,7 +136,7 @@ std::unique_ptr<Rule> MultiSourceMultiSinkRule::from_json(
        JsonValidation::nonempty_array(value, "partial_sinks")) {
     for (const auto& label : labels) {
       partial_sink_kinds.insert(
-          PartialKind::from_json(sink_kind, label, context));
+          PartialKind::from_rule_json(sink_kind, label, context));
     }
   }
 
diff --git a/source/PartialKind.cpp b/source/PartialKind.cpp
@@ -15,7 +15,30 @@ void PartialKind::show(std::ostream& out) const {
   out << "Partial:" << name_ << ":" << label_;
 }
 
+Json::Value PartialKind::to_json() const {
+  auto nestedValue = Json::Value(Json::objectValue);
+  nestedValue["name"] = name_;
+  nestedValue["partial_label"] = label_;
+
+  auto value = Json::Value(Json::objectValue);
+  value["kind"] = nestedValue;
+
+  return value;
+}
+
 const PartialKind* PartialKind::from_json(
+    const Json::Value& value,
+    Context& context) {
+  // Notable asymmetry with to_json():
+  // to_json() nests the value in a "kind" field to be consistent with the
+  // overridden Kind::to_json().
+  // from_json(value, ...) assumes `value` has been extracted from "kind" field.
+  auto name = JsonValidation::string(value, /* field */ "name");
+  auto label = JsonValidation::string(value, /* field */ "partial_label");
+  return context.kind_factory->get_partial(name, label);
+}
+
+const PartialKind* PartialKind::from_rule_json(
     const Json::Value& value,
     const std::string& label,
     Context& context) {
diff --git a/source/PartialKind.h b/source/PartialKind.h
@@ -38,10 +38,16 @@ class PartialKind final : public Kind {
 
   void show(std::ostream&) const override;
 
+  Json::Value to_json() const override;
   static const PartialKind* from_json(
+      const Json::Value& value,
+      Context& context);
+
+  static const PartialKind* from_rule_json(
       const Json::Value& value,
       const std::string& partial_label,
       Context& context);
+
   std::string to_trace_string() const override;
 
   const auto& name() const {
diff --git a/source/tests/JsonTest.cpp b/source/tests/JsonTest.cpp
@@ -533,7 +533,14 @@ TEST_F(JsonTest, Sanitizer) {
                             .to_json()));
   EXPECT_EQ(
       test::parse_json(
-          R"({"sanitize": "sources", "kinds": [{"kind": "Kind1"}, {"kind": "Kind2"}, {"kind": "Partial:Kind3:a"}]})"),
+          R"({
+            "sanitize": "sources",
+            "kinds": [
+              {"kind": "Kind1"},
+              {"kind": "Kind2"},
+              {"kind": {"name": "Kind3", "partial_label": "a"}}
+            ]
+          })"),
       test::sorted_json(Sanitizer(
                             SanitizerKind::Sources,
                             /* kinds */
@@ -2215,7 +2222,12 @@ TEST_F(JsonTest, Model) {
         "method": "LData;.method:(LData;LData;)V",
         "sanitizers": [
           {"sanitize": "sources", "kinds": [{"kind": "Kind1"}]},
-          {"sanitize": "sinks", "kinds": [{"kind": "Kind1"}, {"kind": "Partial:Kind2:a"}]}
+          {
+            "sanitize": "sinks",
+            "kinds": [
+              {"kind": "Kind1"},
+              {"kind": {"name": "Kind2", "partial_label": "a"}}
+            ]}
         ]
       })#")));
   EXPECT_EQ(
@@ -2299,7 +2311,13 @@ TEST_F(JsonTest, Model) {
       test::sorted_json(test::parse_json(R"#({
       "method": "LData;.method:(LData;LData;)V",
       "sanitizers": [
-        {"sanitize": "sinks", "kinds": [{"kind": "Kind1"}, {"kind": "Partial:Kind2:a"}]},
+        {
+          "sanitize": "sinks",
+          "kinds": [
+            {"kind": "Kind1"},
+            {"kind": {"name": "Kind2", "partial_label": "a"}}
+          ]
+        },
         {"sanitize": "sinks", "port": "Argument(2)", "kinds": [{"kind": "Kind1"}, {"kind": "Kind3"}]},
         {"sanitize": "sources", "port": "Argument(1)"}
       ]
diff --git a/source/tests/KindTest.cpp b/source/tests/KindTest.cpp
@@ -105,18 +105,11 @@ TEST_F(KindTest, SerializationDeserialization) {
         Kind::from_json(transform_kind->to_json(), context), transform_kind);
   }
 
-  // Deserializing Partial and Triggered Kinds are not currently supported.
-  // These are trickier to parse, and for now, are not needed unless
-  // multi-source/sink rules are used when analysis caching where
-  // deserialization of the cached result happens.
   const auto* partial_kind =
       context.kind_factory->get_partial("PartialKind", "label");
-  EXPECT_THROW(
-      Kind::from_json(partial_kind->to_json(), context), KindNotSupportedError);
-  EXPECT_THROW(
-      Kind::from_trace_string(partial_kind->to_trace_string(), context),
-      KindNotSupportedError);
+  EXPECT_EQ(Kind::from_json(partial_kind->to_json(), context), partial_kind);
 
+  // Deserializing Triggered Kinds is not currently supported.
   auto source_kinds_a =
       Rule::KindSet{context.kind_factory->get("NamedSourceKindA")};
   auto source_kinds_b =
diff --git a/source/tests/integration/end-to-end/code/literal_source/expected_output.json b/source/tests/integration/end-to-end/code/literal_source/expected_output.json
@@ -1156,7 +1156,11 @@
           "kinds" : 
           [
             {
-              "kind" : "Partial:TestSink:regex",
+              "kind" : 
+              {
+                "name" : "TestSink",
+                "partial_label" : "regex"
+              },
               "origins" : 
               [
                 {
@@ -1181,7 +1185,11 @@
           "kinds" : 
           [
             {
-              "kind" : "Partial:TestSink:aci",
+              "kind" : 
+              {
+                "name" : "TestSink",
+                "partial_label" : "aci"
+              },
               "origins" : 
               [
                 {
@@ -2335,7 +2343,11 @@
           "kinds" : 
           [
             {
-              "kind" : "Partial:TestSink:regex",
+              "kind" : 
+              {
+                "name" : "TestSink",
+                "partial_label" : "regex"
+              },
               "origins" : 
               [
                 {
@@ -2360,7 +2372,11 @@
           "kinds" : 
           [
             {
-              "kind" : "Partial:TestSink:aci",
+              "kind" : 
+              {
+                "name" : "TestSink",
+                "partial_label" : "aci"
+              },
               "origins" : 
               [
                 {
@@ -2516,7 +2532,11 @@
           "kinds" : 
           [
             {
-              "kind" : "Partial:TestSink:aci",
+              "kind" : 
+              {
+                "name" : "TestSink",
+                "partial_label" : "aci"
+              },
               "origins" : 
               [
                 {
@@ -2526,7 +2546,11 @@
               ]
             },
             {
-              "kind" : "Partial:TestSink:regex",
+              "kind" : 
+              {
+                "name" : "TestSink",
+                "partial_label" : "regex"
+              },
               "origins" : 
               [
                 {
@@ -2551,7 +2575,11 @@
           "kinds" : 
           [
             {
-              "kind" : "Partial:TestSink:aci",
+              "kind" : 
+              {
+                "name" : "TestSink",
+                "partial_label" : "aci"
+              },
               "origins" : 
               [
                 {
@@ -2561,7 +2589,11 @@
               ]
             },
             {
-              "kind" : "Partial:TestSink:regex",
+              "kind" : 
+              {
+                "name" : "TestSink",
+                "partial_label" : "regex"
+              },
               "origins" : 
               [
                 {
@@ -2699,7 +2731,11 @@
           "kinds" : 
           [
             {
-              "kind" : "Partial:TestSink:regex",
+              "kind" : 
+              {
+                "name" : "TestSink",
+                "partial_label" : "regex"
+              },
               "origins" : 
               [
                 {
@@ -2724,7 +2760,11 @@
           "kinds" : 
           [
             {
-              "kind" : "Partial:TestSink:aci",
+              "kind" : 
+              {
+                "name" : "TestSink",
+                "partial_label" : "aci"
+              },
               "origins" : 
               [
                 {
diff --git a/source/tests/integration/end-to-end/code/multi_sources/expected_output.json b/source/tests/integration/end-to-end/code/multi_sources/expected_output.json
@@ -1568,7 +1568,11 @@
               [
                 "sink-a"
               ],
-              "kind" : "Partial:TestSink:a",
+              "kind" : 
+              {
+                "name" : "TestSink",
+                "partial_label" : "a"
+              },
               "origins" : 
               [
                 {
@@ -1597,7 +1601,11 @@
               [
                 "sink-b"
               ],
-              "kind" : "Partial:TestSink:b",
+              "kind" : 
+              {
+                "name" : "TestSink",
+                "partial_label" : "b"
+              },
               "origins" : 
               [
                 {
@@ -2184,7 +2192,11 @@
               [
                 "sink-a"
               ],
-              "kind" : "Partial:TestSinkSingleArg:a",
+              "kind" : 
+              {
+                "name" : "TestSinkSingleArg",
+                "partial_label" : "a"
+              },
               "origins" : 
               [
                 {
@@ -2198,7 +2210,11 @@
               [
                 "sink-b"
               ],
-              "kind" : "Partial:TestSinkSingleArg:b",
+              "kind" : 
+              {
+                "name" : "TestSinkSingleArg",
+                "partial_label" : "b"
+              },
               "origins" : 
               [
                 {

Original file line number	Diff line number	Diff line change
`@@ -136,7 +136,7 @@ std::unique_ptr<Rule> MultiSourceMultiSinkRule::from_json(`
`136`	`136`	`JsonValidation::nonempty_array(value, "partial_sinks")) {`
`137`	`137`	`for (const auto& label : labels) {`
`138`	`138`	`partial_sink_kinds.insert(`
`139`		`- PartialKind::from_json(sink_kind, label, context));`
	`139`	`+ PartialKind::from_rule_json(sink_kind, label, context));`
`140`	`140`	`}`
`141`	`141`	`}`
`142`	`142`
Original file line number	Diff line number	Diff line change
`@@ -1156,7 +1156,11 @@`
`1156`	`1156`	`"kinds" :`
`1157`	`1157`	`[`
`1158`	`1158`	`{`
`1159`		`- "kind" : "Partial:TestSink:regex",`
	`1159`	`+ "kind" :`
	`1160`	`+ {`
	`1161`	`+ "name" : "TestSink",`
	`1162`	`+ "partial_label" : "regex"`
	`1163`	`+ },`
`1160`	`1164`	`"origins" :`
`1161`	`1165`	`[`
`1162`	`1166`	`{`
`@@ -1181,7 +1185,11 @@`
`1181`	`1185`	`"kinds" :`
`1182`	`1186`	`[`
`1183`	`1187`	`{`
`1184`		`- "kind" : "Partial:TestSink:aci",`
	`1188`	`+ "kind" :`
	`1189`	`+ {`
	`1190`	`+ "name" : "TestSink",`
	`1191`	`+ "partial_label" : "aci"`
	`1192`	`+ },`
`1185`	`1193`	`"origins" :`
`1186`	`1194`	`[`
`1187`	`1195`	`{`
`@@ -2335,7 +2343,11 @@`
`2335`	`2343`	`"kinds" :`
`2336`	`2344`	`[`
`2337`	`2345`	`{`
`2338`		`- "kind" : "Partial:TestSink:regex",`
	`2346`	`+ "kind" :`
	`2347`	`+ {`
	`2348`	`+ "name" : "TestSink",`
	`2349`	`+ "partial_label" : "regex"`
	`2350`	`+ },`
`2339`	`2351`	`"origins" :`
`2340`	`2352`	`[`
`2341`	`2353`	`{`
`@@ -2360,7 +2372,11 @@`
`2360`	`2372`	`"kinds" :`
`2361`	`2373`	`[`
`2362`	`2374`	`{`
`2363`		`- "kind" : "Partial:TestSink:aci",`
	`2375`	`+ "kind" :`
	`2376`	`+ {`
	`2377`	`+ "name" : "TestSink",`
	`2378`	`+ "partial_label" : "aci"`
	`2379`	`+ },`
`2364`	`2380`	`"origins" :`
`2365`	`2381`	`[`
`2366`	`2382`	`{`
`@@ -2516,7 +2532,11 @@`
`2516`	`2532`	`"kinds" :`
`2517`	`2533`	`[`
`2518`	`2534`	`{`
`2519`		`- "kind" : "Partial:TestSink:aci",`
	`2535`	`+ "kind" :`
	`2536`	`+ {`
	`2537`	`+ "name" : "TestSink",`
	`2538`	`+ "partial_label" : "aci"`
	`2539`	`+ },`
`2520`	`2540`	`"origins" :`
`2521`	`2541`	`[`
`2522`	`2542`	`{`
`@@ -2526,7 +2546,11 @@`
`2526`	`2546`	`]`
`2527`	`2547`	`},`
`2528`	`2548`	`{`
`2529`		`- "kind" : "Partial:TestSink:regex",`
	`2549`	`+ "kind" :`
	`2550`	`+ {`
	`2551`	`+ "name" : "TestSink",`
	`2552`	`+ "partial_label" : "regex"`
	`2553`	`+ },`
`2530`	`2554`	`"origins" :`
`2531`	`2555`	`[`
`2532`	`2556`	`{`
`@@ -2551,7 +2575,11 @@`
`2551`	`2575`	`"kinds" :`
`2552`	`2576`	`[`
`2553`	`2577`	`{`
`2554`		`- "kind" : "Partial:TestSink:aci",`
	`2578`	`+ "kind" :`
	`2579`	`+ {`
	`2580`	`+ "name" : "TestSink",`
	`2581`	`+ "partial_label" : "aci"`
	`2582`	`+ },`
`2555`	`2583`	`"origins" :`
`2556`	`2584`	`[`
`2557`	`2585`	`{`
`@@ -2561,7 +2589,11 @@`
`2561`	`2589`	`]`
`2562`	`2590`	`},`
`2563`	`2591`	`{`
`2564`		`- "kind" : "Partial:TestSink:regex",`
	`2592`	`+ "kind" :`
	`2593`	`+ {`
	`2594`	`+ "name" : "TestSink",`
	`2595`	`+ "partial_label" : "regex"`
	`2596`	`+ },`
`2565`	`2597`	`"origins" :`
`2566`	`2598`	`[`
`2567`	`2599`	`{`
`@@ -2699,7 +2731,11 @@`
`2699`	`2731`	`"kinds" :`
`2700`	`2732`	`[`
`2701`	`2733`	`{`
`2702`		`- "kind" : "Partial:TestSink:regex",`
	`2734`	`+ "kind" :`
	`2735`	`+ {`
	`2736`	`+ "name" : "TestSink",`
	`2737`	`+ "partial_label" : "regex"`
	`2738`	`+ },`
`2703`	`2739`	`"origins" :`
`2704`	`2740`	`[`
`2705`	`2741`	`{`
`@@ -2724,7 +2760,11 @@`
`2724`	`2760`	`"kinds" :`
`2725`	`2761`	`[`
`2726`	`2762`	`{`
`2727`		`- "kind" : "Partial:TestSink:aci",`
	`2763`	`+ "kind" :`
	`2764`	`+ {`
	`2765`	`+ "name" : "TestSink",`
	`2766`	`+ "partial_label" : "aci"`
	`2767`	`+ },`
`2728`	`2768`	`"origins" :`
`2729`	`2769`	`[`
`2730`	`2770`	`{`
Original file line number	Diff line number	Diff line change
`@@ -1568,7 +1568,11 @@`
`1568`	`1568`	`[`
`1569`	`1569`	`"sink-a"`
`1570`	`1570`	`],`
`1571`		`- "kind" : "Partial:TestSink:a",`
	`1571`	`+ "kind" :`
	`1572`	`+ {`
	`1573`	`+ "name" : "TestSink",`
	`1574`	`+ "partial_label" : "a"`
	`1575`	`+ },`
`1572`	`1576`	`"origins" :`
`1573`	`1577`	`[`
`1574`	`1578`	`{`
`@@ -1597,7 +1601,11 @@`
`1597`	`1601`	`[`
`1598`	`1602`	`"sink-b"`
`1599`	`1603`	`],`
`1600`		`- "kind" : "Partial:TestSink:b",`
	`1604`	`+ "kind" :`
	`1605`	`+ {`
	`1606`	`+ "name" : "TestSink",`
	`1607`	`+ "partial_label" : "b"`
	`1608`	`+ },`
`1601`	`1609`	`"origins" :`
`1602`	`1610`	`[`
`1603`	`1611`	`{`
`@@ -2184,7 +2192,11 @@`
`2184`	`2192`	`[`
`2185`	`2193`	`"sink-a"`
`2186`	`2194`	`],`
`2187`		`- "kind" : "Partial:TestSinkSingleArg:a",`
	`2195`	`+ "kind" :`
	`2196`	`+ {`
	`2197`	`+ "name" : "TestSinkSingleArg",`
	`2198`	`+ "partial_label" : "a"`
	`2199`	`+ },`
`2188`	`2200`	`"origins" :`
`2189`	`2201`	`[`
`2190`	`2202`	`{`
`@@ -2198,7 +2210,11 @@`
`2198`	`2210`	`[`
`2199`	`2211`	`"sink-b"`
`2200`	`2212`	`],`
`2201`		`- "kind" : "Partial:TestSinkSingleArg:b",`
	`2213`	`+ "kind" :`
	`2214`	`+ {`
	`2215`	`+ "name" : "TestSinkSingleArg",`
	`2216`	`+ "partial_label" : "b"`
	`2217`	`+ },`
`2202`	`2218`	`"origins" :`
`2203`	`2219`	`[`
`2204`	`2220`	`{`