diff --git a/infer/src/nullsafe/NullsafeMode.ml b/infer/src/nullsafe/NullsafeMode.ml index bbe946028..ade484d9e 100644 --- a/infer/src/nullsafe/NullsafeMode.ml +++ b/infer/src/nullsafe/NullsafeMode.ml @@ -68,6 +68,18 @@ module Trust = struct is_stricter_trust_list stricter_trust_list weaker_trust_list + let intersect trust1 trust2 = + match (trust1, trust2) with + | Only classes, All -> + Only classes + | All, Only classes -> + Only classes + | Only list1, Only list2 -> + Only (JavaClassName.Set.inter list1 list2) + | All, All -> + All + + let pp fmt t = match t with | All -> @@ -116,9 +128,8 @@ let extract_user_defined_class_name java_class_name = |> Option.value ~default:java_class_name -let of_class tenv class_name = - let user_defined_class = extract_user_defined_class_name class_name in - match PatternMatch.type_name_get_annotation tenv (Typ.JavaClass user_defined_class) with +let extract_mode_from_explicit_class_annotation tenv classname = + match PatternMatch.type_name_get_annotation tenv (Typ.JavaClass classname) with | Some annots -> ( if Annotations.ia_is_nullsafe_strict annots then Strict else @@ -132,6 +143,33 @@ let of_class tenv class_name = Default +(** Get the minimal mode that is stricter or equal than both of given modes *) +let intersect mode1 mode2 = + match (mode1, mode2) with + | Strict, _ | _, Strict -> + Strict + | Local trust1, Local trust2 -> + Local (Trust.intersect trust1 trust2) + | Local trust, Default | Default, Local trust -> + Local trust + | Default, Default -> + Default + + +let of_class tenv class_name = + (* The mode of the class is the strictest over this class's mode annotation and its outer classes *) + let rec of_class_and_outer_classes class_name = + let curr_class_mode = extract_mode_from_explicit_class_annotation tenv class_name in + match JavaClassName.get_outer_class_name class_name with + | Some outer_name -> + intersect curr_class_mode (of_class_and_outer_classes outer_name) + | None -> + curr_class_mode + in + let user_defined_class = extract_user_defined_class_name class_name in + of_class_and_outer_classes user_defined_class + + let of_procname tenv pname = let class_name = match pname with diff --git a/infer/tests/codetoanalyze/java/nullsafe-default/NullsafeModeNestedClasses.java b/infer/tests/codetoanalyze/java/nullsafe-default/NullsafeModeNestedClasses.java new file mode 100644 index 000000000..1662c45ea --- /dev/null +++ b/infer/tests/codetoanalyze/java/nullsafe-default/NullsafeModeNestedClasses.java @@ -0,0 +1,112 @@ +/* + * 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_default; + +import com.facebook.infer.annotation.Nullsafe; + +/** Test to ensure we correctly evaluate mode for nested classes */ +@Nullsafe(Nullsafe.Mode.LOCAL) +class NullsafeLocal { + + public String shouldBeNullsafeModeError() { + return null; + } + + // Mode should be inherited from the parent class + class Nested { + public String shouldBeNullsafeModeError() { + return null; + } + + // Mode propagation should be transitive + class DeeplyNested { + public String shouldBeNullsafeModeError() { + return null; + } + } + + // This is Local, but not Strict mode + public String returningDefaultNotNullIsOK() { + return Default.getString(); + } + } + + // It is OK to make nested classes more strict + @Nullsafe(Nullsafe.Mode.STRICT) + class NestedStrict { + public String returningDefaultNotNullIsError() { + return Default.getString(); + } + } + + // No need to repeat the mode - it is redundant + @Nullsafe(Nullsafe.Mode.LOCAL) + class NestedExplicitLocal { + public String shouldBeNullsafeModeError() { + return null; + } + } +} + +@Nullsafe(Nullsafe.Mode.STRICT) +class NullsafeStrict { + public String returningDefaultNotNullIsError() { + return Default.getString(); + } + + // STRICT mode is propagated to the nested class + class Nested { + public String returningDefaultNotNullIsError() { + return Default.getString(); + } + + // Impossible to downgrade the level of nested class, even if the nested mode + // is implicit + @Nullsafe(Nullsafe.Mode.LOCAL) + class DeeplyNestedLocalIsStillStrict { + public String returningDefaultNotNullIsError() { + return Default.getString(); + } + } + } + + // Impossible to downgrade the level of nested class + @Nullsafe(Nullsafe.Mode.LOCAL) + class NestedLocalIsStillStrict { + public String returningDefaultNotNullIsError() { + return Default.getString(); + } + } +} + +class Default { + public static String getString() { + return ""; + } + + // OK for nested to be @Nullsafe but the outer is not + @Nullsafe(Nullsafe.Mode.LOCAL) + class NestedLocal { + public String shouldBeNullsafeModeError() { + return null; + } + + // This is Local, but not Strict mode + public String returningDefaultNotNullIsOK() { + return Default.getString(); + } + + // And we can increase strictness even more + @Nullsafe(Nullsafe.Mode.STRICT) + class DeeplyNestedStrict { + public String returningDefaultNotNullIsError() { + return Default.getString(); + } + } + } +} diff --git a/infer/tests/codetoanalyze/java/nullsafe-default/issues.exp b/infer/tests/codetoanalyze/java/nullsafe-default/issues.exp index cf9a1cac3..4273e23e8 100644 --- a/infer/tests/codetoanalyze/java/nullsafe-default/issues.exp +++ b/infer/tests/codetoanalyze/java/nullsafe-default/issues.exp @@ -262,6 +262,29 @@ codetoanalyze/java/nullsafe-default/NullsafeMode.java, codetoanalyze.java.nullsa codetoanalyze/java/nullsafe-default/NullsafeMode.java, codetoanalyze.java.nullsafe_default.NullsafeMode$TrustSomeNullsafe.BAD_returnNullFromNonNulsafe():java.lang.String, 0, ERADICATE_RETURN_NOT_NULLABLE, no_bucket, ERROR, [`BAD_returnNullFromNonNulsafe()`: return type is declared non-nullable but the method returns a nullable value: call to returnNull() at line 144.] codetoanalyze/java/nullsafe-default/NullsafeMode.java, codetoanalyze.java.nullsafe_default.NullsafeMode$TrustSomeNullsafe.FP_OK_accessFieldFromNonNullsafe():java.lang.String, 1, ERADICATE_UNCHECKED_USAGE_IN_NULLSAFE, no_bucket, ERROR, [`NullsafeMode$NonNullsafe.valField`: `@NullsafeLocal(trust=selected)` mode prohibits using values coming from non-nullsafe classes without a check. This field is used at line 147. Either add a local check for null or assertion, or make NullsafeMode$NonNullsafe nullsafe.] codetoanalyze/java/nullsafe-default/NullsafeMode.java, codetoanalyze.java.nullsafe_default.NullsafeMode$TrustSomeNullsafe.OK_returnFromUntrustedNonNullsafeAsNullable():java.lang.String, 0, ERADICATE_RETURN_OVER_ANNOTATED, no_bucket, ADVICE, [Method `OK_returnFromUntrustedNonNullsafeAsNullable()` is annotated with `@Nullable` but never returns null.] +codetoanalyze/java/nullsafe-default/NullsafeModeNestedClasses.java, Linters_dummy_method, 14, ERADICATE_META_CLASS_NEEDS_IMPROVEMENT, no_bucket, INFO, [], NullsafeLocal, codetoanalyze.java.nullsafe_default, issues: 1, curr_mode: "LocalTrustAll" +codetoanalyze/java/nullsafe-default/NullsafeModeNestedClasses.java, Linters_dummy_method, 21, ERADICATE_META_CLASS_NEEDS_IMPROVEMENT, no_bucket, INFO, [], NullsafeLocal$Nested, codetoanalyze.java.nullsafe_default, issues: 1, curr_mode: "LocalTrustAll" +codetoanalyze/java/nullsafe-default/NullsafeModeNestedClasses.java, Linters_dummy_method, 27, ERADICATE_META_CLASS_NEEDS_IMPROVEMENT, no_bucket, INFO, [], NullsafeLocal$Nested$DeeplyNested, codetoanalyze.java.nullsafe_default, issues: 1, curr_mode: "LocalTrustAll" +codetoanalyze/java/nullsafe-default/NullsafeModeNestedClasses.java, Linters_dummy_method, 41, ERADICATE_META_CLASS_NEEDS_IMPROVEMENT, no_bucket, INFO, [], NullsafeLocal$NestedStrict, codetoanalyze.java.nullsafe_default, issues: 1, curr_mode: "Strict" +codetoanalyze/java/nullsafe-default/NullsafeModeNestedClasses.java, Linters_dummy_method, 49, ERADICATE_META_CLASS_NEEDS_IMPROVEMENT, no_bucket, INFO, [], NullsafeLocal$NestedExplicitLocal, codetoanalyze.java.nullsafe_default, issues: 1, curr_mode: "LocalTrustAll" +codetoanalyze/java/nullsafe-default/NullsafeModeNestedClasses.java, Linters_dummy_method, 57, ERADICATE_META_CLASS_NEEDS_IMPROVEMENT, no_bucket, INFO, [], NullsafeStrict, codetoanalyze.java.nullsafe_default, issues: 1, curr_mode: "Strict" +codetoanalyze/java/nullsafe-default/NullsafeModeNestedClasses.java, Linters_dummy_method, 63, ERADICATE_META_CLASS_NEEDS_IMPROVEMENT, no_bucket, INFO, [], NullsafeStrict$Nested, codetoanalyze.java.nullsafe_default, issues: 1, curr_mode: "Strict" +codetoanalyze/java/nullsafe-default/NullsafeModeNestedClasses.java, Linters_dummy_method, 71, ERADICATE_META_CLASS_NEEDS_IMPROVEMENT, no_bucket, INFO, [], NullsafeStrict$Nested$DeeplyNestedLocalIsStillStrict, codetoanalyze.java.nullsafe_default, issues: 1, curr_mode: "Strict" +codetoanalyze/java/nullsafe-default/NullsafeModeNestedClasses.java, Linters_dummy_method, 80, ERADICATE_META_CLASS_NEEDS_IMPROVEMENT, no_bucket, INFO, [], NullsafeStrict$NestedLocalIsStillStrict, codetoanalyze.java.nullsafe_default, issues: 1, curr_mode: "Strict" +codetoanalyze/java/nullsafe-default/NullsafeModeNestedClasses.java, Linters_dummy_method, 87, ERADICATE_META_CLASS_CAN_BE_NULLSAFE, no_bucket, ADVICE, [Congrats! `Default` is free of nullability issues. Mark it `@Nullsafe(value = Nullsafe.Mode.LOCAL, trustOnly = @Nullsafe.TrustList({}))` to prevent regressions.], Default, codetoanalyze.java.nullsafe_default, issues: 0, curr_mode: "Default", promote_mode: "Strict" +codetoanalyze/java/nullsafe-default/NullsafeModeNestedClasses.java, Linters_dummy_method, 94, ERADICATE_META_CLASS_NEEDS_IMPROVEMENT, no_bucket, INFO, [], Default$NestedLocal, codetoanalyze.java.nullsafe_default, issues: 1, curr_mode: "LocalTrustAll" +codetoanalyze/java/nullsafe-default/NullsafeModeNestedClasses.java, Linters_dummy_method, 106, ERADICATE_META_CLASS_NEEDS_IMPROVEMENT, no_bucket, INFO, [], Default$NestedLocal$DeeplyNestedStrict, codetoanalyze.java.nullsafe_default, issues: 1, curr_mode: "Strict" +codetoanalyze/java/nullsafe-default/NullsafeModeNestedClasses.java, codetoanalyze.java.nullsafe_default.Default$NestedLocal$DeeplyNestedStrict.returningDefaultNotNullIsError():java.lang.String, 1, ERADICATE_UNCHECKED_USAGE_IN_NULLSAFE, no_bucket, ERROR, [`Default.getString()`: `@NullsafeStrict` mode prohibits using values coming from non-strict classes without a check. Result of this call is used at line 107. Either add a local check for null or assertion, or make Default nullsafe strict.] +codetoanalyze/java/nullsafe-default/NullsafeModeNestedClasses.java, codetoanalyze.java.nullsafe_default.Default$NestedLocal.shouldBeNullsafeModeError():java.lang.String, 0, ERADICATE_RETURN_NOT_NULLABLE, no_bucket, ERROR, [`shouldBeNullsafeModeError()`: return type is declared non-nullable but the method returns `null`: null constant at line 96.] +codetoanalyze/java/nullsafe-default/NullsafeModeNestedClasses.java, codetoanalyze.java.nullsafe_default.NullsafeLocal$Nested$DeeplyNested.shouldBeNullsafeModeError():java.lang.String, 0, ERADICATE_RETURN_NOT_NULLABLE, no_bucket, ERROR, [`shouldBeNullsafeModeError()`: return type is declared non-nullable but the method returns `null`: null constant at line 29.] +codetoanalyze/java/nullsafe-default/NullsafeModeNestedClasses.java, codetoanalyze.java.nullsafe_default.NullsafeLocal$Nested.shouldBeNullsafeModeError():java.lang.String, 0, ERADICATE_RETURN_NOT_NULLABLE, no_bucket, ERROR, [`shouldBeNullsafeModeError()`: return type is declared non-nullable but the method returns `null`: null constant at line 23.] +codetoanalyze/java/nullsafe-default/NullsafeModeNestedClasses.java, codetoanalyze.java.nullsafe_default.NullsafeLocal$NestedExplicitLocal.shouldBeNullsafeModeError():java.lang.String, 0, ERADICATE_RETURN_NOT_NULLABLE, no_bucket, ERROR, [`shouldBeNullsafeModeError()`: return type is declared non-nullable but the method returns `null`: null constant at line 51.] +codetoanalyze/java/nullsafe-default/NullsafeModeNestedClasses.java, codetoanalyze.java.nullsafe_default.NullsafeLocal$NestedStrict.returningDefaultNotNullIsError():java.lang.String, 1, ERADICATE_UNCHECKED_USAGE_IN_NULLSAFE, no_bucket, ERROR, [`Default.getString()`: `@NullsafeStrict` mode prohibits using values coming from non-strict classes without a check. Result of this call is used at line 42. Either add a local check for null or assertion, or make Default nullsafe strict.] +codetoanalyze/java/nullsafe-default/NullsafeModeNestedClasses.java, codetoanalyze.java.nullsafe_default.NullsafeLocal.shouldBeNullsafeModeError():java.lang.String, 0, ERADICATE_RETURN_NOT_NULLABLE, no_bucket, ERROR, [`shouldBeNullsafeModeError()`: return type is declared non-nullable but the method returns `null`: null constant at line 17.] +codetoanalyze/java/nullsafe-default/NullsafeModeNestedClasses.java, codetoanalyze.java.nullsafe_default.NullsafeStrict$Nested$DeeplyNestedLocalIsStillStrict.returningDefaultNotNullIsError():java.lang.String, 1, ERADICATE_UNCHECKED_USAGE_IN_NULLSAFE, no_bucket, ERROR, [`Default.getString()`: `@NullsafeStrict` mode prohibits using values coming from non-strict classes without a check. Result of this call is used at line 72. Either add a local check for null or assertion, or make Default nullsafe strict.] +codetoanalyze/java/nullsafe-default/NullsafeModeNestedClasses.java, codetoanalyze.java.nullsafe_default.NullsafeStrict$Nested.returningDefaultNotNullIsError():java.lang.String, 1, ERADICATE_UNCHECKED_USAGE_IN_NULLSAFE, no_bucket, ERROR, [`Default.getString()`: `@NullsafeStrict` mode prohibits using values coming from non-strict classes without a check. Result of this call is used at line 64. Either add a local check for null or assertion, or make Default nullsafe strict.] +codetoanalyze/java/nullsafe-default/NullsafeModeNestedClasses.java, codetoanalyze.java.nullsafe_default.NullsafeStrict$NestedLocalIsStillStrict.returningDefaultNotNullIsError():java.lang.String, 1, ERADICATE_UNCHECKED_USAGE_IN_NULLSAFE, no_bucket, ERROR, [`Default.getString()`: `@NullsafeStrict` mode prohibits using values coming from non-strict classes without a check. Result of this call is used at line 81. Either add a local check for null or assertion, or make Default nullsafe strict.] +codetoanalyze/java/nullsafe-default/NullsafeModeNestedClasses.java, codetoanalyze.java.nullsafe_default.NullsafeStrict.returningDefaultNotNullIsError():java.lang.String, 1, ERADICATE_UNCHECKED_USAGE_IN_NULLSAFE, no_bucket, ERROR, [`Default.getString()`: `@NullsafeStrict` mode prohibits using values coming from non-strict classes without a check. Result of this call is used at line 58. Either add a local check for null or assertion, or make Default nullsafe strict.] codetoanalyze/java/nullsafe-default/ParameterNotNullable.java, Linters_dummy_method, 20, ERADICATE_META_CLASS_NEEDS_IMPROVEMENT, no_bucket, INFO, [], ParameterNotNullable, codetoanalyze.java.nullsafe_default, issues: 37, curr_mode: "Default" codetoanalyze/java/nullsafe-default/ParameterNotNullable.java, Linters_dummy_method, 64, ERADICATE_META_CLASS_CAN_BE_NULLSAFE, no_bucket, ADVICE, [Congrats! `ParameterNotNullable$Builder` is free of nullability issues. Mark it `@Nullsafe(value = Nullsafe.Mode.LOCAL, trustOnly = @Nullsafe.TrustList({}))` to prevent regressions.], ParameterNotNullable$Builder, codetoanalyze.java.nullsafe_default, issues: 0, curr_mode: "Default", promote_mode: "Strict" codetoanalyze/java/nullsafe-default/ParameterNotNullable.java, Linters_dummy_method, 93, ERADICATE_META_CLASS_NEEDS_IMPROVEMENT, no_bucket, INFO, [], ParameterNotNullable$ConstructorCall, codetoanalyze.java.nullsafe_default, issues: 1, curr_mode: "Default"