Update comments + clean up for the exploitability origin changes

Summary: As titiled, updates code and test comments, removes unused methods that was missed.

Reviewed By: arthaud

Differential Revision: D62978375

fbshipit-source-id: ce16b20db39b4112ba2c4f109b3a6bed66b2a863

source/Origin.cpp

@@ -118,10 +118,6 @@ std::optional<std::string> ExploitabilityOrigin::to_model_validator_string()
   return std::nullopt;
 }
 
-std::string ExploitabilityOrigin::issue_handle_callee() const {
-  return fmt::format("{}:{}", exploitability_root_->show(), callee_->str());
-}
-
 Json::Value ExploitabilityOrigin::to_json() const {
   auto value = Json::Value(Json::objectValue);
   value["exploitability_root"] = exploitability_root_->to_json();

source/Origin.h

@@ -148,8 +148,14 @@ class StringOrigin final : public Origin {
  * Similar to how method/field/string origins are first added when creating
  * user-declared models (see Taint::add_origins_if_declaration()),
  * exploitability-origins are added when we first infer the source-as-transform
- * sinks. It tracks the (caller method + sink callee) pair where
- * the source-as-transform sink was materialized/originated.
+ * sinks. It tracks the (caller method + sink callee + position) tuple where
+ * the source-as-transform sink was materialized/originated. This helps us track
+ * the information related to the original source/sink flow and is used in the
+ * computation of issue grouping (and hence issue handles) when the
+ * exploitability rule is fulfilled. We create one issue per
+ * exploitability-origin and use it as the "callee" of the Issue, maintaining
+ * the issue grouping from the original source/sink flow. See
+ * exploitability_rules integration test for more information.
  *
  * From the SAPP UI perspective: Similar to how method/field/string origins
  * indicates the leaf frames of the source/sink traces, exploitability-origins
@@ -183,12 +189,6 @@ class ExploitabilityOrigin final : public Origin {
     return callee_;
   }
 
-  const Position* position() const {
-    return position_;
-  }
-
-  std::string issue_handle_callee() const;
-
  private:
   const Method* exploitability_root_;
   const DexString* callee_;

source/tests/integration/end-to-end/code/exploitability_rule/ExploitabilityRule.java

@@ -65,9 +65,6 @@ protected void testMultiplePathsToExploitabilityRoot() {
     // This method is Exported _and_ has call-chain flows to the same exploitability-root via
     // multiple paths.
     // Expect: a single issue here.
-    // TODO: T201904683: Currently we find multiple issues but create the same issue handle. This is
-    // because we use the position of the callsite in this method in IssueSet, but that is ignored
-    // in the sapp parser when creating the handle.
 
     // Path 1: Direct path to issue.
     ExploitabilityRule.testExported(this);