From fe674937a4b43e4ac4d16aa63299a22572ad1852 Mon Sep 17 00:00:00 2001 From: Mitya Lyubarskiy Date: Mon, 9 Sep 2019 01:54:36 -0700 Subject: [PATCH] [nullsafe] Improve the test for checking `onDestroy` handling Summary: 1. Let's make the intention of the test more visible, also let's provide an example when the error does occur. 2. `onDestroy` silence "field not nullable" warnigs not only for `View`, but for any objects, so let's use `String` (as an example of a trivial object) instead. Original diff that introduced the test: D10024458 Reviewed By: artempyanykh Differential Revision: D17202839 fbshipit-source-id: 037d937e4 --- .../nullsafe-default/FieldNotNullable.java | 22 +++++++++++++++---- .../java/nullsafe-default/issues.exp | 8 +++---- 2 files changed, 22 insertions(+), 8 deletions(-) diff --git a/infer/tests/codetoanalyze/java/nullsafe-default/FieldNotNullable.java b/infer/tests/codetoanalyze/java/nullsafe-default/FieldNotNullable.java index 649b17a00..5b40a8736 100644 --- a/infer/tests/codetoanalyze/java/nullsafe-default/FieldNotNullable.java +++ b/infer/tests/codetoanalyze/java/nullsafe-default/FieldNotNullable.java @@ -10,7 +10,6 @@ package codetoanalyze.java.nullsafe_default; import android.app.Activity; import android.os.Bundle; import android.support.v4.app.Fragment; -import android.view.View; import com.facebook.infer.annotation.Assertions; import com.facebook.infer.annotation.Cleanup; import com.facebook.infer.annotation.Initializer; @@ -24,13 +23,28 @@ abstract class A { } } -class FragmentExample extends Fragment { +/** + * It is common in Android code to recycle the objects (e.g. views) by nullifying them in the + * onDestroy() or onDestroyView() methods. This allows the GC to recycle it not waiting the outer + * object to be freed. This is safe because the lifecycle of the object is over so those field are + * not going to be accessed. so it is not necessary to annotate those fields with @Nullable. + */ +class CanAssignNullInDestroyMethods extends Fragment { - View view; + String someObject = ""; @Override public void onDestroyView() { - view = null; + someObject = null; + } + + @Override + public void onDestroy() { + someObject = null; + } + + public void assignNullDissalowedInAnyOtherMethod() { + someObject = null; // BAD: field not nullable } } diff --git a/infer/tests/codetoanalyze/java/nullsafe-default/issues.exp b/infer/tests/codetoanalyze/java/nullsafe-default/issues.exp index 5a5b58f1c..034d0d854 100644 --- a/infer/tests/codetoanalyze/java/nullsafe-default/issues.exp +++ b/infer/tests/codetoanalyze/java/nullsafe-default/issues.exp @@ -22,11 +22,11 @@ codetoanalyze/java/nullsafe-default/FieldNotInitialized.java, codetoanalyze.java codetoanalyze/java/nullsafe-default/FieldNotInitialized.java, codetoanalyze.java.nullsafe_default.FieldNotInitialized$Swap.(codetoanalyze.java.nullsafe_default.FieldNotInitialized), 0, ERADICATE_FIELD_NOT_INITIALIZED, no_bucket, WARNING, [Field `FieldNotInitialized$Swap.o1` is not initialized in the constructor and is not declared `@Nullable`] codetoanalyze/java/nullsafe-default/FieldNotInitialized.java, codetoanalyze.java.nullsafe_default.FieldNotInitialized$WriteItself.(codetoanalyze.java.nullsafe_default.FieldNotInitialized), 0, ERADICATE_FIELD_NOT_INITIALIZED, no_bucket, WARNING, [Field `FieldNotInitialized$WriteItself.o` is not initialized in the constructor and is not declared `@Nullable`] codetoanalyze/java/nullsafe-default/FieldNotInitialized.java, codetoanalyze.java.nullsafe_default.FieldNotInitialized.(), 0, ERADICATE_FIELD_NOT_INITIALIZED, no_bucket, WARNING, [Field `FieldNotInitialized.a` is not initialized in the constructor and is not declared `@Nullable`] -codetoanalyze/java/nullsafe-default/FieldNotNullable.java, codetoanalyze.java.nullsafe_default.FieldNotNullable.(java.lang.Integer), -25, ERADICATE_FIELD_NOT_NULLABLE, no_bucket, WARNING, [Field `FieldNotNullable.static_s` can be null but is not declared `@Nullable`. (Origin: null constant at line 41)] -codetoanalyze/java/nullsafe-default/FieldNotNullable.java, codetoanalyze.java.nullsafe_default.FieldNotNullable.(java.lang.String), -2, ERADICATE_FIELD_NOT_NULLABLE, no_bucket, WARNING, [Field `FieldNotNullable.static_s` can be null but is not declared `@Nullable`. (Origin: null constant at line 41)] -codetoanalyze/java/nullsafe-default/FieldNotNullable.java, codetoanalyze.java.nullsafe_default.FieldNotNullable.setYNull():void, 1, ERADICATE_FIELD_NOT_NULLABLE, no_bucket, WARNING, [Field `FieldNotNullable.y` can be null but is not declared `@Nullable`. (Origin: null constant at line 59)] +codetoanalyze/java/nullsafe-default/FieldNotNullable.java, codetoanalyze.java.nullsafe_default.CanAssignNullInDestroyMethods.assignNullDissalowedInAnyOtherMethod():void, 1, ERADICATE_FIELD_NOT_NULLABLE, no_bucket, WARNING, [Field `CanAssignNullInDestroyMethods.someObject` can be null but is not declared `@Nullable`. (Origin: null constant at line 47)] +codetoanalyze/java/nullsafe-default/FieldNotNullable.java, codetoanalyze.java.nullsafe_default.FieldNotNullable.(java.lang.Integer), -25, ERADICATE_FIELD_NOT_NULLABLE, no_bucket, WARNING, [Field `FieldNotNullable.static_s` can be null but is not declared `@Nullable`. (Origin: null constant at line 55)] +codetoanalyze/java/nullsafe-default/FieldNotNullable.java, codetoanalyze.java.nullsafe_default.FieldNotNullable.(java.lang.String), -2, ERADICATE_FIELD_NOT_NULLABLE, no_bucket, WARNING, [Field `FieldNotNullable.static_s` can be null but is not declared `@Nullable`. (Origin: null constant at line 55)] +codetoanalyze/java/nullsafe-default/FieldNotNullable.java, codetoanalyze.java.nullsafe_default.FieldNotNullable.setYNull():void, 1, ERADICATE_FIELD_NOT_NULLABLE, no_bucket, WARNING, [Field `FieldNotNullable.y` can be null but is not declared `@Nullable`. (Origin: null constant at line 73)] codetoanalyze/java/nullsafe-default/FieldNotNullable.java, codetoanalyze.java.nullsafe_default.FieldNotNullable.setYNullable(java.lang.String):void, 1, ERADICATE_FIELD_NOT_NULLABLE, no_bucket, WARNING, [Field `FieldNotNullable.y` can be null but is not declared `@Nullable`. (Origin: method parameter s)] -codetoanalyze/java/nullsafe-default/FieldNotNullable.java, codetoanalyze.java.nullsafe_default.FragmentExample.(), 0, ERADICATE_FIELD_NOT_INITIALIZED, no_bucket, WARNING, [Field `FragmentExample.view` is not initialized in the constructor and is not declared `@Nullable`] codetoanalyze/java/nullsafe-default/FieldNotNullable.java, codetoanalyze.java.nullsafe_default.TestInitializerBuilder.build():codetoanalyze.java.nullsafe_default.TestInitializer, 2, ERADICATE_CONDITION_REDUNDANT, no_bucket, WARNING, [The condition TestInitializerBuilder.required1 is always true according to the existing annotations.] codetoanalyze/java/nullsafe-default/FieldNotNullable.java, codetoanalyze.java.nullsafe_default.TestInitializerBuilder.build():codetoanalyze.java.nullsafe_default.TestInitializer, 2, ERADICATE_CONDITION_REDUNDANT, no_bucket, WARNING, [The condition TestInitializerBuilder.required2 is always true according to the existing annotations.] 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 not `@Nullable` but is declared `@Nullable`in the parent class method `SomeExternalClass.externalMethod2(...)`.]