From f9b0d0682618a7a162755a98c751ba5831cf3310 Mon Sep 17 00:00:00 2001 From: Mitya Lyubarskiy Date: Mon, 28 Oct 2019 03:38:14 -0700 Subject: [PATCH] [nullsafe] Primitive types are always Nonnull Summary: Primitive types are not annotated. Because of that, we used to implicitly derive `DeclaredNonnull` type for them. This worked fine, but this leads to errors in Strict mode, which does not believe DeclaredNonnull type. Now lets offifically teach nullsafe that primitive types are non-nullable. Reviewed By: jvillard Differential Revision: D18114623 fbshipit-source-id: 227217931 --- infer/src/nullsafe/AnnotatedField.ml | 3 ++- infer/src/nullsafe/AnnotatedNullability.ml | 18 +++++++++---- infer/src/nullsafe/AnnotatedNullability.mli | 5 ++-- infer/src/nullsafe/AnnotatedSignature.ml | 25 ++++++++++++------- .../java/nullsafe-default/StrictMode.java | 10 ++++++++ .../java/nullsafe-default/issues.exp | 2 +- 6 files changed, 45 insertions(+), 18 deletions(-) diff --git a/infer/src/nullsafe/AnnotatedField.ml b/infer/src/nullsafe/AnnotatedField.ml index 02bde8fc4..5dd3c079a 100644 --- a/infer/src/nullsafe/AnnotatedField.ml +++ b/infer/src/nullsafe/AnnotatedField.ml @@ -28,7 +28,8 @@ let get tenv fn typ = { annotation_deprecated= annotation ; annotated_type= AnnotatedType. - {nullability= AnnotatedNullability.of_annot_item annotation ~is_strict_mode; typ} } + { nullability= AnnotatedNullability.of_type_and_annotation typ annotation ~is_strict_mode + ; typ } } in Option.map (Typ.Struct.get_field_type_and_annotation ~lookup fn typ) diff --git a/infer/src/nullsafe/AnnotatedNullability.ml b/infer/src/nullsafe/AnnotatedNullability.ml index c067c031e..df11cec63 100644 --- a/infer/src/nullsafe/AnnotatedNullability.ml +++ b/infer/src/nullsafe/AnnotatedNullability.ml @@ -40,6 +40,7 @@ and declared_nonnull_origin = and nonnull_origin = | ModelledNonnull (** nullsafe knows it is non-nullable via its internal models *) | StrictMode (** under strict mode we consider non-null declarations to be trusted *) + | PrimitiveType (** Primitive types are non-nullable by language design *) [@@deriving compare] let get_nullability = function @@ -67,7 +68,13 @@ let pp fmt t = match origin with AnnotatedNonnull -> "@" | ImplicitlyNonnull -> "implicit" in let string_of_nonnull_origin nonnull_origin = - match nonnull_origin with ModelledNonnull -> "model" | StrictMode -> "strict" + match nonnull_origin with + | ModelledNonnull -> + "model" + | StrictMode -> + "strict" + | PrimitiveType -> + "primitive" in match t with | Nullable origin -> @@ -78,14 +85,15 @@ let pp fmt t = F.fprintf fmt "Nonnull[%s]" (string_of_nonnull_origin origin) -let of_annot_item ~is_strict_mode ia = - if Annotations.ia_is_nullable ia then +let of_type_and_annotation ~is_strict_mode typ annotations = + if not (PatternMatch.type_is_class typ) then Nonnull PrimitiveType + else if Annotations.ia_is_nullable annotations then let nullable_origin = - if Annotations.ia_is_propagates_nullable ia then AnnotatedPropagatesNullable + if Annotations.ia_is_propagates_nullable annotations then AnnotatedPropagatesNullable else AnnotatedNullable in Nullable nullable_origin else if is_strict_mode then Nonnull StrictMode - else if Annotations.ia_is_nonnull ia then DeclaredNonnull AnnotatedNonnull + else if Annotations.ia_is_nonnull annotations then DeclaredNonnull AnnotatedNonnull (* Currently, we treat not annotated types as nonnull *) else DeclaredNonnull ImplicitlyNonnull diff --git a/infer/src/nullsafe/AnnotatedNullability.mli b/infer/src/nullsafe/AnnotatedNullability.mli index 17a460b26..04a0b0d3b 100644 --- a/infer/src/nullsafe/AnnotatedNullability.mli +++ b/infer/src/nullsafe/AnnotatedNullability.mli @@ -43,12 +43,13 @@ and declared_nonnull_origin = and nonnull_origin = | ModelledNonnull (** nullsafe knows it is non-nullable via its internal models *) | StrictMode (** under strict mode we consider non-null declarations to be trusted *) + | PrimitiveType (** Primitive types are non-nullable by language design *) [@@deriving compare] val get_nullability : t -> Nullability.t -val of_annot_item : is_strict_mode:bool -> Annot.Item.t -> t -(** Converts the information from the annotation to nullability. +val of_type_and_annotation : is_strict_mode:bool -> Typ.t -> Annot.Item.t -> t +(** Given the type and its annotations, returns its nullability. NOTE: it does not take into account models etc., so this is intended to be used as a helper function for more high-level annotation processing. *) diff --git a/infer/src/nullsafe/AnnotatedSignature.ml b/infer/src/nullsafe/AnnotatedSignature.ml index d72a212f0..0889ac340 100644 --- a/infer/src/nullsafe/AnnotatedSignature.ml +++ b/infer/src/nullsafe/AnnotatedSignature.ml @@ -28,8 +28,11 @@ and param_signature = [@@deriving compare] (* get nullability of method's return type given its annotations and information about its params *) -let nullability_for_return ia ~is_strict_mode ~has_propagates_nullable_in_param = - let nullability = AnnotatedNullability.of_annot_item ~is_strict_mode ia in +let nullability_for_return ret_type ret_annotations ~is_strict_mode + ~has_propagates_nullable_in_param = + let nullability = + AnnotatedNullability.of_type_and_annotation ~is_strict_mode ret_type ret_annotations + in (* if any param is annotated with propagates nullable, then the result is nullable *) match nullability with | AnnotatedNullability.Nullable _ -> @@ -43,9 +46,10 @@ let nullability_for_return ia ~is_strict_mode ~has_propagates_nullable_in_param (* Given annotations for method signature, extract nullability information for return type and params *) -let extract_nullability ~is_strict_mode return_annotation param_annotations = +let extract_nullability ~is_strict_mode ret_type ret_annotations param_annotated_types = let params_nullability = - List.map param_annotations ~f:(AnnotatedNullability.of_annot_item ~is_strict_mode) + List.map param_annotated_types ~f:(fun (typ, annotations) -> + AnnotatedNullability.of_type_and_annotation typ annotations ~is_strict_mode ) in let has_propagates_nullable_in_param = List.exists params_nullability ~f:(function @@ -55,13 +59,14 @@ let extract_nullability ~is_strict_mode return_annotation param_annotations = false ) in let return_nullability = - nullability_for_return return_annotation ~is_strict_mode ~has_propagates_nullable_in_param + nullability_for_return ret_type ret_annotations ~is_strict_mode + ~has_propagates_nullable_in_param in (return_nullability, params_nullability) let get ~is_strict_mode proc_attributes : t = - let Annot.Method.{return= return_annotation; params= original_params_annotation} = + let Annot.Method.{return= ret_annotation; params= original_params_annotation} = proc_attributes.ProcAttributes.method_annotation in let formals = proc_attributes.ProcAttributes.formals in @@ -85,12 +90,14 @@ let get ~is_strict_mode proc_attributes : t = in List.rev (zip_params (List.rev original_params_annotation) (List.rev formals)) in - let _, final_params_annotation = List.unzip params_with_annotations in + let param_annotated_types = + List.map params_with_annotations ~f:(fun ((_, typ), annotations) -> (typ, annotations)) + in let return_nullability, params_nullability = - extract_nullability ~is_strict_mode return_annotation final_params_annotation + extract_nullability ~is_strict_mode ret_type ret_annotation param_annotated_types in let ret = - { ret_annotation_deprecated= return_annotation + { ret_annotation_deprecated= ret_annotation ; ret_annotated_type= AnnotatedType.{nullability= return_nullability; typ= ret_type} } in let params = diff --git a/infer/tests/codetoanalyze/java/nullsafe-default/StrictMode.java b/infer/tests/codetoanalyze/java/nullsafe-default/StrictMode.java index 7562ac44b..8d47a9f0b 100644 --- a/infer/tests/codetoanalyze/java/nullsafe-default/StrictMode.java +++ b/infer/tests/codetoanalyze/java/nullsafe-default/StrictMode.java @@ -135,6 +135,12 @@ class Strict { return (new NonStrict()).getNullable(); } + public int usingPrimitiveTypesFromNonStrictIsOK() { + // Of course, primitive types can not be nullable so it + // does not matter if they are used from NonStrict + return NonStrict.getPrimitiveTypeValue(); + } + private String nonStrictClass_convertingNonnullToNonnullIsBad() { // even that it is declared as nonnull, can not convert it to nonnull it without checking before return (new NonStrict()).getNonnull(); @@ -235,4 +241,8 @@ class NonStrict { public static String staticNonnull() { return ""; } + + public static int getPrimitiveTypeValue() { + return 0; + } } diff --git a/infer/tests/codetoanalyze/java/nullsafe-default/issues.exp b/infer/tests/codetoanalyze/java/nullsafe-default/issues.exp index 8991c6eb1..a488b80dd 100644 --- a/infer/tests/codetoanalyze/java/nullsafe-default/issues.exp +++ b/infer/tests/codetoanalyze/java/nullsafe-default/issues.exp @@ -216,7 +216,7 @@ codetoanalyze/java/nullsafe-default/ReturnNotNullable.java, codetoanalyze.java.n codetoanalyze/java/nullsafe-default/ReturnNotNullable.java, codetoanalyze.java.nullsafe_default.ReturnNotNullable.return_null_in_catch():java.lang.String, 0, ERADICATE_RETURN_NOT_NULLABLE, no_bucket, WARNING, [Method `return_null_in_catch()` may return null but it is not annotated with `@Nullable`. (Origin: null constant at line 160)] codetoanalyze/java/nullsafe-default/ReturnNotNullable.java, codetoanalyze.java.nullsafe_default.ReturnNotNullable.return_null_in_catch_after_throw():java.lang.String, 0, ERADICATE_RETURN_NOT_NULLABLE, no_bucket, WARNING, [Method `return_null_in_catch_after_throw()` may return null but it is not annotated with `@Nullable`. (Origin: null constant at line 172)] codetoanalyze/java/nullsafe-default/ReturnNotNullable.java, codetoanalyze.java.nullsafe_default.ReturnNotNullable.tryWithResourcesReturnNullable(java.lang.String):java.lang.Object, 0, ERADICATE_RETURN_NOT_NULLABLE, no_bucket, WARNING, [Method `tryWithResourcesReturnNullable(...)` may return null but it is not annotated with `@Nullable`. (Origin: call to nullToNullableIsOK() at line 142)] -codetoanalyze/java/nullsafe-default/StrictMode.java, codetoanalyze.java.nullsafe_default.Strict.nonStrictClass_convertingNonnullToNonnullIsBad():java.lang.String, 0, ERADICATE_RETURN_NOT_NULLABLE, no_bucket, WARNING, [Method `nonStrictClass_convertingNonnullToNonnullIsBad()` may return null but it is not annotated with `@Nullable`. (Origin: call to getNonnull() at line 140)] +codetoanalyze/java/nullsafe-default/StrictMode.java, codetoanalyze.java.nullsafe_default.Strict.nonStrictClass_convertingNonnullToNonnullIsBad():java.lang.String, 0, ERADICATE_RETURN_NOT_NULLABLE, no_bucket, WARNING, [Method `nonStrictClass_convertingNonnullToNonnullIsBad()` may return null but it is not annotated with `@Nullable`. (Origin: call to getNonnull() at line 146)] codetoanalyze/java/nullsafe-default/StrictMode.java, codetoanalyze.java.nullsafe_default.Strict.nonStrictClass_convertingNonnullToNullableIsOK():java.lang.String, 0, ERADICATE_RETURN_OVER_ANNOTATED, no_bucket, WARNING, [Method `nonStrictClass_convertingNonnullToNullableIsOK()` is annotated with `@Nullable` but never returns null.] codetoanalyze/java/nullsafe-default/StrictMode.java, codetoanalyze.java.nullsafe_default.Strict.nonStrictClass_convertingNullableToNonnullIsBad():java.lang.String, 0, ERADICATE_RETURN_NOT_NULLABLE, no_bucket, WARNING, [Method `nonStrictClass_convertingNullableToNonnullIsBad()` may return null but it is not annotated with `@Nullable`. (Origin: call to getNullable() at line 135)] codetoanalyze/java/nullsafe-default/StrictMode.java, codetoanalyze.java.nullsafe_default.Strict.nonStrictClass_dereferenceNonnullFieldAfterCheckIsOK():void, 2, ERADICATE_CONDITION_REDUNDANT, no_bucket, WARNING, [The condition NonStrict.nonnull is always true according to the existing annotations.]