From 666650c664a710aca859bd90d124b0036e31e6e8 Mon Sep 17 00:00:00 2001
From: lukes <lukes@google.com>
Date: Thu, 25 Aug 2016 10:21:00 -0700
Subject: [PATCH] Improve ArrowType.hashCode and FunctionType.hashCode to make
 hash collisions less likely.

For ArrowType:
The previous version simply summed all the parameter hashcodes, but since + is commutative this means that function(a,b){} and function(b,a){} would have the same hashCode.

Also it included the 'returnTypeInferred' field in the hashCode though it was not part of equals().  This has been fixed.

For FunctionType:
All functions with the same ArrowType would get the same hashcode.  To resolve this we instead just hash more data.

Doing this revealed a bug in the equals() method for FunctionType where it was assymmetric and allowed constructors to be equal to ordinary functions in some circumstances.  This has been also been fixed.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=141193824
---
 .../javascript/rhino/jstype/ArrowType.java    |  7 +---
 .../javascript/rhino/jstype/FunctionType.java | 41 ++++++++++++-------
 2 files changed, 27 insertions(+), 21 deletions(-)

diff --git a/src/com/google/javascript/rhino/jstype/ArrowType.java b/src/com/google/javascript/rhino/jstype/ArrowType.java
index 9a55eefa1a5..9ba04bf930f 100644
--- a/src/com/google/javascript/rhino/jstype/ArrowType.java
+++ b/src/com/google/javascript/rhino/jstype/ArrowType.java
@@ -224,16 +224,11 @@ public int hashCode() {
     if (returnType != null) {
       hashCode += returnType.hashCode();
     }
-    if (returnTypeInferred) {
-      hashCode += 1;
-    }
     if (parameters != null) {
       Node param = parameters.getFirstChild();
       while (param != null) {
         JSType paramType = param.getJSType();
-        if (paramType != null) {
-          hashCode += paramType.hashCode();
-        }
+        hashCode = hashCode * 31 + (paramType == null ? 0 : paramType.hashCode());
         param = param.getNext();
       }
     }
diff --git a/src/com/google/javascript/rhino/jstype/FunctionType.java b/src/com/google/javascript/rhino/jstype/FunctionType.java
index d6a6489e041..25c0fc70a22 100644
--- a/src/com/google/javascript/rhino/jstype/FunctionType.java
+++ b/src/com/google/javascript/rhino/jstype/FunctionType.java
@@ -956,29 +956,40 @@ public ObjectType getTopMostDefiningType(String propertyName) {
    */
   boolean checkFunctionEquivalenceHelper(
       FunctionType that, EquivalenceMethod eqMethod, EqCache eqCache) {
-    if (isConstructor()) {
-      if (that.isConstructor()) {
-        return this == that;
-      }
-      return false;
+    if (this == that) {
+      return true;
     }
-    if (isInterface()) {
-      if (that.isInterface()) {
-        return getReferenceName().equals(that.getReferenceName());
-      }
+    if (kind != that.kind) {
       return false;
     }
-    if (that.isInterface()) {
-      return false;
+    switch (kind) {
+      case CONSTRUCTOR:
+        return false;  // constructors use identity semantics, which we already checked for above.
+      case INTERFACE:
+        return getReferenceName().equals(that.getReferenceName());
+      case ORDINARY:
+        return typeOfThis.checkEquivalenceHelper(that.typeOfThis, eqMethod, eqCache)
+            && call.checkArrowEquivalenceHelper(that.call, eqMethod, eqCache);
+      default:
+        throw new AssertionError();
     }
-
-    return typeOfThis.checkEquivalenceHelper(that.typeOfThis, eqMethod, eqCache) &&
-        call.checkArrowEquivalenceHelper(that.call, eqMethod, eqCache);
   }
 
   @Override
   public int hashCode() {
-    return isInterface() ? getReferenceName().hashCode() : call.hashCode();
+    int hc = kind.hashCode();
+    switch (kind) {
+      case CONSTRUCTOR:
+        return 31 * hc + System.identityHashCode(this);  // constructors use identity semantics
+      case INTERFACE:
+        return 31 * hc + getReferenceName().hashCode();
+      case ORDINARY:
+        hc =  31 * hc + typeOfThis.hashCode();
+        hc =  31 * hc + call.hashCode();
+        return hc;
+      default:
+        throw new AssertionError();
+    }
   }
 
   public boolean hasEqualCallType(FunctionType otherType) {