|
|
|
/*
|
|
|
|
* 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.
|
|
|
|
*/
|
|
|
|
|
[nullsafe] Prepare to introduce gradual mode: split tests
Summary:
In next diff, we are going to introduce a new mode of nullsafe
(gradual). For testing, we are going to employ the strategy used by jvillard
for Pulse.
In this diff we split tests into two subfolders, one for the default and one for the gradual
mode.
We are planning to make the gradual mode default eventually. For that, most
new features will make sense for gradual mode, and we will mostly evolve
tests for that mode.
As for 'default' mode, we need to preserve tests mostly to ensure we don't introduce
regressions.
Occasionally, we might make changes that make sense for both modes, in
this (expected relatively rare) cases we will make changes to both set
of tests.
An alternative strategy would be to have two sets of issues.exp files,
one for gradual and one for default mode. This has an advantage of each
java file to be always tested twice, but disadvantage is that it will be
harder to write meaningful test code so that it makes sense for both
modes simultaneously.
Reviewed By: ngorogiannis
Differential Revision: D17156724
fbshipit-source-id: a92a9208f
5 years ago
|
|
|
package codetoanalyze.java.nullsafe_default;
|
|
|
|
|
|
|
|
import android.text.TextUtils;
|
|
|
|
import com.facebook.infer.annotation.FalseOnNull;
|
|
|
|
import com.facebook.infer.annotation.PropagatesNullable;
|
|
|
|
import com.facebook.infer.annotation.TrueOnNull;
|
|
|
|
import com.google.common.base.Strings;
|
|
|
|
import javax.annotation.Nullable;
|
|
|
|
|
|
|
|
public class CustomAnnotations {
|
|
|
|
|
|
|
|
// Example of API that benefits from annotating with @TrueOnNull and @FalseOnNull
|
|
|
|
static class AnnotatedTextUtils {
|
|
|
|
|
|
|
|
@TrueOnNull
|
|
|
|
static boolean isEmpty(@Nullable CharSequence s) {
|
|
|
|
return s == null || s.equals("");
|
|
|
|
}
|
|
|
|
|
|
|
|
@FalseOnNull
|
|
|
|
static boolean isNotEmpty(@Nullable CharSequence s) {
|
|
|
|
return s != null && s.length() > 0;
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
// The same API, but not annotated
|
|
|
|
static class NotAnnotatedTextUtils {
|
|
|
|
static boolean isEmpty(@Nullable CharSequence s) {
|
|
|
|
return s == null || s.equals("");
|
|
|
|
}
|
|
|
|
|
|
|
|
static boolean isNotEmpty(@Nullable CharSequence s) {
|
|
|
|
return s != null && s.length() > 0;
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
class Test {
|
|
|
|
void testTrueOnNull(@Nullable CharSequence s) {
|
|
|
|
// Explicitly annotated
|
|
|
|
if (!AnnotatedTextUtils.isEmpty(s)) {
|
|
|
|
s.toString(); // OK: if we are here, we know that s is not null
|
|
|
|
}
|
|
|
|
|
|
|
|
// Not annotated
|
|
|
|
if (!NotAnnotatedTextUtils.isEmpty(s)) {
|
|
|
|
s.toString(); // BAD: the typecker does not know s can not be null
|
|
|
|
}
|
|
|
|
|
|
|
|
if (AnnotatedTextUtils.isEmpty(s)) {
|
|
|
|
s.toString(); // BAD: s can be null or an empty string
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
void testFalseOnNull(@Nullable CharSequence s) {
|
|
|
|
// Explicitly annotated
|
|
|
|
if (AnnotatedTextUtils.isNotEmpty(s)) {
|
|
|
|
s.toString(); // OK: if we are here, we know that `s` is not null
|
|
|
|
}
|
|
|
|
|
|
|
|
// Not annotated
|
|
|
|
if (NotAnnotatedTextUtils.isNotEmpty(s)) {
|
|
|
|
s.toString(); // BAD: the typecker does not know `s` can not be null
|
|
|
|
}
|
|
|
|
|
|
|
|
if (!AnnotatedTextUtils.isNotEmpty(s)) {
|
|
|
|
s.toString(); // BAD: `s` can be null or an empty string
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
void testModelledTrueOnNull(String s) {
|
|
|
|
// TextUtils.isEmpty is modelled as TrueOnNull
|
|
|
|
if (!TextUtils.isEmpty(s)) {
|
|
|
|
s.toString(); // OK: if we are here, we know that `s` is not null
|
|
|
|
}
|
|
|
|
|
|
|
|
// Strings.isNullOrEmpty is modelled as TrueOnNull
|
|
|
|
if (!Strings.isNullOrEmpty(s)) {
|
|
|
|
s.toString(); // OK: if we are here, we know that `s` is not null
|
|
|
|
}
|
|
|
|
|
|
|
|
if (!NotAnnotatedTextUtils.isEmpty(s)) {
|
|
|
|
s.toString(); // BAD: the typechecker can not figure this out for not modelled class
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
void testEarlyReturn(@Nullable CharSequence s1, @Nullable CharSequence s2) {
|
|
|
|
if (AnnotatedTextUtils.isEmpty(s1) || NotAnnotatedTextUtils.isEmpty(s2)) {
|
|
|
|
return;
|
|
|
|
}
|
|
|
|
|
|
|
|
s1.toString(); // OK: if `s1` was null, we would no rech this point
|
|
|
|
s2.toString(); // BAD: typechecker can not figure this for not annotated class
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
// Tests for the annotation @PropagatesNullable
|
|
|
|
class TestPropagatesNullable {
|
|
|
|
|
|
|
|
class TestOneParameter {
|
|
|
|
|
|
|
|
// means return null iff s is null
|
|
|
|
String propagatesNullable(@PropagatesNullable String s) {
|
|
|
|
return s;
|
|
|
|
}
|
|
|
|
|
|
|
|
@Nullable
|
|
|
|
String nullable(@Nullable String s) {
|
|
|
|
return s;
|
|
|
|
}
|
|
|
|
|
|
|
|
void test(@Nullable String sNullable, String sNonnull) {
|
|
|
|
// null constant
|
|
|
|
propagatesNullable(null).length(); // BAD: will be NPE
|
|
|
|
nullable(null).length(); // BAD: result might be null
|
|
|
|
|
|
|
|
// nullable
|
|
|
|
propagatesNullable(sNullable).length(); // BAD: result can be null
|
|
|
|
nullable(sNullable).length(); // BAD: result can be null
|
|
|
|
|
|
|
|
// string literal
|
|
|
|
propagatesNullable("").length(); // OK: result can not be null
|
|
|
|
nullable("").length(); // BAD: typechecker can not figure out that this is safe
|
|
|
|
|
|
|
|
// nonnull
|
|
|
|
propagatesNullable(sNonnull).length(); // OK: result can not be null
|
|
|
|
nullable(sNonnull).length(); // BAD: typechecker can not figure out that this is safe
|
|
|
|
|
|
|
|
// flow sensitive nonnull FALSE POSITIVE
|
|
|
|
if (sNullable != null) {
|
|
|
|
// here we know that sNullable is not null, so it should be safe, but it is not the case
|
|
|
|
// TODO(T53770056) fix it.
|
|
|
|
propagatesNullable(sNullable).length();
|
|
|
|
nullable(sNullable).length();
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
// limitation: we currently cannot check the body, and just trust the annotation
|
|
|
|
String cannotCheckBody(@PropagatesNullable String s) {
|
|
|
|
return null; // nothing is reported here
|
|
|
|
}
|
|
|
|
|
|
|
|
void illustrateFalseNegativeAsCannotCheckBody() {
|
|
|
|
cannotCheckBody("").length(); // this is an NPE but is not found
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
class TestSecondParameter {
|
|
|
|
// means return null iff s2 is null
|
|
|
|
String propagatesNullable(@Nullable String s1, @PropagatesNullable String s2) {
|
|
|
|
return s2;
|
|
|
|
}
|
|
|
|
|
|
|
|
@Nullable
|
|
|
|
String nullable(@Nullable String s1, @Nullable String s2) {
|
|
|
|
return s2;
|
|
|
|
}
|
|
|
|
|
|
|
|
// Let's ensure that @PropagatesNullable applies only to the parameter
|
|
|
|
// that was annotated with it, and not to other params.
|
|
|
|
void test(@Nullable String sNullable, String sNonnull) {
|
|
|
|
// Both nullable
|
|
|
|
propagatesNullable(sNullable, sNullable).length(); // BAD: result can be null
|
|
|
|
nullable(sNullable, sNullable).length(); // BAD: result can be null
|
|
|
|
|
|
|
|
// First is nonnull, second is nullable
|
|
|
|
propagatesNullable(sNonnull, sNullable).length(); // BAD: result can be null
|
|
|
|
nullable(sNonnull, sNullable).length(); // BAD: result can be null
|
|
|
|
|
|
|
|
// First is nullable, second is nonnull
|
|
|
|
propagatesNullable(sNullable, sNonnull).length(); // OK: result can not be null
|
|
|
|
nullable(sNullable, sNonnull).length(); // BAD: typechecker can not figure this out
|
|
|
|
|
|
|
|
// Both nonnullable
|
|
|
|
propagatesNullable(sNonnull, sNonnull).length(); // OK: result can not be null
|
|
|
|
nullable(sNonnull, sNonnull).length(); // BAD: typechecker can not figure this out
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
class TestBothParams {
|
|
|
|
// both parameters are annotated:
|
|
|
|
// means return null iff either s1 or s2 is null
|
|
|
|
String propagatesNullable(@PropagatesNullable String s1, @PropagatesNullable String s2) {
|
|
|
|
return s1 == null ? s1 : s2;
|
|
|
|
}
|
|
|
|
|
|
|
|
void testBothParamsShouldBeNonnull(@Nullable String sNullable, String sNonnull) {
|
|
|
|
propagatesNullable(sNullable, sNullable).length(); // BAD: result can be null
|
|
|
|
propagatesNullable(sNonnull, sNullable).length(); // BAD: result can be null
|
|
|
|
propagatesNullable(sNullable, sNonnull).length(); // BAD: result can be null
|
|
|
|
propagatesNullable(sNonnull, sNonnull).length(); // OK: result can be null
|
|
|
|
}
|
|
|
|
}
|
[nullsafe] When inferring type based on the formal type, respect NullsafeType instead of reading annotations
Summary:
This continues work for eliminating Annot.Item.t from Nullsafe low-level
code.
The introduced function `from_nullsafe_type` is called when we infer
initial type of the equation based on the function or field formal signature.
Before that, we did it via reading the annotation directly, which
complicates the logic and making introducing Unknown nullability tricky.
## Clarifying the semantics of PropagatesNullable
This diff also clarifies (and changes) the behavior of PropagatesNullable params.
Previously, if the return value of a function that has PropagatesNullable params was
annotated as Nullable, nullsafe was effectively ignoring PropagatesNullable effect.
This is especially bad because one can add Nullable annotation based on the logic "if the function can return `null`, it should be annotated with Nullable`.
In the new design, there is no possibility for such a misuse: the code that
applies the rule "any param is PropagatesNullable hence the return
value is nullable even if not explicitly annotated" lives in NullsafeType.ml, so
this will be automatically taken into account.
Meaning that now we implicitly deduce Nullable annotation for the return value, and providing it explicitly as an alternative that does not change the effect.
In the future, we might consider annotating the return value with `Nullable` explicit.
Reviewed By: jvillard
Differential Revision: D17479157
fbshipit-source-id: 66c2c8777
5 years ago
|
|
|
|
|
|
|
// For convenience, we do not require to annotate return type with @Nullable,
|
|
|
|
// make sure it is respected.
|
|
|
|
class TestReturnValueAnnotationIsAutomaticallyInferred {
|
|
|
|
|
|
|
|
// Ensure that we do not warn with "return not nullable" even if we did not annotate the
|
|
|
|
// return with @Nullable
|
|
|
|
|
|
|
|
String notAnnotatingReturnWhenThereIsPropagatesNullableIsOK(@PropagatesNullable String s) {
|
|
|
|
return null; // OK: treat is as implicitly nullable
|
|
|
|
}
|
|
|
|
|
|
|
|
String notAnnotatingReturnWhenThereAreNoPropagatesNullableIsBAD(@Nullable String s) {
|
|
|
|
return null; // BAD: return not nullable
|
|
|
|
}
|
|
|
|
|
|
|
|
// Ensure that the behavior remains the same for explicitly and implicitly annotated functions
|
|
|
|
|
|
|
|
@Nullable
|
|
|
|
String annotatedReturn(@PropagatesNullable String s) {
|
|
|
|
return s;
|
|
|
|
}
|
|
|
|
|
|
|
|
String notAnnotatedReturn(@PropagatesNullable String s) {
|
|
|
|
return s;
|
|
|
|
}
|
|
|
|
|
|
|
|
// 1. Both versions equally catch non-legit usages
|
|
|
|
|
|
|
|
void annotated_dereferencingAfterPassingNullableIsBAD(@Nullable String s) {
|
|
|
|
annotatedReturn(s).toString(); // BAD: nullable dereference
|
|
|
|
}
|
|
|
|
|
|
|
|
void notAnnotated_dereferencingAfterPassingNullableIsBAD(@Nullable String s) {
|
|
|
|
notAnnotatedReturn(s).toString(); // BAD: nullable dereference
|
|
|
|
}
|
|
|
|
|
|
|
|
// 2. Both versions equally allow legit usages
|
|
|
|
|
|
|
|
void annotated_dereferencingAfterPassingNonnullIsOK(String s) {
|
|
|
|
annotatedReturn(s).toString(); // OK: inferred to be non-nullable
|
|
|
|
}
|
|
|
|
|
|
|
|
void notAnnotated_dereferencingAfterPassingNonnullIsOK(String s) {
|
|
|
|
notAnnotatedReturn(s).toString(); // OK: inferred to be non-nullable
|
|
|
|
}
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
}
|