[nullsafe] More consistent version of anonymous classes in JavaClassName

Summary:
There is a case where the class name is "A$1$B$2".
In this case, we would correctly say it is an anonymous class, and
return A$1$B as user defined, which is bad: now we can get the outer
class which will be A$1 that will be anonymous again.

This was leading to tricky bugs.

Now, get_user_defined_class name will return something that is indeeed
contains only user defined names.

There is one tricky pathological case left: when the outermost class is
anonymous, which can in theory occur. This might lead to other tricky
bugs, so the follow up will be to rewrite API of JavaClassName.t so that
this case is made clear for the client.

But lets unbreak nullsafe fore now first.

Reviewed By: ngorogiannis

Differential Revision: D21449686

fbshipit-source-id: b0ba4702e

infer/src/IR/JavaClassName.ml

@@ -67,31 +67,6 @@ let get_outer_class_name {package; classname} =
   String.rsplit2 classname ~on:'$' |> Option.map ~f:(fun (outer, _) -> {package; classname= outer})
 
 
-(* Strips $<int> suffixes from the class name, and return how many were stripped *)
-let strip_anonymous_suffixes_if_present classname =
-  let rec strip_recursively classname nesting_level =
-    match String.rsplit2 classname ~on:'$' with
-    | Some (outer, suffix) when is_int suffix ->
-        (* Suffix is an integer - that was an anonymous class.
-           But it could be nested inside another anonymous class as well *)
-        strip_recursively outer (nesting_level + 1)
-    | _ ->
-        (* Suffix is not an integer or not present - not an anonymous class *)
-        (classname, nesting_level)
-  in
-  strip_recursively classname 0
-
-
-(* Strips everything after $Lambda$ (if it is a lambda-class),
-   and returns the result string together with if it was stripped *)
-let strip_lambda_if_present classname =
-  match String.substr_index classname ~pattern:"$Lambda$" with
-  | Some index ->
-      (String.prefix classname index, true)
-  | None ->
-      (classname, false)
-
-
 (*
  Anonymous classes have two forms:
  - classic anonymous classes: suffixes in form of $<int>.
@@ -101,10 +76,30 @@ let strip_lambda_if_present classname =
  In general case anonymous class name looks something like
  Class$NestedClass$1$17$5$Lambda$_1_2, and we need to return Class$NestedClass *)
 let get_user_defined_class_if_anonymous_inner {package; classname} =
-  let without_lambda, was_lambda_stripped = strip_lambda_if_present classname in
-  let outer_class_name, nesting_level = strip_anonymous_suffixes_if_present without_lambda in
-  let was_stripped = was_lambda_stripped || nesting_level > 0 in
-  if was_stripped then Some {package; classname= outer_class_name} else None
+  let is_anonymous_name = function
+    | "Lambda" ->
+        true
+    | name when is_int name ->
+        true
+    | _ ->
+        false
+  in
+  let pieces = String.split classname ~on:'$' in
+  let first_anonymous_name = List.findi pieces ~f:(fun _index name -> is_anonymous_name name) in
+  Option.bind first_anonymous_name ~f:(fun (index, _name) ->
+      (* Everything before this index did not have anonymous prefixes. Deem it user defined. *)
+      match List.take pieces index with
+      | [] ->
+          (* This is a weird situation - our class _starts_ with an anonymous name.
+             This should not happen normally, but we can not rule this out completely since there is no physical limitations on
+             the bytecode names.
+             In this case, we return [None] because formally this is not an anonymous _inner_ class, but anonymous outermost class instead.
+             TODO: redesign this API so this case is modelled directly
+          *)
+          None
+      | list ->
+          (* Assemble back all pieces together *)
+          Some {package; classname= String.concat ~sep:"$" list} )
 
 
 let is_anonymous_inner_class_name t = get_user_defined_class_if_anonymous_inner t |> is_some

infer/src/unit/JavaClassNameTests.ml

@@ -80,23 +80,27 @@ let test_anonymous =
   |> assert_some
   |> assert_equal_to ~expected_package:(Some "some.package")
        ~expected_classname:"SomeClass$NestedClass$AgainNestedClass" ;
-  (* If it is a lambda class, we expect this to be detected *)
+  (* Some name inside of anonymous - this is still anonymous  *)
   get_user_defined_class_if_anonymous_inner
-    (make ~package:(Some "some.package") ~classname:"SomeClass$Lambda$_4_1")
+    (make ~package:(Some "some.package") ~classname:"SomeClass$1$SomeName")
   |> assert_some
   |> assert_equal_to ~expected_package:(Some "some.package") ~expected_classname:"SomeClass" ;
-  (* Lambda might be inside anonymous (or several ones in general case) *)
+  (* Some names inside anonymous classes - should still return the innermost user defined *)
   get_user_defined_class_if_anonymous_inner
-    (make ~package:(Some "some.package") ~classname:"SomeClass$1$7$Lambda$_4_1")
+    (make ~package:(Some "some.package") ~classname:"A$B$1$C$2")
   |> assert_some
-  |> assert_equal_to ~expected_package:(Some "some.package") ~expected_classname:"SomeClass" ;
-  (* The most general case: nested class, lambda, and anonymous mixed *)
+  |> assert_equal_to ~expected_package:(Some "some.package") ~expected_classname:"A$B" ;
+  (* Pathological case - this is anonymous class on the outer level. Should return None because it is not inner. *)
+  get_user_defined_class_if_anonymous_inner (make ~package:(Some "some.package") ~classname:"23")
+  |> assert_none ;
+  (* Pathological case - this is anonymous class on the outer level. Should return None because it is not inner. *)
+  get_user_defined_class_if_anonymous_inner (make ~package:(Some "some.package") ~classname:"1$A")
+  |> assert_none ;
+  (* If it is a lambda class, we expect this to be detected *)
   get_user_defined_class_if_anonymous_inner
-    (make ~package:(Some "some.package")
-       ~classname:"SomeClass$NestedClass$7$1$Lambda$_4_1$2$Lambda$_7_18$19$16")
+    (make ~package:(Some "some.package") ~classname:"SomeClass$Lambda$_4_1")
   |> assert_some
-  |> assert_equal_to ~expected_package:(Some "some.package")
-       ~expected_classname:"SomeClass$NestedClass" ;
+  |> assert_equal_to ~expected_package:(Some "some.package") ~expected_classname:"SomeClass" ;
   (* If package was empty, everything should still work *)
   get_user_defined_class_if_anonymous_inner
     (make ~package:None ~classname:"SomeClass$NestedClass$AgainNestedClass$17$23$1")