From 090fa92c153f24553a8614cefaec8cddc4ec76ee Mon Sep 17 00:00:00 2001 From: Mitya Lyubarskiy Date: Mon, 9 Sep 2019 01:54:40 -0700 Subject: [PATCH] [nullsafe] Make FieldNotNullable test specific about both positive and negative cases Summary: 1. Remove manipulations with "shadowed" fields and abstract class, I don't believe they produced high quality signal (and no related warnings in the test output). 2. For each failure case provide corresponding success case and the reverse Reviewed By: artempyanykh Differential Revision: D17203240 fbshipit-source-id: c809857ed --- .../nullsafe-default/FieldNotNullable.java | 53 ++++++++----------- .../java/nullsafe-default/issues.exp | 11 ++-- 2 files changed, 28 insertions(+), 36 deletions(-) diff --git a/infer/tests/codetoanalyze/java/nullsafe-default/FieldNotNullable.java b/infer/tests/codetoanalyze/java/nullsafe-default/FieldNotNullable.java index 5b40a8736..4522561df 100644 --- a/infer/tests/codetoanalyze/java/nullsafe-default/FieldNotNullable.java +++ b/infer/tests/codetoanalyze/java/nullsafe-default/FieldNotNullable.java @@ -15,14 +15,6 @@ import com.facebook.infer.annotation.Cleanup; import com.facebook.infer.annotation.Initializer; import javax.annotation.Nullable; -abstract class A { - final String fld; - - A(String s) { - this.fld = s; - } -} - /** * 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 @@ -48,39 +40,38 @@ class CanAssignNullInDestroyMethods extends Fragment { } } -public class FieldNotNullable extends A { - @Nullable String x; - String y; - String fld; // Shadow the field defined in A - String static_s = null; // Static initializer error +public class FieldNotNullable { + @Nullable String nullable = ""; + String notNullable = ""; - FieldNotNullable(String s) { - super(s); - x = null; - y = s; - this.fld = s; - } + String initializeNonNullableWithNullIsBAD = null; + @Nullable String initializeNullableWithNullIsOK = null; - void setXNull() { - x = null; + @Nullable + String getNullable() { + return ""; } - void setXNullable(@Nullable String s) { - x = s; + String getNotNullable() { + return ""; } - void setYNull() { - y = null; + void setNullableToNotNullableIsBAD(@Nullable String s) { + notNullable = null; // BAD + notNullable = s; // BAD + notNullable = getNullable(); // BAD (even though getNullable() does not really return null) } - void setYNullable(@Nullable String s) { - y = s; + void setNullableToNullableIsOK(@Nullable String s) { + nullable = null; // OK + nullable = s; // OK + nullable = getNullable(); // OK } - FieldNotNullable(Integer n) { - super(""); - this.fld = ""; - y = x == null ? "abc" : "def"; + void setNotNullableToNotNullableIsOK(String s) { + notNullable = "abc"; // OK + notNullable = s; // OK + notNullable = getNotNullable(); // OK } } diff --git a/infer/tests/codetoanalyze/java/nullsafe-default/issues.exp b/infer/tests/codetoanalyze/java/nullsafe-default/issues.exp index 034d0d854..f3a34d95c 100644 --- a/infer/tests/codetoanalyze/java/nullsafe-default/issues.exp +++ b/infer/tests/codetoanalyze/java/nullsafe-default/issues.exp @@ -22,11 +22,12 @@ 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.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.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 39)] +codetoanalyze/java/nullsafe-default/FieldNotNullable.java, codetoanalyze.java.nullsafe_default.FieldNotNullable.(), 4, ERADICATE_FIELD_NOT_NULLABLE, no_bucket, WARNING, [Field `FieldNotNullable.initializeNonNullableWithNullIsBAD` 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.getNullable():java.lang.String, 0, ERADICATE_RETURN_OVER_ANNOTATED, no_bucket, WARNING, [Method `getNullable()` is annotated with `@Nullable` but never returns null.] +codetoanalyze/java/nullsafe-default/FieldNotNullable.java, codetoanalyze.java.nullsafe_default.FieldNotNullable.setNullableToNotNullableIsBAD(java.lang.String):void, 1, ERADICATE_FIELD_NOT_NULLABLE, no_bucket, WARNING, [Field `FieldNotNullable.notNullable` can be null but is not declared `@Nullable`. (Origin: null constant at line 60)] +codetoanalyze/java/nullsafe-default/FieldNotNullable.java, codetoanalyze.java.nullsafe_default.FieldNotNullable.setNullableToNotNullableIsBAD(java.lang.String):void, 2, ERADICATE_FIELD_NOT_NULLABLE, no_bucket, WARNING, [Field `FieldNotNullable.notNullable` can be null but is not declared `@Nullable`. (Origin: method parameter s)] +codetoanalyze/java/nullsafe-default/FieldNotNullable.java, codetoanalyze.java.nullsafe_default.FieldNotNullable.setNullableToNotNullableIsBAD(java.lang.String):void, 3, ERADICATE_FIELD_NOT_NULLABLE, no_bucket, WARNING, [Field `FieldNotNullable.notNullable` can be null but is not declared `@Nullable`. (Origin: call to getNullable() at line 62)] 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(...)`.]