From b640b6faf77f7af955e64bd03ae630ce2fb09627 Mon Sep 17 00:00:00 2001 From: Dulmandakh Date: Fri, 8 Feb 2019 04:08:37 -0800 Subject: [PATCH] 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 --- .../facebook/react/bridge/JavaOnlyArray.java | 17 ++++---- .../facebook/react/bridge/JavaOnlyMap.java | 42 ++++++++++--------- .../facebook/react/bridge/ReadableArray.java | 15 ++++--- .../facebook/react/bridge/ReadableMap.java | 27 ++++++------ .../react/bridge/ReadableNativeArray.java | 14 ++++--- .../react/bridge/ReadableNativeMap.java | 32 +++++++------- .../facebook/react/bridge/WritableArray.java | 8 ++-- .../facebook/react/bridge/WritableMap.java | 18 ++++---- .../react/bridge/WritableNativeArray.java | 9 ++-- .../react/bridge/WritableNativeMap.java | 20 +++++---- 10 files changed, 114 insertions(+), 88 deletions(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/JavaOnlyArray.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/JavaOnlyArray.java index 83684d7a245ed2..9f215e3d26eaff 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/bridge/JavaOnlyArray.java +++ b/ReactAndroid/src/main/java/com/facebook/react/bridge/JavaOnlyArray.java @@ -11,6 +11,9 @@ 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 int getInt(int index) { } @Override - public String getString(int index) { + public @Nullable String getString(int index) { return (String) mBackingList.get(index); } @@ -115,12 +118,12 @@ public JavaOnlyMap getMap(int index) { } @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 void pushInt(int value) { } @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 void pushNull() { } @Override - public ArrayList toArrayList() { + public @Nonnull ArrayList toArrayList() { return new ArrayList(mBackingList); } diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/JavaOnlyMap.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/JavaOnlyMap.java index ba565105fe740c..77d64f65636a55 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/bridge/JavaOnlyMap.java +++ b/ReactAndroid/src/main/java/com/facebook/react/bridge/JavaOnlyMap.java @@ -11,6 +11,8 @@ 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 JavaOnlyMap() { } @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 ReadableType getType(String name) { } @Override - public ReadableMapKeySetIterator keySetIterator() { + public @Nonnull ReadableMapKeySetIterator keySetIterator() { return new ReadableMapKeySetIterator() { Iterator mIterator = mBackingMap.keySet().iterator(); @@ -162,47 +164,47 @@ public String nextKey() { } @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 toHashMap() { + public @Nonnull HashMap toHashMap() { return new HashMap(mBackingMap); } diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/ReadableArray.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/ReadableArray.java index 3a7a31517e15ab..79b3bb63985141 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/bridge/ReadableArray.java +++ b/ReactAndroid/src/main/java/com/facebook/react/bridge/ReadableArray.java @@ -9,6 +9,9 @@ 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 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 toArrayList(); } diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/ReadableMap.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/ReadableMap.java index bad731e709c099..8a2ae68dbdbaf9 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/bridge/ReadableMap.java +++ b/ReactAndroid/src/main/java/com/facebook/react/bridge/ReadableMap.java @@ -9,23 +9,26 @@ 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 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 toHashMap(); } diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/ReadableNativeArray.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/ReadableNativeArray.java index 80f360adc1d63f..94acf671b9dbae 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/bridge/ReadableNativeArray.java +++ b/ReactAndroid/src/main/java/com/facebook/react/bridge/ReadableNativeArray.java @@ -13,6 +13,8 @@ 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 int getInt(int index) { 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 String getString(int index) { 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 ReadableNativeArray getArray(int index) { 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 ReadableNativeMap getMap(int index) { 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 ReadableType getType(int index) { 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 toArrayList() { + public @Nonnull ArrayList toArrayList() { ArrayList arrayList = new ArrayList<>(); for (int i = 0; i < this.size(); i++) { diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/ReadableNativeMap.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/ReadableNativeMap.java index 505486650164db..ef661c06d94429 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/bridge/ReadableNativeMap.java +++ b/ReactAndroid/src/main/java/com/facebook/react/bridge/ReadableNativeMap.java @@ -13,6 +13,8 @@ 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 @@ private HashMap getLocalMap() { private native String[] importKeys(); private native Object[] importValues(); - private HashMap getLocalTypeMap() { + private @Nonnull HashMap getLocalTypeMap() { // Fast and non-blocking return for common case if (mLocalTypeMap != null) { return mLocalTypeMap; @@ -93,7 +95,7 @@ private HashMap getLocalTypeMap() { 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 boolean hasKey(String name) { 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 boolean isNull(String name) { } 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 @@ private void checkInstance(String name, Object value, Class type) { } @Override - public boolean getBoolean(String name) { + public boolean getBoolean(@Nonnull String name) { if (ReactFeatureFlags.useMapNativeAccessor) { mJniCallCounter++; return getBooleanNative(name); @@ -160,7 +162,7 @@ public boolean getBoolean(String name) { 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 double getDouble(String name) { 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 int getInt(String name) { 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 int getInt(String name) { 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 int getInt(String name) { 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 int getInt(String name) { 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 ReadableType getType(String name) { 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 toHashMap() { + public @Nonnull HashMap toHashMap() { if (ReactFeatureFlags.useMapNativeAccessor) { ReadableMapKeySetIterator iterator = keySetIterator(); HashMap hashMap = new HashMap<>(); diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/WritableArray.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/WritableArray.java index f0006c81df447f..a5a7b69a70705b 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/bridge/WritableArray.java +++ b/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); } diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/WritableMap.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/WritableMap.java index 1aaef7cc51d25f..8c30a4e7803617 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/bridge/WritableMap.java +++ b/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); } diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/WritableNativeArray.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/WritableNativeArray.java index 074446485c282f..5875f14a5c6fe4 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/bridge/WritableNativeArray.java +++ b/ReactAndroid/src/main/java/com/facebook/react/bridge/WritableNativeArray.java @@ -11,6 +11,9 @@ 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 WritableNativeArray() { @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 void pushArray(WritableArray array) { // 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); diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/WritableNativeMap.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/WritableNativeMap.java index 4c7365a53810dd..ad70213e6e18d1 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/bridge/WritableNativeMap.java +++ b/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 void putMap(String key, WritableMap value) { // 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 void putArray(String key, WritableArray value) { // 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); }