diff --git a/Makefile b/Makefile index 9d9eff7bb..2dd5051ec 100644 --- a/Makefile +++ b/Makefile @@ -153,6 +153,7 @@ DIRECT_TESTS += \ java_bufferoverrun \ java_checkers \ java_nullsafe \ + java_nullsafe-annotation-graph \ java_hoisting \ java_hoistingExpensive \ java_impurity \ diff --git a/infer/src/integration/IssuesTest.ml b/infer/src/integration/IssuesTest.ml index e964c3baa..cb62ec84e 100644 --- a/infer/src/integration/IssuesTest.ml +++ b/infer/src/integration/IssuesTest.ml @@ -13,7 +13,9 @@ let pp_method_info fmt Jsonbug_t.{class_name; package; method_name; call_line} = let pp_nullsafe_extra fmt - Jsonbug_t.{class_name; package; meta_issue_info; unvetted_3rd_party; nullable_methods} = + Jsonbug_t. + {class_name; package; meta_issue_info; unvetted_3rd_party; nullable_methods; annotation_graph} + = F.fprintf fmt "%s, %s" class_name (Option.value package ~default:"") ; Option.iter unvetted_3rd_party ~f:(fun unvetted_3rd_party -> let third_party_str = String.concat unvetted_3rd_party ~sep:"," in @@ -29,7 +31,10 @@ let pp_nullsafe_extra fmt in F.fprintf fmt ", issues: %d, curr_mode: %s%s" num_issues (Jsonbug_j.string_of_nullsafe_mode curr_nullsafe_mode) - can_be_promoted_to_str ) + can_be_promoted_to_str ) ; + Option.iter annotation_graph ~f:(fun annotation_graph -> + F.fprintf fmt "\nAnnotationGraph:@\n @[%a@]" NullsafeAnnotationGraphUtils.pp_annotation_graph + annotation_graph ) let pp_trace fmt trace comma = diff --git a/infer/src/integration/NullsafeAnnotationGraphUtils.ml b/infer/src/integration/NullsafeAnnotationGraphUtils.ml new file mode 100644 index 000000000..a1a0e1696 --- /dev/null +++ b/infer/src/integration/NullsafeAnnotationGraphUtils.ml @@ -0,0 +1,55 @@ +(* + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + *) + +open! IStd + +let pp_annotation_point fmt + Jsonbug_t.{id; kind; method_info; field_name; param_num; num_violations; dependent_point_ids} = + let pp_id fmt = Format.fprintf fmt "id: %s@\n" id in + let pp_kind fmt = + let kind_str = match kind with `Field -> "Field" | `Method -> "Method" | `Param -> "Param" in + Format.fprintf fmt "kind: %s@\n" kind_str + in + let pp_method_info fmt = + Option.iter method_info ~f:(fun Jsonbug_t.{method_name; params; access_level} -> + let access_level_str = + match access_level with + | `Public -> + "Public" + | `Private -> + "Private" + | `Protected -> + "Protected" + | `Default -> + "Default" + in + Format.fprintf fmt "method_info:@\n @[method_name: %s@\nparams: %s@\naccess_level: %s@]@\n" + method_name (String.concat ~sep:", " params) access_level_str ) + in + let pp_field_name fmt = + Option.iter field_name ~f:(fun field_name -> Format.fprintf fmt "field_name: %s@\n" field_name) + in + let pp_param_num fmt = + Option.iter param_num ~f:(fun param_num -> Format.fprintf fmt "param_num: %d@\n" param_num) + in + let pp_num_violations fmt = Format.fprintf fmt "num_violations: %d@\n" num_violations in + let pp_dependent_point_ids fmt = + Format.fprintf fmt "dependent_point_ids: [%s]@\n" (String.concat ~sep:", " dependent_point_ids) + in + let pp_point fmt = + pp_id fmt ; + pp_kind fmt ; + pp_method_info fmt ; + pp_field_name fmt ; + pp_param_num fmt ; + pp_num_violations fmt ; + pp_dependent_point_ids fmt + in + Format.fprintf fmt "Annotation point:@\n @[%t@]@\n" pp_point + + +let pp_annotation_graph = Pp.seq ~sep:"" pp_annotation_point diff --git a/infer/src/integration/NullsafeAnnotationGraphUtils.mli b/infer/src/integration/NullsafeAnnotationGraphUtils.mli new file mode 100644 index 000000000..32f0d8e90 --- /dev/null +++ b/infer/src/integration/NullsafeAnnotationGraphUtils.mli @@ -0,0 +1,12 @@ +(* + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + *) + +open! IStd + +(** A helper module to deal with Nullsafe annotation graph's JSON representation *) + +val pp_annotation_graph : Format.formatter -> Jsonbug_t.annotation_point list -> unit diff --git a/infer/src/nullsafe/AnnotationGraph.ml b/infer/src/nullsafe/AnnotationGraph.ml index 8aa8d96ec..2ad12d229 100644 --- a/infer/src/nullsafe/AnnotationGraph.ml +++ b/infer/src/nullsafe/AnnotationGraph.ml @@ -60,6 +60,8 @@ let build_graph_nodes tenv class_struct class_name = let class_typ = Typ.mk_struct class_name in let field_annotations = class_struct.Struct.fields + (* Sorting to get the stable list *) + |> List.sort ~compare:Struct.compare_field |> List.filter_map ~f:(fun (field_name, _, _) -> let AnnotatedField.{annotated_type= {nullability}} = Option.value_exn @@ -68,7 +70,9 @@ let build_graph_nodes tenv class_struct class_name = get_provisional_annotation nullability ) in let method_signatures = + (* Sorting to get the stable list *) class_struct.Struct.methods + |> List.sort ~compare:Procname.compare |> List.map ~f:(fun proc_name -> let proc_attributes = Option.value_exn (PatternMatch.lookup_attributes tenv proc_name) in AnnotatedSignature.get_for_class_under_analysis tenv proc_attributes ) diff --git a/infer/tests/codetoanalyze/java/nullsafe-annotation-graph/.inferconfig b/infer/tests/codetoanalyze/java/nullsafe-annotation-graph/.inferconfig new file mode 100644 index 000000000..0d8fd5da3 --- /dev/null +++ b/infer/tests/codetoanalyze/java/nullsafe-annotation-graph/.inferconfig @@ -0,0 +1,3 @@ +{ + "nullsafe-annotation-graph": true +} diff --git a/infer/tests/codetoanalyze/java/nullsafe-annotation-graph/AnnotationGraph.java b/infer/tests/codetoanalyze/java/nullsafe-annotation-graph/AnnotationGraph.java new file mode 100644 index 000000000..d10a05ec9 --- /dev/null +++ b/infer/tests/codetoanalyze/java/nullsafe-annotation-graph/AnnotationGraph.java @@ -0,0 +1,81 @@ +/* + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +package codetoanalyze.java.nullsafe_annotation_graph; + +import javax.annotation.Nullable; + +public class AnnotationGraph { + public String fieldA; + public String fieldB; + public @Nullable String fieldC; + public String fieldD; + + // methodA() depends on `p` and on `fieldD` + private String methodA(String p, boolean flag) { + // fieldA depends on p + fieldA = p; + if (flag) { + return p; + } else { + return fieldD; + } + } + + // methodB() depends on methodA()'s return + private String methodB() { + return methodA("", true); + } + + public String methodC() { + String a = methodB(); + // fieldC depends on methodB() + fieldC = a; + + // return does NOT depend on methodB(): already checked for null + if (a != null) { + return a; + } + + return ""; + } + + private void methodD() { + // fieldB depends on fieldA + fieldB = fieldA; + } + + private void methodE() { + // violation for fieldD + SomeExternalClass.acceptsNull(fieldD); + // violation for fieldD + fieldD.toString(); + if (fieldD != null) { + // no violation for fieldD + SomeExternalClass.acceptsNull(fieldD); + } + // no violation for fieldB + SomeExternalClass.doesNotAcceptNull(fieldB); + + if (methodC() != null) { + methodC().toString(); // no violation for methodC + } + } + + private void methodF() { + // violation for fieldA + fieldA.toString(); + + methodC().toString(); // violation for methodC + } +} + +class SomeExternalClass { + public static void acceptsNull(@Nullable String a) {} + + public static void doesNotAcceptNull(String a) {} +} diff --git a/infer/tests/codetoanalyze/java/nullsafe-annotation-graph/Makefile b/infer/tests/codetoanalyze/java/nullsafe-annotation-graph/Makefile new file mode 100644 index 000000000..074683141 --- /dev/null +++ b/infer/tests/codetoanalyze/java/nullsafe-annotation-graph/Makefile @@ -0,0 +1,14 @@ +# Copyright (c) Facebook, Inc. and its affiliates. +# +# This source code is licensed under the MIT license found in the +# LICENSE file in the root directory of this source tree. + +TESTS_DIR = ../../.. + +INFER_OPTIONS = \ + --eradicate-only \ + --debug-exceptions +INFERPRINT_OPTIONS = --issues-tests +SOURCES = $(wildcard *.java) + +include $(TESTS_DIR)/javac.make diff --git a/infer/tests/codetoanalyze/java/nullsafe-annotation-graph/issues.exp b/infer/tests/codetoanalyze/java/nullsafe-annotation-graph/issues.exp new file mode 100644 index 000000000..df0f93c7e --- /dev/null +++ b/infer/tests/codetoanalyze/java/nullsafe-annotation-graph/issues.exp @@ -0,0 +1,172 @@ +codetoanalyze/java/nullsafe-annotation-graph/AnnotationGraph.java, Linters_dummy_method, 12, ERADICATE_ANNOTATION_GRAPH, no_bucket, INFO, [], AnnotationGraph, codetoanalyze.java.nullsafe_annotation_graph +AnnotationGraph: + Annotation point: + id: f0 + kind: Field + field_name: fieldA + num_violations: 1 + dependent_point_ids: [f1] + + Annotation point: + id: f1 + kind: Field + field_name: fieldB + num_violations: 1 + dependent_point_ids: [] + + Annotation point: + id: f2 + kind: Field + field_name: fieldD + num_violations: 1 + dependent_point_ids: [m4] + + Annotation point: + id: m4 + kind: Method + method_info: + method_name: methodA + params: java.lang.String, boolean + access_level: Private + num_violations: 0 + dependent_point_ids: [m7] + + Annotation point: + id: m7 + kind: Method + method_info: + method_name: methodB + params: + access_level: Private + num_violations: 0 + dependent_point_ids: [] + + Annotation point: + id: m9 + kind: Method + method_info: + method_name: methodC + params: + access_level: Public + num_violations: 1 + dependent_point_ids: [] + + Annotation point: + id: p10 + kind: Param + method_info: + method_name: methodC + params: + access_level: Public + param_num: 0 + num_violations: 0 + dependent_point_ids: [] + + Annotation point: + id: p11 + kind: Param + method_info: + method_name: methodD + params: + access_level: Private + param_num: 0 + num_violations: 0 + dependent_point_ids: [] + + Annotation point: + id: p12 + kind: Param + method_info: + method_name: methodE + params: + access_level: Private + param_num: 0 + num_violations: 0 + dependent_point_ids: [] + + Annotation point: + id: p13 + kind: Param + method_info: + method_name: methodF + params: + access_level: Private + param_num: 0 + num_violations: 0 + dependent_point_ids: [] + + Annotation point: + id: p3 + kind: Param + method_info: + method_name: + params: + access_level: Public + param_num: 0 + num_violations: 0 + dependent_point_ids: [] + + Annotation point: + id: p5 + kind: Param + method_info: + method_name: methodA + params: java.lang.String, boolean + access_level: Private + param_num: 0 + num_violations: 0 + dependent_point_ids: [] + + Annotation point: + id: p6 + kind: Param + method_info: + method_name: methodA + params: java.lang.String, boolean + access_level: Private + param_num: 1 + num_violations: 0 + dependent_point_ids: [f0, m4] + + Annotation point: + id: p8 + kind: Param + method_info: + method_name: methodB + params: + access_level: Private + param_num: 0 + num_violations: 0 + dependent_point_ids: [] + + +codetoanalyze/java/nullsafe-annotation-graph/AnnotationGraph.java, Linters_dummy_method, 12, ERADICATE_META_CLASS_NEEDS_IMPROVEMENT, no_bucket, INFO, [], AnnotationGraph, codetoanalyze.java.nullsafe_annotation_graph, issues: 3, curr_mode: "Default" +codetoanalyze/java/nullsafe-annotation-graph/AnnotationGraph.java, Linters_dummy_method, 77, ERADICATE_ANNOTATION_GRAPH, no_bucket, INFO, [], SomeExternalClass, codetoanalyze.java.nullsafe_annotation_graph +AnnotationGraph: + Annotation point: + id: p0 + kind: Param + method_info: + method_name: + params: + access_level: Default + param_num: 0 + num_violations: 0 + dependent_point_ids: [] + + Annotation point: + id: p1 + kind: Param + method_info: + method_name: doesNotAcceptNull + params: java.lang.String + access_level: Public + param_num: 0 + num_violations: 0 + dependent_point_ids: [] + + +codetoanalyze/java/nullsafe-annotation-graph/AnnotationGraph.java, Linters_dummy_method, 77, ERADICATE_META_CLASS_CAN_BE_NULLSAFE, no_bucket, ADVICE, [Congrats! `SomeExternalClass` is free of nullability issues. Mark it `@Nullsafe(Nullsafe.Mode.LOCAL)` to prevent regressions.], SomeExternalClass, codetoanalyze.java.nullsafe_annotation_graph, issues: 0, curr_mode: "Default", promote_mode: "Strict" +codetoanalyze/java/nullsafe-annotation-graph/AnnotationGraph.java, codetoanalyze.java.nullsafe_annotation_graph.AnnotationGraph.(), 0, ERADICATE_FIELD_NOT_INITIALIZED, no_bucket, WARNING, [Field `fieldD` is declared non-nullable, so it should be initialized in the constructor or in an `@Initializer` method], AnnotationGraph, codetoanalyze.java.nullsafe_annotation_graph +codetoanalyze/java/nullsafe-annotation-graph/AnnotationGraph.java, codetoanalyze.java.nullsafe_annotation_graph.AnnotationGraph.(), 0, ERADICATE_FIELD_NOT_INITIALIZED, no_bucket, WARNING, [Field `fieldB` is declared non-nullable, so it should be initialized in the constructor or in an `@Initializer` method], AnnotationGraph, codetoanalyze.java.nullsafe_annotation_graph +codetoanalyze/java/nullsafe-annotation-graph/AnnotationGraph.java, codetoanalyze.java.nullsafe_annotation_graph.AnnotationGraph.(), 0, ERADICATE_FIELD_NOT_INITIALIZED, no_bucket, WARNING, [Field `fieldA` is declared non-nullable, so it should be initialized in the constructor or in an `@Initializer` method], AnnotationGraph, codetoanalyze.java.nullsafe_annotation_graph