[nullsafe] Convert InheritanceRule to Procname.Java.t

Summary: This is part of work aimed to reduce usage of language-agnostics modules in Java-specific parts of nullsafe. Reviewed By: artempyanykh Differential Revision: D23052773 fbshipit-source-id: aacd07f27
4 years ago · 1e9f5f1bd3
parent 94f7d6aeeb
commit 1e9f5f1bd3
6 changed files with 65 additions and 65 deletions
--- a/infer/src/nullsafe/InheritanceRule.ml
+++ b/infer/src/nullsafe/InheritanceRule.ml
@ -31,24 +31,19 @@ module ReportableViolation = struct
    else Some {nullsafe_mode; violation}


-  let is_java_lang_object_equals = function
-    | Procname.Java java_procname -> (
-      match
-        (Procname.Java.get_class_name java_procname, Procname.Java.get_method java_procname)
-      with
-      | "java.lang.Object", "equals" ->
-          true
-      | _ ->
-          false )
+  let is_java_lang_object_equals procname =
+    match (Procname.Java.get_class_name procname, Procname.Java.get_method procname) with
+    | "java.lang.Object", "equals" ->
+        true
    | _ ->
        false


  let get_description _ violation_type ~base_proc_name ~overridden_proc_name =
    let module MF = MarkupFormatter in
-    let base_method_descr = Procname.to_simplified_string ~withclass:true base_proc_name in
+    let base_method_descr = Procname.Java.to_simplified_string ~withclass:true base_proc_name in
    let overridden_method_descr =
-      Procname.to_simplified_string ~withclass:true overridden_proc_name
+      Procname.Java.to_simplified_string ~withclass:true overridden_proc_name
    in
    match violation_type with
    | InconsistentReturn ->
--- a/infer/src/nullsafe/InheritanceRule.mli
+++ b/infer/src/nullsafe/InheritanceRule.mli
@ -42,7 +42,11 @@ module ReportableViolation : sig
  (** Severity of the violation to be reported *)

  val get_description :
-    t -> violation_type -> base_proc_name:Procname.t -> overridden_proc_name:Procname.t -> string
+       t
+    -> violation_type
+    -> base_proc_name:Procname.Java.t
+    -> overridden_proc_name:Procname.Java.t
+    -> string
  (** Given context around violation, return error message together with the info where to put this
      message *)
 end
--- a/infer/src/nullsafe/eradicate.ml
+++ b/infer/src/nullsafe/eradicate.ml
@ -177,7 +177,7 @@ let analyze_one_procedure ~java_pname
  do_final_typestate final_typestate_opt calls_this ;
  if checks.TypeCheck.eradicate then
    EradicateChecks.check_overridden_annotations analysis_data find_canonical_duplicate
-      annotated_signature ;
+      ~proc_name:java_pname annotated_signature ;
  ()


--- a/infer/src/nullsafe/eradicateChecks.ml
+++ b/infer/src/nullsafe/eradicateChecks.ml
@ -438,10 +438,8 @@ let check_call_parameters ({IntraproceduralAnalysis.tenv; _} as analysis_data) ~
  List.iter ~f:check resolved_params


-let check_inheritance_rule_for_return
-    ({IntraproceduralAnalysis.proc_desc= overridden_proc_desc; _} as analysis_data)
-    find_canonical_duplicate loc ~nullsafe_mode ~base_proc_name ~base_nullability
-    ~overridden_nullability =
+let check_inheritance_rule_for_return analysis_data find_canonical_duplicate loc ~nullsafe_mode
+    ~overridden_proc_name ~base_proc_name ~base_nullability ~overridden_nullability =
  Result.iter_error
    (InheritanceRule.check InheritanceRule.Ret ~base:base_nullability
       ~overridden:overridden_nullability) ~f:(fun inheritance_violation ->
@ -449,15 +447,14 @@ let check_inheritance_rule_for_return
        (Inconsistent_subclass
           { inheritance_violation
           ; violation_type= InconsistentReturn
-           ; overridden_proc_name= Procdesc.get_proc_name overridden_proc_desc
+           ; overridden_proc_name
           ; base_proc_name })
        None ~nullsafe_mode loc )


-let check_inheritance_rule_for_param
-    ({IntraproceduralAnalysis.proc_desc= overridden_proc_desc; _} as analysis_data)
-    find_canonical_duplicate loc ~nullsafe_mode ~overridden_param_name ~base_proc_name
-    ~param_position ~base_nullability ~overridden_nullability =
+let check_inheritance_rule_for_param analysis_data find_canonical_duplicate loc ~nullsafe_mode
+    ~overridden_param_name ~base_proc_name ~param_position ~base_nullability ~overridden_nullability
+    ~overridden_proc_name =
  Result.iter_error
    (InheritanceRule.check InheritanceRule.Param ~base:base_nullability
       ~overridden:overridden_nullability) ~f:(fun inheritance_violation ->
@ -468,12 +465,12 @@ let check_inheritance_rule_for_param
               InconsistentParam
                 {param_position; param_description= Mangled.to_string overridden_param_name}
           ; base_proc_name
-           ; overridden_proc_name= Procdesc.get_proc_name overridden_proc_desc })
+           ; overridden_proc_name })
        None ~nullsafe_mode loc )


 let check_inheritance_rule_for_params analysis_data find_canonical_duplicate loc ~nullsafe_mode
-    ~base_proc_name ~base_signature ~overridden_signature =
+    ~base_proc_name ~base_signature ~overridden_signature ~overridden_proc_name =
  let base_params = base_signature.AnnotatedSignature.params in
  let overridden_params = overridden_signature.AnnotatedSignature.params in
  let zipped_params = List.zip base_params overridden_params in
@ -492,6 +489,7 @@ let check_inheritance_rule_for_params analysis_data find_canonical_duplicate loc
            ~overridden_param_name ~base_proc_name
            ~param_position:(if should_index_from_zero then index else index + 1)
            ~base_nullability:(AnnotatedNullability.get_nullability annotated_nullability_base)
+            ~overridden_proc_name
            ~overridden_nullability:
              (AnnotatedNullability.get_nullability annotated_nullability_overridden) )
  | Unequal_lengths ->
@ -502,55 +500,58 @@ let check_inheritance_rule_for_params analysis_data find_canonical_duplicate loc

 (** Check both params and return values for complying for co- and contravariance *)
 let check_inheritance_rule_for_signature analysis_data find_canonical_duplicate loc ~nullsafe_mode
-    ~base_proc_name ~base_signature ~overridden_signature =
+    ~base_proc_name ~base_signature ~overridden_signature ~overridden_proc_name =
  (* Check params *)
  check_inheritance_rule_for_params analysis_data find_canonical_duplicate loc ~nullsafe_mode
-    ~base_proc_name ~base_signature ~overridden_signature ;
+    ~base_proc_name ~base_signature ~overridden_signature ~overridden_proc_name ;
  (* Check return value *)
-  match base_proc_name with
-  (* TODO model this as unknown nullability and get rid of that check *)
-  | Procname.Java java_pname when not (Procname.Java.is_external java_pname) ->
-      (* Check if return value is consistent with the base *)
-      let base_nullability =
-        AnnotatedNullability.get_nullability
-          base_signature.AnnotatedSignature.ret.ret_annotated_type.nullability
-      in
-      let overridden_nullability =
-        AnnotatedNullability.get_nullability
-          overridden_signature.AnnotatedSignature.ret.ret_annotated_type.nullability
-      in
-      check_inheritance_rule_for_return analysis_data find_canonical_duplicate loc ~nullsafe_mode
-        ~base_proc_name ~base_nullability ~overridden_nullability
-  | _ ->
-      (* the analysis should not report return type inconsistencies with external code *)
-      ()
+  if Procname.Java.is_external base_proc_name then
+    (* the analysis should not report return type inconsistencies with external code *) ()
+  else
+    (* Check if return value is consistent with the base *)
+    let base_nullability =
+      AnnotatedNullability.get_nullability
+        base_signature.AnnotatedSignature.ret.ret_annotated_type.nullability
+    in
+    let overridden_nullability =
+      AnnotatedNullability.get_nullability
+        overridden_signature.AnnotatedSignature.ret.ret_annotated_type.nullability
+    in
+    check_inheritance_rule_for_return analysis_data find_canonical_duplicate loc ~nullsafe_mode
+      ~base_proc_name ~base_nullability ~overridden_nullability ~overridden_proc_name


 (** Checks if the annotations are consistent with the derived classes and with the implemented
    interfaces *)
 let check_overridden_annotations ({IntraproceduralAnalysis.tenv; proc_desc; _} as analysis_data)
-    find_canonical_duplicate annotated_signature =
+    find_canonical_duplicate annotated_signature ~proc_name =
  let start_node = Procdesc.get_start_node proc_desc in
  let loc = Procdesc.Node.get_loc start_node in
  let check_if_base_signature_matches_current base_proc_name =
-    match PatternMatch.lookup_attributes tenv base_proc_name with
-    | Some base_attributes ->
-        let base_signature =
-          (* TODO(T62825735): fully support trusted callees. Note that for inheritance
-             rule it doesn't make much difference, but would be nice to refactor anyway. *)
-          Models.get_modelled_annotated_signature ~is_callee_in_trust_list:false tenv
-            base_attributes
-        in
-        check_inheritance_rule_for_signature analysis_data
-          ~nullsafe_mode:annotated_signature.AnnotatedSignature.nullsafe_mode
-          find_canonical_duplicate loc ~base_proc_name ~base_signature
-          ~overridden_signature:annotated_signature
-    | None ->
-        (* Could not find the attributes - optimistically skipping the check *)
-        (* TODO(T54687014) ensure this is not an issue in practice *)
+    match base_proc_name with
+    | Procname.Java base_java_proc_name -> (
+      match PatternMatch.lookup_attributes tenv base_proc_name with
+      | Some base_attributes ->
+          let base_signature =
+            (* TODO(T62825735): fully support trusted callees. Note that for inheritance
+               rule it doesn't make much difference, but would be nice to refactor anyway. *)
+            Models.get_modelled_annotated_signature ~is_callee_in_trust_list:false tenv
+              base_attributes
+          in
+          check_inheritance_rule_for_signature analysis_data
+            ~nullsafe_mode:annotated_signature.AnnotatedSignature.nullsafe_mode
+            find_canonical_duplicate loc ~base_proc_name:base_java_proc_name ~base_signature
+            ~overridden_signature:annotated_signature ~overridden_proc_name:proc_name
+      | None ->
+          (* Could not find the attributes - optimistically skipping the check *)
+          (* TODO(T54687014) ensure this is not an issue in practice *)
+          () )
+    | _ ->
+        (* The base method is not a Java method. This is a weird situation and should not happen in practice.
+           TODO(T54687014) ensure this is not an issue in practice
+        *)
        ()
  in
-  let proc_name = Procdesc.get_proc_name proc_desc in
  (* Iterate over all methods the current method overrides and see the current
     method is compatible with all of them *)
-  PatternMatch.override_iter check_if_base_signature_matches_current tenv proc_name
+  PatternMatch.override_iter check_if_base_signature_matches_current tenv (Procname.Java proc_name)
--- a/infer/src/nullsafe/typeErr.ml
+++ b/infer/src/nullsafe/typeErr.ml
@ -65,8 +65,8 @@ type err_instance =
  | Inconsistent_subclass of
      { inheritance_violation: InheritanceRule.violation
      ; violation_type: InheritanceRule.ReportableViolation.violation_type
-      ; base_proc_name: Procname.t
-      ; overridden_proc_name: Procname.t }
+      ; base_proc_name: Procname.Java.t
+      ; overridden_proc_name: Procname.Java.t }
  | Field_not_initialized of {field_name: Fieldname.t}
  | Over_annotation of
      { over_annotated_violation: OverAnnotatedRule.violation
--- a/infer/src/nullsafe/typeErr.mli
+++ b/infer/src/nullsafe/typeErr.mli
@ -39,8 +39,8 @@ type err_instance =
  | Inconsistent_subclass of
      { inheritance_violation: InheritanceRule.violation
      ; violation_type: InheritanceRule.ReportableViolation.violation_type
-      ; base_proc_name: Procname.t
-      ; overridden_proc_name: Procname.t }
+      ; base_proc_name: Procname.Java.t
+      ; overridden_proc_name: Procname.Java.t }
  | Field_not_initialized of {field_name: Fieldname.t}
  | Over_annotation of
      { over_annotated_violation: OverAnnotatedRule.violation