Cleanup a number of warnings in the code. (#137)

* Eliminate 'badimport' warning

It was already reasonably obvious what this is, so the warning is mostly
spurious. But it's easier and less boilerplate to just add the extra
qualifier than to suppress the warning.

https://errorprone.info/bugpattern/BadImport

* Suppress EqualsGetClass warnings for public classes

These are effectively final and are documented as not being final only
to allow mocking within tests.

https://errorprone.info/bugpattern/EqualsGetClass

* Mark a number of classes final

Mostly to avoid a warning, though it's generally recommended to do this
anyways. (Note that this makes mocking impossible; if any of these
classes need to be mocked, we might be better of just suppressing the
warning or using instanceof.)

https://errorprone.info/bugpattern/EqualsGetClass

* Switch .getClass() to instanceof in some .equals() methods

This is for cases where we're dealing with a non-leaf node in the
inheritance hierarchy. (Leaf nodes in the heirarchy can just mark the
entire class as final.) In order to preserve symmetry of .equals(), also
mark the .equals() methods themselves as final.

In this case, "non-leaf" nodes also counts effectively final classes
which are mocked. (i.e. ViewSnapshot.)

https://errorprone.info/bugpattern/EqualsGetClass

* Provide toString on FieldMask

Since it's used by PatchMutation

https://errorprone.info/bugpattern/ObjectToString

* Fix a 'missing override' warning

https://errorprone.info/bugpattern/MissingOverride

* Rework FieldMask to use a Set of FieldPaths rather than a Collection.

FieldMask uses .equals() on the container, and that's not well defined
for a Collection. The fix is to switch to either a Set or a List. Set
seems more appropriate.

https://errorprone.info/bugpattern/UndefinedEquals

* Add TestUtil.fieldMask(String...) to simplify creating FieldMasks.

Author: rsgowman

firebase-firestore/src/main/java/com/google/firebase/firestore/DocumentReference.java

@@ -555,7 +555,7 @@ public boolean equals(Object o) {
     if (this == o) {
       return true;
     }
-    if (o == null || getClass() != o.getClass()) {
+    if (!(o instanceof DocumentReference)) {
       return false;
     }
 

firebase-firestore/src/main/java/com/google/firebase/firestore/Query.java

@@ -905,7 +905,7 @@ public boolean equals(Object o) {
     if (this == o) {
       return true;
     }
-    if (o == null || getClass() != o.getClass()) {
+    if (!(o instanceof Query)) {
       return false;
     }
 

firebase-firestore/src/main/java/com/google/firebase/firestore/SetOptions.java

@@ -20,8 +20,9 @@
 import android.support.annotation.Nullable;
 import com.google.firebase.annotations.PublicApi;
 import com.google.firebase.firestore.model.mutation.FieldMask;
-import java.util.ArrayList;
+import java.util.HashSet;
 import java.util.List;
+import java.util.Set;
 
 /**
  * An options object that configures the behavior of set() calls. By providing one of the SetOptions
@@ -78,13 +79,13 @@ public static SetOptions merge() {
   @NonNull
   @PublicApi
   public static SetOptions mergeFields(List<String> fields) {
-    List<com.google.firebase.firestore.model.FieldPath> fieldPaths = new ArrayList<>();
+    Set<com.google.firebase.firestore.model.FieldPath> fieldPaths = new HashSet<>();
 
     for (String field : fields) {
       fieldPaths.add(FieldPath.fromDotSeparatedPath(field).getInternalPath());
     }
 
-    return new SetOptions(true, FieldMask.fromCollection(fieldPaths));
+    return new SetOptions(true, FieldMask.fromSet(fieldPaths));
   }
 
   /**
@@ -100,13 +101,13 @@ public static SetOptions mergeFields(List<String> fields) {
   @NonNull
   @PublicApi
   public static SetOptions mergeFields(String... fields) {
-    List<com.google.firebase.firestore.model.FieldPath> fieldPaths = new ArrayList<>();
+    Set<com.google.firebase.firestore.model.FieldPath> fieldPaths = new HashSet<>();
 
     for (String field : fields) {
       fieldPaths.add(FieldPath.fromDotSeparatedPath(field).getInternalPath());
     }
 
-    return new SetOptions(true, FieldMask.fromCollection(fieldPaths));
+    return new SetOptions(true, FieldMask.fromSet(fieldPaths));
   }
 
   /**
@@ -121,13 +122,13 @@ public static SetOptions mergeFields(String... fields) {
   @NonNull
   @PublicApi
   public static SetOptions mergeFieldPaths(List<FieldPath> fields) {
-    List<com.google.firebase.firestore.model.FieldPath> fieldPaths = new ArrayList<>();
+    Set<com.google.firebase.firestore.model.FieldPath> fieldPaths = new HashSet<>();
 
     for (FieldPath field : fields) {
       fieldPaths.add(field.getInternalPath());
     }
 
-    return new SetOptions(true, FieldMask.fromCollection(fieldPaths));
+    return new SetOptions(true, FieldMask.fromSet(fieldPaths));
   }
 
   @Override

firebase-firestore/src/main/java/com/google/firebase/firestore/core/EventManager.java

@@ -137,6 +137,7 @@ public void onError(Query query, Status error) {
     queries.remove(query);
   }
 
+  @Override
   public void handleOnlineStateChange(OnlineState onlineState) {
     this.onlineState = onlineState;
     for (QueryListenersInfo info : queries.values()) {

firebase-firestore/src/main/java/com/google/firebase/firestore/core/UserData.java

@@ -29,9 +29,9 @@
 import com.google.firebase.firestore.model.value.ObjectValue;
 import com.google.firebase.firestore.util.Assert;
 import java.util.ArrayList;
+import java.util.HashSet;
 import java.util.List;
-import java.util.SortedSet;
-import java.util.TreeSet;
+import java.util.Set;
 import java.util.regex.Pattern;
 import javax.annotation.Nullable;
 
@@ -72,16 +72,16 @@ public static class ParseAccumulator {
      */
     private final Source dataSource;
 
-    /** Accumulates a list of the field paths found while parsing the data. */
-    private final SortedSet<FieldPath> fieldMask;
+    /** Accumulates a set of the field paths found while parsing the data. */
+    private final Set<FieldPath> fieldMask;
 
     /** Accumulates a list of field transforms found while parsing the data. */
     private final ArrayList<FieldTransform> fieldTransforms;
 
     /** @param dataSource Indicates what kind of API method this data came from. */
     public ParseAccumulator(Source dataSource) {
       this.dataSource = dataSource;
-      this.fieldMask = new TreeSet<>();
+      this.fieldMask = new HashSet<>();
       this.fieldTransforms = new ArrayList<>();
     }
 
@@ -135,7 +135,7 @@ void addToFieldTransforms(FieldPath fieldPath, TransformOperation transformOpera
      */
     public ParsedSetData toMergeData(ObjectValue data) {
       return new ParsedSetData(
-          data, FieldMask.fromCollection(fieldMask), unmodifiableList(fieldTransforms));
+          data, FieldMask.fromSet(fieldMask), unmodifiableList(fieldTransforms));
     }
 
     /**
@@ -181,7 +181,7 @@ public ParsedSetData toSetData(ObjectValue data) {
      */
     public ParsedUpdateData toUpdateData(ObjectValue data) {
       return new ParsedUpdateData(
-          data, FieldMask.fromCollection(fieldMask), unmodifiableList(fieldTransforms));
+          data, FieldMask.fromSet(fieldMask), unmodifiableList(fieldTransforms));
     }
   }
 

firebase-firestore/src/main/java/com/google/firebase/firestore/core/ViewSnapshot.java

@@ -118,11 +118,11 @@ public boolean excludesMetadataChanges() {
   }
 
   @Override
-  public boolean equals(Object o) {
+  public final boolean equals(Object o) {
     if (this == o) {
       return true;
     }
-    if (o == null || getClass() != o.getClass()) {
+    if (!(o instanceof ViewSnapshot)) {
       return false;
     }
 

firebase-firestore/src/main/java/com/google/firebase/firestore/local/IndexedQueryEngine.java

@@ -22,7 +22,6 @@
 import com.google.firebase.firestore.core.Filter;
 import com.google.firebase.firestore.core.Filter.Operator;
 import com.google.firebase.firestore.core.IndexRange;
-import com.google.firebase.firestore.core.IndexRange.Builder;
 import com.google.firebase.firestore.core.NaNFilter;
 import com.google.firebase.firestore.core.NullFilter;
 import com.google.firebase.firestore.core.Query;
@@ -216,7 +215,7 @@ static IndexRange extractBestIndexRange(Query query) {
    * filter. The determined {@code IndexRange} is likely overselective and requires post-filtering.
    */
   private static IndexRange convertFilterToIndexRange(Filter filter) {
-    Builder indexRange = IndexRange.builder().setFieldPath(filter.getField());
+    IndexRange.Builder indexRange = IndexRange.builder().setFieldPath(filter.getField());
     if (filter instanceof RelationFilter) {
       RelationFilter relationFilter = (RelationFilter) filter;
       FieldValue filterValue = relationFilter.getValue();

firebase-firestore/src/main/java/com/google/firebase/firestore/model/BasePath.java

@@ -161,13 +161,11 @@ public int length() {
   }
 
   @Override
-  @SuppressWarnings("unchecked")
-  public boolean equals(Object o) {
-    if (o == null) {
-      return false;
+  public final boolean equals(Object o) {
+    if (this == o) {
+      return true;
     }
-    // The cast is not unchecked because of the class equality check.
-    return getClass() == o.getClass() && compareTo((B) o) == 0;
+    return (o instanceof BasePath) && compareTo((B) o) == 0;
   }
 
   @Override

firebase-firestore/src/main/java/com/google/firebase/firestore/model/Document.java

@@ -23,7 +23,7 @@
  * Represents a document in Firestore with a key, version, data and whether the data has local
  * mutations applied to it.
  */
-public class Document extends MaybeDocument {
+public final class Document extends MaybeDocument {
 
   /** Describes the `hasPendingWrites` state of a document. */
   public enum DocumentState {

firebase-firestore/src/main/java/com/google/firebase/firestore/model/DocumentKey.java

@@ -23,7 +23,7 @@
 import java.util.List;
 
 /** DocumentKey represents the location of a document in the Firestore database. */
-public class DocumentKey implements Comparable<DocumentKey> {
+public final class DocumentKey implements Comparable<DocumentKey> {
 
   public static final String KEY_FIELD_NAME = "__name__";
 

firebase-firestore/src/main/java/com/google/firebase/firestore/model/DocumentSet.java

@@ -30,7 +30,7 @@
  * An immutable set of documents (unique by key) ordered by the given comparator or ordered by key
  * by default if no document is present.
  */
-public class DocumentSet implements Iterable<Document> {
+public final class DocumentSet implements Iterable<Document> {
 
   /** Returns an empty DocumentSet sorted by the given comparator, then by keys. */
   public static DocumentSet emptySet(final Comparator<Document> comparator) {

firebase-firestore/src/main/java/com/google/firebase/firestore/model/NoDocument.java

@@ -15,7 +15,7 @@
 package com.google.firebase.firestore.model;
 
 /** Represents that no documents exists for the key at the given version. */
-public class NoDocument extends MaybeDocument {
+public final class NoDocument extends MaybeDocument {
 
   private boolean hasCommittedMutations;
 

firebase-firestore/src/main/java/com/google/firebase/firestore/model/UnknownDocument.java

@@ -18,7 +18,7 @@
  * A class representing an existing document whose data is unknown (e.g. a document that was updated
  * without a known base document).
  */
-public class UnknownDocument extends MaybeDocument {
+public final class UnknownDocument extends MaybeDocument {
   public UnknownDocument(DocumentKey key, SnapshotVersion version) {
     super(key, version);
   }

firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/ArrayTransformOperation.java

@@ -51,6 +51,7 @@ public FieldValue applyToRemoteDocument(FieldValue previousValue, FieldValue tra
   }
 
   @Override
+  @SuppressWarnings("EqualsGetClass") // subtype-sensitive equality is intended.
   public boolean equals(Object o) {
     if (this == o) {
       return true;

firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/FieldMask.java

@@ -15,7 +15,7 @@
 package com.google.firebase.firestore.model.mutation;
 
 import com.google.firebase.firestore.model.FieldPath;
-import java.util.Collection;
+import java.util.Set;
 
 /**
  * Provides a set of fields that can be used to partially patch a document. The FieldMask is used in
@@ -25,14 +25,14 @@
  * companion ObjectValue, the field is deleted. foo.bar - Overwrites only the field bar of the
  * object foo. If foo is not an object, foo is replaced with an object containing foo.
  */
-public class FieldMask {
-  public static FieldMask fromCollection(Collection<FieldPath> mask) {
+public final class FieldMask {
+  public static FieldMask fromSet(Set<FieldPath> mask) {
     return new FieldMask(mask);
   }
 
-  private final Collection<FieldPath> mask;
+  private final Set<FieldPath> mask;
 
-  private FieldMask(Collection<FieldPath> mask) {
+  private FieldMask(Set<FieldPath> mask) {
     this.mask = mask;
   }
 
@@ -49,6 +49,11 @@ public boolean equals(Object o) {
     return mask.equals(fieldMask.mask);
   }
 
+  @Override
+  public String toString() {
+    return "FieldMask{mask=" + mask.toString() + "}";
+  }
+
   /**
    * Verifies that 'fieldPath' is included by at least one field in this field mask.
    *
@@ -69,7 +74,7 @@ public int hashCode() {
     return mask.hashCode();
   }
 
-  public Collection<FieldPath> getMask() {
+  public Set<FieldPath> getMask() {
     return mask;
   }
 }

firebase-firestore/src/main/java/com/google/firebase/firestore/remote/RemoteSerializer.java

@@ -98,8 +98,10 @@
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 
 /** Serializer that converts to and from Firestore API protos. */
 public final class RemoteSerializer {
@@ -532,11 +534,11 @@ private DocumentMask encodeDocumentMask(FieldMask mask) {
 
   private FieldMask decodeDocumentMask(DocumentMask mask) {
     int count = mask.getFieldPathsCount();
-    List<FieldPath> paths = new ArrayList<>(count);
+    Set<FieldPath> paths = new HashSet<>(count);
     for (int i = 0; i < count; i++) {
       paths.add(FieldPath.fromServerFormat(mask.getFieldPaths(i)));
     }
-    return FieldMask.fromCollection(paths);
+    return FieldMask.fromSet(paths);
   }
 
   private DocumentTransform.FieldTransform encodeFieldTransform(FieldTransform fieldTransform) {

firebase-firestore/src/main/java/com/google/firebase/firestore/remote/WatchChange.java

@@ -38,7 +38,7 @@ private WatchChange() {
    * A document change represents a change document and a list of target ids to which this change
    * applies. If the document has been deleted, the deleted document will be provided.
    */
-  public static class DocumentChange extends WatchChange {
+  public static final class DocumentChange extends WatchChange {
     // TODO: figure out if we can actually use arrays here for efficiency
     /** The new document applies to all of these targets. */
     private final List<Integer> updatedTargetIds;
@@ -137,7 +137,7 @@ public int hashCode() {
    * An ExistenceFilterWatchChange applies to the targets and is required to verify the current
    * client state against expected state sent from the server.
    */
-  public static class ExistenceFilterWatchChange extends WatchChange {
+  public static final class ExistenceFilterWatchChange extends WatchChange {
     private final int targetId;
 
     private final ExistenceFilter existenceFilter;
@@ -177,7 +177,7 @@ public enum WatchTargetChangeType {
   }
 
   /** The state of a target has changed. This can mean removal, addition, current or reset. */
-  public static class WatchTargetChange extends WatchChange {
+  public static final class WatchTargetChange extends WatchChange {
     private final WatchTargetChangeType changeType;
     private final List<Integer> targetIds;
     private final ByteString resumeToken;

firebase-firestore/src/test/java/com/google/firebase/firestore/local/LocalSerializerTest.java

@@ -17,7 +17,7 @@
 import static com.google.firebase.firestore.testutil.TestUtil.deleteMutation;
 import static com.google.firebase.firestore.testutil.TestUtil.deletedDoc;
 import static com.google.firebase.firestore.testutil.TestUtil.doc;
-import static com.google.firebase.firestore.testutil.TestUtil.field;
+import static com.google.firebase.firestore.testutil.TestUtil.fieldMask;
 import static com.google.firebase.firestore.testutil.TestUtil.key;
 import static com.google.firebase.firestore.testutil.TestUtil.map;
 import static com.google.firebase.firestore.testutil.TestUtil.setMutation;
@@ -33,7 +33,6 @@
 import com.google.firebase.firestore.model.NoDocument;
 import com.google.firebase.firestore.model.SnapshotVersion;
 import com.google.firebase.firestore.model.UnknownDocument;
-import com.google.firebase.firestore.model.mutation.FieldMask;
 import com.google.firebase.firestore.model.mutation.Mutation;
 import com.google.firebase.firestore.model.mutation.MutationBatch;
 import com.google.firebase.firestore.model.mutation.PatchMutation;
@@ -71,7 +70,7 @@ public void testEncodesMutationBatch() {
         new PatchMutation(
             key("bar/baz"),
             TestUtil.wrapObject(map("a", "b", "num", 1)),
-            FieldMask.fromCollection(asList(field("a"))),
+            fieldMask("a"),
             com.google.firebase.firestore.model.mutation.Precondition.exists(true));
     Mutation del = deleteMutation("baz/quux");
     Timestamp writeTime = Timestamp.now();

firebase-firestore/src/test/java/com/google/firebase/firestore/model/MutationTest.java

@@ -18,6 +18,7 @@
 import static com.google.firebase.firestore.testutil.TestUtil.deletedDoc;
 import static com.google.firebase.firestore.testutil.TestUtil.doc;
 import static com.google.firebase.firestore.testutil.TestUtil.field;
+import static com.google.firebase.firestore.testutil.TestUtil.fieldMask;
 import static com.google.firebase.firestore.testutil.TestUtil.key;
 import static com.google.firebase.firestore.testutil.TestUtil.map;
 import static com.google.firebase.firestore.testutil.TestUtil.mutationResult;
@@ -110,7 +111,7 @@ public void testDeletesValuesFromTheFieldMask() {
     Document baseDoc = doc("collection/key", 0, data);
 
     DocumentKey key = key("collection/key");
-    FieldMask mask = FieldMask.fromCollection(Arrays.asList(field("foo.bar")));
+    FieldMask mask = fieldMask("foo.bar");
     Mutation patch = new PatchMutation(key, ObjectValue.emptyObject(), mask, Precondition.NONE);
 
     MaybeDocument patchDoc = patch.applyToLocalView(baseDoc, baseDoc, Timestamp.now());