[nullsafe] Introduce @Nullsafe annotation

Summary:
- Add `Nullsafe` annotation as a general mechanism to specify
type-checking behaviour for nullsafe.
- Document annotation params and provide usage examples with
explanations.
- Add tests to demonstrate the behaviour with different type-checking
modes.

No implementation is added. This diff serves as an RFC to hash out the
details before I dive into code.

Reviewed By: ngorogiannis

Differential Revision: D19578329

fbshipit-source-id: b1a9f6162
master
Artem Pianykh 5 years ago committed by Facebook Github Bot
parent 4677584018
commit 237aac4cd0

@ -0,0 +1,125 @@
/*
* 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 com.facebook.infer.annotation;
import java.lang.annotation.ElementType;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;
@Retention(RetentionPolicy.CLASS)
@Target({ElementType.TYPE})
/**
* Configures nullability checking mode of annotated classes; a more general version of {@link
* NullsafeStrict}.
*
* <p>To specify the null-checking behaviour we need to define the following.
*
* <p>Code can be either *third-party* or *internal*. Whether the code is third-party is defined
* based on files in third-party signatures repo. This can be set up per-project (see nullsafe
* documentation for more details on this).
*
* <p>Code can be either *checked* or *unchecked*.
*
* <ul>
* <ol>
* An internal class is checked if it's annotated as @Nullsafe or @NullsafeStrict.
* </ol>
* <ol>
* An external method is checked when it has nullability signature defined in third-party
* signatures repo.
* </ol>
* <ol>
* The code that has built-in models in nullsafe is considered checked (both internal and
* third-party).
* </ol>
* </ul>
*
* <p>For internal code our default assumption is that lack of {@code @Nullable} annotation is
* equivalent to presence of {@code @NonNull} annotation.
*
* <p>When something is said to be "treated pessimistically" it means that return value is
* considered to be nullable and parameters to be non-nullable unless they have explicit {@code
* Nullable} annotations.
*
* <p>Now let's consider *usage examples* and describe nullability checking behaviour in each case:
*
* <pre>
* import com.facebook.infer.annotation.Nullsafe;
*
* @Nullsafe(Nullsafe.Mode.LOCAL)
* public class LocallyCheckedAgainstDependencies { ... }
* // 1. This mode:
* // a. Requires that all third-party calls are to checked code. Calls to unchecked third-party
* // code are treated pessimistically.
* // b. Calls to internal checked code treated as usual based on existing nullability annotations.
* // c. Calls to internal unchecked code are allowed and default assumptions on the meaning of
* // presence/absence of nullability annotations are in effect.
* // In other words, we trust all checked and unchecked internal code and
* // treat it as if all annotations were correct.
*
*
* @Nullsafe(value = Nullsafe.Mode.LOCAL,
* trustOnly = @Nullsafe.TrustList({UncheckedComponent.class}))
* public class LocallyCheckedWithFewTrustedDependencies { ... }
* // 2. Same as above but calls to unchecked code are allowed only for classes specified
* // in {@code trustOnly} list. Calls to other unchecked code are treated pessimistically
* // by the checker.
*
*
* @Nullsafe(value = Nullsafe.Mode.LOCAL, trustOnly = @Nullsafe.TrustList({}))
* public class LocallyCheckedRequiringOnlyCheckedDirectDependencies { ... }
* // 3. Same as above but with empty trust list, which means that all calls to unchecked code
* // are treated pessimistically.
* // In practice, it usually means that all direct dependencies of the class
* // should be @Nullsafe themselves.
*
*
* @Nullsafe(Nullsafe.Mode.STRICT)
* public class StrictClass { ... }
* // 4. This is analogous to marking a class as @NullsafeStrict. In particular:
* // a. Third-party calls should be to checked code. Calls to unchecked third-party
* // code are treated pessimistically.
* // b. Only calls to internal classes checked under strict mode are trusted.
* // c. All other calls to checked and unchecked code are treated pessimistically.
* // In practice it means that transitive dependencies of the class should
* // themselves be @NullsafeStrict.
* </pre>
*/
public @interface Nullsafe {
enum Mode {
LOCAL,
STRICT
}
@interface TrustList {
Class[] value();
/**
* Analogous to a wildcard "*" in a trust list when set to true. When set to false only classes
* listed in {@code value} parameter are trusted.
*/
boolean trustAll() default false;
}
/**
* Specifies the null-checking mode. The parameter is named {@code value} instead of {@code mode}
* to enable a single-element annotation shorthand for @Nullsafe annotation, i.e.
* use @Nullsafe(Nullsafe.Mode.Strict) instead of @Nullsafe(mode = Nullsafe.Mode.STRICT).
*/
Mode value();
/**
* Provides fine-grained control over which unchecked internal classes to trust. Only affects
* LOCAL null-checking mode, as strict requires all dependencies to be STRICT themselves.
*/
TrustList trustOnly() default
@TrustList(
value = {},
trustAll = true);
}

@ -0,0 +1,95 @@
/*
* 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 com.facebook.infer.annotation.Nullsafe;
import com.facebook.infer.annotation.NullsafeStrict;
import javax.annotation.Nullable;
public class NullsafeMode {
abstract class VariousMethods {
public String returnVal() {
return "OK";
}
@Nullable
public String returnNull() {
return null;
}
}
class NonNullsafe extends VariousMethods {}
class AnotherNonNullsafe extends VariousMethods {}
@Nullsafe(Nullsafe.Mode.LOCAL)
class TrustAllNullsafe extends VariousMethods {
String OK_returnFromAnyNonNullsafe() {
String a = new NonNullsafe().returnVal();
String b = new AnotherNonNullsafe().returnVal();
return a.concat(b);
}
String BAD_returnNullFromNonNulsafe() {
return (new NonNullsafe()).returnNull();
}
}
@Nullsafe(value = Nullsafe.Mode.LOCAL, trustOnly = @Nullsafe.TrustList({NonNullsafe.class}))
class TrustSomeNullsafe extends VariousMethods {
String OK_returnFromNonNullsafe() {
return new NonNullsafe().returnVal();
}
String BAD_returnFromAnotherNonNullsafe() {
return new AnotherNonNullsafe().returnVal();
}
@Nullable
String OK_returnFromAnotherNonNullsafeAsNullable() {
return new AnotherNonNullsafe().returnVal();
}
String BAD_returnNullFromNonNulsafe() {
return new NonNullsafe().returnNull();
}
}
@Nullsafe(value = Nullsafe.Mode.LOCAL, trustOnly = @Nullsafe.TrustList({}))
class TrustNoneNullsafe extends VariousMethods {
String BAD_returnFromNonNullsafe() {
return new NonNullsafe().returnVal();
}
String OK_returnFromNullsafe() {
return new TrustSomeNullsafe().returnVal();
}
}
@Nullsafe(Nullsafe.Mode.STRICT)
class NullsafeWithStrictMode extends VariousMethods {
String BAD_returnFromNonStrict() {
return new TrustNoneNullsafe().returnVal();
}
String OK_returnFromNullsafeStrict() {
return new StrictNullsafe().returnVal();
}
}
@NullsafeStrict
class StrictNullsafe extends VariousMethods {
String BAD_returnFromNonNullsafe() {
return new NonNullsafe().returnVal();
}
String OK_returnFromNullsafeWithStrictMode() {
return new NullsafeWithStrictMode().returnVal();
}
}
}

@ -173,6 +173,11 @@ codetoanalyze/java/nullsafe-default/NullMethodCall.java, codetoanalyze.java.null
codetoanalyze/java/nullsafe-default/NullMethodCall.java, codetoanalyze.java.nullsafe_default.NullMethodCall.testSystemGetenvBad():int, 1, ERADICATE_NULLABLE_DEREFERENCE, no_bucket, WARNING, [`envValue` is nullable and is not locally checked for null when calling `length()`: call to System.getenv(...) at line 240 (nullable according to nullsafe internal models)]
codetoanalyze/java/nullsafe-default/NullMethodCall.java, codetoanalyze.java.nullsafe_default.NullMethodCall.withConditionalAssignemnt(codetoanalyze.java.nullsafe_default.NullMethodCall$AnotherI,boolean,java.lang.Object,java.lang.Object):void, 0, ERADICATE_NULLABLE_DEREFERENCE, no_bucket, WARNING, [`i` is nullable and is not locally checked for null when calling `withObjectParameter(...)`.]
codetoanalyze/java/nullsafe-default/NullMethodCall.java, codetoanalyze.java.nullsafe_default.NullMethodCall.withConjuction(codetoanalyze.java.nullsafe_default.NullMethodCall$AnotherI,boolean,boolean):void, 0, ERADICATE_NULLABLE_DEREFERENCE, no_bucket, WARNING, [`i` is nullable and is not locally checked for null when calling `withBooleanParameter(...)`.]
codetoanalyze/java/nullsafe-default/NullsafeMode.java, codetoanalyze.java.nullsafe_default.NullsafeMode$StrictNullsafe.BAD_returnFromNonNullsafe():java.lang.String, 0, ERADICATE_UNCHECKED_NONSTRICT_FROM_STRICT, no_bucket, ERROR, [`NullsafeMode$VariousMethods.returnVal()`: `@NullsafeStrict` mode prohibits using values coming from non-strict classes without a check. Result of this call is used at line 88. Either add a local check for null or assertion, or strictify NullsafeMode$VariousMethods.]
codetoanalyze/java/nullsafe-default/NullsafeMode.java, codetoanalyze.java.nullsafe_default.NullsafeMode$StrictNullsafe.OK_returnFromNullsafeWithStrictMode():java.lang.String, 0, ERADICATE_UNCHECKED_NONSTRICT_FROM_STRICT, no_bucket, ERROR, [`NullsafeMode$VariousMethods.returnVal()`: `@NullsafeStrict` mode prohibits using values coming from non-strict classes without a check. Result of this call is used at line 92. Either add a local check for null or assertion, or strictify NullsafeMode$VariousMethods.]
codetoanalyze/java/nullsafe-default/NullsafeMode.java, codetoanalyze.java.nullsafe_default.NullsafeMode$TrustAllNullsafe.BAD_returnNullFromNonNulsafe():java.lang.String, 0, ERADICATE_RETURN_NOT_NULLABLE, no_bucket, WARNING, [`BAD_returnNullFromNonNulsafe()`: return type is declared non-nullable but the method returns a nullable value: call to returnNull() at line 39.]
codetoanalyze/java/nullsafe-default/NullsafeMode.java, codetoanalyze.java.nullsafe_default.NullsafeMode$TrustSomeNullsafe.BAD_returnNullFromNonNulsafe():java.lang.String, 0, ERADICATE_RETURN_NOT_NULLABLE, no_bucket, WARNING, [`BAD_returnNullFromNonNulsafe()`: return type is declared non-nullable but the method returns a nullable value: call to returnNull() at line 59.]
codetoanalyze/java/nullsafe-default/NullsafeMode.java, codetoanalyze.java.nullsafe_default.NullsafeMode$TrustSomeNullsafe.OK_returnFromAnotherNonNullsafeAsNullable():java.lang.String, 0, ERADICATE_RETURN_OVER_ANNOTATED, no_bucket, WARNING, [Method `OK_returnFromAnotherNonNullsafeAsNullable()` is annotated with `@Nullable` but never returns null.]
codetoanalyze/java/nullsafe-default/ParameterNotNullable.java, codetoanalyze.java.nullsafe_default.ParameterNotNullable$ConstructorCall.<init>(codetoanalyze.java.nullsafe_default.ParameterNotNullable,int), 0, ERADICATE_PARAMETER_NOT_NULLABLE, no_bucket, WARNING, [`ParameterNotNullable$ConstructorCall(...)`: parameter #2(`s`) is declared non-nullable but the argument is `null`.]
codetoanalyze/java/nullsafe-default/ParameterNotNullable.java, codetoanalyze.java.nullsafe_default.ParameterNotNullable.callNull():void, 1, ERADICATE_PARAMETER_NOT_NULLABLE, no_bucket, WARNING, [`ParameterNotNullable.test(...)`: parameter #1(`s`) is declared non-nullable but the argument `s` is `null`: null constant at line 39.]
codetoanalyze/java/nullsafe-default/ParameterNotNullable.java, codetoanalyze.java.nullsafe_default.ParameterNotNullable.callNullable(java.lang.String):void, 0, ERADICATE_PARAMETER_NOT_NULLABLE, no_bucket, WARNING, [`ParameterNotNullable.test(...)`: parameter #1(`s`) is declared non-nullable but the argument `s` is nullable.]

Loading…
Cancel
Save