Skip to content

Commit

Permalink
fix WritableArray, WritableMap nullable annotations (#23397)
Browse files Browse the repository at this point in the history
Summary:
Recently, I added nullable annotations to ReadableArray, ReadableMap, WritableArray, WritableMap and subclasses to improve Kotlin developer experience. But found that I made mistake with pushArray, pushMap, pushString method of WritableArray, and putArray, putMap, putString methods of WritableMap. This PR fixes previous mistake.

Excerpt from WritableNativeArray.cpp.
```cpp
void WritableNativeArray::pushString(jstring value) {
  if (value == NULL) {
    pushNull();
    return;
  }
  throwIfConsumed();
  array_.push_back(wrap_alias(value)->toStdString());
}

void WritableNativeArray::pushNativeArray(WritableNativeArray* otherArray) {
  if (otherArray == NULL) {
    pushNull();
    return;
  }
  throwIfConsumed();
  array_.push_back(otherArray->consume());
}

void WritableNativeArray::pushNativeMap(WritableNativeMap* map) {
  if (map == NULL) {
    pushNull();
    return;
  }
  throwIfConsumed();
  array_.push_back(map->consume());
}
```

Excerpt from WritableNativeMap.cpp
```cpp
void WritableNativeMap::putString(std::string key, alias_ref<jstring> val) {
  if (!val) {
    putNull(std::move(key));
    return;
  }
  throwIfConsumed();
  map_.insert(std::move(key), val->toString());
}

void WritableNativeMap::putNativeArray(std::string key, WritableNativeArray* otherArray) {
  if (!otherArray) {
    putNull(std::move(key));
    return;
  }
  throwIfConsumed();
  map_.insert(key, otherArray->consume());
}

void WritableNativeMap::putNativeMap(std::string key, WritableNativeMap *otherMap) {
  if (!otherMap) {
    putNull(std::move(key));
    return;
  }
  throwIfConsumed();
  map_.insert(std::move(key), otherMap->consume());
}
```

[Android] [Changed] - fix nullable annotations in WritableArray, WritableMap
Pull Request resolved: #23397

Differential Revision: D14044014

Pulled By: cpojer

fbshipit-source-id: c44ea2e097e7b1156223b516aa640a181f0d4a9b
  • Loading branch information
dulmandakh authored and facebook-github-bot committed Feb 12, 2019
1 parent 77300ca commit 7b33d6b
Show file tree
Hide file tree
Showing 6 changed files with 20 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ public void pushInt(int value) {
}

@Override
public void pushString(@Nonnull String value) {
public void pushString(@Nullable String value) {
mBackingList.add(value);
}

Expand All @@ -170,7 +170,7 @@ public void pushArray(@Nullable WritableArray array) {
}

@Override
public void pushMap(@Nonnull WritableMap map) {
public void pushMap(@Nullable WritableMap map) {
mBackingList.add(map);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import java.util.Map;

import javax.annotation.Nonnull;
import javax.annotation.Nullable;

/**
* Java {@link HashMap} backed implementation of {@link ReadableMap} and {@link WritableMap}
Expand Down Expand Up @@ -179,7 +180,7 @@ public void putInt(@Nonnull String key, int value) {
}

@Override
public void putString(@Nonnull String key, @Nonnull String value) {
public void putString(@Nonnull String key, @Nullable String value) {
mBackingMap.put(key, value);
}

Expand All @@ -189,7 +190,7 @@ public void putNull(@Nonnull String key) {
}

@Override
public void putMap(@Nonnull String key, @Nonnull WritableMap value) {
public void putMap(@Nonnull String key, @Nullable WritableMap value) {
mBackingMap.put(key, value);
}

Expand All @@ -199,7 +200,7 @@ public void merge(@Nonnull ReadableMap source) {
}

@Override
public void putArray(@Nonnull String key, @Nonnull WritableArray value) {
public void putArray(@Nonnull String key, @Nullable WritableArray value) {
mBackingMap.put(key, value);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
package com.facebook.react.bridge;

import javax.annotation.Nonnull;
import javax.annotation.Nullable;

/**
* Interface for a mutable array. Used to pass arguments from Java to JS.
Expand All @@ -18,7 +19,7 @@ public interface WritableArray extends ReadableArray {
void pushBoolean(boolean value);
void pushDouble(double value);
void pushInt(int value);
void pushString(@Nonnull String value);
void pushArray(@Nonnull WritableArray array);
void pushMap(@Nonnull WritableMap map);
void pushString(@Nullable String value);
void pushArray(@Nullable WritableArray array);
void pushMap(@Nullable WritableMap map);
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
package com.facebook.react.bridge;

import javax.annotation.Nonnull;
import javax.annotation.Nullable;

/**
* Interface for a mutable map. Used to pass arguments from Java to JS.
Expand All @@ -18,9 +19,9 @@ public interface WritableMap extends ReadableMap {
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 putString(@Nonnull String key, @Nullable String value);
void putArray(@Nonnull String key, @Nullable WritableArray value);
void putMap(@Nonnull String key, @Nullable WritableMap value);

void merge(@Nonnull ReadableMap source);
}
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ public WritableNativeArray() {
@Override
public native void pushInt(int value);
@Override
public native void pushString(@Nonnull String value);
public native void pushString(@Nullable String value);

// Note: this consumes the map so do not reuse it.
@Override
Expand All @@ -50,7 +50,7 @@ public void pushArray(@Nullable WritableArray array) {

// Note: this consumes the map so do not reuse it.
@Override
public void pushMap(@Nonnull WritableMap map) {
public void pushMap(@Nullable WritableMap map) {
Assertions.assertCondition(
map == null || map instanceof WritableNativeMap, "Illegal type provided");
pushNativeMap((WritableNativeMap) map);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import com.facebook.proguard.annotations.DoNotStrip;

import javax.annotation.Nonnull;
import javax.annotation.Nullable;

/**
* Implementation of a write-only map stored in native memory. Use
Expand All @@ -33,21 +34,21 @@ public class WritableNativeMap extends ReadableNativeMap implements WritableMap
@Override
public native void putInt(@Nonnull String key, int value);
@Override
public native void putString(@Nonnull String key, @Nonnull String value);
public native void putString(@Nonnull String key, @Nullable String value);
@Override
public native void putNull(@NonNull String key);

// Note: this consumes the map so do not reuse it.
@Override
public void putMap(@Nonnull String key, @Nonnull WritableMap value) {
public void putMap(@Nonnull String key, @Nullable WritableMap value) {
Assertions.assertCondition(
value == null || value instanceof WritableNativeMap, "Illegal type provided");
putNativeMap(key, (WritableNativeMap) value);
}

// Note: this consumes the map so do not reuse it.
@Override
public void putArray(@Nonnull String key, @Nonnull WritableArray value) {
public void putArray(@Nonnull String key, @Nullable WritableArray value) {
Assertions.assertCondition(
value == null || value instanceof WritableNativeArray, "Illegal type provided");
putNativeArray(key, (WritableNativeArray) value);
Expand Down

0 comments on commit 7b33d6b

Please sign in to comment.