Summary: See motivation below. This diff is dealing with FieldNotNullable: - move not relevant subclasses into dedicated classes and files - modify the tests so they comply with the standards below --Motivation-- Gradual mode we are going to introduce is an invasive change in how Infer treats nullability semantics. In order to make the change in a controllable way, we need the tests to comply with the following standards and conventions. 1. For each code peace where we expect a bug to happen, the there should be corresponding (minimally different from above) peace of code where we expect a bug to NOT happen. (This is to ensure bug is happening for exact reason we think it is happening). 2. Conversely: for each peace of code where we expect a bug to be NOT present, there shuold be a peace of code where the bug IS happening. (Otherwise there can be too many reasons for a bug NOT to happen). 3. Convention: end corresponding methods IsOK and IsBUG correspondingly. 4. Keep code examples as small as possible. Reviewed By: ngorogiannis Differential Revision: D17183222 fbshipit-source-id: 83d03e67fmaster
Mitya Lyubarskiy
5 years ago
committed by
Facebook Github Bot
parent
3250ff35d2
commit
8add080e4a
@ -0,0 +1,84 @@
|
||||
/*
|
||||
* 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 javax.annotation.Nullable;
|
||||
|
||||
@interface InjectView {}
|
||||
|
||||
/** Support assignments of null to @InjectView fields, generated by butterknife. */
|
||||
public class ButterKnife {
|
||||
@InjectView String injected;
|
||||
String normal = ""; // assign to suppress not initialized warning
|
||||
@Nullable String nullable = ""; // assign to suppress not initialized warning
|
||||
|
||||
void f(String nonNullable) {}
|
||||
|
||||
// When dereferencing, injected should behave as not nullable
|
||||
|
||||
void dereferencingInjectedIsOK() {
|
||||
int n = injected.length();
|
||||
}
|
||||
|
||||
void dereferencingNormalIsOK() {
|
||||
int n = normal.length();
|
||||
}
|
||||
|
||||
void dereferencingNullableIsBAD() {
|
||||
int n = nullable.length();
|
||||
}
|
||||
|
||||
// When returning a value, injected should be treated as non nullable
|
||||
|
||||
String convertingToNotNullableForInjectedIsOK() {
|
||||
return injected;
|
||||
}
|
||||
|
||||
String convertingToNotNullableForNormalIsOK() {
|
||||
return normal;
|
||||
}
|
||||
|
||||
String convertingToNotNullableForNullableIsBAD() {
|
||||
return nullable;
|
||||
}
|
||||
|
||||
// When passing to a non nullable param, injected should be treated as non nullable
|
||||
|
||||
void passingToNullableForInjectedIsOK() {
|
||||
f(injected);
|
||||
}
|
||||
|
||||
void passingToNullableForNormalIsOK() {
|
||||
f(normal);
|
||||
}
|
||||
|
||||
void passingToNullableForNullableIsBAD() {
|
||||
f(nullable);
|
||||
}
|
||||
|
||||
// Assigning null to Injected should be allowed (as if it was nullable)
|
||||
// (Those assignments are generated by Butterknife framework.)
|
||||
|
||||
void assignNullToInjectedIsOK() {
|
||||
injected = null;
|
||||
}
|
||||
|
||||
void assignNullToNullableIsOK() {
|
||||
nullable = null;
|
||||
}
|
||||
|
||||
void assignNullToNormalIsBAD() {
|
||||
normal = null;
|
||||
}
|
||||
|
||||
class TestNotInitialized {
|
||||
@InjectView String notInitializedInjectedIsOK;
|
||||
@Nullable String notInitializedNullableIsOK;
|
||||
String notInitializedNormalIsBAD;
|
||||
}
|
||||
}
|
||||
@ -0,0 +1,29 @@
|
||||
/*
|
||||
* 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 javax.annotation.Nullable;
|
||||
|
||||
/** Nullability checks for captured params */
|
||||
public class CapturedParam {
|
||||
|
||||
void dereferencingNullableIsBAD(@Nullable Object parameter) {
|
||||
parameter.toString();
|
||||
}
|
||||
|
||||
void dereferencingCapturedNullableShouldBeBAD_FIXME(@Nullable Object parameter) {
|
||||
Object object =
|
||||
new Object() {
|
||||
void foo() {
|
||||
// Should be disallowed, but it is not the case
|
||||
// TODO(T53473076) fix the FN.
|
||||
parameter.toString();
|
||||
}
|
||||
};
|
||||
}
|
||||
}
|
||||
@ -0,0 +1,140 @@
|
||||
/*
|
||||
* 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 javax.annotation.Nullable;
|
||||
|
||||
/** Check how we model the behavior of Map nullability */
|
||||
public class MapNullability {
|
||||
|
||||
class TestThatGetIsAllowedOnlyAfterContainsKeyWasChecked {
|
||||
|
||||
void usingGetAfterKeyWasCheckedIsOK(java.util.Map<Integer, String> m) {
|
||||
if (m.containsKey(3)) {
|
||||
m.get(3).isEmpty();
|
||||
}
|
||||
}
|
||||
|
||||
void usingGetWithoutCheckingKeyIsBAD(java.util.Map<Integer, String> m) {
|
||||
m.get(3).isEmpty();
|
||||
}
|
||||
|
||||
void usingGetAfterWrongKeyWasCheckedIsBAD(java.util.Map<Integer, String> m) {
|
||||
if (m.containsKey(3)) {
|
||||
m.get(4).isEmpty();
|
||||
}
|
||||
}
|
||||
|
||||
void usingGetAfterKeyWasCheckedInWhileLoopIsOK(java.util.Map<Integer, String> m) {
|
||||
while (true) {
|
||||
if (m.containsKey(3)) {
|
||||
m.get(3).isEmpty();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
void usingGetAfterWrongKeyWasCheckedInWhileLoopIsBAD(java.util.Map<Integer, String> m) {
|
||||
while (true) {
|
||||
if (m.containsKey(3)) {
|
||||
m.get(4).isEmpty();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
void immutableMap_usingGetAfterKeyWasCheckedIsOK(
|
||||
com.google.common.collect.ImmutableMap<Integer, String> m) {
|
||||
if (m.containsKey(3)) {
|
||||
m.get(3).isEmpty();
|
||||
}
|
||||
}
|
||||
|
||||
void immutableMap_usingGetAfterWrongKeyWasCheckedIsBAD(
|
||||
com.google.common.collect.ImmutableMap<Integer, String> m) {
|
||||
if (m.containsKey(3)) {
|
||||
m.get(4).isEmpty();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
public class TestThatGetAfterPutIsAllowed {
|
||||
String dontAssignNull = "";
|
||||
|
||||
public void getAfterPutIsOK(java.util.Map<String, String> map, String key) {
|
||||
map.put(key, "abc");
|
||||
dontAssignNull = map.get(key);
|
||||
}
|
||||
|
||||
public void getWithoutPutIsBAD(java.util.Map<String, String> map, String key) {
|
||||
dontAssignNull = map.get(key);
|
||||
}
|
||||
|
||||
public void getAfterPutWrongKeyIsBAD(
|
||||
java.util.Map<String, String> map, String key, String wrongKey) {
|
||||
map.put(key, "abc");
|
||||
dontAssignNull = map.get(wrongKey);
|
||||
}
|
||||
|
||||
public void getAfterPutSeveralKeysIsOK(java.util.Map<String, String> map) {
|
||||
map.put("key1", "value1");
|
||||
map.put("key2", "value1");
|
||||
dontAssignNull = map.get("key2");
|
||||
dontAssignNull = map.get("key1");
|
||||
dontAssignNull = map.get("key2");
|
||||
map.put("key3", "value1");
|
||||
dontAssignNull = map.get("key1");
|
||||
dontAssignNull = map.get("key2");
|
||||
dontAssignNull = map.get("key3");
|
||||
}
|
||||
|
||||
public void getAfterPutSeveralKeysButGetWrongOneIsBAD(java.util.Map<String, String> map) {
|
||||
map.put("key1", "value1");
|
||||
map.put("key2", "value1");
|
||||
dontAssignNull = map.get("key2"); // OK
|
||||
dontAssignNull = map.get("wrong_key"); // BAD
|
||||
}
|
||||
|
||||
public void getAfterPutNonnullIsOK(java.util.Map<String, String> map, String nonnullValue) {
|
||||
map.put("key", nonnullValue);
|
||||
dontAssignNull = map.get("key");
|
||||
}
|
||||
|
||||
public void getAfterPutNullableIsBAD(
|
||||
java.util.Map<String, String> map, @Nullable String nullableValue) {
|
||||
map.put("key", nullableValue);
|
||||
dontAssignNull = map.get("key");
|
||||
}
|
||||
|
||||
public void overwriteKeyByNullIsBAD(java.util.Map<String, String> map, String key) {
|
||||
map.put(key, "abc");
|
||||
map.put(key, null); // Parameter not nullable
|
||||
dontAssignNull = map.get(key); // BAD
|
||||
}
|
||||
|
||||
public void overwriteKeyByNonnullIsOK(java.util.Map<String, String> map, String key) {
|
||||
map.put(key, null); // Parameter not nullable
|
||||
map.put(key, "abc");
|
||||
dontAssignNull = map.get(key); // OK
|
||||
}
|
||||
|
||||
public void getAfterConditionalPutIsOK(java.util.Map<String, String> map, String key) {
|
||||
if (!map.containsKey(key)) {
|
||||
map.put(key, "abc");
|
||||
}
|
||||
// OK: map either already contained a key, or we've just put it here!
|
||||
dontAssignNull = map.get(key);
|
||||
}
|
||||
|
||||
public void getAfterConditionalPutWrongKeyIsBAD(
|
||||
java.util.Map<String, String> map, String key, String wrongKey) {
|
||||
if (!map.containsKey(key)) {
|
||||
map.put(wrongKey, "abc");
|
||||
}
|
||||
dontAssignNull = map.get(key);
|
||||
}
|
||||
}
|
||||
}
|
||||
@ -0,0 +1,209 @@
|
||||
/*
|
||||
* 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 javax.annotation.Nullable;
|
||||
|
||||
public class NestedFieldAccess {
|
||||
|
||||
class C {
|
||||
@Nullable String s;
|
||||
}
|
||||
|
||||
class CC {
|
||||
@Nullable C c;
|
||||
}
|
||||
|
||||
class CCC {
|
||||
@Nullable CC cc;
|
||||
}
|
||||
|
||||
/**
|
||||
* Tests nullability check patterns for f1.f2.f3, when all components in the chain are nullable.
|
||||
* (it should require checking of all components in the chain)
|
||||
*/
|
||||
public class TestNullableChains {
|
||||
@Nullable String s;
|
||||
C myc;
|
||||
|
||||
TestNullableChains() {
|
||||
myc = new C();
|
||||
}
|
||||
|
||||
void field_AccessAfterNullCheckIsOK() {
|
||||
if (s != null) {
|
||||
int n = s.length();
|
||||
}
|
||||
}
|
||||
|
||||
void field_AccessWithoutNullCheckIsBad() {
|
||||
int n = s.length();
|
||||
}
|
||||
|
||||
void nestedField_AccessAfterNullCheckIsOK() {
|
||||
if (myc.s != null) {
|
||||
int n = myc.s.length();
|
||||
}
|
||||
}
|
||||
|
||||
void nestedField_AccessWithoutNullCheckIsBad() {
|
||||
int n = myc.s.length();
|
||||
}
|
||||
|
||||
void param_AccessAfterNullCheckIsOK(C c) {
|
||||
if (c.s != null) {
|
||||
int n = c.s.length();
|
||||
}
|
||||
}
|
||||
|
||||
void param_AccessWithoutNullCheckIsBad(C c) {
|
||||
int n = c.s.length();
|
||||
}
|
||||
|
||||
void local_AccessAfterNullCheckIsOK() {
|
||||
C c = new C();
|
||||
if (c.s != null) {
|
||||
int n = c.s.length();
|
||||
}
|
||||
}
|
||||
|
||||
void local_AccessWithoutNullCheckIsBad() {
|
||||
C c = new C();
|
||||
int n = c.s.length();
|
||||
}
|
||||
|
||||
void deep_AccessWithNullCheckIsOK(CC cc) {
|
||||
if (cc.c != null && cc.c.s != null) {
|
||||
int n = cc.c.s.length();
|
||||
}
|
||||
}
|
||||
|
||||
void deep_AccessWithoutNullCheckIsBad(CC cc) {
|
||||
if (cc.c != null /* && cc.c.s != null */) {
|
||||
int n = cc.c.s.length();
|
||||
}
|
||||
}
|
||||
|
||||
void veryDeep_AccessWithNullCheckIsOK(CCC ccc) {
|
||||
if (ccc.cc != null && ccc.cc.c != null && ccc.cc.c.s != null) {
|
||||
int n = ccc.cc.c.s.length();
|
||||
}
|
||||
}
|
||||
|
||||
void veryDeep_AccessWithoutNullCheckIsBad(CCC ccc) {
|
||||
if (ccc.cc != null && ccc.cc.c != null /* && ccc.cc.c.s != null */) {
|
||||
int n = ccc.cc.c.s.length();
|
||||
}
|
||||
}
|
||||
|
||||
void veryDeep_AccessViaOrEarlyReturnIsOK(@Nullable CCC ccc) {
|
||||
if (ccc == null || ccc.cc == null || ccc.cc.c == null || ccc.cc.c.s == null) {
|
||||
} else {
|
||||
int n = ccc.cc.c.s.length();
|
||||
}
|
||||
}
|
||||
|
||||
void veryDeep_IncompleteAccessViaOrEarlyReturnIsBad(@Nullable CCC ccc) {
|
||||
if (ccc == null || ccc.cc == null || ccc.cc.c == null /*|| ccc.cc.c.s == null*/) {
|
||||
} else {
|
||||
int n = ccc.cc.c.s.length();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Tests nullability patterns for chains a().a().a().a().nullable(). Basically nullsafe needs to
|
||||
* realize that objects returned by a().a() and a().a().a() are different so it should not learn
|
||||
* anything about the nullability of one based on evidence about the other one.
|
||||
*/
|
||||
class TestFunctionsIdempotent {
|
||||
@Nullable String s;
|
||||
String dontAssignNull = "";
|
||||
|
||||
@Nullable
|
||||
String nullable(int n) {
|
||||
return s;
|
||||
}
|
||||
|
||||
TestFunctionsIdempotent getSelf() {
|
||||
return this;
|
||||
}
|
||||
|
||||
void chainOf0VsChainOf0IsOK() {
|
||||
if (nullable(3) != null) {
|
||||
dontAssignNull = nullable(3);
|
||||
}
|
||||
}
|
||||
|
||||
void chainOf0VsChainOf0ParamsMismatchIsBad() {
|
||||
if (nullable(3) != null) {
|
||||
dontAssignNull = nullable(4);
|
||||
}
|
||||
}
|
||||
|
||||
void otherObjVsItselfIsOK(TestFunctionsIdempotent otherObj) {
|
||||
if (otherObj.nullable(3) != null) {
|
||||
dontAssignNull = otherObj.nullable(3);
|
||||
}
|
||||
}
|
||||
|
||||
void otherObjVsItselfIsOKParamsMismatchIsBAD(TestFunctionsIdempotent otherObj) {
|
||||
if (otherObj.nullable(3) != null) {
|
||||
dontAssignNull = otherObj.nullable(4);
|
||||
}
|
||||
}
|
||||
|
||||
void selfVsOtherObjectIsBAD(TestFunctionsIdempotent otherObj) {
|
||||
if (otherObj.nullable(3) != null) {
|
||||
dontAssignNull = nullable(3);
|
||||
}
|
||||
}
|
||||
|
||||
void chainOf0VsChainOf1IsBad() {
|
||||
if (nullable(3) != null) {
|
||||
dontAssignNull = getSelf().nullable(3);
|
||||
}
|
||||
}
|
||||
|
||||
void chainOf1VsChainOf0IsBad() {
|
||||
if (getSelf().nullable(3) != null) {
|
||||
dontAssignNull = nullable(3);
|
||||
}
|
||||
}
|
||||
|
||||
void chainOf1VsChainOf1IsOK() {
|
||||
if (getSelf().nullable(3) != null) {
|
||||
dontAssignNull = getSelf().nullable(3);
|
||||
}
|
||||
}
|
||||
|
||||
void chainOf1VsChainOf1ParamMismatchIsBad() {
|
||||
if (getSelf().nullable(3) != null) {
|
||||
dontAssignNull = getSelf().nullable(4);
|
||||
}
|
||||
}
|
||||
|
||||
void chainOf2VsChainOf2IsOK() {
|
||||
if (getSelf().getSelf().nullable(3) != null) {
|
||||
dontAssignNull = getSelf().getSelf().nullable(3);
|
||||
}
|
||||
}
|
||||
|
||||
void chainOf1VsChainOf2IsBad() {
|
||||
if (getSelf().nullable(3) != null) {
|
||||
dontAssignNull = getSelf().getSelf().nullable(3);
|
||||
}
|
||||
}
|
||||
|
||||
void chainOf2VsChainOf1IsBad() {
|
||||
if (getSelf().getSelf().nullable(3) != null) {
|
||||
dontAssignNull = getSelf().nullable(3);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
Loading…
Reference in new issue