Skip to content

Commit 14de1c1

Browse files
GijsWeteringsfacebook-github-bot
GijsWeterings
authored and
facebook-github-bot
committed
Fix Nullsafe FIXMEs for StackTraceHelper.java and mark Nullsafe (#50054)
Summary: Pull Request resolved: #50054 Gone trough all the FIXMEs added in the previous diff by the nullsafe tool, marked the class as nullsafe and ensured no remaining violations. Changelog: [Android][Fixed] Made StackTraceHelper.java nullsafe Reviewed By: javache Differential Revision: D71126387 fbshipit-source-id: 3456dde049b50e48ad703fe8312cf57b8d849515
1 parent 3c795e4 commit 14de1c1

3 files changed

Lines changed: 59 additions & 52 deletions

File tree

‎packages/react-native/ReactAndroid/src/main/java/com/facebook/react/devsupport/StackTraceHelper.java‎

Lines changed: 55 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@
88
package com.facebook.react.devsupport;
99

1010
import androidx.annotation.Nullable;
11+
import com.facebook.infer.annotation.Assertions;
12+
import com.facebook.infer.annotation.Nullsafe;
1113
import com.facebook.react.bridge.JavaOnlyArray;
1214
import com.facebook.react.bridge.JavaOnlyMap;
1315
import com.facebook.react.bridge.ReadableArray;
@@ -26,6 +28,7 @@
2628
import org.json.JSONObject;
2729

2830
/** Helper class converting JS and Java stack traces into arrays of {@link StackFrame} objects. */
31+
@Nullsafe(Nullsafe.Mode.LOCAL)
2932
public class StackTraceHelper {
3033

3134
public static final String COLUMN_KEY = "column";
@@ -49,27 +52,29 @@ public class StackTraceHelper {
4952

5053
/** Represents a generic entry in a stack trace, be it originally from JS or Java. */
5154
public static class StackFrameImpl implements StackFrame {
52-
private final String mFile;
55+
@Nullable private final String mFile;
5356
private final String mMethod;
5457
private final int mLine;
5558
private final int mColumn;
56-
private final String mFileName;
59+
@Nullable private final String mFileName;
5760
private final boolean mIsCollapsed;
5861

59-
private StackFrameImpl(String file, String method, int line, int column, boolean isCollapsed) {
62+
private StackFrameImpl(
63+
@Nullable String file, String method, int line, int column, boolean isCollapsed) {
6064
mFile = file;
6165
mMethod = method;
6266
mLine = line;
6367
mColumn = column;
64-
mFileName = file != null ? new File(file).getName() : "";
68+
mFileName = file != null ? new File(file).getName() : null;
6569
mIsCollapsed = isCollapsed;
6670
}
6771

68-
private StackFrameImpl(String file, String method, int line, int column) {
72+
private StackFrameImpl(@Nullable String file, String method, int line, int column) {
6973
this(file, method, line, column, false);
7074
}
7175

72-
private StackFrameImpl(String file, String fileName, String method, int line, int column) {
76+
private StackFrameImpl(
77+
@Nullable String file, @Nullable String fileName, String method, int line, int column) {
7378
mFile = file;
7479
mFileName = fileName;
7580
mMethod = method;
@@ -84,7 +89,8 @@ private StackFrameImpl(String file, String fileName, String method, int line, in
8489
* <p>JS traces return the full path to the file here, while Java traces only return the file
8590
* name (the path is not known).
8691
*/
87-
public String getFile() {
92+
@Override
93+
public @Nullable String getFile() {
8894
return mFile;
8995
}
9096

@@ -109,7 +115,8 @@ public int getColumn() {
109115
* <p>For JS traces this is different from {@link #getFile()} in that it only returns the file
110116
* name, not the full path. For Java traces there is no difference.
111117
*/
112-
public String getFileName() {
118+
@Override
119+
public @Nullable String getFileName() {
113120
return mFileName;
114121
}
115122

@@ -119,9 +126,10 @@ public boolean isCollapsed() {
119126

120127
/** Convert the stack frame to a JSON representation. */
121128
public JSONObject toJSON() {
129+
String file = getFile();
122130
return new JSONObject(
123131
MapBuilder.of(
124-
"file", getFile(),
132+
"file", (file == null) ? "" : file,
125133
"methodName", getMethod(),
126134
"lineNumber", getLine(),
127135
"column", getColumn(),
@@ -136,36 +144,33 @@ public JSONObject toJSON() {
136144
public static StackFrame[] convertJsStackTrace(@Nullable ReadableArray stack) {
137145
int size = stack != null ? stack.size() : 0;
138146
StackFrame[] result = new StackFrame[size];
139-
for (int i = 0; i < size; i++) {
140-
// NULLSAFE_FIXME[Nullable Dereference]
141-
ReadableType type = stack.getType(i);
142-
if (type == ReadableType.Map) {
143-
// NULLSAFE_FIXME[Nullable Dereference]
144-
ReadableMap frame = stack.getMap(i);
145-
// NULLSAFE_FIXME[Nullable Dereference]
146-
String methodName = frame.getString("methodName");
147-
// NULLSAFE_FIXME[Nullable Dereference]
148-
String fileName = frame.getString("file");
149-
boolean collapse =
150-
// NULLSAFE_FIXME[Nullable Dereference]
151-
frame.hasKey("collapse") && !frame.isNull("collapse") && frame.getBoolean("collapse");
152-
int lineNumber = -1;
153-
// NULLSAFE_FIXME[Nullable Dereference]
154-
if (frame.hasKey(LINE_NUMBER_KEY) && !frame.isNull(LINE_NUMBER_KEY)) {
155-
// NULLSAFE_FIXME[Nullable Dereference]
156-
lineNumber = frame.getInt(LINE_NUMBER_KEY);
157-
}
158-
int columnNumber = -1;
159-
// NULLSAFE_FIXME[Nullable Dereference]
160-
if (frame.hasKey(COLUMN_KEY) && !frame.isNull(COLUMN_KEY)) {
161-
// NULLSAFE_FIXME[Nullable Dereference]
162-
columnNumber = frame.getInt(COLUMN_KEY);
147+
if (stack != null) {
148+
for (int i = 0; i < size; i++) {
149+
ReadableType type = stack.getType(i);
150+
if (type == ReadableType.Map) {
151+
ReadableMap frame = stack.getMap(i);
152+
Assertions.assertNotNull(frame);
153+
String methodName = frame.getString("methodName");
154+
String fileName = frame.getString("file");
155+
Assertions.assertNotNull(fileName);
156+
Assertions.assertNotNull(methodName);
157+
boolean collapse =
158+
frame.hasKey("collapse") && !frame.isNull("collapse") && frame.getBoolean("collapse");
159+
int lineNumber = -1;
160+
if (frame.hasKey(LINE_NUMBER_KEY) && !frame.isNull(LINE_NUMBER_KEY)) {
161+
lineNumber = frame.getInt(LINE_NUMBER_KEY);
162+
}
163+
int columnNumber = -1;
164+
if (frame.hasKey(COLUMN_KEY) && !frame.isNull(COLUMN_KEY)) {
165+
columnNumber = frame.getInt(COLUMN_KEY);
166+
}
167+
result[i] = new StackFrameImpl(fileName, methodName, lineNumber, columnNumber, collapse);
168+
} else if (type == ReadableType.String) {
169+
String stackFrame = stack.getString(i);
170+
if (stackFrame != null) {
171+
result[i] = new StackFrameImpl(null, stackFrame, -1, -1);
172+
}
163173
}
164-
// NULLSAFE_FIXME[Parameter Not Nullable]
165-
result[i] = new StackFrameImpl(fileName, methodName, lineNumber, columnNumber, collapse);
166-
} else if (type == ReadableType.String) {
167-
// NULLSAFE_FIXME[Parameter Not Nullable, Nullable Dereference]
168-
result[i] = new StackFrameImpl(null, stack.getString(i), -1, -1);
169174
}
170175
}
171176
return result;
@@ -214,19 +219,22 @@ public static StackFrame[] convertJsStackTrace(String stack) {
214219
} else if (matcher1.find()) {
215220
matcher = matcher1;
216221
} else {
217-
// NULLSAFE_FIXME[Parameter Not Nullable]
218-
result[i] = new StackFrameImpl(null, stackTrace[i], -1, -1);
222+
String unmatchedStackFrame = stackTrace[i];
223+
if (unmatchedStackFrame != null) {
224+
result[i] = new StackFrameImpl(null, unmatchedStackFrame, -1, -1);
225+
}
226+
continue;
227+
}
228+
String fileName = matcher.group(2);
229+
String methodName = matcher.group(1) == null ? "(unknown)" : matcher.group(1);
230+
String lineString = matcher.group(3);
231+
String columnString = matcher.group(4);
232+
if (methodName == null || fileName == null || lineString == null || columnString == null) {
219233
continue;
220234
}
221235
result[i] =
222236
new StackFrameImpl(
223-
// NULLSAFE_FIXME[Parameter Not Nullable]
224-
matcher.group(2),
225-
matcher.group(1) == null ? "(unknown)" : matcher.group(1),
226-
// NULLSAFE_FIXME[Parameter Not Nullable]
227-
Integer.parseInt(matcher.group(3)),
228-
// NULLSAFE_FIXME[Parameter Not Nullable]
229-
Integer.parseInt(matcher.group(4)));
237+
fileName, methodName, Integer.parseInt(lineString), Integer.parseInt(columnString));
230238
}
231239
return result;
232240
}
@@ -239,7 +247,6 @@ public static StackFrame[] convertJavaStackTrace(Throwable exception) {
239247
result[i] =
240248
new StackFrameImpl(
241249
stackTrace[i].getClassName(),
242-
// NULLSAFE_FIXME[Parameter Not Nullable]
243250
stackTrace[i].getFileName(),
244251
stackTrace[i].getMethodName(),
245252
stackTrace[i].getLineNumber(),

‎packages/react-native/ReactAndroid/src/main/java/com/facebook/react/devsupport/interfaces/StackFrame.kt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ public interface StackFrame {
2020
public val file: String?
2121

2222
/** Get the name of the method this frame points to. */
23-
public val method: String?
23+
public val method: String
2424

2525
/** Get the line number this frame points to in the file returned by [.getFile]. */
2626
public val line: Int
@@ -40,5 +40,5 @@ public interface StackFrame {
4040
public val isCollapsed: Boolean
4141

4242
/** Convert the stack frame to a JSON representation. */
43-
public fun toJSON(): JSONObject?
43+
public fun toJSON(): JSONObject
4444
}

‎packages/react-native/ReactAndroid/src/test/java/com/facebook/react/devsupport/StackTraceHelperTest.kt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ class StackTraceHelperTest {
4747
fun testParseStackFrameWithInvalidFrame() {
4848
val frame = StackTraceHelper.convertJsStackTrace("Test.bundle:ten:twenty").get(0)
4949
assertThat(frame.method).isEqualTo("Test.bundle:ten:twenty")
50-
assertThat(frame.fileName).isEqualTo("")
50+
assertThat(frame.fileName).isEqualTo(null)
5151
assertThat(frame.line).isEqualTo(-1)
5252
assertThat(frame.column).isEqualTo(-1)
5353
}
@@ -56,7 +56,7 @@ class StackTraceHelperTest {
5656
fun testParseStackFrameWithNativeCodeFrame() {
5757
val frame = StackTraceHelper.convertJsStackTrace("forEach@[native code]").get(0)
5858
assertThat(frame.method).isEqualTo("forEach@[native code]")
59-
assertThat(frame.fileName).isEqualTo("")
59+
assertThat(frame.fileName).isEqualTo(null)
6060
assertThat(frame.line).isEqualTo(-1)
6161
assertThat(frame.column).isEqualTo(-1)
6262
}

0 commit comments

Comments
 (0)