nullable annotations to ReadableMap, WritableMap, ReadableArray, Writable. (#23329)

Summary:
Kotlin is getting traction and more developers write RN native modules in it. This PR adds nullable annotations to help with Kotlin null inference and improve developer experience. Also it'll help checking code quality using lint.

I skimmed through JavaOnlyMap.java, JavaOnlyArray.java, ReadableNativeArray.java, ReadableNativeMap.java, WritableNativeArray.java and WritableNativeMap.java to infer nullability.

This is breaking change to Kotlin code.

[Android] [Changed] - Add nullable annotations to ReadableMap, WritableMap, ReadableArray, Writable.
Pull Request resolved: https://github.com/facebook/react-native/pull/23329

Differential Revision: D14002571

Pulled By: cpojer

fbshipit-source-id: 899d8b3b0a5dad43e8300e6c4ea4208cca0f01a9

ReactAndroid/src/main/java/com/facebook/react/bridge/JavaOnlyArray.java

@@ -11,6 +11,9 @@ import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
 
+import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
+
 /**
  * Java {@link ArrayList} backed implementation of {@link ReadableArray} and {@link WritableArray}
  * Instances of this class SHOULD NOT be used for communication between java and JS, use instances
@@ -95,7 +98,7 @@ public class JavaOnlyArray implements ReadableArray, WritableArray {
   }
 
   @Override
-  public String getString(int index) {
+  public @Nullable String getString(int index) {
     return (String) mBackingList.get(index);
   }
 
@@ -115,12 +118,12 @@ public class JavaOnlyArray implements ReadableArray, WritableArray {
   }
 
   @Override
-  public Dynamic getDynamic(int index) {
+  public @Nonnull Dynamic getDynamic(int index) {
     return DynamicFromArray.create(this, index);
   }
 
   @Override
-  public ReadableType getType(int index) {
+  public @Nonnull ReadableType getType(int index) {
     Object object = mBackingList.get(index);
 
     if (object == null) {
@@ -157,17 +160,17 @@ public class JavaOnlyArray implements ReadableArray, WritableArray {
   }
 
   @Override
-  public void pushString(String value) {
+  public void pushString(@Nonnull String value) {
     mBackingList.add(value);
   }
 
   @Override
-  public void pushArray(WritableArray array) {
+  public void pushArray(@Nullable WritableArray array) {
     mBackingList.add(array);
   }
 
   @Override
-  public void pushMap(WritableMap map) {
+  public void pushMap(@Nonnull WritableMap map) {
     mBackingList.add(map);
   }
 
@@ -177,7 +180,7 @@ public class JavaOnlyArray implements ReadableArray, WritableArray {
   }
 
   @Override
-  public ArrayList<Object> toArrayList() {
+  public @Nonnull ArrayList<Object> toArrayList() {
     return new ArrayList<Object>(mBackingList);
   }
 

ReactAndroid/src/main/java/com/facebook/react/bridge/JavaOnlyMap.java

@@ -11,6 +11,8 @@ import java.util.HashMap;
 import java.util.Iterator;
 import java.util.Map;
 
+import javax.annotation.Nonnull;
+
 /**
  * Java {@link HashMap} backed implementation of {@link ReadableMap} and {@link WritableMap}
  * Instances of this class SHOULD NOT be used for communication between java and JS, use instances
@@ -77,52 +79,52 @@ public class JavaOnlyMap implements ReadableMap, WritableMap {
   }
 
   @Override
-  public boolean hasKey(String name) {
+  public boolean hasKey(@Nonnull String name) {
     return mBackingMap.containsKey(name);
   }
 
   @Override
-  public boolean isNull(String name) {
+  public boolean isNull(@Nonnull String name) {
     return mBackingMap.get(name) == null;
   }
 
   @Override
-  public boolean getBoolean(String name) {
+  public boolean getBoolean(@Nonnull String name) {
     return (Boolean) mBackingMap.get(name);
   }
 
   @Override
-  public double getDouble(String name) {
+  public double getDouble(@Nonnull String name) {
     return ((Number) mBackingMap.get(name)).doubleValue();
   }
 
   @Override
-  public int getInt(String name) {
+  public int getInt(@Nonnull String name) {
     return ((Number) mBackingMap.get(name)).intValue();
   }
 
   @Override
-  public String getString(String name) {
+  public String getString(@Nonnull String name) {
     return (String) mBackingMap.get(name);
   }
 
   @Override
-  public ReadableMap getMap(String name) {
+  public ReadableMap getMap(@Nonnull String name) {
     return (ReadableMap) mBackingMap.get(name);
   }
 
   @Override
-  public JavaOnlyArray getArray(String name) {
+  public JavaOnlyArray getArray(@Nonnull String name) {
     return (JavaOnlyArray) mBackingMap.get(name);
   }
 
   @Override
-  public Dynamic getDynamic(String name) {
+  public @Nonnull Dynamic getDynamic(@Nonnull String name) {
     return DynamicFromMap.create(this, name);
   }
 
   @Override
-  public ReadableType getType(String name) {
+  public @Nonnull ReadableType getType(@Nonnull String name) {
     Object value = mBackingMap.get(name);
     if (value == null) {
       return ReadableType.Null;
@@ -145,7 +147,7 @@ public class JavaOnlyMap implements ReadableMap, WritableMap {
   }
 
   @Override
-  public ReadableMapKeySetIterator keySetIterator() {
+  public @Nonnull ReadableMapKeySetIterator keySetIterator() {
     return new ReadableMapKeySetIterator() {
       Iterator<String> mIterator = mBackingMap.keySet().iterator();
 
@@ -162,47 +164,47 @@ public class JavaOnlyMap implements ReadableMap, WritableMap {
   }
 
   @Override
-  public void putBoolean(String key, boolean value) {
+  public void putBoolean(@Nonnull String key, boolean value) {
     mBackingMap.put(key, value);
   }
 
   @Override
-  public void putDouble(String key, double value) {
+  public void putDouble(@Nonnull String key, double value) {
     mBackingMap.put(key, value);
   }
 
   @Override
-  public void putInt(String key, int value) {
+  public void putInt(@Nonnull String key, int value) {
     mBackingMap.put(key, value);
   }
 
   @Override
-  public void putString(String key, String value) {
+  public void putString(@Nonnull String key, @Nonnull String value) {
     mBackingMap.put(key, value);
   }
 
   @Override
-  public void putNull(String key) {
+  public void putNull(@Nonnull String key) {
     mBackingMap.put(key, null);
   }
 
   @Override
-  public void putMap(String key, WritableMap value) {
+  public void putMap(@Nonnull String key, @Nonnull WritableMap value) {
     mBackingMap.put(key, value);
   }
 
   @Override
-  public void merge(ReadableMap source) {
+  public void merge(@Nonnull ReadableMap source) {
     mBackingMap.putAll(((JavaOnlyMap) source).mBackingMap);
   }
 
   @Override
-  public void putArray(String key, WritableArray value) {
+  public void putArray(@Nonnull String key, @Nonnull WritableArray value) {
     mBackingMap.put(key, value);
   }
 
   @Override
-  public HashMap<String, Object> toHashMap() {
+  public @Nonnull HashMap<String, Object> toHashMap() {
     return new HashMap<String, Object>(mBackingMap);
   }
 

ReactAndroid/src/main/java/com/facebook/react/bridge/ReadableArray.java

@@ -9,6 +9,9 @@ package com.facebook.react.bridge;
 
 import java.util.ArrayList;
 
+import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
+
 /**
  * Interface for an array that allows typed access to its members. Used to pass parameters from JS
  * to Java.
@@ -20,11 +23,11 @@ public interface ReadableArray {
   boolean getBoolean(int index);
   double getDouble(int index);
   int getInt(int index);
-  String getString(int index);
-  ReadableArray getArray(int index);
-  ReadableMap getMap(int index);
-  Dynamic getDynamic(int index);
-  ReadableType getType(int index);
-  ArrayList<Object> toArrayList();
+  @Nullable String getString(int index);
+  @Nullable ReadableArray getArray(int index);
+  @Nullable ReadableMap getMap(int index);
+  @Nonnull Dynamic getDynamic(int index);
+  @Nonnull ReadableType getType(int index);
+  @Nonnull ArrayList<Object> toArrayList();
 
 }

ReactAndroid/src/main/java/com/facebook/react/bridge/ReadableMap.java

@@ -9,23 +9,26 @@ package com.facebook.react.bridge;
 
 import java.util.HashMap;
 
+import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
+
 /**
  * Interface for a map that allows typed access to its members. Used to pass parameters from JS to
  * Java.
  */
 public interface ReadableMap {
 
-  boolean hasKey(String name);
-  boolean isNull(String name);
-  boolean getBoolean(String name);
-  double getDouble(String name);
-  int getInt(String name);
-  String getString(String name);
-  ReadableArray getArray(String name);
-  ReadableMap getMap(String name);
-  Dynamic getDynamic(String name);
-  ReadableType getType(String name);
-  ReadableMapKeySetIterator keySetIterator();
-  HashMap<String, Object> toHashMap();
+  boolean hasKey(@Nonnull String name);
+  boolean isNull(@Nonnull String name);
+  boolean getBoolean(@Nonnull String name);
+  double getDouble(@Nonnull String name);
+  int getInt(@Nonnull String name);
+  @Nullable String getString(@Nonnull String name);
+  @Nullable ReadableArray getArray(@Nonnull String name);
+  @Nullable ReadableMap getMap(@Nonnull String name);
+  @Nonnull Dynamic getDynamic(@Nonnull String name);
+  @Nonnull ReadableType getType(@Nonnull String name);
+  @Nonnull ReadableMapKeySetIterator keySetIterator();
+  @Nonnull HashMap<String, Object> toHashMap();
 
 }

ReactAndroid/src/main/java/com/facebook/react/bridge/ReadableNativeArray.java

@@ -13,6 +13,8 @@ import com.facebook.proguard.annotations.DoNotStrip;
 import com.facebook.react.config.ReactFeatureFlags;
 import java.util.ArrayList;
 import java.util.Arrays;
+
+import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 
 /**
@@ -125,7 +127,7 @@ public class ReadableNativeArray extends NativeArray implements ReadableArray {
   private native int getIntNative(int index);
 
   @Override
-  public String getString(int index) {
+  public @Nullable String getString(int index) {
     if (ReactFeatureFlags.useArrayNativeAccessor) {
       jniPassCounter++;
       return getStringNative(index);
@@ -135,7 +137,7 @@ public class ReadableNativeArray extends NativeArray implements ReadableArray {
   private native String getStringNative(int index);
 
   @Override
-  public ReadableNativeArray getArray(int index) {
+  public @Nullable ReadableNativeArray getArray(int index) {
     if (ReactFeatureFlags.useArrayNativeAccessor) {
       jniPassCounter++;
       return getArrayNative(index);
@@ -145,7 +147,7 @@ public class ReadableNativeArray extends NativeArray implements ReadableArray {
   private native ReadableNativeArray getArrayNative(int index);
 
   @Override
-  public ReadableNativeMap getMap(int index) {
+  public @Nullable ReadableNativeMap getMap(int index) {
     if (ReactFeatureFlags.useArrayNativeAccessor) {
       jniPassCounter++;
       return getMapNative(index);
@@ -155,7 +157,7 @@ public class ReadableNativeArray extends NativeArray implements ReadableArray {
   private native ReadableNativeMap getMapNative(int index);
 
   @Override
-  public ReadableType getType(int index) {
+  public @Nonnull ReadableType getType(int index) {
     if (ReactFeatureFlags.useArrayNativeAccessor) {
       jniPassCounter++;
       return getTypeNative(index);
@@ -166,12 +168,12 @@ public class ReadableNativeArray extends NativeArray implements ReadableArray {
   private native ReadableType getTypeNative(int index);
 
   @Override
-  public Dynamic getDynamic(int index) {
+  public @Nonnull Dynamic getDynamic(int index) {
     return DynamicFromArray.create(this, index);
   }
 
   @Override
-  public ArrayList<Object> toArrayList() {
+  public @Nonnull ArrayList<Object> toArrayList() {
     ArrayList<Object> arrayList = new ArrayList<>();
 
     for (int i = 0; i < this.size(); i++) {

ReactAndroid/src/main/java/com/facebook/react/bridge/ReadableNativeMap.java

@@ -13,6 +13,8 @@ import com.facebook.proguard.annotations.DoNotStrip;
 import com.facebook.react.config.ReactFeatureFlags;
 import java.util.HashMap;
 import java.util.Iterator;
+
+import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 
 /**
@@ -66,7 +68,7 @@ public class ReadableNativeMap extends NativeMap implements ReadableMap {
   private native String[] importKeys();
   private native Object[] importValues();
 
-  private HashMap<String,ReadableType> getLocalTypeMap() {
+  private @Nonnull HashMap<String,ReadableType> getLocalTypeMap() {
     // Fast and non-blocking return for common case
     if (mLocalTypeMap != null) {
       return mLocalTypeMap;
@@ -93,7 +95,7 @@ public class ReadableNativeMap extends NativeMap implements ReadableMap {
   private native Object[] importTypes();
 
   @Override
-  public boolean hasKey(String name) {
+  public boolean hasKey(@Nonnull String name) {
     if (ReactFeatureFlags.useMapNativeAccessor) {
       mJniCallCounter++;
       return hasKeyNative(name);
@@ -103,7 +105,7 @@ public class ReadableNativeMap extends NativeMap implements ReadableMap {
   private native boolean hasKeyNative(String name);
 
   @Override
-  public boolean isNull(String name) {
+  public boolean isNull(@Nonnull String name) {
     if (ReactFeatureFlags.useMapNativeAccessor) {
       mJniCallCounter++;
       return isNullNative(name);
@@ -113,9 +115,9 @@ public class ReadableNativeMap extends NativeMap implements ReadableMap {
     }
     throw new NoSuchKeyException(name);
   }
-  private native boolean isNullNative(String name);
+  private native boolean isNullNative(@Nonnull String name);
 
-  private Object getValue(String name) {
+  private @Nonnull Object getValue(@Nonnull String name) {
     if (hasKey(name) && !(isNull(name))) {
       return Assertions.assertNotNull(getLocalMap().get(name));
     }
@@ -150,7 +152,7 @@ public class ReadableNativeMap extends NativeMap implements ReadableMap {
   }
 
   @Override
-  public boolean getBoolean(String name) {
+  public boolean getBoolean(@Nonnull String name) {
     if (ReactFeatureFlags.useMapNativeAccessor) {
       mJniCallCounter++;
       return getBooleanNative(name);
@@ -160,7 +162,7 @@ public class ReadableNativeMap extends NativeMap implements ReadableMap {
   private native boolean getBooleanNative(String name);
 
   @Override
-  public double getDouble(String name) {
+  public double getDouble(@Nonnull String name) {
     if (ReactFeatureFlags.useMapNativeAccessor) {
       mJniCallCounter++;
       return getDoubleNative(name);
@@ -170,7 +172,7 @@ public class ReadableNativeMap extends NativeMap implements ReadableMap {
   private native double getDoubleNative(String name);
 
   @Override
-  public int getInt(String name) {
+  public int getInt(@Nonnull String name) {
     if (ReactFeatureFlags.useMapNativeAccessor) {
       mJniCallCounter++;
       return getIntNative(name);
@@ -182,7 +184,7 @@ public class ReadableNativeMap extends NativeMap implements ReadableMap {
   private native int getIntNative(String name);
 
   @Override
-  public @Nullable String getString(String name) {
+  public @Nullable String getString(@Nonnull String name) {
     if (ReactFeatureFlags.useMapNativeAccessor) {
       mJniCallCounter++;
       return getStringNative(name);
@@ -192,7 +194,7 @@ public class ReadableNativeMap extends NativeMap implements ReadableMap {
   private native String getStringNative(String name);
 
   @Override
-  public @Nullable ReadableArray getArray(String name) {
+  public @Nullable ReadableArray getArray(@Nonnull String name) {
     if (ReactFeatureFlags.useMapNativeAccessor) {
       mJniCallCounter++;
       return getArrayNative(name);
@@ -202,7 +204,7 @@ public class ReadableNativeMap extends NativeMap implements ReadableMap {
   private native ReadableNativeArray getArrayNative(String name);
 
   @Override
-  public @Nullable ReadableNativeMap getMap(String name) {
+  public @Nullable ReadableNativeMap getMap(@Nonnull String name) {
     if (ReactFeatureFlags.useMapNativeAccessor) {
       mJniCallCounter++;
       return getMapNative(name);
@@ -212,7 +214,7 @@ public class ReadableNativeMap extends NativeMap implements ReadableMap {
   private native ReadableNativeMap getMapNative(String name);
 
   @Override
-  public ReadableType getType(String name) {
+  public @Nonnull ReadableType getType(@Nonnull String name) {
     if (ReactFeatureFlags.useMapNativeAccessor) {
       mJniCallCounter++;
       return getTypeNative(name);
@@ -225,17 +227,17 @@ public class ReadableNativeMap extends NativeMap implements ReadableMap {
   private native ReadableType getTypeNative(String name);
 
   @Override
-  public Dynamic getDynamic(String name) {
+  public @Nonnull Dynamic getDynamic(@Nonnull String name) {
     return DynamicFromMap.create(this, name);
   }
 
   @Override
-  public ReadableMapKeySetIterator keySetIterator() {
+  public @Nonnull ReadableMapKeySetIterator keySetIterator() {
     return new ReadableNativeMapKeySetIterator(this);
   }
 
   @Override
-  public HashMap<String, Object> toHashMap() {
+  public @Nonnull HashMap<String, Object> toHashMap() {
     if (ReactFeatureFlags.useMapNativeAccessor) {
       ReadableMapKeySetIterator iterator = keySetIterator();
       HashMap<String, Object> hashMap = new HashMap<>();

ReactAndroid/src/main/java/com/facebook/react/bridge/WritableArray.java

@@ -7,6 +7,8 @@
 
 package com.facebook.react.bridge;
 
+import javax.annotation.Nonnull;
+
 /**
  * Interface for a mutable array. Used to pass arguments from Java to JS.
  */
@@ -16,7 +18,7 @@ public interface WritableArray extends ReadableArray {
   void pushBoolean(boolean value);
   void pushDouble(double value);
   void pushInt(int value);
-  void pushString(String value);
-  void pushArray(WritableArray array);
-  void pushMap(WritableMap map);
+  void pushString(@Nonnull String value);
+  void pushArray(@Nonnull WritableArray array);
+  void pushMap(@Nonnull WritableMap map);
 }

ReactAndroid/src/main/java/com/facebook/react/bridge/WritableMap.java

@@ -7,18 +7,20 @@
 
 package com.facebook.react.bridge;
 
+import javax.annotation.Nonnull;
+
 /**
  * Interface for a mutable map. Used to pass arguments from Java to JS.
  */
 public interface WritableMap extends ReadableMap {
 
-  void putNull(String key);
-  void putBoolean(String key, boolean value);
-  void putDouble(String key, double value);
-  void putInt(String key, int value);
-  void putString(String key, String value);
-  void putArray(String key, WritableArray value);
-  void putMap(String key, WritableMap value);
+  void putNull(@Nonnull String key);
+  void putBoolean(@Nonnull String key, boolean value);
+  void putDouble(@Nonnull String key, double value);
+  void putInt(@Nonnull String key, int value);
+  void putString(@Nonnull String key, @Nonnull String value);
+  void putArray(@Nonnull String key, @Nonnull WritableArray value);
+  void putMap(@Nonnull String key, @Nonnull WritableMap value);
 
-  void merge(ReadableMap source);
+  void merge(@Nonnull ReadableMap source);
 }

ReactAndroid/src/main/java/com/facebook/react/bridge/WritableNativeArray.java

@@ -11,6 +11,9 @@ import com.facebook.infer.annotation.Assertions;
 import com.facebook.jni.HybridData;
 import com.facebook.proguard.annotations.DoNotStrip;
 
+import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
+
 /**
  * Implementation of a write-only array stored in native memory. Use
  * {@link Arguments#createArray()} if you need to stub out creating this class in a test.
@@ -35,11 +38,11 @@ public class WritableNativeArray extends ReadableNativeArray implements Writable
   @Override
   public native void pushInt(int value);
   @Override
-  public native void pushString(String value);
+  public native void pushString(@Nonnull String value);
 
   // Note: this consumes the map so do not reuse it.
   @Override
-  public void pushArray(WritableArray array) {
+  public void pushArray(@Nullable WritableArray array) {
     Assertions.assertCondition(
         array == null || array instanceof WritableNativeArray, "Illegal type provided");
     pushNativeArray((WritableNativeArray) array);
@@ -47,7 +50,7 @@ public class WritableNativeArray extends ReadableNativeArray implements Writable
 
   // Note: this consumes the map so do not reuse it.
   @Override
-  public void pushMap(WritableMap map) {
+  public void pushMap(@Nonnull WritableMap map) {
     Assertions.assertCondition(
         map == null || map instanceof WritableNativeMap, "Illegal type provided");
     pushNativeMap((WritableNativeMap) map);

ReactAndroid/src/main/java/com/facebook/react/bridge/WritableNativeMap.java

@@ -7,10 +7,14 @@
 
 package com.facebook.react.bridge;
 
+import android.support.annotation.NonNull;
+
 import com.facebook.jni.HybridData;
 import com.facebook.infer.annotation.Assertions;
 import com.facebook.proguard.annotations.DoNotStrip;
 
+import javax.annotation.Nonnull;
+
 /**
  * Implementation of a write-only map stored in native memory. Use
  * {@link Arguments#createMap()} if you need to stub out creating this class in a test.
@@ -23,19 +27,19 @@ public class WritableNativeMap extends ReadableNativeMap implements WritableMap
   }
 
   @Override
-  public native void putBoolean(String key, boolean value);
+  public native void putBoolean(@Nonnull String key, boolean value);
   @Override
-  public native void putDouble(String key, double value);
+  public native void putDouble(@Nonnull String key, double value);
   @Override
-  public native void putInt(String key, int value);
+  public native void putInt(@Nonnull String key, int value);
   @Override
-  public native void putString(String key, String value);
+  public native void putString(@Nonnull String key, @Nonnull String value);
   @Override
-  public native void putNull(String key);
+  public native void putNull(@NonNull String key);
 
   // Note: this consumes the map so do not reuse it.
   @Override
-  public void putMap(String key, WritableMap value) {
+  public void putMap(@Nonnull String key, @Nonnull WritableMap value) {
     Assertions.assertCondition(
         value == null || value instanceof WritableNativeMap, "Illegal type provided");
     putNativeMap(key, (WritableNativeMap) value);
@@ -43,7 +47,7 @@ public class WritableNativeMap extends ReadableNativeMap implements WritableMap
 
   // Note: this consumes the map so do not reuse it.
   @Override
-  public void putArray(String key, WritableArray value) {
+  public void putArray(@Nonnull String key, @Nonnull WritableArray value) {
     Assertions.assertCondition(
         value == null || value instanceof WritableNativeArray, "Illegal type provided");
     putNativeArray(key, (WritableNativeArray) value);
@@ -51,7 +55,7 @@ public class WritableNativeMap extends ReadableNativeMap implements WritableMap
 
   // Note: this **DOES NOT** consume the source map
   @Override
-  public void merge(ReadableMap source) {
+  public void merge(@Nonnull ReadableMap source) {
     Assertions.assertCondition(source instanceof ReadableNativeMap, "Illegal type provided");
     mergeNativeMap((ReadableNativeMap) source);
   }