Move HashFunction out of FileSystem, and turn it into a class, instead of an enum.

Now that we aren't using enum names for the hash functions, we also accept the standard names, such as SHA-256.

RELNOTES: None.
PiperOrigin-RevId: 201624286

8 files changed, 123 insertions, 54 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/vfs/AbstractFileSystem.java b/src/main/java/com/google/devtools/build/lib/vfs/AbstractFileSystem.java
index 3159c21d34..096ca500f4 100644
--- a/src/main/java/com/google/devtools/build/lib/vfs/AbstractFileSystem.java
+++ b/src/main/java/com/google/devtools/build/lib/vfs/AbstractFileSystem.java
@@ -32,7 +32,7 @@ public abstract class AbstractFileSystem extends FileSystem {
 
   public AbstractFileSystem() {}
 
-  public AbstractFileSystem(HashFunction digestFunction) {
+  public AbstractFileSystem(DigestHashFunction digestFunction) {
     super(digestFunction);
   }
 
diff --git a/src/main/java/com/google/devtools/build/lib/vfs/AbstractFileSystemWithCustomStat.java b/src/main/java/com/google/devtools/build/lib/vfs/AbstractFileSystemWithCustomStat.java
index 875df98bab..2a2b06b50a 100644
--- a/src/main/java/com/google/devtools/build/lib/vfs/AbstractFileSystemWithCustomStat.java
+++ b/src/main/java/com/google/devtools/build/lib/vfs/AbstractFileSystemWithCustomStat.java
@@ -24,7 +24,7 @@ public abstract class AbstractFileSystemWithCustomStat extends AbstractFileSyste
 
   public AbstractFileSystemWithCustomStat() {}
 
-  public AbstractFileSystemWithCustomStat(HashFunction hashFunction) {
+  public AbstractFileSystemWithCustomStat(DigestHashFunction hashFunction) {
     super(hashFunction);
   }
 
diff --git a/src/main/java/com/google/devtools/build/lib/vfs/DigestHashFunction.java b/src/main/java/com/google/devtools/build/lib/vfs/DigestHashFunction.java
new file mode 100644
index 0000000000..be18f0ac92
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/vfs/DigestHashFunction.java
@@ -0,0 +1,101 @@
+// Copyright 2018 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.devtools.build.lib.vfs;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.hash.HashFunction;
+import com.google.common.hash.Hashing;
+import com.google.devtools.common.options.Converter;
+import com.google.devtools.common.options.OptionsParsingException;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map.Entry;
+
+/** Type of hash function to use for digesting files. */
+// The underlying HashFunctions are immutable and thread safe.
+public class DigestHashFunction {
+  private static final HashMap<String, DigestHashFunction> hashFunctionRegistry = new HashMap<>();
+
+  public static final DigestHashFunction MD5 = DigestHashFunction.register(Hashing.md5(), "MD5");
+  public static final DigestHashFunction SHA1 =
+      DigestHashFunction.register(Hashing.sha1(), "SHA-1", "SHA1");
+  public static final DigestHashFunction SHA256 =
+      DigestHashFunction.register(Hashing.sha256(), "SHA-256", "SHA256");
+
+  private final HashFunction hash;
+  private final String name;
+
+  private DigestHashFunction(HashFunction hash, String name) {
+    this.hash = hash;
+    this.name = name;
+  }
+
+  public HashFunction getHash() {
+    return hash;
+  }
+
+  public boolean isValidDigest(byte[] digest) {
+    // TODO(b/109764197): Remove this check to accept variable-length hashes.
+    return digest != null && digest.length * 8 == hash.bits();
+  }
+
+  @Override
+  public String toString() {
+    return name;
+  }
+
+  /**
+   * Creates a new DigestHashFunction that is registered to be recognized by its name in {@link
+   * DigestFunctionConverter}.
+   *
+   * @param hashName the canonical name for this hash function.
+   * @param altNames alternative names that will be mapped to this function by the converter but
+   *     will not serve as the canonical name for the DigestHashFunction.
+   * @param hash The {@link HashFunction} to register.
+   * @throws IllegalArgumentException if the name is already registered.
+   */
+  public static DigestHashFunction register(
+      HashFunction hash, String hashName, String... altNames) {
+    DigestHashFunction hashFunction = new DigestHashFunction(hash, hashName);
+    List<String> names = ImmutableList.<String>builder().add(hashName).add(altNames).build();
+    synchronized (hashFunctionRegistry) {
+      for (String name : names) {
+        if (hashFunctionRegistry.containsKey(name)) {
+          throw new IllegalArgumentException("Hash function " + name + " is already registered.");
+        }
+        hashFunctionRegistry.put(name, hashFunction);
+      }
+    }
+    return hashFunction;
+  }
+
+  /** Converts a string to its registered {@link DigestHashFunction}. */
+  public static class DigestFunctionConverter implements Converter<DigestHashFunction> {
+    @Override
+    public DigestHashFunction convert(String input) throws OptionsParsingException {
+      for (Entry<String, DigestHashFunction> possibleFunctions : hashFunctionRegistry.entrySet()) {
+        if (possibleFunctions.getKey().equalsIgnoreCase(input)) {
+          return possibleFunctions.getValue();
+        }
+      }
+      throw new OptionsParsingException("Not a valid hash function.");
+    }
+
+    @Override
+    public String getTypeDescription() {
+      return "hash function";
+    }
+  }
+}
diff --git a/src/main/java/com/google/devtools/build/lib/vfs/FileSystem.java b/src/main/java/com/google/devtools/build/lib/vfs/FileSystem.java
index 2169c18b9c..8258dccfc0 100644
--- a/src/main/java/com/google/devtools/build/lib/vfs/FileSystem.java
+++ b/src/main/java/com/google/devtools/build/lib/vfs/FileSystem.java
@@ -18,11 +18,9 @@ import static java.nio.charset.StandardCharsets.ISO_8859_1;
 
 import com.google.common.base.Preconditions;
 import com.google.common.collect.Lists;
-import com.google.common.hash.Hashing;
 import com.google.common.io.ByteSource;
 import com.google.common.io.CharStreams;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
-import com.google.devtools.common.options.EnumConverter;
 import java.io.FileNotFoundException;
 import java.io.IOException;
 import java.io.InputStream;
@@ -38,47 +36,17 @@ import java.util.List;
 @ThreadSafe
 public abstract class FileSystem {
 
-  /** Type of hash function to use for digesting files. */
-  // The underlying HashFunctions are immutable and thread safe.
-  @SuppressWarnings("ImmutableEnumChecker")
-  public enum HashFunction {
-    MD5(Hashing.md5()),
-    SHA1(Hashing.sha1()),
-    SHA256(Hashing.sha256());
-
-    private final com.google.common.hash.HashFunction hash;
-
-    HashFunction(com.google.common.hash.HashFunction hash) {
-      this.hash = hash;
-    }
-
-    /** Converts to {@link HashFunction}. */
-    public static class Converter extends EnumConverter<HashFunction> {
-      public Converter() {
-        super(HashFunction.class, "hash function");
-      }
-    }
-
-    public com.google.common.hash.HashFunction getHash() {
-      return hash;
-    }
-
-    public boolean isValidDigest(byte[] digest) {
-      return digest != null && digest.length * 8 == hash.bits();
-    }
-  }
-
-  private final HashFunction digestFunction;
+  private final DigestHashFunction digestFunction;
 
   public FileSystem() {
-    this(HashFunction.MD5);
+    this(DigestHashFunction.MD5);
   }
 
-  public FileSystem(HashFunction digestFunction) {
+  public FileSystem(DigestHashFunction digestFunction) {
     this.digestFunction = Preconditions.checkNotNull(digestFunction);
   }
 
-  public HashFunction getDigestFunction() {
+  public DigestHashFunction getDigestFunction() {
     return digestFunction;
   }
 
@@ -266,11 +234,11 @@ public abstract class FileSystem {
   }
 
   /**
-   * Gets a fast digest for the given path and hash function type, or {@code null} if there
-   * isn't one available or the filesystem doesn't support them. This digest should be
-   * suitable for detecting changes to the file.
+   * Gets a fast digest for the given path and hash function type, or {@code null} if there isn't
+   * one available or the filesystem doesn't support them. This digest should be suitable for
+   * detecting changes to the file.
    */
-  protected byte[] getFastDigest(Path path, HashFunction hashFunction) throws IOException {
+  protected byte[] getFastDigest(Path path, DigestHashFunction hashFunction) throws IOException {
     return null;
   }
 
@@ -291,15 +259,15 @@ public abstract class FileSystem {
   }
 
   /**
-   * Returns the digest of the file denoted by the path, following
-   * symbolic links, for the given hash digest function.
+   * Returns the digest of the file denoted by the path, following symbolic links, for the given
+   * hash digest function.
+   *
+   * <p>Subclasses may (and do) optimize this computation for particular digest functions.
    *
    * @return a new byte array containing the file's digest
    * @throws IOException if the digest could not be computed for any reason
-   *
-   * Subclasses may (and do) optimize this computation for particular digest functions.
    */
-  protected byte[] getDigest(final Path path, HashFunction hashFunction) throws IOException {
+  protected byte[] getDigest(final Path path, DigestHashFunction hashFunction) throws IOException {
     return new ByteSource() {
       @Override
       public InputStream openStream() throws IOException {
diff --git a/src/main/java/com/google/devtools/build/lib/vfs/JavaIoFileSystem.java b/src/main/java/com/google/devtools/build/lib/vfs/JavaIoFileSystem.java
index ecda1b4ba1..ea541b4a1e 100644
--- a/src/main/java/com/google/devtools/build/lib/vfs/JavaIoFileSystem.java
+++ b/src/main/java/com/google/devtools/build/lib/vfs/JavaIoFileSystem.java
@@ -56,7 +56,7 @@ public class JavaIoFileSystem extends AbstractFileSystemWithCustomStat {
     this(new JavaClock());
   }
 
-  public JavaIoFileSystem(HashFunction hashFunction) {
+  public JavaIoFileSystem(DigestHashFunction hashFunction) {
     super(hashFunction);
     this.clock = new JavaClock();
   }
@@ -392,7 +392,7 @@ public class JavaIoFileSystem extends AbstractFileSystemWithCustomStat {
   }
 
   @Override
-  protected byte[] getDigest(Path path, HashFunction hashFunction) throws IOException {
+  protected byte[] getDigest(Path path, DigestHashFunction hashFunction) throws IOException {
     String name = path.toString();
     long startTime = Profiler.nanoTimeMaybe();
     try {
diff --git a/src/main/java/com/google/devtools/build/lib/vfs/Path.java b/src/main/java/com/google/devtools/build/lib/vfs/Path.java
index f9bc9d18fc..6928f0de19 100644
--- a/src/main/java/com/google/devtools/build/lib/vfs/Path.java
+++ b/src/main/java/com/google/devtools/build/lib/vfs/Path.java
@@ -19,7 +19,6 @@ import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
 import com.google.devtools.build.lib.skylarkinterface.SkylarkPrintable;
 import com.google.devtools.build.lib.skylarkinterface.SkylarkPrinter;
 import com.google.devtools.build.lib.util.FileType;
-import com.google.devtools.build.lib.vfs.FileSystem.HashFunction;
 import java.io.File;
 import java.io.FileNotFoundException;
 import java.io.IOException;
@@ -766,7 +765,7 @@ public class Path
    * @return a new byte array containing the file's digest
    * @throws IOException if the digest could not be computed for any reason
    */
-  public byte[] getDigest(HashFunction hashFunction) throws IOException {
+  public byte[] getDigest(DigestHashFunction hashFunction) throws IOException {
     return fileSystem.getDigest(this, hashFunction);
   }
 
diff --git a/src/main/java/com/google/devtools/build/lib/vfs/UnionFileSystem.java b/src/main/java/com/google/devtools/build/lib/vfs/UnionFileSystem.java
index a929f1435d..2136633bc6 100644
--- a/src/main/java/com/google/devtools/build/lib/vfs/UnionFileSystem.java
+++ b/src/main/java/com/google/devtools/build/lib/vfs/UnionFileSystem.java
@@ -193,7 +193,7 @@ public final class UnionFileSystem extends FileSystem {
   }
 
   @Override
-  protected byte[] getDigest(Path path, HashFunction hashFunction) throws IOException {
+  protected byte[] getDigest(Path path, DigestHashFunction hashFunction) throws IOException {
     path = internalResolveSymlink(path);
     FileSystem delegate = getDelegate(path);
     return delegate.getDigest(adjustPath(path, delegate), hashFunction);
@@ -425,7 +425,7 @@ public final class UnionFileSystem extends FileSystem {
   }
 
   @Override
-  protected byte[] getFastDigest(Path path, HashFunction hashFunction) throws IOException {
+  protected byte[] getFastDigest(Path path, DigestHashFunction hashFunction) throws IOException {
     path = internalResolveSymlink(path);
     FileSystem delegate = getDelegate(path);
     return delegate.getFastDigest(adjustPath(path, delegate), hashFunction);
diff --git a/src/main/java/com/google/devtools/build/lib/vfs/inmemoryfs/InMemoryFileSystem.java b/src/main/java/com/google/devtools/build/lib/vfs/inmemoryfs/InMemoryFileSystem.java
index c2a52a5b3d..bebe5b380c 100644
--- a/src/main/java/com/google/devtools/build/lib/vfs/inmemoryfs/InMemoryFileSystem.java
+++ b/src/main/java/com/google/devtools/build/lib/vfs/inmemoryfs/InMemoryFileSystem.java
@@ -19,6 +19,7 @@ import com.google.devtools.build.lib.clock.Clock;
 import com.google.devtools.build.lib.clock.JavaClock;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
 import com.google.devtools.build.lib.util.OS;
+import com.google.devtools.build.lib.vfs.DigestHashFunction;
 import com.google.devtools.build.lib.vfs.FileAccessException;
 import com.google.devtools.build.lib.vfs.FileStatus;
 import com.google.devtools.build.lib.vfs.FileSystem;
@@ -78,7 +79,7 @@ public class InMemoryFileSystem extends FileSystem {
    * Creates a new InMemoryFileSystem with scope checking disabled (all paths are considered to be
    * within scope).
    */
-  public InMemoryFileSystem(Clock clock, HashFunction hashFunction) {
+  public InMemoryFileSystem(Clock clock, DigestHashFunction hashFunction) {
     super(hashFunction);
     this.clock = clock;
     this.rootInode = newRootInode(clock);