From 63a5ffb4dcb4080c3761d4b13db17096990ef726 Mon Sep 17 00:00:00 2001 From: Mitya Lyubarskiy Date: Mon, 9 Sep 2019 01:54:55 -0700 Subject: [PATCH] [nullsafe] Make FieldNotInitialized cover negative cases together with positive ones Summary: let's always have positive and negative case for each feature we test Reviewed By: ngorogiannis Differential Revision: D17206785 fbshipit-source-id: 5791ace48 --- .../nullsafe-default/FieldNotInitialized.java | 148 ++++++++++-------- .../java/nullsafe-default/issues.exp | 18 ++- 2 files changed, 95 insertions(+), 71 deletions(-) diff --git a/infer/tests/codetoanalyze/java/nullsafe-default/FieldNotInitialized.java b/infer/tests/codetoanalyze/java/nullsafe-default/FieldNotInitialized.java index 170c71f17..bdd2bcedb 100644 --- a/infer/tests/codetoanalyze/java/nullsafe-default/FieldNotInitialized.java +++ b/infer/tests/codetoanalyze/java/nullsafe-default/FieldNotInitialized.java @@ -10,7 +10,6 @@ package codetoanalyze.java.nullsafe_default; import android.app.Activity; import android.os.Bundle; import android.support.annotation.NonNull; -import android.widget.EditText; import com.facebook.infer.annotation.Assertions; import com.facebook.infer.annotation.Initializer; import com.facebook.infer.annotation.SuppressViewNullability; @@ -23,60 +22,61 @@ import javax.inject.Inject; public class FieldNotInitialized { - String a; + String notNullIsBAD; // BAD: need to initialize it - @Nullable String b; + @Nullable String nullableIsOK; // OK: will be init with null - @Nonnull String c; // Means: assume it will be initialized to a nonnull value somewhere else. + @Nonnull + String nonnullIsOK; // Means: assume it will be initialized to a nonnull value somewhere else. - @Inject String d; // Means: assume it will be initialized via dependency injection + @NonNull + String nonNullIsOK; // Means: assume it will be initialized to a nonnull value somewhere else. - @NonNull String e; + @Inject String injectIsOK; // Means: assume it will be initialized via dependency injection - @Bind EditText f; // Means: assume it will be initialized, and ignore null assignment + @Bind String bindIsOK; // Means: assume it will be initialized, and ignore null assignment - @SuppressViewNullability EditText g; + // Means: assume it will be initialized, and ignore null assignment + @SuppressViewNullability String suppressViewNullabilityIsOK; - // Eradicate should only report one initialization error FieldNotInitialized() {} void testNullifyFields() { - f = null; // OK the framework could write null into the field - g = null; // OK the framework could write null into the field + bindIsOK = null; // OK: the framework could write null into the field + suppressViewNullabilityIsOK = null; // OK: the framework could write null into the field + nonnullIsOK = null; // BAD: can not nullify this + injectIsOK = null; // BAD: can not nullify this } class OnlyRead { Object o; OnlyRead() { - Object x = o; // not initialized + Object x = o; // BAD: we merely read this variable, but forgot to initialize } } - class WriteItself { - Object o; + class WriteItselfIsBAD { + Object ok; + Object bad; - WriteItself() { - o = o; // not initialized + WriteItselfIsBAD() { + ok = ""; + bad = bad; // BAD: Can not initialize with itself } } - class Swap { + class InitializationOrder { Object o1; Object o2; - Swap() { - o1 = o2; // not initialized + InitializationOrder(int a) { + o1 = o2; // BAD: not initialized o2 = new Object(); } - } - - class SwapOK { - Object o1; - Object o2; - SwapOK() { - o1 = new Object(); + InitializationOrder(double a) { + o1 = new Object(); // OK o2 = o1; } } @@ -95,72 +95,88 @@ public class FieldNotInitialized { } } - class ConditionalFieldInit { - Object o1; - @Nullable Object o2 = null; - - public ConditionalFieldInit() { - if (o2 != null) { - o1 = new Object(); // Not always initialized + class ShouldInitializeInAllBranches { + Object f1; + Object f2; + Object f3; + Object f4; + Object f5; + + public ShouldInitializeInAllBranches(int a) { + f4 = new Object(); + Object f5 = new Object(); // BAD: shadowing; not an initialization + if (a == 42) { + f1 = new Object(); + f2 = new Object(); + } else { + f3 = new Object(); + if (a == 43) { + f1 = new Object(); + // BAD: f2 is not initialized in this branch + } else { + f1 = new Object(); + f2 = new Object(); + } } } } class InitIfNull { - Object o; + Object good; + Object shouldBeGood_FIXME; public InitIfNull() { - if (o == null) o = new Object(); - } - } - - class InitIfNull2 { - Object o; - - public InitIfNull2(Object x) { - if (o == null) o = x; - } - } - - class InitIfNull3 { - Object o; - - Object getNotNull() { - return new Object(); - } - - public InitIfNull3() { - if (o == null) o = getNotNull(); + if (good == null) { + good = new Object(); + // bad is considered to be initialized not in all branches. + // (which is bit weird: we know that good is always null by default) + // TODO(T53537343) teach nullsafe that all fields are initially null in constructor + shouldBeGood_FIXME = new Object(); + } } } class InitCircular { - String s; + String bad; + String stillBad; + String good; + + String knownNotNull = ""; InitCircular() { - String tmp = s; - s = tmp; // s is not initialized: circular initialization + String tmp = bad; + bad = tmp; // BAD: circular initialization + stillBad = bad; // BAD: try to initialize from circularly initalized var + + String tmp2 = knownNotNull; + good = tmp2; // OK } } class InitWithOtherClass { class OtherClass { - String s = ""; + String nonNull = ""; + @Nullable String nullable = ""; } - String s; + String bad; + String good; InitWithOtherClass(OtherClass x) { - s = x.s; + bad = x.nullable; // BAD: might be null + good = x.nonNull; // OK: we know can not be null } } - class InitWithThisClass { - - String s; + // Check that Infer does not confuse things + // in copy constuctors. + class InitWithTheSameClass { + String good; + String bad; - InitWithThisClass(InitWithThisClass x) { - s = x.s; + InitWithTheSameClass(InitWithTheSameClass x) { + good = x.good; // OK: this is not a circular initialization + bad = bad; // BAD: this is a circular initialization } } } diff --git a/infer/tests/codetoanalyze/java/nullsafe-default/issues.exp b/infer/tests/codetoanalyze/java/nullsafe-default/issues.exp index 2e04b14ca..b7acd5753 100644 --- a/infer/tests/codetoanalyze/java/nullsafe-default/issues.exp +++ b/infer/tests/codetoanalyze/java/nullsafe-default/issues.exp @@ -14,13 +14,21 @@ codetoanalyze/java/nullsafe-default/CustomAnnotations.java, codetoanalyze.java.n codetoanalyze/java/nullsafe-default/CustomAnnotations.java, codetoanalyze.java.nullsafe_default.CustomAnnotations$TestTextUtilsIsEmpty.textUtilsIsEmpty(java.lang.CharSequence):void, 1, ERADICATE_PARAMETER_NOT_NULLABLE, no_bucket, WARNING, [`TextUtils.isEmpty(...)` needs a non-null value in parameter 1 but argument `s` can be null. (Origin: method parameter s)] codetoanalyze/java/nullsafe-default/CustomAnnotations.java, codetoanalyze.java.nullsafe_default.CustomAnnotations$TestTextUtilsIsEmpty.textUtilsIsEmpty(java.lang.CharSequence):void, 2, ERADICATE_NULLABLE_DEREFERENCE, no_bucket, WARNING, [The value of `s` in the call to `toString()` is nullable and is not locally checked for null. (Origin: method parameter s)] codetoanalyze/java/nullsafe-default/CustomAnnotations.java, codetoanalyze.java.nullsafe_default.CustomAnnotations$TestTextUtilsIsEmpty.textUtilsNotIsEmpty(java.lang.CharSequence):void, 1, ERADICATE_PARAMETER_NOT_NULLABLE, no_bucket, WARNING, [`TextUtils.isEmpty(...)` needs a non-null value in parameter 1 but argument `s` can be null. (Origin: method parameter s)] -codetoanalyze/java/nullsafe-default/FieldNotInitialized.java, codetoanalyze.java.nullsafe_default.FieldNotInitialized$ConditionalFieldInit.(codetoanalyze.java.nullsafe_default.FieldNotInitialized), 0, ERADICATE_FIELD_NOT_INITIALIZED, no_bucket, WARNING, [Field `FieldNotInitialized$ConditionalFieldInit.o1` is not initialized in the constructor and is not declared `@Nullable`] -codetoanalyze/java/nullsafe-default/FieldNotInitialized.java, codetoanalyze.java.nullsafe_default.FieldNotInitialized$InitCircular.(codetoanalyze.java.nullsafe_default.FieldNotInitialized), 0, ERADICATE_FIELD_NOT_INITIALIZED, no_bucket, WARNING, [Field `FieldNotInitialized$InitCircular.s` is not initialized in the constructor and is not declared `@Nullable`] +codetoanalyze/java/nullsafe-default/FieldNotInitialized.java, codetoanalyze.java.nullsafe_default.FieldNotInitialized$InitCircular.(codetoanalyze.java.nullsafe_default.FieldNotInitialized), 0, ERADICATE_FIELD_NOT_INITIALIZED, no_bucket, WARNING, [Field `FieldNotInitialized$InitCircular.bad` is not initialized in the constructor and is not declared `@Nullable`] +codetoanalyze/java/nullsafe-default/FieldNotInitialized.java, codetoanalyze.java.nullsafe_default.FieldNotInitialized$InitCircular.(codetoanalyze.java.nullsafe_default.FieldNotInitialized), 0, ERADICATE_FIELD_NOT_INITIALIZED, no_bucket, WARNING, [Field `FieldNotInitialized$InitCircular.stillBad` is not initialized in the constructor and is not declared `@Nullable`] +codetoanalyze/java/nullsafe-default/FieldNotInitialized.java, codetoanalyze.java.nullsafe_default.FieldNotInitialized$InitIfNull.(codetoanalyze.java.nullsafe_default.FieldNotInitialized), 0, ERADICATE_FIELD_NOT_INITIALIZED, no_bucket, WARNING, [Field `FieldNotInitialized$InitIfNull.shouldBeGood_FIXME` is not initialized in the constructor and is not declared `@Nullable`] +codetoanalyze/java/nullsafe-default/FieldNotInitialized.java, codetoanalyze.java.nullsafe_default.FieldNotInitialized$InitWithOtherClass.(codetoanalyze.java.nullsafe_default.FieldNotInitialized,codetoanalyze.java.nullsafe_default.FieldNotInitialized$InitWithOtherClass$OtherClass), 1, ERADICATE_FIELD_NOT_NULLABLE, no_bucket, WARNING, [Field `FieldNotInitialized$InitWithOtherClass.bad` can be null but is not declared `@Nullable`. (Origin: field FieldNotInitialized$InitWithOtherClass$OtherClass.nullable at line 166)] +codetoanalyze/java/nullsafe-default/FieldNotInitialized.java, codetoanalyze.java.nullsafe_default.FieldNotInitialized$InitWithTheSameClass.(codetoanalyze.java.nullsafe_default.FieldNotInitialized,codetoanalyze.java.nullsafe_default.FieldNotInitialized$InitWithTheSameClass), 0, ERADICATE_FIELD_NOT_INITIALIZED, no_bucket, WARNING, [Field `FieldNotInitialized$InitWithTheSameClass.bad` is not initialized in the constructor and is not declared `@Nullable`] +codetoanalyze/java/nullsafe-default/FieldNotInitialized.java, codetoanalyze.java.nullsafe_default.FieldNotInitialized$InitializationOrder.(codetoanalyze.java.nullsafe_default.FieldNotInitialized,int), 0, ERADICATE_FIELD_NOT_INITIALIZED, no_bucket, WARNING, [Field `FieldNotInitialized$InitializationOrder.o1` is not initialized in the constructor and is not declared `@Nullable`] codetoanalyze/java/nullsafe-default/FieldNotInitialized.java, codetoanalyze.java.nullsafe_default.FieldNotInitialized$OnlyRead.(codetoanalyze.java.nullsafe_default.FieldNotInitialized), 0, ERADICATE_FIELD_NOT_INITIALIZED, no_bucket, WARNING, [Field `FieldNotInitialized$OnlyRead.o` is not initialized in the constructor and is not declared `@Nullable`] codetoanalyze/java/nullsafe-default/FieldNotInitialized.java, codetoanalyze.java.nullsafe_default.FieldNotInitialized$OnlyReadIndirect.(codetoanalyze.java.nullsafe_default.FieldNotInitialized), 0, ERADICATE_FIELD_NOT_INITIALIZED, no_bucket, WARNING, [Field `FieldNotInitialized$OnlyReadIndirect.o1` is not initialized in the constructor and is not declared `@Nullable`] -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/FieldNotInitialized.java, codetoanalyze.java.nullsafe_default.FieldNotInitialized$ShouldInitializeInAllBranches.(codetoanalyze.java.nullsafe_default.FieldNotInitialized,int), 0, ERADICATE_FIELD_NOT_INITIALIZED, no_bucket, WARNING, [Field `FieldNotInitialized$ShouldInitializeInAllBranches.f3` is not initialized in the constructor and is not declared `@Nullable`] +codetoanalyze/java/nullsafe-default/FieldNotInitialized.java, codetoanalyze.java.nullsafe_default.FieldNotInitialized$ShouldInitializeInAllBranches.(codetoanalyze.java.nullsafe_default.FieldNotInitialized,int), 0, ERADICATE_FIELD_NOT_INITIALIZED, no_bucket, WARNING, [Field `FieldNotInitialized$ShouldInitializeInAllBranches.f5` is not initialized in the constructor and is not declared `@Nullable`] +codetoanalyze/java/nullsafe-default/FieldNotInitialized.java, codetoanalyze.java.nullsafe_default.FieldNotInitialized$ShouldInitializeInAllBranches.(codetoanalyze.java.nullsafe_default.FieldNotInitialized,int), 0, ERADICATE_FIELD_NOT_INITIALIZED, no_bucket, WARNING, [Field `FieldNotInitialized$ShouldInitializeInAllBranches.f2` is not initialized in the constructor and is not declared `@Nullable`] +codetoanalyze/java/nullsafe-default/FieldNotInitialized.java, codetoanalyze.java.nullsafe_default.FieldNotInitialized$WriteItselfIsBAD.(codetoanalyze.java.nullsafe_default.FieldNotInitialized), 0, ERADICATE_FIELD_NOT_INITIALIZED, no_bucket, WARNING, [Field `FieldNotInitialized$WriteItselfIsBAD.bad` 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.notNullIsBAD` is not initialized in the constructor and is not declared `@Nullable`] +codetoanalyze/java/nullsafe-default/FieldNotInitialized.java, codetoanalyze.java.nullsafe_default.FieldNotInitialized.testNullifyFields():void, 3, ERADICATE_FIELD_NOT_NULLABLE, no_bucket, WARNING, [Field `FieldNotInitialized.nonnullIsOK` can be null but is not declared `@Nullable`. (Origin: null constant at line 47)] +codetoanalyze/java/nullsafe-default/FieldNotInitialized.java, codetoanalyze.java.nullsafe_default.FieldNotInitialized.testNullifyFields():void, 4, ERADICATE_FIELD_NOT_NULLABLE, no_bucket, WARNING, [Field `FieldNotInitialized.injectIsOK` can be null but is not declared `@Nullable`. (Origin: null constant at line 48)] codetoanalyze/java/nullsafe-default/FieldNotInitialized.java, codetoanalyze.java.nullsafe_default.TestInitializerAnnotation.(), 0, ERADICATE_FIELD_NOT_INITIALIZED, no_bucket, WARNING, [Field `TestInitializerAnnotation.dontInitAtAllIsBAD` is not initialized in the constructor and is not declared `@Nullable`] codetoanalyze/java/nullsafe-default/FieldNotInitialized.java, codetoanalyze.java.nullsafe_default.TestInitializerAnnotation.(), 0, ERADICATE_FIELD_NOT_INITIALIZED, no_bucket, WARNING, [Field `TestInitializerAnnotation.initInAnyOtherMethodIsBAD` is not initialized in the constructor and is not declared `@Nullable`] codetoanalyze/java/nullsafe-default/FieldNotInitialized.java, codetoanalyze.java.nullsafe_default.TestInitializerAnnotation.build():java.lang.Object, 5, ERADICATE_CONDITION_REDUNDANT, no_bucket, WARNING, [The condition TestInitializerAnnotation.initInInitilizerMethod2IsOK is always true according to the existing annotations.]