diff --git a/infer/src/nullsafe/InheritanceRule.ml b/infer/src/nullsafe/InheritanceRule.ml index cbf955d72..38f0e205c 100644 --- a/infer/src/nullsafe/InheritanceRule.ml +++ b/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} = diff --git a/infer/src/nullsafe/modelTables.ml b/infer/src/nullsafe/modelTables.ml index f21834434..7e40adc77 100644 --- a/infer/src/nullsafe/modelTables.ml +++ b/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.(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") diff --git a/infer/tests/codetoanalyze/java/nullsafe-default/InconsistentSubclassAnnotation.java b/infer/tests/codetoanalyze/java/nullsafe-default/InconsistentSubclassAnnotation.java index e25623884..cb60681fb 100644 --- a/infer/tests/codetoanalyze/java/nullsafe-default/InconsistentSubclassAnnotation.java +++ b/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; + } +} diff --git a/infer/tests/codetoanalyze/java/nullsafe-default/issues.exp b/infer/tests/codetoanalyze/java/nullsafe-default/issues.exp index 6316d8f2f..0f294671a 100644 --- a/infer/tests/codetoanalyze/java/nullsafe-default/issues.exp +++ b/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`.]