Disallow Method.from_json(None). Removing concept of "leaf" method

Summary:
Currently, `Method.from_json(None)` returns `Method("leaf")` which is strange. If a method doesn't exist, it doesn't exist. "Leaf" is not a method.

This diff replaces "leaf" methods with `Optional[Method]`. In most cases, the method (usually the callee) should not be `None` anyway, otherwise the analysis is emitting a bad json.

Reviewed By: anwesht

Differential Revision: D51187701

fbshipit-source-id: 8f5a0b92b3e1475b5751612ddaad961e32bad8c2

sapp/pipeline/mariana_trench_parser.py

@@ -40,13 +40,8 @@
 class Method(NamedTuple):
     name: str
 
-    def is_leaf(self) -> bool:
-        return self.name == "leaf"
-
     @staticmethod
-    def from_json(method: Union[None, str, Dict[str, Any]]) -> "Method":
-        if method is None:
-            return Method("leaf")
+    def from_json(method: Union[str, Dict[str, Any]]) -> "Method":
         if isinstance(method, str):
             return Method(method)
 
@@ -122,12 +117,12 @@ def default() -> "Position":
         return Position(UNKNOWN_PATH, UNKNOWN_LINE, 0, 0)
 
     @staticmethod
-    def from_json(position: Dict[str, Any], method: Method) -> "Position":
+    def from_json(position: Dict[str, Any], method: Optional[Method]) -> "Position":
         path = position.get("path", UNKNOWN_PATH)
         line = position.get("line", UNKNOWN_LINE)
         start = position.get("start", 0) + 1
         end = max(position.get("end", 0) + 1, start)
-        if path == UNKNOWN_PATH and not method.is_leaf():
+        if path == UNKNOWN_PATH and method:
             path = method.name.split(";")[0]
             path = path.split("$")[0]
             path = path[1:]
@@ -183,7 +178,7 @@ class CallInfo(NamedTuple):
     """Mirrors the CallInfo object in the analysis"""
 
     call_kind: str
-    method: Method
+    method: Optional[Method]
     port: Port
     position: Position
 
@@ -192,7 +187,9 @@ def from_json(
         taint_json: Dict[str, Any], leaf_kind: str, caller_position: Position
     ) -> "CallInfo":
         call_kind = taint_json["call_kind"]
-        method = Method.from_json(taint_json.get("resolves_to"))
+
+        callee = taint_json.get("resolves_to")
+        method = Method.from_json(callee) if callee else None
         port = Port.from_json(taint_json.get("port", "leaf"), leaf_kind)
 
         position_json = taint_json.get("position")
@@ -224,6 +221,10 @@ class Call(NamedTuple):
 
     @staticmethod
     def from_call_info(call_info: CallInfo) -> "Call":
+        if call_info.method is None:
+            raise sapp.ParseError(
+                f"Cannot construct a Call without a valid method {call_info}"
+            )
         return Call(call_info.method, call_info.port, call_info.position)
 
     @staticmethod
@@ -320,7 +321,7 @@ def to_sapp(self) -> sapp.ParseTraceAnnotation:
                     position=self.callee.position.to_sapp(),
                 )
             ]
-            if not self.callee.method.is_leaf()
+            if self.callee.method
             else []
         )
 
@@ -668,6 +669,11 @@ def _parse_issue_conditions(
                 }
             )
 
+            if call_info.is_declaration():
+                raise sapp.ParseError(
+                    f"Unexpected declaration frame at issue {leaf_kind}"
+                )
+
             if call_info.is_origin():
                 for kind in kinds:
                     condition_leaves = [ConditionLeaf.from_kind(kind)]
@@ -682,7 +688,6 @@ def _parse_issue_conditions(
                             )
                         )
             else:
-                # Declaration and CallSite kinds can be handled the same way.
                 condition_leaves = [ConditionLeaf.from_kind(kind) for kind in kinds]
                 extra_traces = set()
                 for kind in kinds:

sapp/pipeline/tests/test_mariana_trench_parser.py

@@ -455,11 +455,11 @@ def testModelWithIssue(self) -> None:
                   "sources": [
                     {
                       "call_info": {
-                        "call_kind": "Declaration"
+                        "call_kind": "Origin"
                       },
                       "kinds": [
                         {
-                          "call_kind": "Declaration",
+                          "call_kind": "Origin",
                           "distance": 0,
                           "kind": "TestSource",
                           "origins": [
@@ -512,7 +512,7 @@ def testModelWithIssue(self) -> None:
                     ],
                     postconditions=[
                         ParseIssueConditionTuple(
-                            callee="leaf",
+                            callee="LSource;.sourceField:LData;",
                             port="source",
                             location=SourceLocation(
                                 line_no=2,