[java] use a package/classname record for java classes instead of string

Summary:
Use a record of package, class name to store (qualified) Java class names. This saves the round trip of concatenating then splitting again, etc, as well as saves some memory in the type environment as now the package paths can be shared across classes of the same package (about 10% in tests).

Also remove some unfortunate APIs.

Reviewed By: jvillard

Differential Revision: D19969325

fbshipit-source-id: f7b7f5a55

infer/src/IR/JavaClassName.ml

@@ -6,11 +6,36 @@
  *)
 
 open! IStd
+module F = Format
+module L = Logging
 
-type t = string [@@deriving compare]
+(** invariant: if [package = Some str] then [not (String.equal str "")] *)
+type t = {classname: string; package: string option} [@@deriving compare]
 
-let from_string str = str
+let from_string str =
+  match String.rsplit2 str ~on:'.' with
+  | None ->
+      {classname= str; package= None}
+  | Some ("", _) ->
+      L.die InternalError "Empty package path in Java qualified classname.@."
+  | Some (pkg, classname) ->
+      {classname; package= Some pkg}
 
-let to_string str = str
 
-let pp = String.pp
+let to_string = function
+  | {classname; package= None} ->
+      classname
+  | {classname; package= Some pkg} ->
+      String.concat ~sep:"." [pkg; classname]
+
+
+let pp fmt = function
+  | {classname; package= None} ->
+      F.pp_print_string fmt classname
+  | {classname; package= Some pkg} ->
+      F.fprintf fmt "%s.%s" pkg classname
+
+
+let package {package} = package
+
+let classname {classname} = classname

infer/src/IR/JavaClassName.mli

@@ -14,3 +14,7 @@ val from_string : string -> t
 val to_string : t -> string
 
 val pp : Format.formatter -> t -> unit
+
+val package : t -> string option
+
+val classname : t -> string

infer/src/IR/Typ.ml

@@ -467,6 +467,12 @@ module Name = struct
             {type_name= package_classname; package= None}
 
 
+      let of_java_class_name java_class_name =
+        let package = JavaClassName.package java_class_name in
+        let type_name = JavaClassName.classname java_class_name in
+        make ?package type_name
+
+
       let package {package} = package
 
       let type_name {type_name} = type_name
@@ -498,10 +504,19 @@ module Name = struct
 
     let java_lang_string = from_string "java.lang.String"
 
-    let split_typename typename = Split.of_string (name typename)
+    let get_java_class_name_exn typename =
+      match typename with
+      | JavaClass java_class_name ->
+          java_class_name
+      | _ ->
+          L.die InternalError "Tried to split a non-java class name into a java split type@."
+
+
+    let split_typename typename = Split.of_java_class_name (get_java_class_name_exn typename)
 
     let is_anonymous_inner_class_name class_name =
-      let class_name_no_package = Split.type_name (split_typename class_name) in
+      let java_class_name = get_java_class_name_exn class_name in
+      let class_name_no_package = JavaClassName.classname java_class_name in
       match String.rsplit2 class_name_no_package ~on:'$' with
       | Some (_, s) ->
           let is_int =
@@ -515,12 +530,9 @@ module Name = struct
           false
 
 
-    let is_external_classname name_string =
-      let {Split.package} = Split.of_string name_string in
-      Option.exists ~f:Config.java_package_is_external package
-
-
-    let is_external t = is_external_classname (name t)
+    let is_external t =
+      get_java_class_name_exn t |> JavaClassName.package
+      |> Option.exists ~f:Config.java_package_is_external
   end
 
   module Cpp = struct

infer/src/IR/Typ.mli

@@ -242,9 +242,6 @@ module Name : sig
     val is_class : t -> bool
     (** [is_class name] holds if [name] names a Java class *)
 
-    val is_external_classname : string -> bool
-    (** return true if the string is in the .inferconfig list of external classes *)
-
     val is_external : t -> bool
     (** return true if the typename is in the .inferconfig list of external classes *)
 

infer/src/cost/costDomain.ml

@@ -15,7 +15,7 @@ module BasicCost = struct
 
   (* NOTE: Increment the version number if you changed the [t] type.  This is for avoiding
      demarshalling failure of cost analysis results in running infer-reportdiff. *)
-  let version = 3
+  let version = 4
 end
 
 (**

infer/src/nullsafe/NullabilitySuggest.ml

@@ -174,7 +174,7 @@ let pretty_field_name proc_data field_name =
 let is_outside_codebase proc_name field_name =
   match proc_name with
   | Procname.Java _ ->
-      Typ.Name.Java.is_external_classname (Typ.Name.name (Fieldname.get_class_name field_name))
+      Typ.Name.Java.is_external (Fieldname.get_class_name field_name)
   | _ ->
       false
 

infer/tests/build_systems/racerd_dedup/issues.exp

@@ -2,7 +2,7 @@ DeDup.java, build_systems.threadsafety.DeDup.colocated_read_write():void, 63, TH
 DeDup.java, build_systems.threadsafety.DeDup.separate_write_to_colocated_read():void, 68, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [access to `this.colocated_read`]
 DeDup.java, build_systems.threadsafety.DeDup.twoWritesOneInCaller():void, 50, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [call to void DeDup.writeToField(),access to `this.field`]
 DeDup.java, build_systems.threadsafety.DeDup.two_fields():void, 20, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [call to void DeDup.foo(),access to `this.fielda`]
-DeDup.java, build_systems.threadsafety.DeDup.two_reads():void, 37, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [<Read trace>,access to `this.field`,<Write trace>,access to `this.field`]
+DeDup.java, build_systems.threadsafety.DeDup.two_reads():void, 37, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [<Read trace>,access to `this.field`,<Write trace>,call to void DeDup.writeToField(),access to `this.field`]
 DeDup.java, build_systems.threadsafety.DeDup.two_writes():void, 30, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [access to `this.field`]
 DeDup.java, build_systems.threadsafety.DeDup.write_read():void, 44, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [access to `this.field`]
-DeDup.java, build_systems.threadsafety.DeDup.write_read():void, 45, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [<Read trace>,access to `this.field`,<Write trace>,access to `this.field`]
+DeDup.java, build_systems.threadsafety.DeDup.write_read():void, 45, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [<Read trace>,access to `this.field`,<Write trace>,call to void DeDup.writeToField(),access to `this.field`]

infer/tests/codetoanalyze/java/racerd/issues.exp

@@ -86,7 +86,7 @@ codetoanalyze/java/racerd/Ownership.java, codetoanalyze.java.checkers.Ownership.
 codetoanalyze/java/racerd/Ownership.java, codetoanalyze.java.checkers.Ownership.writeToNotOwnedInCalleeBad1(codetoanalyze.java.checkers.Obj):void, 157, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [call to void Ownership.writeToFormal(Obj),access to `o.f`]
 codetoanalyze/java/racerd/Ownership.java, codetoanalyze.java.checkers.Ownership.writeToNotOwnedInCalleeBad2():void, 162, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [call to void Ownership.writeToFormal(Obj),access to `o.f`]
 codetoanalyze/java/racerd/Ownership.java, codetoanalyze.java.checkers.Ownership.writeToNotOwnedInCalleeBad3(codetoanalyze.java.checkers.Obj):void, 166, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [call to void Ownership.callWriteToFormal(Obj),call to void Ownership.writeToFormal(Obj),access to `o.f`]
-codetoanalyze/java/racerd/Ownership.java, codetoanalyze.java.checkers.Ownership.writeToOwnedInCalleeOk2():void, 183, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [<Read trace>,access to `this.field`,<Write trace>,call to void Ownership.setField(Obj),access to `this.field`]
+codetoanalyze/java/racerd/Ownership.java, codetoanalyze.java.checkers.Ownership.writeToOwnedInCalleeOk2():void, 183, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [<Read trace>,access to `this.field`,<Write trace>,access to `this.field`]
 codetoanalyze/java/racerd/RaceWithMainThread.java, codetoanalyze.java.checkers.RaceWithMainThread.conditional2_bad(boolean):void, 130, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [access to `this.ff`]
 codetoanalyze/java/racerd/RaceWithMainThread.java, codetoanalyze.java.checkers.RaceWithMainThread.conditionalMainThreadWriteBad():void, 219, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [call to void RaceWithMainThread.conditionalMainThreadWrite2(boolean),access to `this.mOnlyWrittenOnMain`]
 codetoanalyze/java/racerd/RaceWithMainThread.java, codetoanalyze.java.checkers.RaceWithMainThread.conditional_isMainThread_ElseBranch_Bad():void, 152, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [access to `this.ff`]