Fix potential nullable accesss

Summary:
While ramping up to Flipper I found this potential NPE caused by ordering issues.

There are only 3 uses downstream that don't use a constant that's part of the default initializer.
```
λ festevezga-mbp xplat → buck build sonar/android/src/main/java/com/facebook/flipper/plugins/inspector/... 2>&1 | rg ' = descriptorForClass' | awk '{$1=$1};1' | sort | uniq -c
   5 NodeDescriptor descriptor = descriptorForClass(Object.class);
   3 NodeDescriptor descriptor = descriptorForClass(View.class);
   1 NodeDescriptor descriptor = descriptorForClass(node.getClass());
   2 final NodeDescriptor descriptor = descriptorForClass(Dialog.class);
  12 final NodeDescriptor descriptor = descriptorForClass(Fragment.class);
   5 final NodeDescriptor descriptor = descriptorForClass(Object.class);
  26 final NodeDescriptor descriptor = descriptorForClass(View.class);
   2 final NodeDescriptor descriptor = descriptorForClass(Window.class);
   1 final NodeDescriptor descriptor = descriptorForClass(node.getClass());
   1 final NodeDescriptor descriptor = descriptorForClass(topChild.getClass());
   2 final NodeDescriptor objectDescriptor = descriptorForClass(Object.class);
```

Reviewed By: muraziz

Differential Revision: D22284669

fbshipit-source-id: b41c8d78c70c5b5acdc08aa6f0e7afa681f4242d
This commit is contained in:
Paco Estevez Garcia
2020-07-01 11:03:58 -07:00
committed by Facebook GitHub Bot
parent babc88e472
commit c1d9527406
5 changed files with 20 additions and 9 deletions

View File

@@ -14,6 +14,7 @@ import android.view.View;
import android.view.ViewGroup; import android.view.ViewGroup;
import android.view.Window; import android.view.Window;
import android.widget.TextView; import android.widget.TextView;
import androidx.annotation.Nullable;
import com.facebook.flipper.core.FlipperConnection; import com.facebook.flipper.core.FlipperConnection;
import com.facebook.flipper.plugins.inspector.descriptors.ActivityDescriptor; import com.facebook.flipper.plugins.inspector.descriptors.ActivityDescriptor;
import com.facebook.flipper.plugins.inspector.descriptors.ApplicationDescriptor; import com.facebook.flipper.plugins.inspector.descriptors.ApplicationDescriptor;
@@ -66,7 +67,7 @@ public class DescriptorMapping {
mMapping.put(clazz, descriptor); mMapping.put(clazz, descriptor);
} }
public NodeDescriptor<?> descriptorForClass(Class<?> clazz) { public @Nullable NodeDescriptor<?> descriptorForClass(Class<?> clazz) {
while (!mMapping.containsKey(clazz)) { while (!mMapping.containsKey(clazz)) {
clazz = clazz.getSuperclass(); clazz = clazz.getSuperclass();
} }

View File

@@ -17,13 +17,15 @@ import java.util.Collections;
import java.util.List; import java.util.List;
/** /**
* A NodeDescriptor is an object which known how to expose an Object of type T to the ew Inspector. * A NodeDescriptor is an object which known how to expose an Object of type T to the new Inspector.
* This class is the extension point for the Flipper inspector plugin and is how custom classes and * This class is the extension point for the Flipper inspector plugin and is how custom classes and
* data can be exposed to the inspector. * data can be exposed to the inspector.
*/ */
public abstract class NodeDescriptor<T> { public abstract class NodeDescriptor<T> {
@Nullable protected FlipperConnection mConnection; @Nullable protected FlipperConnection mConnection;
private DescriptorMapping mDescriptorMapping;
// This field is not initialized until setDescriptorMapping is called
@Nullable private DescriptorMapping mDescriptorMapping;
void setConnection(FlipperConnection connection) { void setConnection(FlipperConnection connection) {
mConnection = connection; mConnection = connection;
@@ -39,8 +41,11 @@ public abstract class NodeDescriptor<T> {
* object it describes. This is highly encouraged instead of subclassing another descriptor * object it describes. This is highly encouraged instead of subclassing another descriptor
* class. * class.
*/ */
protected final NodeDescriptor<?> descriptorForClass(Class<?> clazz) { protected final @Nullable NodeDescriptor<?> descriptorForClass(Class<?> clazz) {
return mDescriptorMapping.descriptorForClass(clazz); if (mDescriptorMapping != null) {
return mDescriptorMapping.descriptorForClass(clazz);
}
return null;
} }
/** /**

View File

@@ -230,7 +230,9 @@ public class ApplicationDescriptor extends NodeDescriptor<ApplicationWrapper> {
if (childCount > 0) { if (childCount > 0) {
final Object topChild = getChildAt(node, childCount - 1); final Object topChild = getChildAt(node, childCount - 1);
final NodeDescriptor descriptor = descriptorForClass(topChild.getClass()); final NodeDescriptor descriptor = descriptorForClass(topChild.getClass());
descriptor.setHighlighted(topChild, selected, isAlignmentMode); if (descriptor != null) {
descriptor.setHighlighted(topChild, selected, isAlignmentMode);
}
} }
} }

View File

@@ -70,7 +70,8 @@ public class ObjectDescriptor extends NodeDescriptor<Object> {
@Override @Override
public boolean matches(String query, Object node) throws Exception { public boolean matches(String query, Object node) throws Exception {
final NodeDescriptor descriptor = descriptorForClass(node.getClass()); final NodeDescriptor descriptor = descriptorForClass(node.getClass());
final List<Named<String>> attributes = descriptor.getAttributes(node); final List<Named<String>> attributes =
descriptor == null ? Collections.emptyList() : descriptor.getAttributes(node);
for (Named<String> namedString : attributes) { for (Named<String> namedString : attributes) {
if (namedString.getName().equals("id")) { if (namedString.getName().equals("id")) {
if (namedString.getValue().toLowerCase().contains(query)) { if (namedString.getValue().toLowerCase().contains(query)) {
@@ -79,6 +80,6 @@ public class ObjectDescriptor extends NodeDescriptor<Object> {
} }
} }
return descriptor.getName(node).toLowerCase().contains(query); return descriptor == null ? false : descriptor.getName(node).toLowerCase().contains(query);
} }
} }

View File

@@ -74,7 +74,9 @@ public class ViewGroupDescriptor extends NodeDescriptor<ViewGroup> {
if (connected()) { if (connected()) {
if (key.set(node)) { if (key.set(node)) {
NodeDescriptor descriptor = descriptorForClass(node.getClass()); NodeDescriptor descriptor = descriptorForClass(node.getClass());
descriptor.invalidate(node); if (descriptor != null) {
descriptor.invalidate(node);
}
invalidateAX(node); invalidateAX(node);
} }