[nullsafe] Specific error message for overriding Object.equals()

Summary:
This is a common enough case to make error message specific.
Also let's ensure it's modelled.

Reviewed By: artempyanykh

Differential Revision: D19431899

fbshipit-source-id: f34459cb3

infer/src/nullsafe/InheritanceRule.ml

@@ -43,6 +43,17 @@ type violation_type =
   | InconsistentReturn
 [@@deriving compare]
 
+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 )
+  | _ ->
+      false
+
+
 let violation_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
@@ -57,23 +68,30 @@ let violation_description _ violation_type ~base_proc_name ~overridden_proc_name
          mark the parent as `@Nullable` or ensure the child does not return `null`."
         MF.pp_monospaced overridden_method_descr MF.pp_monospaced base_method_descr
   | InconsistentParam {param_description; param_position} ->
-      let translate_position = function
-        | 1 ->
-            "First"
-        | 2 ->
-            "Second"
-        | 3 ->
-            "Third"
-        | n ->
-            string_of_int n ^ "th"
-      in
-      Format.asprintf
-        "%s parameter %a of method %a is missing `@Nullable` declaration when overriding %a. The \
-         parent method declared it can handle `null` for this param, so the child should also \
-         declare that."
-        (translate_position param_position)
-        MF.pp_monospaced param_description MF.pp_monospaced overridden_method_descr MF.pp_monospaced
-        base_method_descr
+      if is_java_lang_object_equals base_proc_name then
+        (* This is a popular enough case to make error message specific *)
+        Format.asprintf
+          "Parameter %a is missing `@Nullable` declaration: according to the Java Specification, \
+           for any object `x` call `x.equals(null)` should properly return false."
+          MF.pp_monospaced param_description
+      else
+        let translate_position = function
+          | 1 ->
+              "First"
+          | 2 ->
+              "Second"
+          | 3 ->
+              "Third"
+          | n ->
+              string_of_int n ^ "th"
+        in
+        Format.asprintf
+          "%s parameter %a of method %a is missing `@Nullable` declaration when overriding %a. The \
+           parent method declared it can handle `null` for this param, so the child should also \
+           declare that."
+          (translate_position param_position)
+          MF.pp_monospaced param_description MF.pp_monospaced overridden_method_descr
+          MF.pp_monospaced base_method_descr
 
 
 let violation_severity {is_strict_mode} =

infer/src/nullsafe/modelTables.ml

@@ -370,6 +370,7 @@ let annotated_list_nullability =
     ; (n1, "java.lang.Integer.equals(java.lang.Object):boolean")
     ; (o1, "java.lang.Integer.parseInt(java.lang.String):int")
     ; (o1, "java.lang.Long.parseLong(java.lang.String):long")
+    ; (n1, "java.lang.Object.equals(java.lang.Object):boolean")
     ; (n2, "java.lang.RuntimeException.<init>(java.lang.String,java.lang.Throwable)")
     ; (n1, "java.lang.String.equals(java.lang.Object):boolean")
     ; (n1, "java.lang.StringBuilder.append(java.lang.String):java.lang.StringBuilder")

infer/tests/codetoanalyze/java/nullsafe-default/InconsistentSubclassAnnotation.java

@@ -171,3 +171,11 @@ class ExtendsExternalLibrary extends SomeExternalClass {
     // subtyping error on the parameter type are reported
   }
 }
+
+// Check that we have a special error message for this method
+class JavaLangEquals {
+  @Override
+  public boolean equals(Object x) {
+    return false;
+  }
+}

infer/tests/codetoanalyze/java/nullsafe-default/issues.exp

@@ -99,6 +99,7 @@ codetoanalyze/java/nullsafe-default/FieldOverAnnotated.java, codetoanalyze.java.
 codetoanalyze/java/nullsafe-default/InconsistentSubclassAnnotation.java, codetoanalyze.java.nullsafe_default.ArgNullToValBAD.nullableArg(java.lang.String):java.lang.String, 0, ERADICATE_INCONSISTENT_SUBCLASS_PARAMETER_ANNOTATION, no_bucket, WARNING, [First parameter `arg` of method `ArgNullToValBAD.nullableArg(...)` is missing `@Nullable` declaration when overriding `VariousMethods.nullableArg(...)`. The parent method declared it can handle `null` for this param, so the child should also declare that.]
 codetoanalyze/java/nullsafe-default/InconsistentSubclassAnnotation.java, codetoanalyze.java.nullsafe_default.ArgNullToValForInterfaceInAnotherFileBAD.implementInAnotherFile(java.lang.String):java.lang.String, 0, ERADICATE_INCONSISTENT_SUBCLASS_PARAMETER_ANNOTATION, no_bucket, WARNING, [First parameter `s` of method `ArgNullToValForInterfaceInAnotherFileBAD.implementInAnotherFile(...)` is missing `@Nullable` declaration when overriding `InconsistentSubclassAnnotationInterface.implementInAnotherFile(...)`. The parent method declared it can handle `null` for this param, so the child should also declare that.]
 codetoanalyze/java/nullsafe-default/InconsistentSubclassAnnotation.java, codetoanalyze.java.nullsafe_default.ExtendsExternalLibrary.externalMethod2(java.lang.Object):void, 0, ERADICATE_INCONSISTENT_SUBCLASS_PARAMETER_ANNOTATION, no_bucket, WARNING, [First parameter `object` of method `ExtendsExternalLibrary.externalMethod2(...)` is missing `@Nullable` declaration when overriding `SomeExternalClass.externalMethod2(...)`. The parent method declared it can handle `null` for this param, so the child should also declare that.]
+codetoanalyze/java/nullsafe-default/InconsistentSubclassAnnotation.java, codetoanalyze.java.nullsafe_default.JavaLangEquals.equals(java.lang.Object):boolean, 0, ERADICATE_INCONSISTENT_SUBCLASS_PARAMETER_ANNOTATION, no_bucket, WARNING, [Parameter `x` is missing `@Nullable` declaration: according to the Java Specification, for any object `x` call `x.equals(null)` should properly return false.]
 codetoanalyze/java/nullsafe-default/InconsistentSubclassAnnotation.java, codetoanalyze.java.nullsafe_default.OverloadExistingIncorrectBAD.overload(java.lang.String,java.lang.String):java.lang.String, 0, ERADICATE_INCONSISTENT_SUBCLASS_RETURN_ANNOTATION, no_bucket, WARNING, [Child method `OverloadExistingIncorrectBAD.overload(...)` is not substitution-compatible with its parent: the return type is declared as nullable, but parent method `Overloads.overload(...)` is missing `@Nullable` declaration. Either mark the parent as `@Nullable` or ensure the child does not return `null`.]
 codetoanalyze/java/nullsafe-default/InconsistentSubclassAnnotation.java, codetoanalyze.java.nullsafe_default.ReturnValToNullBAD.valBoth(java.lang.String):java.lang.String, 0, ERADICATE_INCONSISTENT_SUBCLASS_RETURN_ANNOTATION, no_bucket, WARNING, [Child method `ReturnValToNullBAD.valBoth(...)` is not substitution-compatible with its parent: the return type is declared as nullable, but parent method `VariousMethods.valBoth(...)` is missing `@Nullable` declaration. Either mark the parent as `@Nullable` or ensure the child does not return `null`.]
 codetoanalyze/java/nullsafe-default/InheritanceForStrictMode.java, codetoanalyze.java.nullsafe_default.InheritanceForStrictMode$NonStrictExtendingStrict.badToAddNullableInChildren():java.lang.String, 0, ERADICATE_INCONSISTENT_SUBCLASS_RETURN_ANNOTATION, no_bucket, WARNING, [Child method `InheritanceForStrictMode$NonStrictExtendingStrict.badToAddNullableInChildren()` is not substitution-compatible with its parent: the return type is declared as nullable, but parent method `InheritanceForStrictMode$StrictBase.badToAddNullableInChildren()` is missing `@Nullable` declaration. Either mark the parent as `@Nullable` or ensure the child does not return `null`.]